UAT: _cleveragents/plan/diff A2A handler always returns empty changes list — PlanApplyService.diff() is never called #2510

Open
opened 2026-04-03 18:41:52 +00:00 by freemo · 1 comment
Owner

Background and Context

The _handle_plan_diff method in src/cleveragents/a2a/facade.py (lines 411–423) always returns "changes": [] regardless of what files were changed during plan execution. The method fetches the plan via PlanLifecycleService but never calls PlanApplyService.diff() to retrieve the actual changeset diff.

The CLI agents plan diff command correctly calls PlanApplyService.diff(plan_id, fmt=fmt) and returns the real unified diff. The A2A handler is missing this call entirely, meaning any A2A client (including the local facade used by the CLI in server mode) receives an empty changeset every time.

Current Behavior

_handle_plan_diff always returns:

{"plan_id": "...", "changes": [], "phase": "..."}

The changes list is hardcoded to []. The faulty implementation:

def _handle_plan_diff(self, params: dict[str, Any]) -> dict[str, Any]:
    svc = self._plan_lifecycle_service
    plan_id = params.get("plan_id", "")
    if svc is None:
        return {"plan_id": plan_id, "changes": []}
    if not plan_id:
        raise ValueError("plan_id is required")
    plan = svc.get_plan(plan_id)
    return {
        "plan_id": plan.identity.plan_id,
        "changes": [],  # Always empty — PlanApplyService.diff() never called
        "phase": plan.phase.value,
    }

Code location: src/cleveragents/a2a/facade.py, method _handle_plan_diff (lines 411–423)

Expected Behavior

Per spec §43269, the _cleveragents/plan/diff A2A extension method must map to PlanService.get_diff(). The handler must call PlanApplyService.diff(plan_id) and return the actual unified diff of the changeset in the changes field — consistent with the behaviour of the CLI agents plan diff command.

Acceptance Criteria

  • _handle_plan_diff calls PlanApplyService.diff(plan_id) and returns the real changeset diff in the changes field.
  • When no changes exist the changes field is an empty list (not a hardcoded constant).
  • When PlanLifecycleService is None the handler returns {"plan_id": plan_id, "changes": []} as before (graceful degradation).
  • The A2A response schema for _cleveragents/plan/diff matches the structure returned by the CLI plan diff command.
  • Unit tests cover: normal diff with changes, empty diff, missing plan_id, and None service cases.
  • All nox default sessions pass with no errors.
  • Coverage ≥ 97%.

Supporting Information

  • Faulty file: src/cleveragents/a2a/facade.py — method _handle_plan_diff (lines 411–423)
  • Reference implementation: src/cleveragents/application/services/plan_apply_service.pyPlanApplyService.diff()
  • CLI reference: agents plan diff command (correctly calls PlanApplyService.diff(plan_id, fmt=fmt))
  • Spec reference: §43269 — _cleveragents/plan/diff maps to PlanService.get_diff()
  • Parent Epic: #399 (Epic: Post-MVP Server & Clients)

Metadata

  • Branch: fix/a2a-plan-diff-empty-changes
  • Commit Message: fix(a2a): call PlanApplyService.diff() in _handle_plan_diff to return real changeset
  • Milestone: v3.8.0
  • Parent Epic: #399

Subtasks

  • Inject or resolve PlanApplyService inside _handle_plan_diff (or pass it via constructor alongside _plan_lifecycle_service)
  • Replace the hardcoded "changes": [] with the result of PlanApplyService.diff(plan_id)
  • Ensure graceful degradation when PlanApplyService is unavailable (return empty list, do not raise)
  • Add/update unit tests for _handle_plan_diff covering: real diff, empty diff, missing plan_id, None service
  • Add/update integration or BDD scenario for _cleveragents/plan/diff A2A method returning correct changeset
  • Verify coverage ≥ 97% via nox -s coverage_report
  • Run nox (all default sessions) and fix any errors

Definition of Done

