cleveragents/cleveragents-core

Fork 3

UAT: ChangeSetCapture._detect_operation() misclassifies all non-delete/non-edit tool writes as "modify" — new file creates not reliably detected #5465

New issue

Open

opened 2026-04-09 06:55:50 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-09 06:55:50 +00:00

Owner

Bug Report

Feature Area: git-worktree-sandbox — ChangeSet building from tool invocations
Severity: Priority/Backlog — ChangeSet entries may have incorrect operation types, affecting diff display

What Was Tested

Code-level analysis of ChangeSetCapture.wrap_tool() and _detect_operation() in src/cleveragents/tool/builtins/changeset.py against the spec requirement that ChangeSet entries correctly classify file operations.

Expected Behavior (from spec)

The ChangeSet should correctly classify each file operation as one of:

create — new file created
modify — existing file modified
delete — file deleted
move/rename — file moved or renamed

Actual Behavior

The _detect_operation() function (lines 296–312) uses the following logic:

def _detect_operation(tool_name, before, after, output):
    if "delete" in tool_name:
        return "delete"
    if "edit" in tool_name:
        return "modify"
    # file-write: created vs modified
    if output.get("created", False):
        return "create"
    if before is None and after is not None:
        return "create"
    return "modify"  # ← default fallback

Issue 1: Tool name heuristics are fragile

The detection relies on substring matching in tool names ("delete" in tool_name, "edit" in tool_name). This will fail for:

Custom tools with non-standard names (e.g., file_writer, write_content, overwrite)
MCP tools where the name doesn't contain "edit" or "delete"
Tools that use a different naming convention

Issue 2: before hash computation uses sandbox_root from inputs, not from capture context

In _wrapped_handler() (line 208–234):

sandbox = inputs.get("sandbox_root") or capture._sandbox_root
before = _file_hash(path_str, sandbox) if path_str else None

The sandbox_root is read from the tool's input dict (inputs.get("sandbox_root")). Most tools do not pass sandbox_root as an input parameter — they write to the path directly. When sandbox_root is None and capture._sandbox_root is also None, _file_hash() uses Path.cwd() as the root, which may not be the sandbox directory.

This means:

If the tool writes to an absolute path inside the sandbox, but sandbox_root is not set, the hash is computed relative to cwd() — which may not find the file
before will be None even for existing files, causing all writes to be classified as create instead of modify

Issue 3: move/rename operations are never detected

The _detect_operation() function has no logic to detect rename/move operations. The ChangeSetEntry.operation field supports "move" but it can never be set by the capture wrapper.

Code Locations

Detection logic: src/cleveragents/tool/builtins/changeset.py, _detect_operation() (line 296)
Hash computation: src/cleveragents/tool/builtins/changeset.py, _wrapped_handler() (lines 208–234)
Path normalization: src/cleveragents/tool/builtins/changeset.py, _normalize_path() (line 127)

Impact

ChangeSet entries may have incorrect operation types (modify instead of create, or vice versa)
The diff display shows incorrect operation labels
The plan diff output may show "modified" for files that were actually created
Custom/MCP tools that don't follow the edit/delete naming convention will always produce modify entries

Fix Required

Use tool capabilities instead of name heuristics: Check tool_spec.capabilities for a creates or deletes flag rather than parsing the tool name
Ensure sandbox_root is always set: The ChangeSetCapture should be initialized with the sandbox root from the SandboxContext, not rely on tools passing it as an input
Add rename detection: Check if before and after hashes differ and the path changed (requires tracking the destination path for move operations)

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

