cleveragents/cleveragents-core

Fork 3

UAT: `llm_actors.py` swallows `OSError` on sandbox file write — file system errors silently ignored, no `FileSystemError` raised #5704

New issue

Open

opened 2026-04-09 08:42:19 +00:00 by HAL9000 · 2 comments

HAL9000 commented

2026-04-09 08:42:19 +00:00

Owner

Summary

src/cleveragents/application/services/llm_actors.py catches OSError when writing generated files to the sandbox but only logs a warning without re-raising. This means file system errors (permission denied, disk full, read-only filesystem) are silently swallowed and the plan continues as if the file was written successfully.

Code Location

# llm_actors.py lines 453-461
try:
    with open(full_path, "w") as fh:
        fh.write(content)
except OSError:
    logger.warning(
        "Failed to write generated file to sandbox",
        path=full_path,
        exc_info=True,
    )
    # <-- No re-raise! Execution continues silently

Expected Behavior (per CONTRIBUTING.md and spec)

Per CONTRIBUTING.md:

Only catch exceptions when you can meaningfully handle them (e.g., retry logic, resource cleanup, adding context). Otherwise, let them propagate.

The spec defines FileSystemError in core/exceptions.py specifically for file system operation failures. When a file cannot be written to the sandbox, this is a critical error that should:

Raise FileSystemError with the path and original exception as context
Cause the plan execution to fail with a clear error message
Allow the retry infrastructure to potentially retry the operation

Actual Behavior

When OSError occurs during sandbox file write:

A WARNING is logged (only visible with -vvv or higher verbosity)
Execution continues as if the file was written
The plan may produce incorrect results because some files are missing from the sandbox
The user sees no error and may believe the plan succeeded

Specific Scenarios Affected

Permission denied: Sandbox directory has wrong permissions
Disk full: Host system has no space left
Read-only filesystem: Sandbox mounted as read-only
Path too long: Generated file path exceeds OS limits

Fix Required

try:
    with open(full_path, "w") as fh:
        fh.write(content)
except OSError as exc:
    raise FileSystemError(
        f"Failed to write generated file to sandbox: {exc}",
        path=full_path,
    ) from exc

Or if partial failure is acceptable (some files may not be written), at minimum the error should be tracked and surfaced in the plan's error details:

except OSError as exc:
    logger.warning("Failed to write generated file to sandbox", path=full_path, exc_info=True)
    failed_files.append(full_path)
    # ... and later raise if any files failed

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: uat-tester

## Summary `src/cleveragents/application/services/llm_actors.py` catches `OSError` when writing generated files to the sandbox but only logs a warning without re-raising. This means file system errors (permission denied, disk full, read-only filesystem) are silently swallowed and the plan continues as if the file was written successfully. ## Code Location ```python # llm_actors.py lines 453-461 try: with open(full_path, "w") as fh: fh.write(content) except OSError: logger.warning( "Failed to write generated file to sandbox", path=full_path, exc_info=True, ) # <-- No re-raise! Execution continues silently ``` ## Expected Behavior (per CONTRIBUTING.md and spec) Per CONTRIBUTING.md: > **Only catch exceptions when you can meaningfully handle them** (e.g., retry logic, resource cleanup, adding context). Otherwise, let them propagate. The spec defines `FileSystemError` in `core/exceptions.py` specifically for file system operation failures. When a file cannot be written to the sandbox, this is a critical error that should: 1. Raise `FileSystemError` with the path and original exception as context 2. Cause the plan execution to fail with a clear error message 3. Allow the retry infrastructure to potentially retry the operation ## Actual Behavior When `OSError` occurs during sandbox file write: 1. A WARNING is logged (only visible with `-vvv` or higher verbosity) 2. Execution continues as if the file was written 3. The plan may produce incorrect results because some files are missing from the sandbox 4. The user sees no error and may believe the plan succeeded ## Specific Scenarios Affected - **Permission denied**: Sandbox directory has wrong permissions - **Disk full**: Host system has no space left - **Read-only filesystem**: Sandbox mounted as read-only - **Path too long**: Generated file path exceeds OS limits ## Fix Required ```python try: with open(full_path, "w") as fh: fh.write(content) except OSError as exc: raise FileSystemError( f"Failed to write generated file to sandbox: {exc}", path=full_path, ) from exc ``` Or if partial failure is acceptable (some files may not be written), at minimum the error should be tracked and surfaced in the plan's error details: ```python except OSError as exc: logger.warning("Failed to write generated file to sandbox", path=full_path, exc_info=True) failed_files.append(full_path) # ... and later raise if any files failed ``` --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: uat-tester

