feat(tool): surface execute_shell_command artifacts; fix download filename

This commit is contained in:
matevip 2026-06-30 14:01:48 +08:00
parent 5cf1c46dd4
commit 6854f8cc44
5 changed files with 261 additions and 179 deletions

View File

@ -20,6 +20,7 @@ import vip.mate.skill.runtime.SkillScriptExecutionService;
import vip.mate.skill.runtime.model.ResolvedSkill;
import vip.mate.skill.secret.SkillSecretService;
import vip.mate.tool.document.GeneratedFileCache;
import vip.mate.tool.document.WorkspaceArtifactSurfacer;
import vip.mate.tool.guard.WorkspacePathGuard;
import java.nio.file.Files;
@ -29,8 +30,6 @@ import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;
/**
* Built-in tool: execute LLM-generated source code inline.
@ -63,12 +62,6 @@ public class CodeExecuteTool {
private final ObjectMapper objectMapper;
private final GeneratedFileCache generatedFileCache;
/** Artifact-surfacing bounds: keep heap and noise in check (best-effort feature). */
private static final int ARTIFACT_SCAN_DEPTH = 4;
private static final int MAX_ARTIFACTS = 8;
private static final long MAX_ARTIFACT_BYTES = 20L * 1024 * 1024;
private static final long MAX_TOTAL_ARTIFACT_BYTES = 48L * 1024 * 1024;
@Lazy
@Autowired
private AgentBindingResolver agentBindingResolver;
@ -168,7 +161,7 @@ public class CodeExecuteTool {
// Surface any files the run wrote as one-click downloads so the user can
// grab generated artifacts (xlsx / csv / images / ) without the model
// having to call send_file or echo a server path.
List<String> fileLinks = collectArtifactLinks(workingDir, runStart, ctx);
List<String> fileLinks = WorkspaceArtifactSurfacer.collect(generatedFileCache, workingDir, runStart, ctx);
return formatResult(result, fileLinks);
} catch (Exception e) {
log.error("[CodeExecute] Execution failed: {}", e.getMessage());
@ -176,89 +169,6 @@ public class CodeExecuteTool {
}
}
/**
* Best-effort: register files created or modified in {@code workingDir} during
* this run into the generated-file cache and return their download links in the
* {@code [name](url)} markdown form the chat layer scans for. Never throws a
* failure here must not fail the code run.
*/
List<String> collectArtifactLinks(Path workingDir, long sinceMillis, @Nullable ToolContext ctx) {
if (workingDir == null || !Files.isDirectory(workingDir)) {
return List.of();
}
List<String> links = new ArrayList<>();
long totalBytes = 0L;
try (Stream<Path> walk = Files.walk(workingDir, ARTIFACT_SCAN_DEPTH)) {
List<Path> candidates = walk
.filter(Files::isRegularFile)
.filter(p -> !isNoiseArtifact(p))
.filter(p -> modifiedSince(p, sinceMillis))
.limit(200)
.toList();
for (Path p : candidates) {
if (links.size() >= MAX_ARTIFACTS) {
break;
}
try {
long size = Files.size(p);
if (size <= 0 || size > MAX_ARTIFACT_BYTES || totalBytes + size > MAX_TOTAL_ARTIFACT_BYTES) {
continue;
}
byte[] bytes = Files.readAllBytes(p);
totalBytes += size;
String name = p.getFileName().toString();
String id = generatedFileCache.put(bytes, name, probeMime(p, name));
links.add("[" + name + "](" + generatedFileCache.downloadUrl(id, ctx) + ")");
} catch (Exception perFile) {
log.debug("[CodeExecute] skip artifact {}: {}", p, perFile.getMessage());
}
}
} catch (Exception e) {
log.debug("[CodeExecute] artifact scan failed for {}: {}", workingDir, e.getMessage());
}
return links;
}
private static boolean modifiedSince(Path p, long sinceMillis) {
try {
// 1s slack absorbs filesystem mtime granularity.
return Files.getLastModifiedTime(p).toMillis() >= sinceMillis - 1000L;
} catch (Exception e) {
return false;
}
}
/** Skip hidden files, dependency/cache dirs, and obvious scratch/log files. */
private static boolean isNoiseArtifact(Path p) {
for (Path seg : p) {
String s = seg.toString();
if (s.startsWith(".") || s.equals("__pycache__") || s.equals("node_modules")) {
return true;
}
}
String name = p.getFileName().toString().toLowerCase();
return name.endsWith(".pyc") || name.endsWith(".tmp") || name.endsWith(".lock")
|| name.endsWith(".log") || name.endsWith(".class");
}
private static String probeMime(Path p, String name) {
try {
String mime = Files.probeContentType(p);
if (mime != null && !mime.isBlank()) {
return mime;
}
} catch (Exception ignore) {
// fall through to extension default
}
String lower = name.toLowerCase();
if (lower.endsWith(".xlsx")) return "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet";
if (lower.endsWith(".docx")) return "application/vnd.openxmlformats-officedocument.wordprocessingml.document";
if (lower.endsWith(".pptx")) return "application/vnd.openxmlformats-officedocument.presentationml.presentation";
if (lower.endsWith(".csv")) return "text/csv";
if (lower.endsWith(".pdf")) return "application/pdf";
return "application/octet-stream";
}
/**
* Decode the JSON-encoded {@code args} string into a positional argument list,
* mirroring {@code SkillScriptTool.normalizeArgs}: a JSON array becomes one
@ -293,13 +203,14 @@ public class CodeExecuteTool {
return List.of(trimmed);
}
private String formatResult(SkillScriptExecutionService.ScriptResult result, List<String> fileLinks) {
String formatResult(SkillScriptExecutionService.ScriptResult result, List<String> fileLinks) {
// generatedFiles carries [name](url) markdown so the chat layer surfaces the
// artifacts as one-click downloads, and the model can echo them to the user.
// It's a single JSON *string* (links joined by newlines), not an array a
// JSON array's own '[' sits adjacent to the markdown '[' and the link-
// extraction regex would then capture '"[name' as the filename.
String filesField = (fileLinks == null || fileLinks.isEmpty()) ? "" :
",\n \"generatedFiles\": ["
+ fileLinks.stream().map(this::jsonEscape).collect(Collectors.joining(", "))
+ "]";
",\n \"generatedFiles\": " + jsonEscape(String.join("\n", fileLinks));
return String.format(
"{\n \"exitCode\": %d,\n \"stdout\": %s,\n \"stderr\": %s%s\n}",
result.getExitCode(),

View File

@ -8,6 +8,8 @@ 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.tool.document.GeneratedFileCache;
import vip.mate.tool.document.WorkspaceArtifactSurfacer;
import java.io.IOException;
import java.io.InputStream;
@ -15,6 +17,7 @@ import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.util.List;
import java.util.Locale;
import java.util.concurrent.TimeUnit;
import java.util.function.Predicate;
@ -41,6 +44,7 @@ import java.util.function.Predicate;
public class ShellExecuteTool {
private final vip.mate.i18n.I18nService i18n;
private final GeneratedFileCache generatedFileCache;
private static final int DEFAULT_TIMEOUT_SECONDS = 60;
private static final int MAX_OUTPUT_BYTES = 10_000;
@ -50,7 +54,8 @@ public class ShellExecuteTool {
@vip.mate.tool.ConcurrencyUnsafe("shell command execution can mutate global state in ways the executor can't reason about")
@Tool(description = "Execute a shell command on the local server. For running system commands, viewing files, running scripts. "
+ "Uses cmd.exe on Windows, /bin/sh on Linux/macOS. "
+ "Dangerous operations trigger security approval. Returns structured result with exitCode, stdout, stderr, timedOut.")
+ "Dangerous operations trigger security approval. Returns structured result with exitCode, stdout, stderr, timedOut, "
+ "and (when the command wrote files) a generatedFiles string of [name](url) download links — echo them so the user can download what you produced.")
public String execute_shell_command(
@ToolParam(description = "Shell command to execute") String command,
@ToolParam(description = "Timeout in seconds, default 60", required = false) Integer timeoutSeconds,
@ -107,6 +112,7 @@ public class ShellExecuteTool {
pb.redirectOutput(stdoutFile.toFile());
pb.redirectError(stderrFile.toFile());
long runStart = System.currentTimeMillis();
Process process = pb.start();
boolean completed = process.waitFor(timeout, TimeUnit.SECONDS);
@ -130,6 +136,14 @@ public class ShellExecuteTool {
result.set("stdout", stdout);
result.set("stderr", stderr);
result.set("timedOut", false);
// Surface files the command wrote as one-click downloads (same path
// as execute_code). A single newline-joined string, not a JSON array,
// so the link-extraction regex captures the clean filename.
java.nio.file.Path workingDir = vip.mate.tool.guard.WorkspacePathGuard.getWorkingDirectory(ctx);
List<String> fileLinks = WorkspaceArtifactSurfacer.collect(generatedFileCache, workingDir, runStart, ctx);
if (!fileLinks.isEmpty()) {
result.set("generatedFiles", String.join("\n", fileLinks));
}
}
} catch (Exception e) {

View File

@ -0,0 +1,121 @@
package vip.mate.tool.document;
import lombok.extern.slf4j.Slf4j;
import org.springframework.ai.chat.model.ToolContext;
import org.springframework.lang.Nullable;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.List;
import java.util.stream.Stream;
/**
* Turns files that a tool run wrote into the working directory into one-click
* download links, so a user can grab generated artifacts (xlsx / csv / images /
* ) without the model having to call {@code send_file} or echo a server path.
*
* <p>Each returned entry is a {@code [name](url)} markdown link backed by
* {@link GeneratedFileCache} (7-day, disk-persisted, served by
* {@code GeneratedFileController}). The chat layer already scans tool results for
* exactly this shape and surfaces them as downloads, so callers just need to put
* the links somewhere in their result payload.
*
* <p>Shared by {@code execute_code} and {@code execute_shell_command}. Best-effort
* throughout surfacing a download must never fail the tool run.
*/
@Slf4j
public final class WorkspaceArtifactSurfacer {
private static final int SCAN_DEPTH = 4;
private static final int MAX_ARTIFACTS = 8;
private static final int MAX_SCAN_CANDIDATES = 200;
private static final long MAX_ARTIFACT_BYTES = 20L * 1024 * 1024;
private static final long MAX_TOTAL_ARTIFACT_BYTES = 48L * 1024 * 1024;
private WorkspaceArtifactSurfacer() {}
/**
* Register files created or modified in {@code workingDir} at or after
* {@code sinceMillis} into the cache and return their download links.
* Returns an empty list when there is no persistent working dir (e.g. a
* private scratch dir that gets deleted after the run).
*/
public static List<String> collect(GeneratedFileCache cache, @Nullable Path workingDir,
long sinceMillis, @Nullable ToolContext ctx) {
if (cache == null || workingDir == null || !Files.isDirectory(workingDir)) {
return List.of();
}
List<String> links = new ArrayList<>();
long totalBytes = 0L;
try (Stream<Path> walk = Files.walk(workingDir, SCAN_DEPTH)) {
List<Path> candidates = walk
.filter(Files::isRegularFile)
.filter(p -> !isNoise(p))
.filter(p -> modifiedSince(p, sinceMillis))
.limit(MAX_SCAN_CANDIDATES)
.toList();
for (Path p : candidates) {
if (links.size() >= MAX_ARTIFACTS) {
break;
}
try {
long size = Files.size(p);
if (size <= 0 || size > MAX_ARTIFACT_BYTES || totalBytes + size > MAX_TOTAL_ARTIFACT_BYTES) {
continue;
}
byte[] bytes = Files.readAllBytes(p);
totalBytes += size;
String name = p.getFileName().toString();
String id = cache.put(bytes, name, probeMime(p, name));
links.add("[" + name + "](" + cache.downloadUrl(id, ctx) + ")");
} catch (Exception perFile) {
log.debug("[ArtifactSurfacer] skip {}: {}", p, perFile.getMessage());
}
}
} catch (Exception e) {
log.debug("[ArtifactSurfacer] scan failed for {}: {}", workingDir, e.getMessage());
}
return links;
}
private static boolean modifiedSince(Path p, long sinceMillis) {
try {
// 1s slack absorbs filesystem mtime granularity.
return Files.getLastModifiedTime(p).toMillis() >= sinceMillis - 1000L;
} catch (Exception e) {
return false;
}
}
/** Skip hidden files, dependency/cache dirs, and obvious scratch/log files. */
private static boolean isNoise(Path p) {
for (Path seg : p) {
String s = seg.toString();
if (s.startsWith(".") || s.equals("__pycache__") || s.equals("node_modules")) {
return true;
}
}
String name = p.getFileName().toString().toLowerCase();
return name.endsWith(".pyc") || name.endsWith(".tmp") || name.endsWith(".lock")
|| name.endsWith(".log") || name.endsWith(".class");
}
private static String probeMime(Path p, String name) {
try {
String mime = Files.probeContentType(p);
if (mime != null && !mime.isBlank()) {
return mime;
}
} catch (Exception ignore) {
// fall through to extension default
}
String lower = name.toLowerCase();
if (lower.endsWith(".xlsx")) return "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet";
if (lower.endsWith(".docx")) return "application/vnd.openxmlformats-officedocument.wordprocessingml.document";
if (lower.endsWith(".pptx")) return "application/vnd.openxmlformats-officedocument.presentationml.presentation";
if (lower.endsWith(".csv")) return "text/csv";
if (lower.endsWith(".pdf")) return "application/pdf";
return "application/octet-stream";
}
}

