Files
cleveragents-core/features/plan_executor_coverage.feature
hamza.khyari a740d9c15a fix: address PR review findings - narrow exception scope and fix test mocks
Address CoreRasurae's review comments:

1. Narrow exception scope in _build_decisions (line 846) to catch only
   json.JSONDecodeError and ValidationError instead of bare Exception.
   This prevents swallowing unintended errors like memory issues.

2. Update feature file section header and scenario titles from
   _run_execute_with_stub to _run_execute_with_actor for consistency.

3. Add spec to MagicMock in failing execute scenario (line 138) to
   match the pattern used in succeeding scenario (line 119).

Note: Master feature entry point - Behave auto-discovers all .feature
files, no explicit entry point needed. Tests run successfully.

ISSUES CLOSED: #10874
2026-04-30 14:56:33 +00:00

424 lines
20 KiB
Gherkin

Feature: PlanExecutor comprehensive coverage
As a developer
I want thorough test coverage of plan_executor.py
So that all branches and edge cases are exercised
# ------------------------------------------------------------------
# StrategizeStubActor._parse_steps parsing branches
# ------------------------------------------------------------------
Scenario: Parse steps with dash bullet prefix
Given a fresh StrategizeStubActor for cov2
When I cov2 parse steps from "- First item\n- Second item"
Then the cov2 parsed steps count should be "2"
And the cov2 parsed step at index "0" should be "First item"
And the cov2 parsed step at index "1" should be "Second item"
Scenario: Parse steps with asterisk bullet prefix
Given a fresh StrategizeStubActor for cov2
When I cov2 parse steps from "* Alpha\n* Beta"
Then the cov2 parsed steps count should be "2"
And the cov2 parsed step at index "0" should be "Alpha"
And the cov2 parsed step at index "1" should be "Beta"
Scenario: Parse steps with bullet character prefix
Given a fresh StrategizeStubActor for cov2
When I cov2 parse steps from "\u2022 Uno\n\u2022 Dos"
Then the cov2 parsed steps count should be "2"
And the cov2 parsed step at index "0" should be "Uno"
And the cov2 parsed step at index "1" should be "Dos"
Scenario: Parse steps with dot-numbered items
Given a fresh StrategizeStubActor for cov2
When I cov2 parse steps from "1. Do alpha\n2. Do beta\n10. Do gamma"
Then the cov2 parsed steps count should be "3"
And the cov2 parsed step at index "0" should be "Do alpha"
And the cov2 parsed step at index "1" should be "Do beta"
And the cov2 parsed step at index "2" should be "Do gamma"
Scenario: Parse steps with paren-numbered items
Given a fresh StrategizeStubActor for cov2
When I cov2 parse steps from "1) Task one\n2) Task two"
Then the cov2 parsed steps count should be "2"
And the cov2 parsed step at index "0" should be "Task one"
And the cov2 parsed step at index "1" should be "Task two"
Scenario: Parse steps skips blank lines in input
Given a fresh StrategizeStubActor for cov2
When I cov2 parse steps from "Line A\n\n\nLine B"
Then the cov2 parsed steps count should be "2"
And the cov2 parsed step at index "0" should be "Line A"
And the cov2 parsed step at index "1" should be "Line B"
Scenario: Parse steps with prefix that leaves empty content returns default
Given a fresh StrategizeStubActor for cov2
When I cov2 parse steps from "- "
Then the cov2 parsed steps should be the default objective
Scenario: Parse steps with empty string returns default
Given a fresh StrategizeStubActor for cov2
When I cov2 parse steps from an empty string
Then the cov2 parsed steps should be the default objective
Scenario: Parse steps with whitespace-only returns default
Given a fresh StrategizeStubActor for cov2
When I cov2 parse steps from " \n\t "
Then the cov2 parsed steps should be the default objective
Scenario: Parse steps with plain text lines
Given a fresh StrategizeStubActor for cov2
When I cov2 parse steps from "Just a sentence"
Then the cov2 parsed steps count should be "1"
And the cov2 parsed step at index "0" should be "Just a sentence"
# ------------------------------------------------------------------
# StrategizeStubActor.execute - validation and edge cases
# ------------------------------------------------------------------
Scenario: Strategize stub rejects empty plan_id
Given a fresh StrategizeStubActor for cov2
When I cov2 execute strategize with an empty plan_id
Then a cov2 ValidationError should be raised with message containing "plan_id"
Scenario: Strategize stub with None definition of done produces default decision
Given a fresh StrategizeStubActor for cov2
When I cov2 execute strategize with plan_id "PLAN01" and no definition
Then the cov2 strategize result should have "1" decisions
And the cov2 first decision text should be "Complete the plan objectives"
Scenario: Strategize stub with multi-line definition produces correct decisions
Given a fresh StrategizeStubActor for cov2
When I cov2 execute strategize with plan_id "PLAN02" and definition "Step A\nStep B\nStep C"
Then the cov2 strategize result should have "3" decisions
And the cov2 root decision should have no parent
And the cov2 non-root decisions should reference root as parent
Scenario: Strategize stub with invariants produces invariant records
Given a fresh StrategizeStubActor for cov2
When I cov2 execute strategize with invariants
Then the cov2 strategize result should have "2" invariant records
And each cov2 invariant record should have enforced set to true
Scenario: Strategize stub emits all stream events
Given a fresh StrategizeStubActor for cov2
And a cov2 stream event collector
When I cov2 execute strategize with stream callback
Then the cov2 stream events should include "strategize_started"
And the cov2 stream events should include "strategize_decisions"
And the cov2 stream events should include "strategize_complete"
Scenario: Strategize stub without stream callback does not error
Given a fresh StrategizeStubActor for cov2
When I cov2 execute strategize with plan_id "PLAN03" and definition "Do thing" without callback
Then the cov2 strategize result should have "1" decisions
# ------------------------------------------------------------------
# ExecuteStubActor - tool_runner, sandbox_root, stream, read_only
# ------------------------------------------------------------------
Scenario: Execute stub rejects empty plan_id
Given a fresh ExecuteStubActor for cov2
When I cov2 execute execute-stub with empty plan_id
Then a cov2 ValidationError should be raised with message containing "plan_id"
Scenario: Execute stub with tool_runner counts discovered tools
Given a fresh ExecuteStubActor for cov2
And a cov2 mock tool runner that discovers 3 tools
When I cov2 execute execute-stub with tool_runner and 2 decisions
Then the cov2 execute result tool_calls_count should be "6"
Scenario: Execute stub with sandbox_root populates sandbox_refs
Given a fresh ExecuteStubActor for cov2
When I cov2 execute execute-stub with sandbox_root "/tmp/sandbox"
Then the cov2 execute result sandbox_refs should contain "/tmp/sandbox"
Scenario: Execute stub without sandbox_root has empty sandbox_refs
Given a fresh ExecuteStubActor for cov2
When I cov2 execute execute-stub without sandbox_root
Then the cov2 execute result sandbox_refs should be empty
Scenario: Execute stub emits all stream events for each decision
Given a fresh ExecuteStubActor for cov2
And a cov2 stream event collector
When I cov2 execute execute-stub with 2 decisions and stream callback
Then the cov2 stream events should include "execute_started"
And the cov2 stream events should include "execute_step"
And the cov2 stream events should include "execute_complete"
Scenario: Execute stub with read_only passes to changeset capture
Given a fresh ExecuteStubActor for cov2
When I cov2 execute execute-stub with read_only true
Then the cov2 execute result should have a valid changeset_id
# ------------------------------------------------------------------
# PlanExecutor.__init__ - validation
# ------------------------------------------------------------------
Scenario: PlanExecutor rejects None lifecycle_service
When I cov2 construct PlanExecutor with None lifecycle
Then a cov2 ValidationError should be raised with message containing "lifecycle_service"
Scenario: PlanExecutor initialises with valid lifecycle
Given a cov2 mock lifecycle service
When I cov2 construct PlanExecutor with that lifecycle
Then the cov2 PlanExecutor should be initialised without error
# ------------------------------------------------------------------
# PlanExecutor properties
# ------------------------------------------------------------------
Scenario: has_runtime is False without execution context
Given a cov2 mock lifecycle service
And a cov2 PlanExecutor without execution context
Then the cov2 executor has_runtime should be "False"
Scenario: has_runtime is True with execution context
Given a cov2 mock lifecycle service
And a cov2 mock execution context
And a cov2 PlanExecutor with execution context
Then the cov2 executor has_runtime should be "True"
Scenario: changeset_store is None without execution context
Given a cov2 mock lifecycle service
And a cov2 PlanExecutor without execution context
Then the cov2 executor changeset_store should be None
Scenario: changeset_store returns store from execution context
Given a cov2 mock lifecycle service
And a cov2 mock execution context with changeset store
And a cov2 PlanExecutor with that execution context
Then the cov2 executor changeset_store should be the mock store
Scenario: execution_context property returns None without context
Given a cov2 mock lifecycle service
And a cov2 PlanExecutor without execution context
Then the cov2 executor execution_context should be None
Scenario: execution_context property returns context when set
Given a cov2 mock lifecycle service
And a cov2 mock execution context
And a cov2 PlanExecutor with execution context
Then the cov2 executor execution_context should not be None
# ------------------------------------------------------------------
# PlanExecutor.run_strategize - guards and happy path
# ------------------------------------------------------------------
Scenario: run_strategize rejects empty plan_id
Given a cov2 mock lifecycle service
And a cov2 PlanExecutor without execution context
When I cov2 call run_strategize with empty plan_id
Then a cov2 ValidationError should be raised with message containing "plan_id"
Scenario: run_strategize rejects plan not in Strategize phase
Given a cov2 mock lifecycle service
And a cov2 plan in Execute phase
And a cov2 PlanExecutor without execution context
When I cov2 call run_strategize
Then a cov2 PlanError should be raised containing "not in Strategize phase"
Scenario: run_strategize happy path succeeds
Given a cov2 mock lifecycle service
And a cov2 plan in Strategize phase with definition "Build feature\nAdd tests"
And a cov2 PlanExecutor without execution context
When I cov2 call run_strategize
Then the cov2 strategize run result should be a StrategizeResult
And the cov2 lifecycle should have called start_strategize
And the cov2 lifecycle should have called complete_strategize
And the cov2 lifecycle should have called _commit_plan
Scenario: run_strategize sets decision_root_id on execution context
Given a cov2 mock lifecycle service
And a cov2 mock execution context
And a cov2 plan in Strategize phase with definition "Do work"
And a cov2 PlanExecutor with execution context
When I cov2 call run_strategize
Then the cov2 execution context decision_root_id should be set
# ------------------------------------------------------------------
# PlanExecutor.run_strategize - exception path
# ------------------------------------------------------------------
Scenario: run_strategize calls fail_strategize on exception
Given a cov2 mock lifecycle service
And a cov2 plan in Strategize phase with definition "Do work"
And a cov2 PlanExecutor with failing strategize actor
When I cov2 call run_strategize expecting exception
Then a cov2 RuntimeError should have been raised
And the cov2 lifecycle should have called fail_strategize
And the cov2 plan error_details should contain exception_type
Scenario: run_strategize records error via error_recovery service
Given a cov2 mock lifecycle service
And a cov2 plan in Strategize phase with definition "Do work"
And a cov2 mock error recovery service
And a cov2 PlanExecutor with error recovery and failing strategize actor
When I cov2 call run_strategize expecting exception
Then the cov2 error recovery should have recorded a strategize error
# ------------------------------------------------------------------
# PlanExecutor._guard_execute - all branches
# ------------------------------------------------------------------
Scenario: guard_execute rejects plan not in Execute phase
Given a cov2 mock lifecycle service
And a cov2 plan in Strategize phase with definition "Do work"
And a cov2 PlanExecutor without execution context
When I cov2 call guard_execute
Then a cov2 PlanError should be raised containing "not in Execute phase"
Scenario: guard_execute rejects plan not in queued state
Given a cov2 mock lifecycle service
And a cov2 plan in Execute phase but Processing state
And a cov2 PlanExecutor without execution context
When I cov2 call guard_execute
Then a cov2 PlanError should be raised containing "not queued"
Scenario: guard_execute rejects plan with no decision_root_id
Given a cov2 mock lifecycle service
And a cov2 plan in Execute-Queued state without decision root
And a cov2 PlanExecutor without execution context
When I cov2 call guard_execute
Then a cov2 PlanError should be raised containing "no decision tree"
# ------------------------------------------------------------------
# PlanExecutor.run_execute - routing
# ------------------------------------------------------------------
Scenario: run_execute rejects empty plan_id
Given a cov2 mock lifecycle service
And a cov2 PlanExecutor without execution context
When I cov2 call run_execute with empty plan_id
Then a cov2 ValidationError should be raised with message containing "plan_id"
Scenario: run_execute routes to stub when no execution context
Given a cov2 mock lifecycle service
And a cov2 plan in Execute-Queued state with decision root and definition
And a cov2 PlanExecutor without execution context
When I cov2 call run_execute
Then the cov2 execute run result should be an ExecuteResult
And the cov2 lifecycle should have called complete_execute
# ------------------------------------------------------------------
# PlanExecutor._run_execute_with_runtime
# ------------------------------------------------------------------
Scenario: run_execute routes to runtime when execution context is set
Given a cov2 mock lifecycle service
And a cov2 mock execution context
And a cov2 plan in Execute-Queued state with decision root and definition
And a cov2 PlanExecutor with execution context and tool runner
When I cov2 call run_execute
Then the cov2 execute run result should be a RuntimeExecuteResult
And the cov2 lifecycle should have called complete_execute
Scenario: run_execute runtime mode raises PlanError when no ToolRunner
Given a cov2 mock lifecycle service
And a cov2 mock execution context
And a cov2 plan in Execute-Queued state with decision root and definition
And a cov2 PlanExecutor with execution context but no tool runner
When I cov2 call run_execute expecting exception
Then a cov2 PlanError should be raised containing "ToolRunner"
Scenario: run_execute runtime mode calls fail_execute on exception
Given a cov2 mock lifecycle service
And a cov2 mock execution context
And a cov2 plan in Execute-Queued state with decision root and definition
And a cov2 PlanExecutor with execution context and tool runner and failing runtime
When I cov2 call run_execute expecting exception
Then a cov2 RuntimeError should have been raised
And the cov2 lifecycle should have called fail_execute
# ------------------------------------------------------------------
# PlanExecutor._run_execute_with_actor - error recovery / retry
# ------------------------------------------------------------------
Scenario: run_execute actor calls fail_execute on exception without recovery
Given a cov2 mock lifecycle service
And a cov2 plan in Execute-Queued state with decision root and definition
And a cov2 PlanExecutor with failing execute actor and no recovery
When I cov2 call run_execute expecting exception
Then a cov2 RuntimeError should have been raised
And the cov2 lifecycle should have called fail_execute
Scenario: run_execute actor retries once with error recovery then succeeds
Given a cov2 mock lifecycle service
And a cov2 plan in Execute-Queued state with decision root and definition
And a cov2 PlanExecutor with execute actor that fails once then succeeds
When I cov2 call run_execute
Then the cov2 execute run result should be an ExecuteResult
And the cov2 error recovery should have recorded an execute error
Scenario: run_execute actor exhausts retries with error recovery and fails
Given a cov2 mock lifecycle service
And a cov2 plan in Execute-Queued state with decision root and definition
And a cov2 PlanExecutor with always-failing execute actor and exhausted retries
When I cov2 call run_execute expecting exception
Then a cov2 RuntimeError should have been raised
And the cov2 lifecycle should have called fail_execute
And the cov2 error recovery should have recorded an execute error
Scenario: run_execute actor with error recovery records error and no retry
Given a cov2 mock lifecycle service
And a cov2 plan in Execute-Queued state with decision root and definition
And a cov2 PlanExecutor with failing execute actor and recovery denying retry
When I cov2 call run_execute expecting exception
Then a cov2 RuntimeError should have been raised
And the cov2 error recovery should have recorded an execute error
And the cov2 lifecycle should have called fail_execute
# ------------------------------------------------------------------
# PlanExecutor._build_decisions
# ------------------------------------------------------------------
Scenario: _build_decisions creates decisions from plan definition_of_done
Given a cov2 mock lifecycle service
And a cov2 plan in Execute-Queued state with decision root and definition
And a cov2 PlanExecutor without execution context
When I cov2 call build_decisions
Then the cov2 built decisions should have "2" entries
And the cov2 first built decision should use the plan decision_root_id
And the cov2 second built decision should have root as parent
# ------------------------------------------------------------------
# Result model edge cases
# ------------------------------------------------------------------
Scenario: StrategyDecision model validates sequence is non-negative
When I cov2 create a StrategyDecision with negative sequence
Then a cov2 validation error should be raised for the model
Scenario: ExecuteResult defaults are correct
When I cov2 create an ExecuteResult with minimal fields
Then the cov2 ExecuteResult tool_calls_count should default to "0"
And the cov2 ExecuteResult sandbox_refs should default to empty list
# ------------------------------------------------------------------
# _build_decisions: stored strategy_decisions_json path (M6 wiring)
# ------------------------------------------------------------------
Scenario: _build_decisions uses stored strategy_decisions_json when present
Given a cov2 mock lifecycle service
And a cov2 plan with stored strategy_decisions_json in error_details
And a cov2 PlanExecutor without execution context
When I cov2 call build_decisions
Then the cov2 built decisions should match the stored JSON decisions
Scenario: _build_decisions falls back to definition_of_done on corrupt JSON
Given a cov2 mock lifecycle service
And a cov2 plan with corrupt strategy_decisions_json in error_details
And a cov2 PlanExecutor without execution context
When I cov2 call build_decisions
Then the cov2 built decisions should have "2" entries
Scenario: run_strategize passes resources from project_links to actor
Given a cov2 mock lifecycle service
And a cov2 plan in Strategize-Queued state with project links
And a cov2 PlanExecutor with a capturing strategize actor
When I cov2 run strategize
Then the cov2 strategize actor received resources from project links
Scenario: StrategizeStubActor.execute accepts extra keyword arguments
When I cov2 call StrategizeStubActor execute with extra kwargs
Then the cov2 stub result should have decisions