UAT: InlineToolExecutor module docstring claims "background thread" execution but implementation uses subprocess — misleading safety contract #3733

New issue

Open

opened 2026-04-05 22:20:52 +00:00 by freemo · 0 comments

freemo commented

2026-04-05 22:20:52 +00:00

Owner

Metadata

Branch: fix/inline-executor-docstring-subprocess
Commit Message: fix(skills): correct InlineToolExecutor module docstring to describe subprocess execution
Milestone: Backlog
Parent Epic: #3374

Background

The InlineToolExecutor in src/cleveragents/skills/inline_executor.py has a module-level docstring that states execution uses "a background thread" with timeout, but the actual implementation uses subprocess.Popen. This creates a misleading safety contract for callers and violates the spec's documentation accuracy requirements.

What Was Tested

Code-level analysis of src/cleveragents/skills/inline_executor.py.

Expected Behavior (from spec)

The module docstring accurately describes the execution mechanism. The spec requires documentation to match implementation.

Actual Behavior

Module docstring (lines 18-20):

- **Timeout**: Execution is bounded by ``max_runtime_seconds``
  (default 30 s).  A background thread runs the code; if it does
  not complete in time the result reports a timeout error.

Actual implementation (_run_with_timeout method):

proc = subprocess.Popen(
    [sys.executable, "-I", "-c", wrapper, json.dumps(input_data), tool.code or ""],
    stdout=subprocess.PIPE,
    stderr=subprocess.PIPE,
    env=child_env,
)

The implementation uses subprocess.Popen (a child process), not a thread. The _run_with_timeout docstring correctly explains the subprocess approach, but the module-level docstring contradicts it.

Impact

Callers reading the module docstring have incorrect expectations about isolation level
Thread-based execution and subprocess-based execution have different security/isolation properties
The module docstring says "thread" but subprocess provides stronger isolation (separate process)
This discrepancy could cause confusion when debugging or extending the executor

Code Location

src/cleveragents/skills/inline_executor.py — module-level docstring, line 18-20

Subtasks

Update module-level docstring to accurately describe subprocess-based execution
Verify _run_with_timeout docstring is consistent with module docstring
Add/update Behave tests for timeout behavior
Run nox (all default sessions), fix any errors
Verify coverage >= 97% via nox -e coverage_report

Definition of Done

Module docstring accurately describes subprocess execution
No contradictions between module docstring and method docstrings
nox -e unit_tests passes
All subtasks above are completed and checked off
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done
All nox stages pass
Coverage >= 97%

Backlog note: This issue was discovered during autonomous operation
on milestone v3.6.0. It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/inline-executor-docstring-subprocess` - **Commit Message**: `fix(skills): correct InlineToolExecutor module docstring to describe subprocess execution` - **Milestone**: Backlog - **Parent Epic**: #3374 ## Background The `InlineToolExecutor` in `src/cleveragents/skills/inline_executor.py` has a module-level docstring that states execution uses "a background thread" with timeout, but the actual implementation uses `subprocess.Popen`. This creates a misleading safety contract for callers and violates the spec's documentation accuracy requirements. ## What Was Tested Code-level analysis of `src/cleveragents/skills/inline_executor.py`. ## Expected Behavior (from spec) The module docstring accurately describes the execution mechanism. The spec requires documentation to match implementation. ## Actual Behavior **Module docstring (lines 18-20)**: ``` - **Timeout**: Execution is bounded by ``max_runtime_seconds`` (default 30 s). A background thread runs the code; if it does not complete in time the result reports a timeout error. ``` **Actual implementation** (`_run_with_timeout` method): ```python proc = subprocess.Popen( [sys.executable, "-I", "-c", wrapper, json.dumps(input_data), tool.code or ""], stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=child_env, ) ``` The implementation uses `subprocess.Popen` (a child process), not a thread. The `_run_with_timeout` docstring correctly explains the subprocess approach, but the module-level docstring contradicts it. ## Impact - Callers reading the module docstring have incorrect expectations about isolation level - Thread-based execution and subprocess-based execution have different security/isolation properties - The module docstring says "thread" but subprocess provides stronger isolation (separate process) - This discrepancy could cause confusion when debugging or extending the executor ## Code Location `src/cleveragents/skills/inline_executor.py` — module-level docstring, line 18-20 ## Subtasks - [ ] Update module-level docstring to accurately describe subprocess-based execution - [ ] Verify `_run_with_timeout` docstring is consistent with module docstring - [ ] Add/update Behave tests for timeout behavior - [ ] Run `nox` (all default sessions), fix any errors - [ ] Verify coverage >= 97% via `nox -e coverage_report` ## Definition of Done - [ ] Module docstring accurately describes subprocess execution - [ ] No contradictions between module docstring and method docstrings - [ ] `nox -e unit_tests` passes - [ ] All subtasks above are completed and checked off - [ ] A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation - [ ] The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly - [ ] The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done - All nox stages pass - Coverage >= 97% > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.6.0. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added a new dependency

2026-04-05 22:20:59 +00:00

#3374 Epic: E2E Workflow Specification Tests & Code Review Tool Examples

freemo referenced this issue

2026-04-05 22:22:14 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo referenced this issue

2026-04-05 22:22:42 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654

freemo referenced this issue

2026-04-05 22:25:59 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 (Resumed) #3654