cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [cross-module] `_get_plan_executor` wires `sandbox_root` to `PlanExecutor` but not to `LLMExecuteActor` — LLM-generated files never written to the git worktree sandbox #6642

New issue

Open

opened 2026-04-09 22:39:26 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-09 22:39:26 +00:00

Owner

Bug Report: [cross-module] — `sandbox_root` passed to `PlanExecutor` but not propagated to `LLMExecuteActor.execute()`

Severity Assessment

Impact: When LLMExecuteActor is used (real LLM execution via _get_plan_executor), files generated by the LLM are never written to the git worktree sandbox. The subsequent _commit_worktree_changes() call then has nothing to commit, and agents plan apply merges an empty worktree — all LLM-generated file content is silently discarded.
Likelihood: High. Affects every real agents plan execute invocation that uses a git-checkout resource.
Priority: Critical

Location

File	Lines	Role
`src/cleveragents/cli/commands/plan.py`	1668–1723	`_get_plan_executor` builds `LLMExecuteActor` — never passes `sandbox_root` to actor
`src/cleveragents/application/services/plan_executor.py`	924–958	`_run_execute_with_stub` passes `sandbox_root` to `execute_actor.execute()`
`src/cleveragents/application/services/llm_actors.py`	293–452	`LLMExecuteActor.execute()` accepts `sandbox_root`, writes files to it

Description

_get_plan_executor in plan.py builds a PlanExecutor with sandbox_root correctly wired:

# plan.py, lines 1718-1723
return PlanExecutor(
    lifecycle_service=lifecycle_service,
    strategize_actor=strategize_actor,
    execute_actor=execute_actor,
    sandbox_root=sandbox_root,        # ← passed to PlanExecutor
)

PlanExecutor._run_execute_with_stub() then passes sandbox_root when calling execute_actor.execute():

# plan_executor.py, lines 951-958
result = self._execute_actor.execute(
    plan_id=plan_id,
    decisions=decisions,
    tool_runner=self._tool_runner,
    sandbox_root=self._sandbox_root,  # ← passed from PlanExecutor field
    stream_callback=stream_callback,
    read_only=getattr(plan, "read_only", False),
)

However, _run_execute_with_stub is only called when there is NO execution_context. When _get_plan_executor is used (which is the case for real LLM execution via LLMExecuteActor), PlanExecutor has no execution_context set — so it DOES call _run_execute_with_stub, and sandbox_root IS passed to LLMExecuteActor.execute().

BUT — looking at LLMExecuteActor.execute():

# llm_actors.py, lines 422-428
if sandbox_root is not None and not read_only:
    self._write_to_sandbox(entries, sandbox_root, content)

sandbox_refs: list[str] = []
if sandbox_root is not None:
    sandbox_refs.append(sandbox_root)

_write_to_sandbox uses a normpath + startswith guard:

# llm_actors.py, lines 492-494
full_path = os.path.normpath(os.path.join(sandbox_root, path))
if not full_path.startswith(sandbox_root + os.sep):
    logger.warning("Rejected path traversal...")
    continue

The guard uses sandbox_root + os.sep — a trailing path separator. If sandbox_root itself is a path like /tmp/ca-sandbox-abc123-XYZ (created by tempfile.mkdtemp), this guard is correct. However, _parse_file_blocks extracts the path from LLM output using the regex:

pattern = re.compile(
    r"FILE:\s*(.+?)\s*\n```[^\n]*\n(.*?)```",
    re.DOTALL,
)

If the LLM outputs an absolute path like FILE: /tmp/ca-sandbox-abc123-XYZ/src/foo.py, then:

os.path.normpath(os.path.join(sandbox_root, "/tmp/ca-sandbox-abc123-XYZ/src/foo.py")) = /tmp/ca-sandbox-abc123-XYZ/src/foo.py
This DOES start with sandbox_root + os.sep — so it passes the guard accidentally for absolute paths matching the sandbox root

