Skip to content

修复 harness 中 memory glob 路径归一化问题 #1481

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
guslegend0510 wants to merge 4 commits into
base: main
Choose a base branch
from
Open

修复 harness 中 memory glob 路径归一化问题 #1481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
505d6cb
fix(harness): normalize memory glob paths
guslegend May 24, 2026
8ecc062
Merge upstream/main into fix/issue-1466
guslegend Jun 9, 2026
d0d8bf8
fix(harness): address path normalization review
guslegend Jun 10, 2026
71179af
Merge branch 'main' into fix/issue-1466
guslegend0510 Jun 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 22 additions & 20 deletions agentscope-harness/src/main/java/io/agentscope/harness/agent/memory/MemoryConsolidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
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.InvalidPathException;
import java.nio.file.Path;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Comparator;
Expand Down Expand Up @@ -214,27 +216,30 @@ private String readDailyEntries(RuntimeContext rc, Instant watermark) {
return "";
}

List<FileInfo> eligible = new ArrayList<>();
List<String> 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(rc, 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");
}
}
Expand All @@ -258,23 +263,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 (InvalidPathException 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(RuntimeContext rc, String content) {
workspaceManager.writeUtf8WorkspaceRelative(rc, "MEMORY.md", content);
}
Expand Down
58 changes: 51 additions & 7 deletions agentscope-harness/src/main/java/io/agentscope/harness/agent/workspace/WorkspaceManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.InvalidPathException;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.time.Duration;
Expand Down Expand Up @@ -116,6 +117,7 @@ public class WorkspaceManager implements AutoCloseable {
private final Map<String, ReentrantLock> pathLocks = new ConcurrentHashMap<>();

private final Path workspace;
private final Path workspaceRoot;
private final AbstractFilesystem filesystem;

/** Best-effort local file index; may be {@code null} if SQLite is unavailable. */
Expand Down Expand Up @@ -158,6 +160,7 @@ private WorkspaceManager(
NamespaceFactory namespaceFactory,
boolean ownsIndex) {
this.workspace = workspace;
this.workspaceRoot = workspace.toAbsolutePath().normalize();
this.filesystem = filesystem;
this.index = index;
this.namespaceFactory = namespaceFactory;
Expand Down Expand Up @@ -225,6 +228,43 @@ public Path getWorkspace() {
return workspace;
}

/**
* Normalizes a filesystem-reported path to a workspace-relative path when possible.
*
* <p>This handles three common cases:
* <ul>
* <li>already-relative workspace paths such as {@code memory/2026-05-20.md}</li>
* <li>virtual/remote paths that start with {@code /}</li>
* <li>local absolute paths that live under the workspace root</li>
* </ul>
*/
public String toWorkspaceRelativePath(String path) {
if (path == null || path.isBlank()) {
return "";
}

String normalized = path.strip().replace('\\', '/');
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 (InvalidPathException ignored) {
// Fall through to string-based normalization.
}

while (normalized.startsWith("/")) {
normalized = normalized.substring(1);
}
return normalized;
}

/**
* Resolves a workspace-relative path for runtime user data, applying namespace prefix.
* Use for paths that contain per-user data (sessions, tasks, memory).
Expand All @@ -233,14 +273,15 @@ public Path getWorkspace() {
* identity (user/session) drives the namespace, not a shared mutable reference.
*/
public Path resolveRuntimeDataPath(RuntimeContext rc, String relativePath) {
String normalized = normalizeRelativePath(relativePath);
if (namespaceFactory == null) {
return workspace.resolve(relativePath);
return workspace.resolve(normalized);
}
List<String> ns = namespaceFactory.getNamespace(rc != null ? rc : RuntimeContext.empty());
if (ns == null || ns.isEmpty()) {
return workspace.resolve(relativePath);
return workspace.resolve(normalized);
}
return workspace.resolve(String.join("/", ns)).resolve(relativePath);
return workspace.resolve(String.join("/", ns)).resolve(normalized);
}

/** Reads AGENTS.md content, returns empty string if not found. */
Expand All @@ -266,7 +307,7 @@ public String readManagedWorkspaceFileUtf8(RuntimeContext rc, String relativePat
if (relativePath == null || relativePath.isBlank()) {
return "";
}
String normalized = normalizeRelativePath(relativePath);
String normalized = toWorkspaceRelativePath(relativePath);
if (normalized.isEmpty()) {
return "";
}
Expand Down Expand Up @@ -298,7 +339,10 @@ public List<Path> listKnowledgeFiles(RuntimeContext rc) {
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);
}
}
}
}
Expand Down Expand Up @@ -946,7 +990,7 @@ public List<String> listMemoryFilePaths(RuntimeContext rc) {
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);
}
Expand Down Expand Up @@ -983,7 +1027,7 @@ public List<String> listSessionLogFiles(RuntimeContext rc) {
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);
}
Expand Down
118 changes: 59 additions & 59 deletions ...ss/src/test/java/io/agentscope/harness/agent/memory/MemoryConsolidatorFilesystemTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,16 @@
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.agent.RuntimeContext;
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.filesystem.remote.store.InMemoryStore;
import io.agentscope.harness.agent.workspace.WorkspaceManager;
Expand All @@ -30,123 +38,115 @@
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 {

private static void seedStoreFile(
InMemoryStore store, List<String> ns, String path, String content, Instant modifiedAt) {
Map<String, Object> value =
Map.of(
"content",
content,
"encoding",
"utf-8",
"modified_at",
modifiedAt.toString());
store.put(ns, path, value);
}

// ======================================================================
// readWatermark: returns EPOCH when state file absent
// ======================================================================
private static final RuntimeContext RC = RuntimeContext.empty();

@Test
void readWatermark_returnsEpochWhenStateAbsent(@TempDir Path tmp) throws Exception {
void readWatermark_returnsEpochWhenStateAbsent(@TempDir Path tmp) {
InMemoryStore store = new InMemoryStore();
List<String> ns = List.of("test-ns");
RemoteFilesystem fs = new RemoteFilesystem(store, ns);
try (WorkspaceManager wsm = new WorkspaceManager(tmp, fs)) {
MemoryConsolidator consolidator = new MemoryConsolidator(wsm, null);

assertEquals(Instant.EPOCH, consolidator.readWatermark(RuntimeContext.empty()));
assertEquals(Instant.EPOCH, consolidator.readWatermark(RC));
}
}

// ======================================================================
// readWatermark / writeWatermark round-trip through filesystem
// ======================================================================

@Test
void watermark_roundTripThroughFilesystem(@TempDir Path tmp) throws Exception {
void watermark_roundTripThroughFilesystem(@TempDir Path tmp) {
InMemoryStore store = new InMemoryStore();
List<String> ns = List.of("test-ns");
RemoteFilesystem fs = new RemoteFilesystem(store, ns);
try (WorkspaceManager wsm = new WorkspaceManager(tmp, fs)) {
MemoryConsolidator consolidator = new MemoryConsolidator(wsm, null);

Instant ts = Instant.parse("2025-06-15T12:00:00Z");
wsm.writeUtf8WorkspaceRelative(
RuntimeContext.empty(), MemoryConsolidator.STATE_REL_PATH, ts.toString());
wsm.writeUtf8WorkspaceRelative(RC, MemoryConsolidator.STATE_REL_PATH, ts.toString());

assertEquals(ts, consolidator.readWatermark(RuntimeContext.empty()));
assertEquals(ts, consolidator.readWatermark(RC));
}
}

// ======================================================================
// readWatermark: no local file is touched — only the filesystem
// ======================================================================

@Test
void watermark_doesNotCreateLocalFile(@TempDir Path tmp) throws Exception {
void watermark_doesNotCreateLocalFile(@TempDir Path tmp) {
InMemoryStore store = new InMemoryStore();
List<String> ns = List.of("test-ns");
RemoteFilesystem fs = new RemoteFilesystem(store, ns);
try (WorkspaceManager wsm = new WorkspaceManager(tmp, fs)) {
MemoryConsolidator consolidator = new MemoryConsolidator(wsm, null);

Instant ts = Instant.now();
wsm.writeUtf8WorkspaceRelative(
RuntimeContext.empty(), MemoryConsolidator.STATE_REL_PATH, ts.toString());
wsm.writeUtf8WorkspaceRelative(RC, 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(RuntimeContext.empty()));
assertEquals(ts, consolidator.readWatermark(RC));
}
}

// ======================================================================
// 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);
try (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(RC).block();

assertEquals("updated memory", wsm.readMemoryMd(RC));
assertTrue(consolidator.readWatermark(RC).isAfter(Instant.EPOCH));
}
}

@Test
void watermark_localFallback_whenNoFilesystem(@TempDir Path tmp) throws Exception {
WorkspaceManager wsm = new WorkspaceManager(tmp);
try (WorkspaceManager wsm = new WorkspaceManager(tmp)) {
MemoryConsolidator consolidator = new MemoryConsolidator(wsm, null);

MemoryConsolidator consolidator = new MemoryConsolidator(wsm, null);
assertEquals(Instant.EPOCH, consolidator.readWatermark(RC));

// No file → EPOCH
assertEquals(Instant.EPOCH, consolidator.readWatermark(RuntimeContext.empty()));
Instant ts = Instant.parse("2025-03-10T09:00:00Z");
wsm.writeUtf8WorkspaceRelative(RC, MemoryConsolidator.STATE_REL_PATH, ts.toString());

// Write via WorkspaceManager (falls to local disk)
Instant ts = Instant.parse("2025-03-10T09:00:00Z");
wsm.writeUtf8WorkspaceRelative(
RuntimeContext.empty(), MemoryConsolidator.STATE_REL_PATH, ts.toString());
assertEquals(ts, consolidator.readWatermark(RC));

assertEquals(ts, consolidator.readWatermark(RuntimeContext.empty()));
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");
}
}

// 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;
}
}
Loading
Loading