git.iko.soy/crush
on main help sign in
handle errors correctly in the edit tool
921f5eeKujtim Hoxha
internal/llm/tools/diagnostics.go+2−2
compare with main
unifiedsplit
@@ -82,7 +82,7 @@ func (b *diagnosticsTool) Run(ctx context.Context, call ToolCall) (ToolResponse,
82 waitForLspDiagnostics(ctx, params.FilePath, lsps)
82 waitForLspDiagnostics(ctx, params.FilePath, lsps)
83 }
83 }
84
84
85− output := appendDiagnostics(params.FilePath, lsps)
85+ output := getDiagnostics(params.FilePath, lsps)
86
86
87 return NewTextResponse(output), nil
87 return NewTextResponse(output), nil
88}
88}
@@ -154,7 +154,7 @@ func hasDiagnosticsChanged(current, original map[protocol.DocumentUri][]protocol
154 return false
154 return false
155}
155}
156
156
157−func appendDiagnostics(filePath string, lsps map[string]*lsp.Client) string {
157+func getDiagnostics(filePath string, lsps map[string]*lsp.Client) string {
158 fileDiagnostics := []string{}
158 fileDiagnostics := []string{}
159 projectDiagnostics := []string{}
159 projectDiagnostics := []string{}
160
160
internal/llm/tools/edit.go+74−78
@@ -131,68 +131,54 @@ func (e *editTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error)
131 params.FilePath = filepath.Join(wd, params.FilePath)
131 params.FilePath = filepath.Join(wd, params.FilePath)
132 }
132 }
133
133
134+ var response ToolResponse
135+ var err error
136+
134 if params.OldString == "" {
137 if params.OldString == "" {
135− result, err := e.createNewFile(ctx, params.FilePath, params.NewString)
138+ response, err = e.createNewFile(ctx, params.FilePath, params.NewString)
136 if err != nil {
139 if err != nil {
137− return NewTextErrorResponse(fmt.Sprintf("error creating file: %s", err)), nil
140+ return response, nil
138 }
141 }
139− return WithResponseMetadata(NewTextResponse(result.text), EditResponseMetadata{
140− Additions: result.additions,
141− Removals: result.removals,
142− }), nil
143 }
142 }
144
143
145 if params.NewString == "" {
144 if params.NewString == "" {
146− result, err := e.deleteContent(ctx, params.FilePath, params.OldString)
145+ response, err = e.deleteContent(ctx, params.FilePath, params.OldString)
147 if err != nil {
146 if err != nil {
148− return NewTextErrorResponse(fmt.Sprintf("error deleting content: %s", err)), nil
147+ return response, nil
149 }
148 }
150− return WithResponseMetadata(NewTextResponse(result.text), EditResponseMetadata{
151− Additions: result.additions,
152− Removals: result.removals,
153− }), nil
154 }
149 }
155
150
156− result, err := e.replaceContent(ctx, params.FilePath, params.OldString, params.NewString)
151+ response, err = e.replaceContent(ctx, params.FilePath, params.OldString, params.NewString)
157 if err != nil {
152 if err != nil {
158− return NewTextErrorResponse(fmt.Sprintf("error replacing content: %s", err)), nil
153+ return response, nil
159 }
154 }
160
155
161 waitForLspDiagnostics(ctx, params.FilePath, e.lspClients)
156 waitForLspDiagnostics(ctx, params.FilePath, e.lspClients)
162− text := fmt.Sprintf("<result>\n%s\n</result>\n", result.text)
157+ text := fmt.Sprintf("<result>\n%s\n</result>\n", response.Content)
163− text += appendDiagnostics(params.FilePath, e.lspClients)
158+ text += getDiagnostics(params.FilePath, e.lspClients)
164− return WithResponseMetadata(NewTextResponse(text), EditResponseMetadata{
159+ response.Content = text
165− Additions: result.additions,
160+ return response, nil
166− Removals: result.removals,
167− }), nil
168−}
169−
170−type editResponse struct {
171− text string
172− additions int
173− removals int
174}
161}
175
162
176−func (e *editTool) createNewFile(ctx context.Context, filePath, content string) (editResponse, error) {
163+func (e *editTool) createNewFile(ctx context.Context, filePath, content string) (ToolResponse, error) {
177− er := editResponse{}
178 fileInfo, err := os.Stat(filePath)
164 fileInfo, err := os.Stat(filePath)
179 if err == nil {
165 if err == nil {
180 if fileInfo.IsDir() {
166 if fileInfo.IsDir() {
181− return er, fmt.Errorf("path is a directory, not a file: %s", filePath)
167+ return NewTextErrorResponse(fmt.Sprintf("path is a directory, not a file: %s", filePath)), nil
182 }
168 }
183− return er, fmt.Errorf("file already exists: %s. Use the Replace tool to overwrite an existing file", filePath)
169+ return NewTextErrorResponse(fmt.Sprintf("file already exists: %s", filePath)), nil
184 } else if !os.IsNotExist(err) {
170 } else if !os.IsNotExist(err) {
185− return er, fmt.Errorf("failed to access file: %w", err)
171+ return ToolResponse{}, fmt.Errorf("failed to access file: %w", err)
186 }
172 }
187
173
188 dir := filepath.Dir(filePath)
174 dir := filepath.Dir(filePath)
189 if err = os.MkdirAll(dir, 0o755); err != nil {
175 if err = os.MkdirAll(dir, 0o755); err != nil {
190− return er, fmt.Errorf("failed to create parent directories: %w", err)
176+ return ToolResponse{}, fmt.Errorf("failed to create parent directories: %w", err)
191 }
177 }
192
178
193 sessionID, messageID := GetContextValues(ctx)
179 sessionID, messageID := GetContextValues(ctx)
194 if sessionID == "" || messageID == "" {
180 if sessionID == "" || messageID == "" {
195− return er, fmt.Errorf("session ID and message ID are required for creating a new file")
181+ return ToolResponse{}, fmt.Errorf("session ID and message ID are required for creating a new file")
196 }
182 }
197
183
198 diff, stats, err := git.GenerateGitDiffWithStats(
184 diff, stats, err := git.GenerateGitDiffWithStats(
@@ -201,7 +187,7 @@ func (e *editTool) createNewFile(ctx context.Context, filePath, content string)
201 content,
187 content,
202 )
188 )
203 if err != nil {
189 if err != nil {
204− return er, fmt.Errorf("failed to get file diff: %w", err)
190+ return ToolResponse{}, fmt.Errorf("failed to get file diff: %w", err)
205 }
191 }
206 p := e.permissions.Request(
192 p := e.permissions.Request(
207 permission.CreatePermissionRequest{
193 permission.CreatePermissionRequest{
@@ -216,63 +202,67 @@ func (e *editTool) createNewFile(ctx context.Context, filePath, content string)
216 },
202 },
217 )
203 )
218 if !p {
204 if !p {
219− return er, fmt.Errorf("permission denied")
205+ return ToolResponse{}, permission.ErrorPermissionDenied
220 }
206 }
221
207
222 err = os.WriteFile(filePath, []byte(content), 0o644)
208 err = os.WriteFile(filePath, []byte(content), 0o644)
223 if err != nil {
209 if err != nil {
224− return er, fmt.Errorf("failed to write file: %w", err)
210+ return ToolResponse{}, fmt.Errorf("failed to write file: %w", err)
225 }
211 }
226
212
227 recordFileWrite(filePath)
213 recordFileWrite(filePath)
228 recordFileRead(filePath)
214 recordFileRead(filePath)
229
215
216+ return WithResponseMetadata(
230− er.text = "File created: " + filePath
217+ NewTextResponse("File created: "+filePath),
218+ EditResponseMetadata{
231− er.additions = stats.Additions
219+ Additions: stats.Additions,
232− er.removals = stats.Removals
220+ Removals: stats.Removals,
221+ },
233− return er, nil
222+ ), nil
234}
223}
235
224
236−func (e *editTool) deleteContent(ctx context.Context, filePath, oldString string) (editResponse, error) {
225+func (e *editTool) deleteContent(ctx context.Context, filePath, oldString string) (ToolResponse, error) {
237− er := editResponse{}
238 fileInfo, err := os.Stat(filePath)
226 fileInfo, err := os.Stat(filePath)
239 if err != nil {
227 if err != nil {
240 if os.IsNotExist(err) {
228 if os.IsNotExist(err) {
241− return er, fmt.Errorf("file not found: %s", filePath)
229+ return NewTextErrorResponse(fmt.Sprintf("file not found: %s", filePath)), nil
242 }
230 }
243− return er, fmt.Errorf("failed to access file: %w", err)
231+ return ToolResponse{}, fmt.Errorf("failed to access file: %w", err)
244 }
232 }
245
233
246 if fileInfo.IsDir() {
234 if fileInfo.IsDir() {
247− return er, fmt.Errorf("path is a directory, not a file: %s", filePath)
235+ return NewTextErrorResponse(fmt.Sprintf("path is a directory, not a file: %s", filePath)), nil
248 }
236 }
249
237
250 if getLastReadTime(filePath).IsZero() {
238 if getLastReadTime(filePath).IsZero() {
251− return er, fmt.Errorf("you must read the file before editing it. Use the View tool first")
239+ return NewTextErrorResponse("you must read the file before editing it. Use the View tool first"), nil
252 }
240 }
253
241
254 modTime := fileInfo.ModTime()
242 modTime := fileInfo.ModTime()
255 lastRead := getLastReadTime(filePath)
243 lastRead := getLastReadTime(filePath)
256 if modTime.After(lastRead) {
244 if modTime.After(lastRead) {
245+ return NewTextErrorResponse(
257− return er, fmt.Errorf("file %s has been modified since it was last read (mod time: %s, last read: %s)",
246+ fmt.Sprintf("file %s has been modified since it was last read (mod time: %s, last read: %s)",
258− filePath, modTime.Format(time.RFC3339), lastRead.Format(time.RFC3339))
247+ filePath, modTime.Format(time.RFC3339), lastRead.Format(time.RFC3339),
248+ )), nil
259 }
249 }
260
250
261 content, err := os.ReadFile(filePath)
251 content, err := os.ReadFile(filePath)
262 if err != nil {
252 if err != nil {
263− return er, fmt.Errorf("failed to read file: %w", err)
253+ return ToolResponse{}, fmt.Errorf("failed to read file: %w", err)
264 }
254 }
265
255
266 oldContent := string(content)
256 oldContent := string(content)
267
257
268 index := strings.Index(oldContent, oldString)
258 index := strings.Index(oldContent, oldString)
269 if index == -1 {
259 if index == -1 {
270− return er, fmt.Errorf("old_string not found in file. Make sure it matches exactly, including whitespace and line breaks")
260+ return NewTextErrorResponse("old_string not found in file. Make sure it matches exactly, including whitespace and line breaks"), nil
271 }
261 }
272
262
273 lastIndex := strings.LastIndex(oldContent, oldString)
263 lastIndex := strings.LastIndex(oldContent, oldString)
274 if index != lastIndex {
264 if index != lastIndex {
275− return er, fmt.Errorf("old_string appears multiple times in the file. Please provide more context to ensure a unique match")
265+ return NewTextErrorResponse("old_string appears multiple times in the file. Please provide more context to ensure a unique match"), nil
276 }
266 }
277
267
278 newContent := oldContent[:index] + oldContent[index+len(oldString):]
268 newContent := oldContent[:index] + oldContent[index+len(oldString):]
@@ -280,7 +270,7 @@ func (e *editTool) deleteContent(ctx context.Context, filePath, oldString string
280 sessionID, messageID := GetContextValues(ctx)
270 sessionID, messageID := GetContextValues(ctx)
281
271
282 if sessionID == "" || messageID == "" {
272 if sessionID == "" || messageID == "" {
283− return er, fmt.Errorf("session ID and message ID are required for creating a new file")
273+ return ToolResponse{}, fmt.Errorf("session ID and message ID are required for creating a new file")
284 }
274 }
285
275
286 diff, stats, err := git.GenerateGitDiffWithStats(
276 diff, stats, err := git.GenerateGitDiffWithStats(
@@ -289,7 +279,7 @@ func (e *editTool) deleteContent(ctx context.Context, filePath, oldString string
289 newContent,
279 newContent,
290 )
280 )
291 if err != nil {
281 if err != nil {
292− return er, fmt.Errorf("failed to get file diff: %w", err)
282+ return ToolResponse{}, fmt.Errorf("failed to get file diff: %w", err)
293 }
283 }
294
284
295 p := e.permissions.Request(
285 p := e.permissions.Request(
@@ -305,62 +295,66 @@ func (e *editTool) deleteContent(ctx context.Context, filePath, oldString string
305 },
295 },
306 )
296 )
307 if !p {
297 if !p {
308− return er, fmt.Errorf("permission denied")
298+ return ToolResponse{}, permission.ErrorPermissionDenied
309 }
299 }
310
300
311 err = os.WriteFile(filePath, []byte(newContent), 0o644)
301 err = os.WriteFile(filePath, []byte(newContent), 0o644)
312 if err != nil {
302 if err != nil {
313− return er, fmt.Errorf("failed to write file: %w", err)
303+ return ToolResponse{}, fmt.Errorf("failed to write file: %w", err)
314 }
304 }
315 recordFileWrite(filePath)
305 recordFileWrite(filePath)
316 recordFileRead(filePath)
306 recordFileRead(filePath)
317
307
308+ return WithResponseMetadata(
318− er.text = "Content deleted from file: " + filePath
309+ NewTextResponse("Content deleted from file: "+filePath),
310+ EditResponseMetadata{
319− er.additions = stats.Additions
311+ Additions: stats.Additions,
320− er.removals = stats.Removals
312+ Removals: stats.Removals,
313+ },
321− return er, nil
314+ ), nil
322}
315}
323
316
324−func (e *editTool) replaceContent(ctx context.Context, filePath, oldString, newString string) (editResponse, error) {
317+func (e *editTool) replaceContent(ctx context.Context, filePath, oldString, newString string) (ToolResponse, error) {
325− er := editResponse{}
326 fileInfo, err := os.Stat(filePath)
318 fileInfo, err := os.Stat(filePath)
327 if err != nil {
319 if err != nil {
328 if os.IsNotExist(err) {
320 if os.IsNotExist(err) {
329− return er, fmt.Errorf("file not found: %s", filePath)
321+ return NewTextErrorResponse(fmt.Sprintf("file not found: %s", filePath)), nil
330 }
322 }
331− return er, fmt.Errorf("failed to access file: %w", err)
323+ return ToolResponse{}, fmt.Errorf("failed to access file: %w", err)
332 }
324 }
333
325
334 if fileInfo.IsDir() {
326 if fileInfo.IsDir() {
335− return er, fmt.Errorf("path is a directory, not a file: %s", filePath)
327+ return NewTextErrorResponse(fmt.Sprintf("path is a directory, not a file: %s", filePath)), nil
336 }
328 }
337
329
338 if getLastReadTime(filePath).IsZero() {
330 if getLastReadTime(filePath).IsZero() {
339− return er, fmt.Errorf("you must read the file before editing it. Use the View tool first")
331+ return NewTextErrorResponse("you must read the file before editing it. Use the View tool first"), nil
340 }
332 }
341
333
342 modTime := fileInfo.ModTime()
334 modTime := fileInfo.ModTime()
343 lastRead := getLastReadTime(filePath)
335 lastRead := getLastReadTime(filePath)
344 if modTime.After(lastRead) {
336 if modTime.After(lastRead) {
337+ return NewTextErrorResponse(
345− return er, fmt.Errorf("file %s has been modified since it was last read (mod time: %s, last read: %s)",
338+ fmt.Sprintf("file %s has been modified since it was last read (mod time: %s, last read: %s)",
346− filePath, modTime.Format(time.RFC3339), lastRead.Format(time.RFC3339))
339+ filePath, modTime.Format(time.RFC3339), lastRead.Format(time.RFC3339),
340+ )), nil
347 }
341 }
348
342
349 content, err := os.ReadFile(filePath)
343 content, err := os.ReadFile(filePath)
350 if err != nil {
344 if err != nil {
351− return er, fmt.Errorf("failed to read file: %w", err)
345+ return ToolResponse{}, fmt.Errorf("failed to read file: %w", err)
352 }
346 }
353
347
354 oldContent := string(content)
348 oldContent := string(content)
355
349
356 index := strings.Index(oldContent, oldString)
350 index := strings.Index(oldContent, oldString)
357 if index == -1 {
351 if index == -1 {
358− return er, fmt.Errorf("old_string not found in file. Make sure it matches exactly, including whitespace and line breaks")
352+ return NewTextErrorResponse("old_string not found in file. Make sure it matches exactly, including whitespace and line breaks"), nil
359 }
353 }
360
354
361 lastIndex := strings.LastIndex(oldContent, oldString)
355 lastIndex := strings.LastIndex(oldContent, oldString)
362 if index != lastIndex {
356 if index != lastIndex {
363− return er, fmt.Errorf("old_string appears multiple times in the file. Please provide more context to ensure a unique match")
357+ return NewTextErrorResponse("old_string appears multiple times in the file. Please provide more context to ensure a unique match"), nil
364 }
358 }
365
359
366 newContent := oldContent[:index] + newString + oldContent[index+len(oldString):]
360 newContent := oldContent[:index] + newString + oldContent[index+len(oldString):]
@@ -368,7 +362,7 @@ func (e *editTool) replaceContent(ctx context.Context, filePath, oldString, newS
368 sessionID, messageID := GetContextValues(ctx)
362 sessionID, messageID := GetContextValues(ctx)
369
363
370 if sessionID == "" || messageID == "" {
364 if sessionID == "" || messageID == "" {
371− return er, fmt.Errorf("session ID and message ID are required for creating a new file")
365+ return ToolResponse{}, fmt.Errorf("session ID and message ID are required for creating a new file")
372 }
366 }
373 diff, stats, err := git.GenerateGitDiffWithStats(
367 diff, stats, err := git.GenerateGitDiffWithStats(
374 removeWorkingDirectoryPrefix(filePath),
368 removeWorkingDirectoryPrefix(filePath),
@@ -376,7 +370,7 @@ func (e *editTool) replaceContent(ctx context.Context, filePath, oldString, newS
376 newContent,
370 newContent,
377 )
371 )
378 if err != nil {
372 if err != nil {
379− return er, fmt.Errorf("failed to get file diff: %w", err)
373+ return ToolResponse{}, fmt.Errorf("failed to get file diff: %w", err)
380 }
374 }
381
375
382 p := e.permissions.Request(
376 p := e.permissions.Request(
@@ -393,19 +387,21 @@ func (e *editTool) replaceContent(ctx context.Context, filePath, oldString, newS
393 },
387 },
394 )
388 )
395 if !p {
389 if !p {
396− return er, fmt.Errorf("permission denied")
390+ return ToolResponse{}, permission.ErrorPermissionDenied
397 }
391 }
398
392
399 err = os.WriteFile(filePath, []byte(newContent), 0o644)
393 err = os.WriteFile(filePath, []byte(newContent), 0o644)
400 if err != nil {
394 if err != nil {
401− return er, fmt.Errorf("failed to write file: %w", err)
395+ return ToolResponse{}, fmt.Errorf("failed to write file: %w", err)
402 }
396 }
403
397
404 recordFileWrite(filePath)
398 recordFileWrite(filePath)
405 recordFileRead(filePath)
399 recordFileRead(filePath)
406− er.text = "Content replaced in file: " + filePath
407− er.additions = stats.Additions
408− er.removals = stats.Removals
409
400
401+ return WithResponseMetadata(
402+ NewTextResponse("Content replaced in file: "+filePath),
403+ EditResponseMetadata{
404+ Additions: stats.Additions,
405+ Removals: stats.Removals,
410− return er, nil
406+ }), nil
411}
407}
internal/llm/tools/view.go+1−1
@@ -177,7 +177,7 @@ func (v *viewTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error)
177 params.Offset+len(strings.Split(content, "\n")))
177 params.Offset+len(strings.Split(content, "\n")))
178 }
178 }
179 output += "\n</file>\n"
179 output += "\n</file>\n"
180− output += appendDiagnostics(filePath, v.lspClients)
180+ output += getDiagnostics(filePath, v.lspClients)
181 recordFileRead(filePath)
181 recordFileRead(filePath)
182 return NewTextResponse(output), nil
182 return NewTextResponse(output), nil
183}
183}
internal/llm/tools/write.go+1−1
@@ -183,7 +183,7 @@ func (w *writeTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error
183
183
184 result := fmt.Sprintf("File successfully written: %s", filePath)
184 result := fmt.Sprintf("File successfully written: %s", filePath)
185 result = fmt.Sprintf("<result>\n%s\n</result>", result)
185 result = fmt.Sprintf("<result>\n%s\n</result>", result)
186− result += appendDiagnostics(filePath, w.lspClients)
186+ result += getDiagnostics(filePath, w.lspClients)
187 return WithResponseMetadata(NewTextResponse(result),
187 return WithResponseMetadata(NewTextResponse(result),
188 WriteResponseMetadata{
188 WriteResponseMetadata{
189 Additions: stats.Additions,
189 Additions: stats.Additions,