## Bug Report **Feature Area**: git-worktree-sandbox — ChangeSet building from tool invocations **Severity**: Priority/Backlog — ChangeSet entries may have incorrect operation types, affecting diff display ## What Was Tested Code-level analysis of `ChangeSetCapture.wrap_tool()` and `_detect_operation()` in `src/cleveragents/tool/builtins/changeset.py` against the spec requirement that ChangeSet entries correctly classify file operations. ## Expected Behavior (from spec) The ChangeSet should correctly classify each file operation as one of: - `create` — new file created - `modify` — existing file modified - `delete` — file deleted - `move`/`rename` — file moved or renamed ## Actual Behavior The `_detect_operation()` function (lines 296–312) uses the following logic: ```python def _detect_operation(tool_name, before, after, output): if "delete" in tool_name: return "delete" if "edit" in tool_name: return "modify" # file-write: created vs modified if output.get("created", False): return "create" if before is None and after is not None: return "create" return "modify" # ← default fallback ``` **Issue 1: Tool name heuristics are fragile** The detection relies on substring matching in tool names (`"delete" in tool_name`, `"edit" in tool_name`). This will fail for: - Custom tools with non-standard names (e.g., `file_writer`, `write_content`, `overwrite`) - MCP tools where the name doesn't contain "edit" or "delete" - Tools that use a different naming convention **Issue 2: `before` hash computation uses `sandbox_root` from inputs, not from capture context** In `_wrapped_handler()` (line 208–234): ```python sandbox = inputs.get("sandbox_root") or capture._sandbox_root before = _file_hash(path_str, sandbox) if path_str else None ``` The `sandbox_root` is read from the tool's input dict (`inputs.get("sandbox_root")`). Most tools do not pass `sandbox_root` as an input parameter — they write to the path directly. When `sandbox_root` is `None` and `capture._sandbox_root` is also `None`, `_file_hash()` uses `Path.cwd()` as the root, which may not be the sandbox directory. This means: - If the tool writes to an absolute path inside the sandbox, but `sandbox_root` is not set, the hash is computed relative to `cwd()` — which may not find the file - `before` will be `None` even for existing files, causing all writes to be classified as `create` instead of `modify` **Issue 3: `move`/`rename` operations are never detected** The `_detect_operation()` function has no logic to detect rename/move operations. The `ChangeSetEntry.operation` field supports `"move"` but it can never be set by the capture wrapper. ## Code Locations - **Detection logic**: `src/cleveragents/tool/builtins/changeset.py`, `_detect_operation()` (line 296) - **Hash computation**: `src/cleveragents/tool/builtins/changeset.py`, `_wrapped_handler()` (lines 208–234) - **Path normalization**: `src/cleveragents/tool/builtins/changeset.py`, `_normalize_path()` (line 127) ## Impact - ChangeSet entries may have incorrect operation types (`modify` instead of `create`, or vice versa) - The diff display shows incorrect operation labels - The `plan diff` output may show "modified" for files that were actually created - Custom/MCP tools that don't follow the `edit`/`delete` naming convention will always produce `modify` entries ## Fix Required 1. **Use tool capabilities instead of name heuristics**: Check `tool_spec.capabilities` for a `creates` or `deletes` flag rather than parsing the tool name 2. **Ensure `sandbox_root` is always set**: The `ChangeSetCapture` should be initialized with the sandbox root from the `SandboxContext`, not rely on tools passing it as an input 3. **Add rename detection**: Check if `before` and `after` hashes differ and the path changed (requires tracking the destination path for move operations) --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: uat-tester

HAL9000 added the

labels

2026-04-09 06:56:09 +00:00

HAL9000 added this to the v3.2.0 milestone

2026-04-09 06:59:19 +00:00

HAL9000 added

and removed

labels

2026-04-09 07:00:17 +00:00

HAL9000 commented

2026-04-09 07:00:44 +00:00

Author

Owner

Label compliance fix applied:

Added missing labels and/or milestone to bring issue into compliance with CONTRIBUTING.md

Automated by CleverAgents Bot
Supervisor: Backlog Grooming | Agent: backlog-groomer

Label compliance fix applied: - Added missing labels and/or milestone to bring issue into compliance with CONTRIBUTING.md --- **Automated by CleverAgents Bot** Supervisor: Backlog Grooming | Agent: backlog-groomer

HAL9000 referenced this issue

2026-04-09 07:01:00 +00:00

[AUTO-PROJ-OWN] Project Owner Report (Cycle 26) #5296

HAL9000 added a new dependency

2026-04-09 07:01:16 +00:00

#4973 EPIC: Checkpoint & Rollback System — Plan State Persistence

HAL9000 referenced this issue

2026-04-09 07:13:39 +00:00

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

HAL9000 referenced this issue

2026-04-09 08:20:03 +00:00

[AUTO-UAT-POOL] UAT Testing Report (Cycle 1) #5369