From 7c5ca240d2da13351236b17e18f6c4b320a23f9e Mon Sep 17 00:00:00 2001 From: zerone0x Date: Fri, 27 Mar 2026 00:11:27 +0800 Subject: [PATCH] fix: verify file edit was applied and return confirmation snippet The environment_file_edit tool could report success without verifying that the patch was actually applied to the file. This caused silent failures where the tool returned "edited successfully" but the file contents were unchanged. Changes: - After applying the patch, read the file back and compare against the expected content to ensure the edit was actually applied - Return a context snippet around the replacement text so the calling agent can confirm what changed - Change FileEdit to return a FileEditResult containing the verification snippet Fixes #320 Co-Authored-By: Claude Opus 4.6 --- environment/filesystem.go | 40 +++++++++++++++++++++++++++++++-------- mcpserver/tools.go | 9 +++++---- 2 files changed, 37 insertions(+), 12 deletions(-) diff --git a/environment/filesystem.go b/environment/filesystem.go index f1eee215..0fcb28e0 100644 --- a/environment/filesystem.go +++ b/environment/filesystem.go @@ -54,15 +54,21 @@ func (env *Environment) FileWrite(ctx context.Context, explanation, targetFile, return nil } -func (env *Environment) FileEdit(ctx context.Context, explanation, targetFile, search, replace, matchID string) error { +// FileEditResult contains information about a successful file edit +type FileEditResult struct { + // Snippet is a short context snippet around the replaced text + Snippet string +} + +func (env *Environment) FileEdit(ctx context.Context, explanation, targetFile, search, replace, matchID string) (*FileEditResult, error) { // Check if the file is within a submodule if err := env.validateNotSubmoduleFile(targetFile); err != nil { - return err + return nil, err } contents, err := env.container().File(targetFile).Contents(ctx) if err != nil { - return err + return nil, err } // Find all matches of the search text @@ -79,7 +85,7 @@ func (env *Environment) FileEdit(ctx context.Context, explanation, targetFile, s } if len(matches) == 0 { - return fmt.Errorf("search text not found in file %s", targetFile) + return nil, fmt.Errorf("search text not found in file %s", targetFile) } // If there are multiple matches and no matchID is provided, return an error with all matches @@ -95,7 +101,7 @@ func (env *Environment) FileEdit(ctx context.Context, explanation, targetFile, s matchDescriptions = append(matchDescriptions, fmt.Sprintf("Match %d (ID: %s):\n%s", i+1, id, context)) } - return fmt.Errorf("multiple matches found for search text in %s. Please specify which_match parameter with one of the following IDs:\n\n%s", + return nil, fmt.Errorf("multiple matches found for search text in %s. Please specify which_match parameter with one of the following IDs:\n\n%s", targetFile, strings.Join(matchDescriptions, "\n\n")) } @@ -115,7 +121,7 @@ func (env *Environment) FileEdit(ctx context.Context, explanation, targetFile, s } } if !found { - return fmt.Errorf("match ID %s not found", matchID) + return nil, fmt.Errorf("match ID %s not found", matchID) } } @@ -128,10 +134,28 @@ func (env *Environment) FileEdit(ctx context.Context, explanation, targetFile, s ctr := env.container() err = env.apply(ctx, ctr.WithDirectory(".", ctr.Directory(".").WithPatch(patch))) if err != nil { - return fmt.Errorf("failed applying file edit, skipping git propagation: %w", err) + return nil, fmt.Errorf("failed applying file edit, skipping git propagation: %w", err) + } + + // Verify the edit was actually applied by reading the file back + verifiedContents, err := env.container().File(targetFile).Contents(ctx) + if err != nil { + return nil, fmt.Errorf("failed to verify edit was applied: %w", err) } + + if verifiedContents != newContents { + return nil, fmt.Errorf("edit verification failed: file contents of %s after applying the patch do not match expected result", targetFile) + } + + // Generate a context snippet around the replacement for confirmation + replaceIndex := targetMatchIndex + if replaceIndex >= len(verifiedContents) { + replaceIndex = max(0, len(verifiedContents)-1) + } + snippet := getMatchContext(verifiedContents, replaceIndex) + env.Notes.Add("Edit %s", targetFile) - return nil + return &FileEditResult{Snippet: snippet}, nil } func (env *Environment) FileDelete(ctx context.Context, explanation, targetFile string) error { diff --git a/mcpserver/tools.go b/mcpserver/tools.go index 392322f0..ccd5a9e2 100644 --- a/mcpserver/tools.go +++ b/mcpserver/tools.go @@ -732,21 +732,22 @@ func createEnvironmentFileEditTool(singleTenant bool) *Tool { return nil, err } - if err := env.FileEdit(ctx, + result, err := env.FileEdit(ctx, request.GetString("explanation", ""), targetFile, search, replace, request.GetString("which_match", ""), - ); err != nil { - return mcp.NewToolResultErrorFromErr("failed to write file", err), nil + ) + if err != nil { + return mcp.NewToolResultErrorFromErr("failed to edit file", err), nil } if err := repo.UpdateFile(ctx, env, targetFile, request.GetString("explanation", "")); err != nil { return mcp.NewToolResultErrorFromErr("unable to update the environment", err), nil } - return mcp.NewToolResultText(fmt.Sprintf("file %s edited successfully and committed to container-use/%s remote ref", targetFile, env.ID)), nil + return mcp.NewToolResultText(fmt.Sprintf("file %s edited successfully and committed to container-use/%s remote ref\n\nVerified edit - snippet around replacement:\n%s", targetFile, env.ID, result.Snippet)), nil }, } }