From 14df331ce6f4624d61910f03dfe2fbba49f07318 Mon Sep 17 00:00:00 2001 From: matevip Date: Fri, 21 Aug 2026 22:05:26 -0400 Subject: [PATCH] fix: resolve workspace context for file mutations (#617) --- .../vip/mate/tool/builtin/EditFileTool.java | 15 ++-- .../vip/mate/tool/builtin/WriteFileTool.java | 15 ++-- .../builtin/FileMutationToolContextTest.java | 75 +++++++++++++++++++ 3 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 mateclaw-server/src/test/java/vip/mate/tool/builtin/FileMutationToolContextTest.java diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/EditFileTool.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/EditFileTool.java index 8eed7643..c73fadd7 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/EditFileTool.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/EditFileTool.java @@ -3,14 +3,18 @@ package vip.mate.tool.builtin; import cn.hutool.json.JSONObject; import cn.hutool.json.JSONUtil; import lombok.extern.slf4j.Slf4j; +import org.springframework.ai.chat.model.ToolContext; import org.springframework.ai.tool.annotation.Tool; import org.springframework.ai.tool.annotation.ToolParam; +import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; +import vip.mate.i18n.I18nService; +import vip.mate.tool.ConcurrencyUnsafe; +import vip.mate.tool.guard.WorkspacePathGuard; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; /** * 内置工具：编辑文件（查找替换） @@ -31,9 +35,9 @@ import java.nio.file.Paths; @lombok.RequiredArgsConstructor public class EditFileTool { - private final vip.mate.i18n.I18nService i18n; + private final I18nService i18n; - @vip.mate.tool.ConcurrencyUnsafe("in-place file edit — must not race with reads/writes on the same path") + @ConcurrencyUnsafe("in-place file edit — must not race with reads/writes on the same path") @Tool(description = "Edit file content via find-and-replace. Finds exact match of old_text and replaces with new_text. " + "Returns structured JSON with filePath, replacements count. " + "May require user approval when security rules flag the edit. " @@ -42,7 +46,8 @@ public class EditFileTool { @ToolParam(description = "Absolute or relative file path") String filePath, @ToolParam(description = "Original text to find (exact match)") String oldText, @ToolParam(description = "Replacement text") String newText, - @ToolParam(description = "Replace all occurrences, default false (first only)", required = false) Boolean replaceAll) { + @ToolParam(description = "Replace all occurrences, default false (first only)", required = false) Boolean replaceAll, + @Nullable ToolContext ctx) { JSONObject result = new JSONObject(); result.set("filePath", filePath); @@ -63,7 +68,7 @@ public class EditFileTool { Path path; try { - path = vip.mate.tool.guard.WorkspacePathGuard.validatePath(filePath); + path = WorkspacePathGuard.validatePath(filePath, ctx); } catch (IllegalArgumentException e) { return errorResult(filePath, e.getMessage()); } diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/WriteFileTool.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/WriteFileTool.java index 8bd6ce5b..b7edc4e1 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/WriteFileTool.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/WriteFileTool.java @@ -3,14 +3,18 @@ package vip.mate.tool.builtin; import cn.hutool.json.JSONObject; import cn.hutool.json.JSONUtil; import lombok.extern.slf4j.Slf4j; +import org.springframework.ai.chat.model.ToolContext; import org.springframework.ai.tool.annotation.Tool; import org.springframework.ai.tool.annotation.ToolParam; +import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; +import vip.mate.i18n.I18nService; +import vip.mate.tool.ConcurrencyUnsafe; +import vip.mate.tool.guard.WorkspacePathGuard; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; /** * 内置工具：写入文件 @@ -31,15 +35,16 @@ import java.nio.file.Paths; @lombok.RequiredArgsConstructor public class WriteFileTool { - private final vip.mate.i18n.I18nService i18n; + private final I18nService i18n; - @vip.mate.tool.ConcurrencyUnsafe("file write — must serialize with reads/writes on overlapping paths") + @ConcurrencyUnsafe("file write — must serialize with reads/writes on overlapping paths") @Tool(description = "Write content to a file. Overwrites if exists, creates if not (auto-creates parent directories). " + "Returns structured JSON with filePath, bytesWritten. " + "May require user approval when security rules flag the write.") public String write_file( @ToolParam(description = "Absolute or relative file path") String filePath, - @ToolParam(description = "Content to write to the file") String content) { + @ToolParam(description = "Content to write to the file") String content, + @Nullable ToolContext ctx) { JSONObject result = new JSONObject(); result.set("filePath", filePath); @@ -54,7 +59,7 @@ public class WriteFileTool { Path path; try { - path = vip.mate.tool.guard.WorkspacePathGuard.validatePath(filePath); + path = WorkspacePathGuard.validatePath(filePath, ctx); } catch (IllegalArgumentException e) { return errorResult(filePath, e.getMessage()); } diff --git a/mateclaw-server/src/test/java/vip/mate/tool/builtin/FileMutationToolContextTest.java b/mateclaw-server/src/test/java/vip/mate/tool/builtin/FileMutationToolContextTest.java new file mode 100644 index 00000000..e52bb2a4 --- /dev/null +++ b/mateclaw-server/src/test/java/vip/mate/tool/builtin/FileMutationToolContextTest.java @@ -0,0 +1,75 @@ +package vip.mate.tool.builtin; + +import cn.hutool.json.JSONUtil; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import vip.mate.agent.context.ChatOrigin; +import vip.mate.i18n.I18nService; +import vip.mate.tool.guard.WorkspacePathGuard; + +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; + +/** + * Regression coverage for issue #617: file mutation tools must use the + * explicit Spring AI ToolContext instead of relying on legacy thread-local + * workspace state. + */ +class FileMutationToolContextTest { + + @AfterEach + void tearDown() { + WorkspacePathGuard.setDefaultRoot(null); + ToolExecutionContext.clear(); + } + + @Test + @DisplayName("write_file resolves relative paths against ToolContext workspace root (#617)") + void writeFileUsesToolContextWorkspaceRoot(@TempDir Path tempDir) throws Exception { + Path defaultRoot = Files.createDirectory(tempDir.resolve("default-root")); + Path contextRoot = Files.createDirectory(tempDir.resolve("context-root")); + WorkspacePathGuard.setDefaultRoot(defaultRoot.toString()); + WriteFileTool tool = new WriteFileTool(i18n()); + + String result = tool.write_file( + "deck.md", + "# Deck", + ChatOrigin.web("conv-617", "alice", 1L, contextRoot.toString()).toToolContext()); + + assertThat(JSONUtil.parseObj(result).getBool("error", false)).isFalse(); + assertThat(contextRoot.resolve("deck.md")).hasContent("# Deck"); + assertThat(defaultRoot.resolve("deck.md")).doesNotExist(); + } + + @Test + @DisplayName("edit_file resolves relative paths against ToolContext workspace root (#617)") + void editFileUsesToolContextWorkspaceRoot(@TempDir Path tempDir) throws Exception { + Path defaultRoot = Files.createDirectory(tempDir.resolve("default-root")); + Path contextRoot = Files.createDirectory(tempDir.resolve("context-root")); + WorkspacePathGuard.setDefaultRoot(defaultRoot.toString()); + Files.writeString(contextRoot.resolve("deck.md"), "old title", StandardCharsets.UTF_8); + EditFileTool tool = new EditFileTool(i18n()); + + String result = tool.edit_file( + "deck.md", + "old", + "new", + false, + ChatOrigin.web("conv-617", "alice", 1L, contextRoot.toString()).toToolContext()); + + assertThat(JSONUtil.parseObj(result).getBool("error", false)).isFalse(); + assertThat(contextRoot.resolve("deck.md")).hasContent("new title"); + assertThat(defaultRoot.resolve("deck.md")).doesNotExist(); + } + + private static I18nService i18n() { + return mock(I18nService.class, inv -> + "msg".equals(inv.getMethod().getName()) ? inv.getArgument(0) : null); + } +}