View File

@ -1,107 +1,51 @@
package vip.mate.tool.builtin;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import vip.mate.tool.document.GeneratedFileCache;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Comparator;
import java.util.ArrayList;
import java.util.List;
import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* After a code run, files it wrote to the working dir should surface as
* one-click downloads (issue #191): the tool registers them in the
* generated-file cache and returns {@code [name](url)} links the chat layer
* already scans for. Pre-existing files, scratch/noise files, and oversized
* files must be left out.
* execute_code embeds artifact download links into its result; the chat layer
* then extracts them (issue #191). This pins the link shape so a JSON-array
* regression can't sneak back: an array's own '[' sits next to the markdown '['
* and the extractor would capture '"[name' as the filename.
*/
class CodeExecuteToolArtifactTest {
// Mirror of ChatController.GENERATED_FILE_LINK_PATTERN so we assert the links
// are in the exact shape the chat layer extracts.
// Mirror of ChatController.GENERATED_FILE_LINK_PATTERN.
private static final Pattern LINK = Pattern.compile(
"\\[([^\\]]+)\\]\\(((?:https?://[^/\\s)\\]]+)?/api/v1/files/generated/[A-Za-z0-9-]+)\\)");
private Path tmp;
private Path cacheDir;
@Test
@DisplayName("formatResult embeds links so extraction yields the clean filename, not '\"[name'")
void formatResultExtractsCleanFilename() {
CodeExecuteTool tool = new CodeExecuteTool(null, null, null, null, null);
var result = vip.mate.skill.runtime.SkillScriptExecutionService.ScriptResult.error(0, "");
String out = tool.formatResult(result, List.of(
"[report.csv](http://localhost:18088/api/v1/files/generated/abc-123)",
"[data.xlsx](http://localhost:18088/api/v1/files/generated/def-456)"));
private CodeExecuteTool newTool(GeneratedFileCache cache) {
// Only generatedFileCache is exercised here; the other collaborators are
// unused by collectArtifactLinks.
return new CodeExecuteTool(null, null, null, null, cache);
}
@AfterEach
void cleanup() throws Exception {
for (Path root : new Path[]{tmp, cacheDir}) {
if (root != null && Files.exists(root)) {
try (var s = Files.walk(root)) {
s.sorted(Comparator.reverseOrder()).forEach(p -> {
try { Files.deleteIfExists(p); } catch (Exception ignore) { }
});
}
}
List<String> names = new ArrayList<>();
Matcher m = LINK.matcher(out);
while (m.find()) {
names.add(m.group(1));
}
assertEquals(List.of("report.csv", "data.xlsx"), names,
"extracted filenames must be clean, full result was: " + out);
}
@Test
@DisplayName("Files written during the run surface as download links; old/noise files do not")
void surfacesNewlyWrittenFiles() throws Exception {
tmp = Files.createTempDirectory("codeexec-artifacts-");
cacheDir = Files.createTempDirectory("codeexec-cache-");
GeneratedFileCache cache = new GeneratedFileCache(cacheDir);
// Pre-existing file, modified well before the run window must be ignored.
Path old = Files.write(tmp.resolve("old.txt"), "old".getBytes());
Files.setLastModifiedTime(old, java.nio.file.attribute.FileTime.fromMillis(1_000L));
long runStart = System.currentTimeMillis();
Thread.sleep(5);
// Files produced "by the run".
byte[] xlsx = "PK fake-xlsx-bytes".getBytes();
Files.write(tmp.resolve("report.xlsx"), xlsx);
Files.write(tmp.resolve("data.csv"), "a,b\n1,2\n".getBytes());
// Noise that must be filtered out.
Files.write(tmp.resolve("scratch.tmp"), "x".getBytes());
Files.write(tmp.resolve(".hidden"), "x".getBytes());
List<String> links = newTool(cache).collectArtifactLinks(tmp, runStart, null);
// report.xlsx + data.csv only.
assertEquals(2, links.size(), "expected the two real artifacts, got: " + links);
assertTrue(links.stream().anyMatch(l -> l.contains("report.xlsx")));
assertTrue(links.stream().anyMatch(l -> l.contains("data.csv")));
assertFalse(links.stream().anyMatch(l -> l.contains("old.txt")), "pre-existing file must not surface");
assertFalse(links.stream().anyMatch(l -> l.contains("scratch.tmp")), ".tmp must not surface");
assertFalse(links.stream().anyMatch(l -> l.contains(".hidden")), "hidden file must not surface");
// Each link is in the exact shape the chat layer extracts, and the bytes
// round-trip through the cache (so the download endpoint can serve them).
for (String link : links) {
Matcher m = LINK.matcher(link);
assertTrue(m.matches(), "link not in extractable form: " + link);
String id = m.group(2).substring(m.group(2).lastIndexOf('/') + 1);
Optional<GeneratedFileCache.Entry> entry = cache.get(id);
assertTrue(entry.isPresent(), "cached artifact must be retrievable: " + link);
}
}
@Test
@DisplayName("Null / non-existent working dir yields no links and does not throw")
void nullWorkingDirIsSafe() throws Exception {
cacheDir = Files.createTempDirectory("codeexec-cache-");
CodeExecuteTool tool = newTool(new GeneratedFileCache(cacheDir));
assertTrue(tool.collectArtifactLinks(null, 0L, null).isEmpty());
assertTrue(tool.collectArtifactLinks(Path.of("/no/such/dir/xyz"), 0L, null).isEmpty());
@DisplayName("No artifacts → no generatedFiles field")
void noArtifactsNoField() {
CodeExecuteTool tool = new CodeExecuteTool(null, null, null, null, null);
var result = vip.mate.skill.runtime.SkillScriptExecutionService.ScriptResult.error(0, "ok");
String out = tool.formatResult(result, List.of());
assertEquals(false, out.contains("generatedFiles"), out);
}
}

View File

@ -0,0 +1,92 @@
package vip.mate.tool.document;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileTime;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* Files a tool run writes to the working dir should surface as one-click
* downloads (issue #191): registered in the generated-file cache and returned as
* {@code [name](url)} links the chat layer scans for. Pre-existing, scratch, and
* hidden files are excluded.
*/
class WorkspaceArtifactSurfacerTest {
private static final Pattern LINK = Pattern.compile(
"\\[([^\\]]+)\\]\\(((?:https?://[^/\\s)\\]]+)?/api/v1/files/generated/[A-Za-z0-9-]+)\\)");
private Path tmp;
private Path cacheDir;
@AfterEach
void cleanup() throws Exception {
for (Path root : new Path[]{tmp, cacheDir}) {
if (root != null && Files.exists(root)) {
try (var s = Files.walk(root)) {
s.sorted(Comparator.reverseOrder()).forEach(p -> {
try { Files.deleteIfExists(p); } catch (Exception ignore) { }
});
}
}
}
}
@Test
@DisplayName("Files written during the run surface as links; old/noise files do not")
void surfacesNewlyWrittenFiles() throws Exception {
tmp = Files.createTempDirectory("artifacts-");
cacheDir = Files.createTempDirectory("cache-");
GeneratedFileCache cache = new GeneratedFileCache(cacheDir);
Path old = Files.write(tmp.resolve("old.txt"), "old".getBytes());
Files.setLastModifiedTime(old, FileTime.fromMillis(1_000L));
long runStart = System.currentTimeMillis();
Thread.sleep(5);
Files.write(tmp.resolve("report.xlsx"), "PK fake-xlsx".getBytes());
Files.write(tmp.resolve("data.csv"), "a,b\n1,2\n".getBytes());
Files.write(tmp.resolve("scratch.tmp"), "x".getBytes());
Files.write(tmp.resolve(".hidden"), "x".getBytes());
List<String> links = WorkspaceArtifactSurfacer.collect(cache, tmp, runStart, null);
assertEquals(2, links.size(), "expected the two real artifacts, got: " + links);
assertTrue(links.stream().anyMatch(l -> l.contains("report.xlsx")));
assertTrue(links.stream().anyMatch(l -> l.contains("data.csv")));
assertFalse(links.stream().anyMatch(l -> l.contains("old.txt")), "pre-existing file must not surface");
assertFalse(links.stream().anyMatch(l -> l.contains(".tmp")), ".tmp must not surface");
assertFalse(links.stream().anyMatch(l -> l.contains(".hidden")), "hidden file must not surface");
for (String link : links) {
Matcher m = LINK.matcher(link);
assertTrue(m.matches(), "link not in extractable form: " + link);
String id = m.group(2).substring(m.group(2).lastIndexOf('/') + 1);
Optional<GeneratedFileCache.Entry> entry = cache.get(id);
assertTrue(entry.isPresent(), "cached artifact must be retrievable: " + link);
}
}
@Test
@DisplayName("Null / non-existent working dir and null cache are safe no-ops")
void edgeCasesAreSafe() throws Exception {
cacheDir = Files.createTempDirectory("cache-");
GeneratedFileCache cache = new GeneratedFileCache(cacheDir);
assertTrue(WorkspaceArtifactSurfacer.collect(cache, null, 0L, null).isEmpty());
assertTrue(WorkspaceArtifactSurfacer.collect(cache, Path.of("/no/such/dir/xyz"), 0L, null).isEmpty());
assertTrue(WorkspaceArtifactSurfacer.collect(null, Path.of("."), 0L, null).isEmpty());
}
}