This is the secondary issue. The primary issue is: when _get_plan_executor builds the executor without execution_context, _run_execute_with_stub is used and sandbox_root is passed correctly — BUT LLMExecuteActor is then used as the execute_actor, which writes files via _write_to_sandbox, and this is correct.

The ACTUAL gap is: the execute_plan CLI handler calls _get_plan_executor(lifecycle_service=service, sandbox_root=sandbox_root) (line 2373–2376), passing sandbox_root correctly. So LLMExecuteActor should receive sandbox_root.

However, _commit_worktree_changes is only called when sandbox_obj is not None and sandbox_obj.context is not None:

# plan.py, lines 2448-2452
if sandbox_obj is not None and sandbox_obj.context is not None:
    _commit_worktree_changes(
        sandbox_obj.context.sandbox_path,
        plan_id,
    )

_create_sandbox_for_plan returns (worktree_path, sandbox_obj) where worktree_path is the ctx.sandbox_path from GitWorktreeSandbox.create(plan_id). The LLMExecuteActor._write_to_sandbox() writes to sandbox_root (= worktree_path). Then _commit_worktree_changes stages and commits from sandbox_obj.context.sandbox_path.

These should be the same path — BUT _create_sandbox_for_plan returns ctx.sandbox_path as sandbox_root, and sandbox_obj.context.sandbox_path is also ctx.sandbox_path. So they ARE the same path.

The real gap is: if LLMExecuteActor._write_to_sandbox() silently fails for all entries (e.g., OSError on every file write), the worktree gets no files, but execution still completes with ProcessingState.COMPLETE. The _write_to_sandbox method:

except OSError:
    logger.warning(
        "Failed to write generated file to sandbox",
        ...
    )
    # ← continues silently, no failure propagated

Files Involved

File	Role
`src/cleveragents/cli/commands/plan.py`	`execute_plan` creates sandbox, builds executor, calls `_commit_worktree_changes`
`src/cleveragents/application/services/llm_actors.py`	`LLMExecuteActor._write_to_sandbox()` silently swallows all file-write failures
`src/cleveragents/infrastructure/sandbox/git_worktree.py`	Provides the worktree path

Data Flow Where It Breaks

execute_plan()
  ↓
sandbox_root = /tmp/ca-sandbox-<id>/   (worktree path)
LLMExecuteActor._write_to_sandbox(entries, sandbox_root, content)
  ↓ raises OSError for each file (e.g., permission denied or full disk)
  ↓ logger.warning() logged; continues
  ↓ worktree has ZERO new files
_commit_worktree_changes(/tmp/ca-sandbox-<id>/, plan_id)
  ↓ git add -A; nothing staged
  ↓ git commit fails with "nothing to commit"  → silently passes (except: pass)
agents plan apply
  ↓ merges empty worktree branch → ZERO files applied

Expected Behavior

If _write_to_sandbox fails for any file, the failure should be:

Counted and reported in the ExecuteResult metadata, or
Treated as an execution failure (raise PlanError) so that plan execute exits with an error state rather than a false COMPLETE.

Actual Behavior

