Skip to content

Commit aa628c0

Browse files
fix(tools): normalize LIST_PARAMS when TRACK_INVOCATIONS is off
Local models often double-encode EditText `edits` as a JSON string. That normalization only ran inside the invocation-tracking block, so tools with TRACK_INVOCATIONS=False (EditText, ReadRange) still saw raw strings and failed with "edits parameter must be an array". Also harden EditText format_output to coerce string edits (same pattern as Grep/UpdateTodoList) and stop emitting Python tracebacks for expected ToolError responses from process_response. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent cce336f commit aa628c0

3 files changed

Lines changed: 142 additions & 16 deletions

File tree

cecli/tools/edit_text.py

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
import json
2-
1+
from cecli.helpers import responses
32
from cecli.helpers.hashline import (
43
ContentHashError,
54
apply_hashline_operations,
@@ -12,6 +11,7 @@
1211
apply_change,
1312
format_tool_result,
1413
handle_tool_error,
14+
normalize_json_array,
1515
validate_file_for_edit,
1616
)
1717
from cecli.tools.utils.output import color_markers, tool_footer, tool_header
@@ -96,6 +96,23 @@ class Tool(BaseTool):
9696
},
9797
}
9898

99+
@classmethod
100+
def _coerce_edits(cls, edits) -> list[dict]:
101+
"""Normalize ``edits`` for display (local models often double-encode arrays)."""
102+
try:
103+
normalized = normalize_json_array(edits, param_name="edits", allow_empty=True)
104+
except ToolError:
105+
return []
106+
out: list[dict] = []
107+
for item in normalized:
108+
if isinstance(item, dict):
109+
out.append(item)
110+
elif isinstance(item, str):
111+
parsed = responses.try_parse_json_value(item)
112+
if isinstance(parsed, dict):
113+
out.append(parsed)
114+
return out
115+
99116
@classmethod
100117
def execute(
101118
cls,
@@ -117,16 +134,18 @@ def execute(
117134

118135
tool_name = "EditText"
119136
try:
120-
# 1. Validate edits parameter
121-
if not isinstance(edits, list):
122-
raise ToolError("edits parameter must be an array")
137+
edits = normalize_json_array(edits, param_name="edits")
123138

124139
if len(edits) == 0:
125140
raise ToolError("edits array cannot be empty")
126141

127142
# 2. Group edits by file_path
128143
edits_by_file = {}
129144
for i, edit in enumerate(edits):
145+
if not isinstance(edit, dict):
146+
raise ToolError(
147+
f"Edit {i + 1} must be an object, got {type(edit).__name__}"
148+
)
130149
edit_file_path = edit.get("file_path")
131150
if edit_file_path is None:
132151
raise ToolError(f"Edit {i + 1} missing required file_path parameter")
@@ -370,17 +389,20 @@ def execute(
370389
def format_output(cls, coder, mcp_server, tool_response):
371390
color_start, color_end = color_markers(coder)
372391

373-
try:
374-
params = json.loads(tool_response.function.arguments)
375-
except json.JSONDecodeError:
376-
coder.io.tool_error("Invalid Tool JSON")
392+
params = responses.parse_tool_arguments(tool_response.function.arguments or "")
393+
if "@error" in params:
394+
coder.io.tool_error(f"Invalid Tool JSON: {params['@error']}")
395+
return
377396

378397
tool_header(coder=coder, mcp_server=mcp_server, tool_response=tool_response)
379398

380399
# Group edits by file_path for display
381400
edits_by_file = {}
401+
edits = cls._coerce_edits(params.get("edits", []))
382402

383-
for i, edit in enumerate(params.get("edits", [])):
403+
for i, edit in enumerate(edits):
404+
if not isinstance(edit, dict):
405+
continue
384406
edit_file_path = edit.get("file_path")
385407
if edit_file_path not in edits_by_file:
386408
edits_by_file[edit_file_path] = []
@@ -397,7 +419,7 @@ def format_output(cls, coder, mcp_server, tool_response):
397419
for edit_index, edit in file_edits:
398420
operation = edit.get("operation", "replace")
399421

400-
if len(params.get("edits", [])) > 1:
422+
if len(edits) > 1:
401423
coder.io.tool_output(
402424
f"{color_start}{OPERATION_NOUNS[operation]}_{edit_index + 1}:{color_end}"
403425
)

cecli/tools/utils/base_tool.py

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from abc import ABC, abstractmethod
22

3-
from cecli.tools.utils.helpers import handle_tool_error, normalize_json_array
3+
from cecli.tools.utils.helpers import ToolError, handle_tool_error, normalize_json_array
44
from cecli.tools.utils.output import print_tool_response
55

66

@@ -82,6 +82,10 @@ def process_response(cls, coder, params):
8282
)
8383
return handle_tool_error(coder, tool_name, ValueError(error_msg))
8484

85+
for param in cls.LIST_PARAMS:
86+
if param in params:
87+
params[param] = normalize_json_array(params[param], param_name=param)
88+
8589
# Check for repeated invocations if TRACK_INVOCATIONS is enabled
8690
if cls.TRACK_INVOCATIONS:
8791
tool_name = None
@@ -122,10 +126,6 @@ def process_response(cls, coder, params):
122126
coder, tool_name, ValueError(error_msg), add_traceback=False
123127
)
124128

125-
for param in cls.LIST_PARAMS:
126-
if param in params:
127-
params[param] = normalize_json_array(params[param], param_name=param)
128-
129129
# Add current invocation to history (keeping only last 3)
130130
if params:
131131
cls._invocations[tool_name].append((current_params_tuple, params))
@@ -134,6 +134,9 @@ def process_response(cls, coder, params):
134134

135135
try:
136136
return cls.execute(coder, **params)
137+
except ToolError as e:
138+
tool_name = (cls.SCHEMA or {}).get("function", {}).get("name", cls.__name__)
139+
return handle_tool_error(coder, tool_name, e, add_traceback=False)
137140
except Exception as e:
138141
return handle_tool_error(coder, cls.SCHEMA.get("function").get("name"), e)
139142

tests/tools/test_edit_text.py

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
"""EditText tool — double-encoded edits and format_output safety."""
2+
3+
import json
4+
from types import SimpleNamespace
5+
from unittest.mock import Mock
6+
7+
from cecli.tools import edit_text
8+
from cecli.tools.utils.base_tool import BaseTool
9+
10+
11+
class DummyIO:
12+
def __init__(self):
13+
self.tool_output = Mock()
14+
self.tool_error = Mock()
15+
self.tool_warning = Mock()
16+
17+
18+
class DummyCoder:
19+
def __init__(self):
20+
self.io = DummyIO()
21+
self.pretty = False
22+
self.verbose = False
23+
24+
25+
class _NoTrackTool(BaseTool):
26+
"""Minimal tool mirroring EditText LIST_PARAMS + TRACK_INVOCATIONS=False."""
27+
28+
NORM_NAME = "notrack"
29+
TRACK_INVOCATIONS = False
30+
LIST_PARAMS = ["edits"]
31+
SCHEMA = {
32+
"function": {
33+
"name": "NoTrack",
34+
"parameters": {
35+
"type": "object",
36+
"properties": {
37+
"edits": {"type": "array"},
38+
},
39+
"required": ["edits"],
40+
},
41+
}
42+
}
43+
44+
@classmethod
45+
def execute(cls, coder, edits=None, **kwargs):
46+
if not isinstance(edits, list):
47+
return f"edits type={type(edits).__name__}"
48+
return f"edits len={len(edits)}"
49+
50+
51+
def test_list_params_normalized_when_track_invocations_disabled():
52+
coder = DummyCoder()
53+
edits_json = json.dumps(
54+
[{"file_path": "pubspec.yaml", "operation": "replace", "start_line": "@000", "end_line": "@000"}]
55+
)
56+
result = _NoTrackTool.process_response(coder, {"edits": edits_json})
57+
assert result == "edits len=1"
58+
59+
60+
def test_format_output_accepts_edits_as_json_string():
61+
coder = DummyCoder()
62+
edits_json = json.dumps(
63+
[
64+
{
65+
"file_path": "pubspec.yaml",
66+
"operation": "replace",
67+
"start_line": "@000",
68+
"end_line": "@000",
69+
"text": "name: demo",
70+
}
71+
]
72+
)
73+
args = json.dumps({"edits": edits_json})
74+
tool_response = SimpleNamespace(function=SimpleNamespace(name="EditText", arguments=args))
75+
76+
edit_text.Tool.format_output(
77+
coder,
78+
mcp_server=SimpleNamespace(name="test"),
79+
tool_response=tool_response,
80+
)
81+
82+
output_text = "\n".join(call.args[0] for call in coder.io.tool_output.call_args_list)
83+
assert "pubspec.yaml" in output_text
84+
coder.io.tool_error.assert_not_called()
85+
86+
87+
def test_format_output_string_edits_does_not_crash():
88+
"""Regression: iterating a JSON string used to raise AttributeError in format_output."""
89+
coder = DummyCoder()
90+
edits_json = json.dumps([{"file_path": "a.txt", "operation": "replace"}])
91+
tool_response = SimpleNamespace(
92+
function=SimpleNamespace(name="EditText", arguments=json.dumps({"edits": edits_json}))
93+
)
94+
95+
edit_text.Tool.format_output(
96+
coder,
97+
mcp_server=SimpleNamespace(name="test"),
98+
tool_response=tool_response,
99+
)
100+
101+
coder.io.tool_error.assert_not_called()

0 commit comments

Comments
 (0)