From a4f746d7e32cc0a35e024287fe511da965b9df0b 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 --- 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 a322c616..0847856a 100644 --- a/environment/filesystem.go +++ b/environment/filesystem.go @@ -63,14 +63,20 @@ type FileEditRequest struct { MatchID string } -func (env *Environment) FileEdit(ctx context.Context, req FileEditRequest) 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, req FileEditRequest) (*FileEditResult, error) { if err := env.validateNotSubmoduleFile(req.TargetFile); err != nil { - return err + return nil, err } contents, err := env.container().File(req.TargetFile).Contents(ctx) if err != nil { - return err + return nil, err } matches := []int{} @@ -86,7 +92,7 @@ func (env *Environment) FileEdit(ctx context.Context, req FileEditRequest) error } if len(matches) == 0 { - return fmt.Errorf("search text not found in file %s", req.TargetFile) + return nil, fmt.Errorf("search text not found in file %s", req.TargetFile) } if len(matches) > 1 && req.MatchID == "" { @@ -97,7 +103,7 @@ func (env *Environment) FileEdit(ctx context.Context, req FileEditRequest) error 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", req.TargetFile, strings.Join(matchDescriptions, "\n\n")) } @@ -115,7 +121,7 @@ func (env *Environment) FileEdit(ctx context.Context, req FileEditRequest) error } } if !found { - return fmt.Errorf("match ID %s not found", req.MatchID) + return nil, fmt.Errorf("match ID %s not found", req.MatchID) } } @@ -125,10 +131,28 @@ func (env *Environment) FileEdit(ctx context.Context, req FileEditRequest) error 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(req.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", req.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", req.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 ba73195d..42691103 100644 --- a/mcpserver/tools.go +++ b/mcpserver/tools.go @@ -788,21 +788,22 @@ func createEnvironmentFileEditTool(singleTenant bool) *Tool { return nil, err } - if err := env.FileEdit(ctx, environment.FileEditRequest{ + result, err := env.FileEdit(ctx, environment.FileEditRequest{ Explanation: request.GetString("explanation", ""), TargetFile: targetFile, Search: search, Replace: replace, MatchID: 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 }, } }