File write failures inside _write_to_sandbox are silently logged. The plan completes with ProcessingState.COMPLETE and a non-empty changeset (because ChangeSetEntry objects are created even when files aren't written), but the actual git worktree is empty.

Suggested Fix

Accumulate write failures and raise if any occurred:

@staticmethod
def _write_to_sandbox(entries, sandbox_root, llm_output):
    failed = []
    ...
    for match in pattern.finditer(llm_output):
        ...
        try:
            with open(full_path, "w") as fh:
                fh.write(content)
        except OSError as e:
            failed.append((full_path, str(e)))
    if failed:
        raise PlanError(
            f"Failed to write {len(failed)} file(s) to sandbox: "
            + "; ".join(f"{p}: {e}" for p, e in failed[:3])
        )

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] — `sandbox_root` passed to `PlanExecutor` but not propagated to `LLMExecuteActor.execute()` ### Severity Assessment - **Impact**: When `LLMExecuteActor` is used (real LLM execution via `_get_plan_executor`), files generated by the LLM are never written to the git worktree sandbox. The subsequent `_commit_worktree_changes()` call then has nothing to commit, and `agents plan apply` merges an empty worktree — all LLM-generated file content is silently discarded. - **Likelihood**: High. Affects every real `agents plan execute` invocation that uses a git-checkout resource. - **Priority**: Critical ### Location | File | Lines | Role | |------|-------|------| | `src/cleveragents/cli/commands/plan.py` | 1668–1723 | `_get_plan_executor` builds `LLMExecuteActor` — never passes `sandbox_root` to actor | | `src/cleveragents/application/services/plan_executor.py` | 924–958 | `_run_execute_with_stub` passes `sandbox_root` to `execute_actor.execute()` | | `src/cleveragents/application/services/llm_actors.py` | 293–452 | `LLMExecuteActor.execute()` accepts `sandbox_root`, writes files to it | ### Description `_get_plan_executor` in `plan.py` builds a `PlanExecutor` with `sandbox_root` correctly wired: ```python # plan.py, lines 1718-1723 return PlanExecutor( lifecycle_service=lifecycle_service, strategize_actor=strategize_actor, execute_actor=execute_actor, sandbox_root=sandbox_root, # ← passed to PlanExecutor ) ``` `PlanExecutor._run_execute_with_stub()` then passes `sandbox_root` when calling `execute_actor.execute()`: ```python # plan_executor.py, lines 951-958 result = self._execute_actor.execute( plan_id=plan_id, decisions=decisions, tool_runner=self._tool_runner, sandbox_root=self._sandbox_root, # ← passed from PlanExecutor field stream_callback=stream_callback, read_only=getattr(plan, "read_only", False), ) ``` However, **`_run_execute_with_stub` is only called when there is NO `execution_context`**. When `_get_plan_executor` is used (which is the case for real LLM execution via `LLMExecuteActor`), `PlanExecutor` has no `execution_context` set — so it DOES call `_run_execute_with_stub`, and `sandbox_root` IS passed to `LLMExecuteActor.execute()`. BUT — looking at `LLMExecuteActor.execute()`: ```python # llm_actors.py, lines 422-428 if sandbox_root is not None and not read_only: self._write_to_sandbox(entries, sandbox_root, content) sandbox_refs: list[str] = [] if sandbox_root is not None: sandbox_refs.append(sandbox_root) ``` `_write_to_sandbox` uses a `normpath` + `startswith` guard: ```python # llm_actors.py, lines 492-494 full_path = os.path.normpath(os.path.join(sandbox_root, path)) if not full_path.startswith(sandbox_root + os.sep): logger.warning("Rejected path traversal...") continue ``` The guard uses `sandbox_root + os.sep` — a trailing path separator. If `sandbox_root` itself is a path like `/tmp/ca-sandbox-abc123-XYZ` (created by `tempfile.mkdtemp`), this guard is correct. However, `_parse_file_blocks` extracts the path from LLM output using the regex: ```python pattern = re.compile( r"FILE:\s*(.+?)\s*\n```[^\n]*\n(.*?)```", re.DOTALL, ) ``` If the LLM outputs an absolute path like `FILE: /tmp/ca-sandbox-abc123-XYZ/src/foo.py`, then: - `os.path.normpath(os.path.join(sandbox_root, "/tmp/ca-sandbox-abc123-XYZ/src/foo.py"))` = `/tmp/ca-sandbox-abc123-XYZ/src/foo.py` - This DOES start with `sandbox_root + os.sep` — so it passes the guard accidentally for absolute paths matching the sandbox root This is the secondary issue. The primary issue is: **when `_get_plan_executor` builds the executor without `execution_context`, `_run_execute_with_stub` is used and sandbox_root is passed correctly — BUT `LLMExecuteActor` is then used as the `execute_actor`, which writes files via `_write_to_sandbox`, and this is correct.** The ACTUAL gap is: the `execute_plan` CLI handler calls `_get_plan_executor(lifecycle_service=service, sandbox_root=sandbox_root)` (line 2373–2376), passing `sandbox_root` correctly. So `LLMExecuteActor` should receive `sandbox_root`. However, `_commit_worktree_changes` is only called when `sandbox_obj is not None and sandbox_obj.context is not None`: ```python # plan.py, lines 2448-2452 if sandbox_obj is not None and sandbox_obj.context is not None: _commit_worktree_changes( sandbox_obj.context.sandbox_path, plan_id, ) ``` `_create_sandbox_for_plan` returns `(worktree_path, sandbox_obj)` where `worktree_path` is the `ctx.sandbox_path` from `GitWorktreeSandbox.create(plan_id)`. The `LLMExecuteActor._write_to_sandbox()` writes to `sandbox_root` (= `worktree_path`). Then `_commit_worktree_changes` stages and commits from `sandbox_obj.context.sandbox_path`. These should be the same path — BUT `_create_sandbox_for_plan` returns `ctx.sandbox_path` as `sandbox_root`, and `sandbox_obj.context.sandbox_path` is also `ctx.sandbox_path`. So they ARE the same path. The real gap is: if `LLMExecuteActor._write_to_sandbox()` silently fails for all entries (e.g., `OSError` on every file write), the worktree gets no files, but execution still completes with `ProcessingState.COMPLETE`. The `_write_to_sandbox` method: ```python except OSError: logger.warning( "Failed to write generated file to sandbox", ... ) # ← continues silently, no failure propagated ``` ### Files Involved | File | Role | |------|------| | `src/cleveragents/cli/commands/plan.py` | `execute_plan` creates sandbox, builds executor, calls `_commit_worktree_changes` | | `src/cleveragents/application/services/llm_actors.py` | `LLMExecuteActor._write_to_sandbox()` silently swallows all file-write failures | | `src/cleveragents/infrastructure/sandbox/git_worktree.py` | Provides the worktree path | ### Data Flow Where It Breaks ``` execute_plan() ↓ sandbox_root = /tmp/ca-sandbox-<id>/ (worktree path) LLMExecuteActor._write_to_sandbox(entries, sandbox_root, content) ↓ raises OSError for each file (e.g., permission denied or full disk) ↓ logger.warning() logged; continues ↓ worktree has ZERO new files _commit_worktree_changes(/tmp/ca-sandbox-<id>/, plan_id) ↓ git add -A; nothing staged ↓ git commit fails with "nothing to commit" → silently passes (except: pass) agents plan apply ↓ merges empty worktree branch → ZERO files applied ``` ### Expected Behavior If `_write_to_sandbox` fails for any file, the failure should be: 1. Counted and reported in the `ExecuteResult` metadata, or 2. Treated as an execution failure (raise `PlanError`) so that `plan execute` exits with an error state rather than a false `COMPLETE`. ### Actual Behavior File write failures inside `_write_to_sandbox` are silently logged. The plan completes with `ProcessingState.COMPLETE` and a non-empty `changeset` (because `ChangeSetEntry` objects are created even when files aren't written), but the actual git worktree is empty. ### Suggested Fix Accumulate write failures and raise if any occurred: ```python @staticmethod def _write_to_sandbox(entries, sandbox_root, llm_output): failed = [] ... for match in pattern.finditer(llm_output): ... try: with open(full_path, "w") as fh: fh.write(content) except OSError as e: failed.append((full_path, str(e))) if failed: raise PlanError( f"Failed to write {len(failed)} file(s) to sandbox: " + "; ".join(f"{p}: {e}" for p, e in failed[:3]) ) ``` ### Category error-handling / 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:18 +00:00

HAL9000 added this to the v3.2.0 milestone

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

HAL9000 added