fix(cli): replace private _commit_plan calls with public save_plan API (PR #8661)

Resolve the SOLID principle violation where CLI layer accessed private
_lifecycle._commit_plan() method directly, breaking encapsulation. All
four CLI call sites and two PlanService call sites now use the public
save_plan() method which is designed as the intended persistence interface.

ISSUES CLOSED: #8628

features/agent_evolution_label_milestone_compliance.feature

@@ -0,0 +1,73 @@
+@phase2 @agent_evolution @labels @milestone
+Feature: Agent Evolution PR Label and Milestone Compliance
+  As a CleverAgents project maintainer
+  I want agent evolution improvement PRs to have consistent labels and milestone assignment
+  So that they are properly categorized, routed, and tracked
+
+  # ---------------------------------------------------------------------------
+  # agent-evolution-worker.md — label requirements
+  # ---------------------------------------------------------------------------
+
+  @agent_evolution_worker_labels
+  Scenario: Agent evolution worker task step includes all required labels
+    Given the file ".opencode/agents/agent-evolution-worker.md" exists
+    And it describes creating a PR in Task step 5
+    Then the task step MUST include "Type/Automation" label reference
+    AND the task step MUST include "State/In Review" label reference
+    AND the task step MUST include "needs feedback" label reference
+
+  @agent_evolution_worker_labels
+  Scenario: Agent evolution worker rules require all required labels
+    Given the file ".opencode/agents/agent-evolution-worker.md" exists
+    And it lists Rules in its rules section
+    Then rule 4 MUST include "Type/Automation" label requirement
+    AND rule 4 MUST include "State/In Review" label requirement
+    AND rule 4 MUST include "needs feedback" label requirement
+
+  @agent_evolution_worker_labels
+  Scenario: Agent evolution worker task permissions include forgejo-label-manager
+    Given the file ".opencode/agents/agent-evolution-worker.md" exists
+    And its permission section includes a task block
+    Then the task block MUST allow "forgejo-label-manager"
+    AND the task block MUST allow "pr-creator"
+
+  @agent_evolution_worker_milestone
+  Scenario: Agent evolution worker requires milestone assignment
+    Given the file ".opencode/agents/agent-evolution-worker.md" exists
+    And it describes creating a PR in Task step 5
+    Then the task step MUST include milestone assignment requirement
+    AND rule 4 MUST mention milestone assignment
+
+  # ---------------------------------------------------------------------------
+  # agent-evolution-pool-supervisor.md — label requirements
+  # ---------------------------------------------------------------------------
+
+  @agent_evolution_supervisor_labels
+  Scenario: Agent evolution supervisor Workers section describes proper labels
+    Given the file ".opencode/agents/agent-evolution-pool-supervisor.md" exists
+    And it has a Workers section
+    Then the Workers description MUST include "Type/Automation" label reference
+    AND it MUST include "State/In Review" label reference
+    AND it MUST include "needs feedback" label reference
+    AND it MUST mention milestone assignment
+
+  @agent_evolution_supervisor_labels
+  Scenario: Agent evolution supervisor Step 2 describes proper labels for PRs
+    Given the file ".opencode/agents/agent-evolution-pool-supervisor.md" exists
+    And it has a Two-Step Proposal Workflow section
+    Then Step 2 (Implementation PR) MUST include "Type/Automation" label reference
+    AND it MUST include "State/In Review" label reference
+    AND it MUST include "needs feedback" label reference
+    AND it MUST mention milestone assignment
+
+  @agent_evolution_supervisor_labels
+  Scenario: Agent evolution supervisor has rule for label management via forgejo-label-manager
+    Given the file ".opencode/agents/agent-evolution-pool-supervisor.md" exists
+    And it has a Rules section
+    Then one of its rules MUST instruct using "forgejo-label-manager"
+
+  @agent_evolution_supervisor_milestone
+  Scenario: Agent evolution supervisor has rule for milestone assignment
+    Given the file ".opencode/agents/agent-evolution-pool-supervisor.md" exists
+    And it has a Rules section
+    Then one of its rules MUST describe milestone assignment for improvement PRs

features/steps/agent_evolution_label_milestone_steps.py

@@ -0,0 +1,270 @@
+"""Step definitions for Agent Evolution PR Label and Milestone Compliance tests."""
+
+from __future__ import annotations
+
+import re
+from pathlib import Path
+
+from behave import given, then, when
+from behave.runner import Context
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _repo_root() -> Path:
+    """Return the repository root directory."""
+    return Path(__file__).resolve().parent.parent.parent
+
+
+def _read_agent_file(filename: str) -> str:
+    """Read an agent definition file from .opencode/agents/."""
+    path = _repo_root() / ".opencode" / "agents" / filename
+    if not path.exists():
+        raise FileNotFoundError(f"Agent file not found: {path}")
+    return path.read_text(encoding="utf-8")
+
+
+def _contains_any(text: str, *patterns: str) -> list[str]:
+    """Return the intersection of matched patterns in text."""
+    return [p for p in patterns if p in text]
+
+
+# ---------------------------------------------------------------------------
+# Given steps
+# ---------------------------------------------------------------------------
+
+
+@given('the file "{filename}" exists')
+def step_file_exists(context: Context, filename: str) -> None:
+    """Assert the agent file exists and is readable."""
+    content = _read_agent_file(filename)
+    context.agent_file_path = filename
+    context.agent_content = content
+
+
+@given('it describes creating a PR in Task step {step_num:d}')
+def step_pr_creation_step(context: Context, step_num: int) -> None:
+    """Extract the nth numbered item from the Task section."""
+    task_section_match = re.search(
+        r"## Task\s*\n(.*?)(?=##|\Z)", context.agent_content, re.DOTALL
+    )
+    assert task_section_match is not None, "No Task section found"
+    task_text = task_section_match.group(1)
+    # Extract numbered items
+    numbered_items = re.findall(r"\d+\.\s+", task_text)
+    assert step_num <= len(numbered_items), (
+        f"Only {len(numbered_items)} numbered steps found in Task section"
+    )
+    context.task_sections = re.split(
+        r"(?<=\n)\n\d+\.\s",
+        task_section_match.group(1).strip(),
+    )
+
+
+@given("it lists Rules in its rules section")
+def step_rules_present(context: Context) -> None:
+    """Extract the Rules section."""
+    rules_match = re.search(
+        r"## Rules\s*\n(.*?)(?:\n## |\Z)", context.agent_content, re.DOTALL
+    )
+    assert rules_match is not None, "No Rules section found"
+    context.rules_text = rules_match.group(1)
+
+
+@given("its permission section includes a task block")
+def step_permission_task_block(context: Context) -> None:
+    """Extract the permission/task section from the YAML frontmatter."""
+    yaml_match = re.search(r"^---\s*\n(.*?)\n^---", context.agent_content, re.DOTALL)
+    assert yaml_match is not None, "No YAML frontmatter found"
+    context.yaml_content = yaml_match.group(1)
+
+
+# ---------------------------------------------------------------------------
+# When steps (no-ops for config validation tests)
+# ---------------------------------------------------------------------------
+
+
+@when('I check the task step {step_num:d}')
+def step_check_task_step(context: Context, step_num: int) -> None:
+    """No-op — the actual checking happens in Then steps."""
+    pass
+
+
+# ---------------------------------------------------------------------------
+# Then steps
+# ---------------------------------------------------------------------------
+
+
+@then('the task step MUST include "{pattern}" label reference')
+def step_label_reference_exists(context: Context, pattern: str) -> None:
+    """Verify a label reference appears in the task PR creation step."""
+    if hasattr(context, "task_sections") and len(context.task_sections) >= 5:
+        # Step 5 is the last numbered item after splitting by "\nN. "
+        step_text = context.task_sections[-1]  # Last split element after removing leading digits
+    else:
+        step_text = context.agent_content
+
+    assert pattern in step_text, (
+        f"Task step does not reference '{pattern}' label\n"
+        f"Content:\n{step_text[:500]}"
+    )
+
+
+@then("the task step MUST include milestone assignment requirement")
+def step_milestone_in_task(context: Context) -> None:
+    """Verify milestone assignment is mentioned in the task PR creation step."""
+    if hasattr(context, "task_sections"):
+        step_text = context.task_sections[-1]
+    else:
+        step_text = context.agent_content
+    assert re.search(r"milestone", step_text, re.IGNORECASE), (
+        f"Milestone assignment not found in task step\nContent:\n{step_text[:500]}"
+    )
+
+
+@then('rule {num:d} MUST include "{pattern}" label requirement')
+def step_rule_label_requirement(context: Context, num: int, pattern: str) -> None:
+    """Verify a specific rule number contains the required label."""
+    rules_match = re.search(
+        rf"(?:^|\n)\s*{num}\.\s+(.*?)(?=\n\d+\.|\Z)",
+        context.rules_text,
+        re.DOTALL,
+    )
+    assert rules_match is not None, (
+        f"Rule {num} not found in Rules section"
+    )
+    rule_text = rules_match.group(1)
+    assert pattern in rule_text, (
+        f"Rule {num} does not mention '{pattern}'\n"
+        f"Rule text:\n{rule_text[:300]}"
+    )
+
+
+@then("rule 4 MUST mention milestone assignment")
+def step_rule_milestone(context: Context) -> None:
+    """Verify rule 4 mentions milestone assignment."""
+    rules_match = re.search(
+        rf"(?:^|\n)\s*4\.\s+(.*?)(?=\n\d+\.|\Z)",
+        context.rules_text,
+        re.DOTALL,
+    )
+    assert rules_match is not None, "Rule 4 not found in Rules section"
+    rule_text = rules_match.group(1)
+    assert re.search(r"milestone", rule_text, re.IGNORECASE), (
+        f"Rule 4 does not mention milestone\n"
+        f"Rule text:\n{rule_text[:300]}"
+    )
+
+
+@then('the task block MUST allow "{tool}"')
+def step_permission_tool_allowed(context: Context, tool: str) -> None:
+    """Verify a tool is explicitly allowed in the task permission block."""
+    # Extract just the task block from YAML
+    task_match = re.search(
+        r"task:\s*\n(?:(?!\n[a-z_]+:|---).)*",
+        context.yaml_content,
+        re.DOTALL,
+    )
+    assert task_match is not None, "No task permission block found"
+    task_block = task_match.group(0)
+    # Look for "tool": allow pattern
+    allowed_pattern = re.search(rf'"{tool}":\s*allow', task_block, re.DOTALL)
+    assert allowed_pattern is not None, (
+        f"Task block does not allow '{tool}'\n"
+        f"Task block content:\n{task_block[:300]}"
+    )
+
+
+@then('the Workers description MUST include "{pattern}" label reference')
+def step_workers_label(context: Context, pattern: str) -> None:
+    """Verify the Workers section mentions the required label."""
+    workers_match = re.search(
+        r"## Workers\s*\n(.*?)(?:\n## |\Z)",
+        context.agent_content,
+        re.DOTALL,
+    )
+    assert workers_match is not None, "No Workers section found"
+    workers_text = workers_match.group(1)
+    assert pattern in workers_text, (
+        f"Workers section does not reference '{pattern}'\n"
+        f"Content:\n{workers_text[:500]}"
+    )
+
+
+@then('it MUST mention milestone assignment')
+def step_workers_milestone(context: Context) -> None:
+    """Verify Workers section mentions milestone assignment."""
+    workers_match = re.search(
+        r"## Workers\s*\n(.*?)(?:\n## |\Z)",
+        context.agent_content,
+        re.DOTALL,
+    )
+    assert workers_match is not None, "No Workers section found"
+    workers_text = workers_match.group(1)
+    assert re.search(r"milestone", workers_text, re.IGNORECASE), (
+        f"Workers section does not mention milestone assignment\n"
+        f"Content:\n{workers_text[:500]}"
+    )
+
+
+@then('Step 2 (Implementation PR) MUST include "{pattern}" label reference')
+def step_implementation_pr_label(context: Context, pattern: str) -> None:
+    """Verify Step 2 mentions the required label."""
+    workflow_match = re.search(
+        r"## Two-Step Proposal Workflow\s*\n(.*?)(?:\n## |\Z)",
+        context.agent_content,
+        re.DOTALL,
+    )
+    assert workflow_match is not None, "No Two-Step Proposal Workflow section found"
+    workflow_text = workflow_match.group(1)
+    # Step 2 should be the second bold line
+    bold_lines = re.findall(r"\*\*(.+?)\*\*", workflow_text)
+    step_2 = bold_lines[1] if len(bold_lines) > 1 else workflow_text
+    assert pattern in step_2, (
+        f"Step 2 does not reference '{pattern}'\n"
+        f"Bold lines: {bold_lines}\n"
+        f"Full workflow text:\n{workflow_text[:500]}"
+    )
+
+
+@then("it MUST mention milestone assignment")
+def step_implementation_pr_milestone(context: Context) -> None:
+    """Verify Step 2 mentions milestone assignment."""
+    workflow_match = re.search(
+        r"## Two-Step Proposal Workflow\s*\n(.*?)(?:\n## |\Z)",
+        context.agent_content,
+        re.DOTALL,
+    )
+    assert workflow_match is not None, "No Two-Step Proposal Workflow section found"
+    workflow_text = workflow_match.group(1)
+    bold_lines = re.findall(r"\*\*(.+?)\*\*", workflow_text)
+    step_2 = bold_lines[1] if len(bold_lines) > 1 else workflow_text
+    assert re.search(r"milestone", step_2, re.IGNORECASE), (
+        f"Step 2 does not mention milestone assignment\n"
+        f"Bold lines: {bold_lines}\n"
+    )
+
+
+@then('one of its rules MUST instruct using "{manager}"')
+def step_rule_label_manager(context: Context, manager: str) -> None:
+    """Verify one of the rules mentions the specified label manager."""
+    assert manager in context.rules_text, (
+        f"No rule mentions '{manager}' in Rules section\n"
+        f"Rules text:\n{context.rules_text[:500]}"
+    )
+
+
+@then('one of its rules MUST describe milestone assignment for improvement PRs')
+def step_rule_milestone_assignment(context: Context) -> None:
+    """Verify one rule mentions milestone assignment specifically."""
+    milestones_found = re.findall(
+        r"(?:^|\n)\s*\d+\.\s+(.*?)(?=\n\d+\.|\Z)",
+        context.rules_text,
+        re.DOTALL,
+    )
+    assert any(re.search(r"milestone", m, re.IGNORECASE) for m in milestones_found), (
+        f"No rule mentions milestone assignment\n"
+        f"All rules:\n{context.rules_text[:500]}"
+    )

src/cleveragents/cli/commands/plan.py

@@ -1976,7 +1976,7 @@ def _recover_errored_execute_plan(
             current_plan.error_details = {
                 "strategy_decisions_json": strategy_json,
             }
-        service._commit_plan(current_plan)
+        service.save_plan(current_plan)
         current_plan = service.get_plan(plan_id)
         if current_plan is None:
             console.print(
@@ -2028,7 +2028,7 @@ def _recover_errored_execute_plan(
         "prior_error_type": error_type,
         "prior_error_details": json.dumps(prior_errors),
     }
-    service._commit_plan(current_plan)
+    service.save_plan(current_plan)
 
     # If reversion succeeded, re-run strategize with error findings
     if current_plan.phase == PlanPhase.STRATEGIZE:
@@ -2684,7 +2684,7 @@ def use_action(
                 execution_environment,
             ]
         ):
-            service._commit_plan(plan)
+            service.save_plan(plan)
 
         if fmt != OutputFormat.RICH.value:
             data = _plan_spec_dict(plan)
@@ -2816,7 +2816,7 @@ def execute_plan(
             pre = service.get_plan(plan_id)
             if pre is not None:
                 pre.execution_environment = execution_environment.lower()
-                service._commit_plan(pre)
+                service.save_plan(pre)
 
         # Create per-resource sandboxes (spec §19310) and build the
         # executor with the sandbox path.