From 505d6cb6ef0d0f82cc8695872d2fe776dd788c04 Mon Sep 17 00:00:00 2001 From: guslegend <1670547022@qq.com> Date: Sun, 24 May 2026 12:26:25 +0800 Subject: [PATCH] fix(harness): normalize memory glob paths --- .../agent/memory/MemoryConsolidator.java | 41 ++++++------ .../agent/workspace/WorkspaceManager.java | 49 ++++++++++++-- .../MemoryConsolidatorFilesystemTest.java | 67 +++++++++++-------- 3 files changed, 106 insertions(+), 51 deletions(-) diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/memory/MemoryConsolidator.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/memory/MemoryConsolidator.java index 324d6726a..d425f9043 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/memory/MemoryConsolidator.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/memory/MemoryConsolidator.java @@ -24,6 +24,7 @@ import io.agentscope.harness.agent.filesystem.model.FileInfo; import io.agentscope.harness.agent.filesystem.model.GlobResult; import io.agentscope.harness.agent.workspace.WorkspaceManager; +import java.nio.file.Path; import java.time.Instant; import java.util.ArrayList; import java.util.Comparator; @@ -194,27 +195,30 @@ private String readDailyEntries(Instant watermark) { return ""; } - List eligible = new ArrayList<>(); + List eligible = new ArrayList<>(); for (FileInfo fi : glob.matches()) { if (fi.isDirectory()) { continue; } - String name = fileName(fi.path()); + String rel = workspaceManager.toWorkspaceRelativePath(fi.path()); + if (rel.isBlank()) { + continue; + } + String name = fileName(rel); if (name.equals(STATE_FILE) || name.equals("archive") || !name.endsWith(".md")) { continue; } if (isModifiedAfter(fi, watermark)) { - eligible.add(fi); + eligible.add(rel); } } - eligible.sort(Comparator.comparing(fi -> fileName(fi.path()))); + eligible.sort(Comparator.comparing(MemoryConsolidator::fileName)); StringBuilder sb = new StringBuilder(); - for (FileInfo fi : eligible) { - String rel = toRelative(fi.path()); + for (String rel : eligible) { String content = workspaceManager.readManagedWorkspaceFileUtf8(rel); if (content != null && !content.isBlank()) { - sb.append("### ").append(fileName(fi.path())).append("\n"); + sb.append("### ").append(fileName(rel)).append("\n"); sb.append(content.strip()).append("\n\n"); } } @@ -238,23 +242,20 @@ private static String fileName(String path) { if (path == null || path.isEmpty()) { return ""; } + try { + Path p = Path.of(path); + Path name = p.getFileName(); + if (name != null) { + return name.toString(); + } + } catch (Exception ignored) { + // Fall through to string-based parsing. + } String stripped = path.endsWith("/") ? path.substring(0, path.length() - 1) : path; - int idx = stripped.lastIndexOf('/'); + int idx = Math.max(stripped.lastIndexOf('/'), stripped.lastIndexOf('\\')); return idx >= 0 ? stripped.substring(idx + 1) : stripped; } - /** - * Converts an absolute filesystem path (e.g. {@code /memory/2025-01-01.md}) to a - * workspace-relative path ({@code memory/2025-01-01.md}) for use with - * {@link WorkspaceManager#readManagedWorkspaceFileUtf8}. - */ - private static String toRelative(String path) { - if (path == null) { - return ""; - } - return path.startsWith("/") ? path.substring(1) : path; - } - private void writeConsolidatedMemory(String content) { workspaceManager.writeUtf8WorkspaceRelative("MEMORY.md", content); } diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/workspace/WorkspaceManager.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/workspace/WorkspaceManager.java index 28e8e005b..1c1ed69c4 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/workspace/WorkspaceManager.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/workspace/WorkspaceManager.java @@ -149,6 +149,44 @@ public Path getWorkspace() { return workspace; } + /** + * Normalizes a filesystem-reported path to a workspace-relative path when possible. + * + *

This handles three common cases: + *

+ */ + public String toWorkspaceRelativePath(String path) { + if (path == null || path.isBlank()) { + return ""; + } + + String normalized = path.strip().replace('\\', '/'); + Path workspaceRoot = workspace.toAbsolutePath().normalize(); + try { + Path candidate = Path.of(path).normalize(); + if (candidate.isAbsolute()) { + Path absoluteCandidate = candidate.toAbsolutePath().normalize(); + if (absoluteCandidate.startsWith(workspaceRoot)) { + return workspaceRoot + .relativize(absoluteCandidate) + .toString() + .replace('\\', '/'); + } + } + } catch (Exception ignored) { + // Fall through to string-based normalization. + } + + while (normalized.startsWith("/")) { + normalized = normalized.substring(1); + } + return normalized; + } + /** Reads AGENTS.md content, returns empty string if not found. */ public String readAgentsMd() { return readWithOverride(AGENTS_MD); @@ -172,7 +210,7 @@ public String readManagedWorkspaceFileUtf8(String relativePath) { if (relativePath == null || relativePath.isBlank()) { return ""; } - String normalized = normalizeRelativePath(relativePath); + String normalized = toWorkspaceRelativePath(relativePath); if (normalized.isEmpty()) { return ""; } @@ -204,7 +242,10 @@ public List listKnowledgeFiles() { if (glob.isSuccess() && glob.matches() != null) { for (FileInfo fi : glob.matches()) { if (fi.path() != null && !fi.path().isBlank()) { - relativePaths.add(normalizeRelativePath(fi.path().trim())); + String rel = toWorkspaceRelativePath(fi.path().trim()); + if (!rel.isEmpty()) { + relativePaths.add(rel); + } } } } @@ -722,7 +763,7 @@ public List listMemoryFilePaths() { if (glob.isSuccess() && glob.matches() != null) { for (FileInfo fi : glob.matches()) { if (fi.path() != null && !fi.path().isBlank()) { - String rel = normalizeRelativePath(fi.path().trim()); + String rel = toWorkspaceRelativePath(fi.path().trim()); if (!rel.isEmpty()) { paths.add(rel); } @@ -759,7 +800,7 @@ public List listSessionLogFiles() { if (glob.isSuccess() && glob.matches() != null) { for (FileInfo fi : glob.matches()) { if (fi.path() != null && !fi.path().isBlank()) { - String rel = normalizeRelativePath(fi.path().trim()); + String rel = toWorkspaceRelativePath(fi.path().trim()); if (!rel.isEmpty()) { paths.add(rel); } diff --git a/agentscope-harness/src/test/java/io/agentscope/harness/agent/memory/MemoryConsolidatorFilesystemTest.java b/agentscope-harness/src/test/java/io/agentscope/harness/agent/memory/MemoryConsolidatorFilesystemTest.java index 6820f73c6..751726982 100644 --- a/agentscope-harness/src/test/java/io/agentscope/harness/agent/memory/MemoryConsolidatorFilesystemTest.java +++ b/agentscope-harness/src/test/java/io/agentscope/harness/agent/memory/MemoryConsolidatorFilesystemTest.java @@ -18,7 +18,15 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; - +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyList; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import io.agentscope.core.message.TextBlock; +import io.agentscope.core.model.ChatResponse; +import io.agentscope.core.model.Model; +import io.agentscope.harness.agent.filesystem.local.LocalFilesystem; import io.agentscope.harness.agent.filesystem.remote.RemoteFilesystem; import io.agentscope.harness.agent.store.InMemoryStore; import io.agentscope.harness.agent.workspace.WorkspaceManager; @@ -29,11 +37,11 @@ import java.util.Map; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import reactor.core.publisher.Flux; /** * Verifies that {@link MemoryConsolidator} reads daily ledgers and writes watermark / MEMORY.md - * entirely through {@link io.agentscope.harness.agent.filesystem.AbstractFilesystem}, making it - * backend-agnostic. + * through the filesystem layer. */ class MemoryConsolidatorFilesystemTest { @@ -50,10 +58,6 @@ private static void seedStoreFile( store.put(ns, path, value); } - // ====================================================================== - // readWatermark: returns EPOCH when state file absent - // ====================================================================== - @Test void readWatermark_returnsEpochWhenStateAbsent(@TempDir Path tmp) { InMemoryStore store = new InMemoryStore(); @@ -66,10 +70,6 @@ void readWatermark_returnsEpochWhenStateAbsent(@TempDir Path tmp) { assertEquals(Instant.EPOCH, consolidator.readWatermark()); } - // ====================================================================== - // readWatermark / writeWatermark round-trip through filesystem - // ====================================================================== - @Test void watermark_roundTripThroughFilesystem(@TempDir Path tmp) { InMemoryStore store = new InMemoryStore(); @@ -85,10 +85,6 @@ void watermark_roundTripThroughFilesystem(@TempDir Path tmp) { assertEquals(ts, consolidator.readWatermark()); } - // ====================================================================== - // readWatermark: no local file is touched — only the filesystem - // ====================================================================== - @Test void watermark_doesNotCreateLocalFile(@TempDir Path tmp) { InMemoryStore store = new InMemoryStore(); @@ -101,28 +97,34 @@ void watermark_doesNotCreateLocalFile(@TempDir Path tmp) { Instant ts = Instant.now(); wsm.writeUtf8WorkspaceRelative(MemoryConsolidator.STATE_REL_PATH, ts.toString()); - // local disk must NOT have the state file — it lives only in the store Path localState = tmp.resolve("memory").resolve(MemoryConsolidator.STATE_FILE); assertFalse( Files.exists(localState), "state file should not be written to local disk when using RemoteFilesystem"); - // but consolidator reads it correctly from the store assertEquals(ts, consolidator.readWatermark()); } - // ====================================================================== - // STATE_FILE constant is preserved - // ====================================================================== - @Test void stateFileRelPath_matchesConstant() { assertEquals("memory/" + MemoryConsolidator.STATE_FILE, MemoryConsolidator.STATE_REL_PATH); } - // ====================================================================== - // Local filesystem (no store) — watermark uses local disk via WorkspaceManager - // ====================================================================== + @Test + void consolidate_readsRootDailyLedgerAndWritesMemoryMd(@TempDir Path tmp) throws Exception { + LocalFilesystem fs = new LocalFilesystem(tmp); + WorkspaceManager wsm = new WorkspaceManager(tmp, fs); + + Path memoryDir = Files.createDirectories(tmp.resolve("memory")); + Files.writeString(memoryDir.resolve("2026-05-20.md"), "root daily entry"); + + MemoryConsolidator consolidator = new MemoryConsolidator(wsm, stubModel("updated memory")); + + consolidator.consolidate().block(); + + assertEquals("updated memory", wsm.readMemoryMd()); + assertTrue(consolidator.readWatermark().isAfter(Instant.EPOCH)); + } @Test void watermark_localFallback_whenNoFilesystem(@TempDir Path tmp) throws Exception { @@ -130,19 +132,30 @@ void watermark_localFallback_whenNoFilesystem(@TempDir Path tmp) throws Exceptio MemoryConsolidator consolidator = new MemoryConsolidator(wsm, null); - // No file → EPOCH assertEquals(Instant.EPOCH, consolidator.readWatermark()); - // Write via WorkspaceManager (falls to local disk) Instant ts = Instant.parse("2025-03-10T09:00:00Z"); wsm.writeUtf8WorkspaceRelative(MemoryConsolidator.STATE_REL_PATH, ts.toString()); assertEquals(ts, consolidator.readWatermark()); - // Verify the local file actually exists Path localState = tmp.resolve("memory").resolve(MemoryConsolidator.STATE_FILE); assertTrue( Files.exists(localState), "state file should be written to local disk when no filesystem is configured"); } + + private static Model stubModel(String assistantText) { + Model model = mock(Model.class); + when(model.getModelName()).thenReturn("stub-model"); + ChatResponse chunk = + new ChatResponse( + "stub-id", + List.of(TextBlock.builder().text(assistantText).build()), + null, + Map.of(), + "stop"); + when(model.stream(anyList(), any(), any())).thenReturn(Flux.just(chunk)); + return model; + } }