BUG-HUNT: [cross-module] `use_action` double-persists plan overrides — `service.save_plan()` then `service._commit_plan()` both called for same overrides, risking stale-read race in multi-process CLI invocations #6646

New issue

Open

opened 2026-04-09 22:41:31 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-09 22:41:31 +00:00

Owner

Bug Report: [cross-module] — Double-persist of plan overrides in `use_action` CLI handler

Severity Assessment

Impact: When CLI overrides (actor, automation_profile, execution environment) are applied after service.use_action(), the plan is persisted twice via two different code paths: service.save_plan(plan) (line 2223) and service._commit_plan(plan) (line 2237). If the two persists race (e.g., in async or multi-process environments) or if _commit_plan is a lower-level write that bypasses save_plan business rules, one write may overwrite the other with stale data, or business rules may be applied inconsistently.
Likelihood: Low in single-process CLI, Medium in multi-process/automation contexts.
Priority: Medium

Location

File: src/cleveragents/cli/commands/plan.py
Function: use_action() (the @app.command("use") handler)
Lines: 2220–2237

Description

After applying CLI overrides to the plan, the use_action command calls two separate persist methods:

# plan.py, lines 2220-2237

# Re-persist the plan so CLI overrides survive across sessions.
# Only call save_plan when overrides were actually applied.
if has_overrides:
    service.save_plan(plan)     # ← FIRST persist (public API)

# Persist any post-creation overrides (profile, actors, env) so
# that subsequent CLI invocations (separate processes) see them.
if any(
    [
        automation_profile,
        strategy_actor,
        execution_actor,
        estimation_actor,
        invariant_actor,
        execution_environment,
    ]
):
    service._commit_plan(plan)  # ← SECOND persist (private, bypasses save logic)

The problem is that both conditions can be true simultaneously for the same plan state. When a user runs:

agents plan use local/my-action my-project --strategy-actor openai/gpt-4

has_overrides = True → service.save_plan(plan) is called (line 2223)
strategy_actor is set → service._commit_plan(plan) is also called (line 2237)

These are two separate database writes to the same plan record with the same in-memory plan object. The sequence:

If save_plan has write-through cache semantics, the second _commit_plan (which bypasses save logic) may clobber any side-effects of save_plan (e.g., domain events, validation, post-save hooks).
_commit_plan is documented as a low-level internal method (prefixed _), bypassing any business rules in save_plan.

Files Involved

File	Role
`src/cleveragents/cli/commands/plan.py`	Calls both `save_plan` and `_commit_plan` for the same plan mutation
`src/cleveragents/application/services/plan_lifecycle_service.py`	Defines both `save_plan` (public) and `_commit_plan` (private/internal)

Data Flow Where It Breaks

use_action(strategy_actor="openai/gpt-4", ...)
  ↓
plan.strategy_actor = "openai/gpt-4"
has_overrides = True
  ↓
service.save_plan(plan)    # first write — public API, may fire domain events
  ↓
# (potential: save_plan emits PlanUpdated event, observers process it)
  ↓
service._commit_plan(plan) # second write — private API, bypasses events/hooks
  ↓
# (potential: second write silently re-writes over first, or partial state seen
#  by concurrent readers between the two writes)

Expected Behavior

There should be exactly ONE persist call for a given set of mutations. Either save_plan handles all override persistence, or _commit_plan is used exclusively — but not both for the same mutation.

Actual Behavior

Two database writes are issued for the same plan state. In the best case this is a redundant no-op. In the worst case it creates a TOCTOU race between the two writes or bypasses business-rule enforcement.

Suggested Fix

Remove the redundant _commit_plan call. save_plan should be the only write:

# plan.py
if has_overrides:
    service.save_plan(plan)   # single authoritative persist — remove the _commit_plan block

Or, if _commit_plan is necessary for some edge case, gate it on not has_overrides to ensure they are mutually exclusive:

if has_overrides:
    service.save_plan(plan)
elif any([automation_profile, strategy_actor, ...]):
    service._commit_plan(plan)

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_, and @tdd_expected_fail to prove the bug exists before fixing it.

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: bug-hunter