This issue is complete when:

  • All subtasks above are completed and checked off.
  • A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly (fix(a2a): call PlanApplyService.diff() in _handle_plan_diff to return real changeset), followed by a blank line, then additional lines providing relevant details about the implementation.
  • The commit is pushed to the remote on the branch matching the Branch in Metadata exactly (fix/a2a-plan-diff-empty-changes).
  • The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
  • All nox stages pass.
  • Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Background and Context The `_handle_plan_diff` method in `src/cleveragents/a2a/facade.py` (lines 411–423) always returns `"changes": []` regardless of what files were changed during plan execution. The method fetches the plan via `PlanLifecycleService` but never calls `PlanApplyService.diff()` to retrieve the actual changeset diff. The CLI `agents plan diff` command correctly calls `PlanApplyService.diff(plan_id, fmt=fmt)` and returns the real unified diff. The A2A handler is missing this call entirely, meaning any A2A client (including the local facade used by the CLI in server mode) receives an empty changeset every time. ## Current Behavior `_handle_plan_diff` always returns: ```json {"plan_id": "...", "changes": [], "phase": "..."} ``` The `changes` list is hardcoded to `[]`. The faulty implementation: ```python def _handle_plan_diff(self, params: dict[str, Any]) -> dict[str, Any]: svc = self._plan_lifecycle_service plan_id = params.get("plan_id", "") if svc is None: return {"plan_id": plan_id, "changes": []} if not plan_id: raise ValueError("plan_id is required") plan = svc.get_plan(plan_id) return { "plan_id": plan.identity.plan_id, "changes": [], # Always empty — PlanApplyService.diff() never called "phase": plan.phase.value, } ``` **Code location**: `src/cleveragents/a2a/facade.py`, method `_handle_plan_diff` (lines 411–423) ## Expected Behavior Per spec §43269, the `_cleveragents/plan/diff` A2A extension method must map to `PlanService.get_diff()`. The handler must call `PlanApplyService.diff(plan_id)` and return the actual unified diff of the changeset in the `changes` field — consistent with the behaviour of the CLI `agents plan diff` command. ## Acceptance Criteria - [ ] `_handle_plan_diff` calls `PlanApplyService.diff(plan_id)` and returns the real changeset diff in the `changes` field. - [ ] When no changes exist the `changes` field is an empty list (not a hardcoded constant). - [ ] When `PlanLifecycleService` is `None` the handler returns `{"plan_id": plan_id, "changes": []}` as before (graceful degradation). - [ ] The A2A response schema for `_cleveragents/plan/diff` matches the structure returned by the CLI `plan diff` command. - [ ] Unit tests cover: normal diff with changes, empty diff, missing `plan_id`, and `None` service cases. - [ ] All `nox` default sessions pass with no errors. - [ ] Coverage ≥ 97%. ## Supporting Information - Faulty file: `src/cleveragents/a2a/facade.py` — method `_handle_plan_diff` (lines 411–423) - Reference implementation: `src/cleveragents/application/services/plan_apply_service.py` — `PlanApplyService.diff()` - CLI reference: `agents plan diff` command (correctly calls `PlanApplyService.diff(plan_id, fmt=fmt)`) - Spec reference: §43269 — `_cleveragents/plan/diff` maps to `PlanService.get_diff()` - Parent Epic: #399 (Epic: Post-MVP Server & Clients) --- ## Metadata - **Branch**: `fix/a2a-plan-diff-empty-changes` - **Commit Message**: `fix(a2a): call PlanApplyService.diff() in _handle_plan_diff to return real changeset` - **Milestone**: v3.8.0 - **Parent Epic**: #399 --- ## Subtasks - [ ] Inject or resolve `PlanApplyService` inside `_handle_plan_diff` (or pass it via constructor alongside `_plan_lifecycle_service`) - [ ] Replace the hardcoded `"changes": []` with the result of `PlanApplyService.diff(plan_id)` - [ ] Ensure graceful degradation when `PlanApplyService` is unavailable (return empty list, do not raise) - [ ] Add/update unit tests for `_handle_plan_diff` covering: real diff, empty diff, missing `plan_id`, `None` service - [ ] Add/update integration or BDD scenario for `_cleveragents/plan/diff` A2A method returning correct changeset - [ ] Verify coverage ≥ 97% via `nox -s coverage_report` - [ ] Run `nox` (all default sessions) and fix any errors --- ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly (`fix(a2a): call PlanApplyService.diff() in _handle_plan_diff to return real changeset`), followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly (`fix/a2a-plan-diff-empty-changes`). - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. - All nox stages pass. - Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator
freemo added this to the v3.8.0 milestone 2026-04-03 18:41:56 +00:00
Author
Owner

Issue triaged by project owner:

  • State: Verified
  • MoSCoW: Should Have — Spec compliance or quality improvement that should be included in the milestone.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **MoSCoW**: Should Have — Spec compliance or quality improvement that should be included in the milestone. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Blocks
#399 Epic: Post-MVP Server & Clients
cleveragents/cleveragents-core
Reference
cleveragents/cleveragents-core#2510
No description provided.