HAL9000 added the

labels

2026-04-09 08:43:59 +00:00

HAL9000 added a new dependency

2026-04-09 08:53:11 +00:00

#5502 EPIC: Actor Execution & Configuration — GRAPH Actor, Config Schema & CLI Compliance (v3.5.0)

HAL9000 referenced this issue

2026-04-09 09:13:58 +00:00

[AUTO-EPIC] Epic Planning Health Report (Cycle 11) #5298

HAL9000 added the

State

Verified

label

2026-04-09 10:16:27 +00:00

HAL9000 commented

2026-04-09 10:20:12 +00:00

Author

Owner

🏷️ Label Fix Applied by Backlog Groomer

The State/Verified label has been added to this issue.

Reason: During a routine backlog grooming pass, this issue was found to have Type/Bug and a Priority/ label but was missing a State/* label entirely — a violation of the CONTRIBUTING.md requirement that every issue must have exactly one State/ label.

Since this issue has been triaged with a priority and type, State/Verified is the appropriate state: the issue has been confirmed as legitimate and is now part of the active backlog.

No other changes were made.

Automated by CleverAgents Bot
Supervisor: Label Management | Agent: forgejo-label-manager

## 🏷️ Label Fix Applied by Backlog Groomer The `State/Verified` label has been added to this issue. **Reason**: During a routine backlog grooming pass, this issue was found to have `Type/Bug` and a `Priority/` label but was missing a `State/*` label entirely — a violation of the CONTRIBUTING.md requirement that every issue must have exactly one `State/` label. Since this issue has been triaged with a priority and type, `State/Verified` is the appropriate state: the issue has been confirmed as legitimate and is now part of the active backlog. No other changes were made. --- **Automated by CleverAgents Bot** Supervisor: Label Management | Agent: forgejo-label-manager

HAL9000 added the

MoSCoW

Could have

Points

labels

2026-04-09 11:14:23 +00:00

HAL9000 commented

2026-04-09 11:22:17 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Backlog — Silent exception swallowing in llm_actors.py for sandbox file write errors. This violates CONTRIBUTING.md error handling rules but doesn't block core functionality.
Milestone: Unassigned — Code quality fix, can be addressed in any milestone.
Story Points: 2 — S size. Replacing bare except with proper exception propagation is a 1-2 hour change.
MoSCoW: Could Have — The system functions without this fix (just silently ignores file write errors). Should be fixed for correctness but doesn't block any milestone acceptance criteria.
Parent Epic: Needs linking to the Plan Lifecycle or Execution Pipeline epic.

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

Issue triaged by project owner: - **State**: Verified - **Priority**: Backlog — Silent exception swallowing in `llm_actors.py` for sandbox file write errors. This violates CONTRIBUTING.md error handling rules but doesn't block core functionality. - **Milestone**: Unassigned — Code quality fix, can be addressed in any milestone. - **Story Points**: 2 — S size. Replacing bare `except` with proper exception propagation is a 1-2 hour change. - **MoSCoW**: Could Have — The system functions without this fix (just silently ignores file write errors). Should be fixed for correctness but doesn't block any milestone acceptance criteria. - **Parent Epic**: Needs linking to the Plan Lifecycle or Execution Pipeline epic. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner

HAL9000 referenced this issue

2026-04-09 12:09:11 +00:00

[AUTO-GROOMER] Backlog Grooming Report (Cycle 59) #5949