## Bug Report: [cross-module] — Double-persist of plan overrides in `use_action` CLI handler ### Severity Assessment - **Impact**: When CLI overrides (actor, automation_profile, execution environment) are applied after `service.use_action()`, the plan is persisted **twice** via two different code paths: `service.save_plan(plan)` (line 2223) and `service._commit_plan(plan)` (line 2237). If the two persists race (e.g., in async or multi-process environments) or if `_commit_plan` is a lower-level write that bypasses `save_plan` business rules, one write may overwrite the other with stale data, or business rules may be applied inconsistently. - **Likelihood**: Low in single-process CLI, Medium in multi-process/automation contexts. - **Priority**: Medium ### Location - **File**: `src/cleveragents/cli/commands/plan.py` - **Function**: `use_action()` (the `@app.command("use")` handler) - **Lines**: 2220–2237 ### Description After applying CLI overrides to the plan, the `use_action` command calls two separate persist methods: ```python # plan.py, lines 2220-2237 # Re-persist the plan so CLI overrides survive across sessions. # Only call save_plan when overrides were actually applied. if has_overrides: service.save_plan(plan) # ← FIRST persist (public API) # Persist any post-creation overrides (profile, actors, env) so # that subsequent CLI invocations (separate processes) see them. if any( [ automation_profile, strategy_actor, execution_actor, estimation_actor, invariant_actor, execution_environment, ] ): service._commit_plan(plan) # ← SECOND persist (private, bypasses save logic) ``` The problem is that both conditions can be true simultaneously for the same plan state. When a user runs: ``` agents plan use local/my-action my-project --strategy-actor openai/gpt-4 ``` 1. `has_overrides = True` → `service.save_plan(plan)` is called (line 2223) 2. `strategy_actor` is set → `service._commit_plan(plan)` is also called (line 2237) These are two separate database writes to the same plan record with the same in-memory `plan` object. The sequence: - If `save_plan` has write-through cache semantics, the second `_commit_plan` (which bypasses save logic) may clobber any side-effects of `save_plan` (e.g., domain events, validation, post-save hooks). - `_commit_plan` is documented as a low-level internal method (prefixed `_`), bypassing any business rules in `save_plan`. ### Files Involved | File | Role | |------|------| | `src/cleveragents/cli/commands/plan.py` | Calls both `save_plan` and `_commit_plan` for the same plan mutation | | `src/cleveragents/application/services/plan_lifecycle_service.py` | Defines both `save_plan` (public) and `_commit_plan` (private/internal) | ### Data Flow Where It Breaks ``` use_action(strategy_actor="openai/gpt-4", ...) ↓ plan.strategy_actor = "openai/gpt-4" has_overrides = True ↓ service.save_plan(plan) # first write — public API, may fire domain events ↓ # (potential: save_plan emits PlanUpdated event, observers process it) ↓ service._commit_plan(plan) # second write — private API, bypasses events/hooks ↓ # (potential: second write silently re-writes over first, or partial state seen # by concurrent readers between the two writes) ``` ### Expected Behavior There should be exactly ONE persist call for a given set of mutations. Either `save_plan` handles all override persistence, or `_commit_plan` is used exclusively — but not both for the same mutation. ### Actual Behavior Two database writes are issued for the same plan state. In the best case this is a redundant no-op. In the worst case it creates a TOCTOU race between the two writes or bypasses business-rule enforcement. ### Suggested Fix Remove the redundant `_commit_plan` call. `save_plan` should be the only write: ```python # plan.py if has_overrides: service.save_plan(plan) # single authoritative persist — remove the _commit_plan block ``` Or, if `_commit_plan` is necessary for some edge case, gate it on `not has_overrides` to ensure they are mutually exclusive: ```python if has_overrides: service.save_plan(plan) elif any([automation_profile, strategy_actor, ...]): service._commit_plan(plan) ``` ### Category consistency / cross-module / data-flow ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: bug-hunter

HAL9000 added the

labels

2026-04-09 22:45:15 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-09 22:47:13 +00:00

HAL9000 added