shumengya/sproutclaw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(agent): harden harness session semantics

Browse Source
This commit is contained in:
Mario Zechner
2026-05-16 00:32:16 +02:00
parent a8af0b5e99
commit 4f40f62b7b
23 changed files with 1112 additions and 873 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
packages/agent/test/harness/compaction.test.ts
View File

@@ -345,7 +345,7 @@ describe("harness compaction", () => {
const u3 = createMessageEntry(createUserMessage("user msg 3"), compaction1.id);
const a3 = createMessageEntry(createAssistantMessage("assistant msg 3", createMockUsage(8000, 2000)), u3.id);
const pathEntries = [u1, a1, u2, a2, compaction1, u3, a3];
const preparation = prepareCompaction(pathEntries, DEFAULT_COMPACTION_SETTINGS);
const preparation = getOrThrow(prepareCompaction(pathEntries, DEFAULT_COMPACTION_SETTINGS));
expect(preparation).toBeDefined();
expect(preparation?.previousSummary).toBe("First summary");
expect(preparation?.firstKeptEntryId).toBeTruthy();
@@ -365,11 +365,13 @@ describe("harness compaction", () => {
};
const u2 = createMessageEntry(createUserMessage("large turn"), compaction1.id);
const a2 = createMessageEntry(createAssistantMessage("large assistant message"), u2.id);
const preparation = prepareCompaction([u1, a1, compaction1, u2, a2], {
enabled: true,
reserveTokens: 100,
keepRecentTokens: 1,
});
const preparation = getOrThrow(
prepareCompaction([u1, a1, compaction1, u2, a2], {
enabled: true,
reserveTokens: 100,
keepRecentTokens: 1,
}),
);
expect(preparation).toMatchObject({ previousSummary: "First summary", isSplitTurn: true });
expect(preparation?.turnPrefixMessages.map((message) => message.role)).toEqual(["user"]);
@@ -398,19 +400,21 @@ describe("harness compaction", () => {
};
const user = createMessageEntry(createUserMessage("keep"), customMessage.id);
const assistant = createMessageEntry(createAssistantMessage("assistant"), user.id);
const preparation = prepareCompaction([branchSummary, customMessage, user, assistant], {
enabled: true,
reserveTokens: 100,
keepRecentTokens: 1,
});
const preparation = getOrThrow(
prepareCompaction([branchSummary, customMessage, user, assistant], {
enabled: true,
reserveTokens: 100,
keepRecentTokens: 1,
}),
);
expect(preparation?.messagesToSummarize.map((message) => message.role)).toEqual(["branchSummary", "custom"]);
});
it("does not prepare compaction when there is nothing valid to compact", () => {
const compaction = createCompactionEntry("already compacted", "entry-keep");
expect(prepareCompaction([compaction], DEFAULT_COMPACTION_SETTINGS)).toBeUndefined();
expect(prepareCompaction([], DEFAULT_COMPACTION_SETTINGS)).toBeUndefined();
expect(getOrThrow(prepareCompaction([compaction], DEFAULT_COMPACTION_SETTINGS))).toBeUndefined();
expect(getOrThrow(prepareCompaction([], DEFAULT_COMPACTION_SETTINGS))).toBeUndefined();
});
it("serializes conversation with truncated tool results", () => {
@@ -535,13 +539,6 @@ describe("harness compaction", () => {
abortedFaux.setResponses([fauxAssistantMessage("", { stopReason: "aborted", errorMessage: "stopped" })]);
const abortedResult = await generateSummary(messages, abortedModel, 2000, "test-key");
expect(abortedResult).toMatchObject({ ok: false, error: { code: "aborted", message: "stopped" } });
const missingProviderModel = { ...abortedModel, api: "missing-api" } as Model<string>;
const unknownResult = await generateSummary(messages, missingProviderModel, 2000, "test-key");
expect(unknownResult).toMatchObject({
ok: false,
error: { code: "unknown", message: "Summarization request failed" },
});
});
it("clamps compaction summary maxTokens to the model output cap", async () => {
@@ -650,12 +647,6 @@ describe("harness compaction", () => {
ok: false,
error: { code: "aborted", message: "prefix stopped" },
});
const missingProviderModel = { ...abortedModel, api: "missing-api" } as Model<string>;
expect(await compact(preparation, missingProviderModel, "test-key")).toMatchObject({
ok: false,
error: { code: "unknown", message: "Turn prefix summarization request failed" },
});
});
it("returns a compaction result with file details", async () => {
@@ -667,7 +658,7 @@ describe("harness compaction", () => {
const a1 = createMessageEntry(assistantMessage, u1.id);
const u2 = createMessageEntry(createUserMessage("continue"), a1.id);
const a2 = createMessageEntry(createAssistantMessage("done", createMockUsage(4000, 500)), u2.id);
const preparation = prepareCompaction([u1, a1, u2, a2], DEFAULT_COMPACTION_SETTINGS);
const preparation = getOrThrow(prepareCompaction([u1, a1, u2, a2], DEFAULT_COMPACTION_SETTINGS));
expect(preparation).toBeDefined();
const { faux, model } = createFauxModel(false);
faux.setResponses([fauxAssistantMessage("## Goal\nTest summary")]);

21
packages/agent/test/harness/nodejs-env.test.ts
View File

@@ -27,6 +27,7 @@ describe("NodeExecutionEnv", () => {
getOrThrow(await env.writeFile("nested/child/file.txt", "hel"));
getOrThrow(await env.appendFile("nested/child/file.txt", "lo"));
expect(getOrThrow(await env.readTextFile("nested/child/file.txt"))).toBe("hello");
expect(getOrThrow(await env.readTextLines("nested/child/file.txt", { maxLines: 1 }))).toEqual(["hello"]);
expect(Buffer.from(getOrThrow(await env.readBinaryFile("nested/child/file.txt"))).toString("utf8")).toBe("hello");
const entries = getOrThrow(await env.listDir("nested/child"));
@@ -91,6 +92,13 @@ describe("NodeExecutionEnv", () => {
]);
});
it("stops reading text lines at the requested limit", async () => {
const root = createTempDir();
const env = new NodeExecutionEnv({ cwd: root });
getOrThrow(await env.writeFile("file.txt", "one\ntwo\nthree"));
expect(getOrThrow(await env.readTextLines("file.txt", { maxLines: 1 }))).toEqual(["one"]);
});
it("returns FileError for missing paths and keeps exists false for missing paths", async () => {
const root = createTempDir();
const env = new NodeExecutionEnv({ cwd: root });
@@ -155,7 +163,7 @@ describe("NodeExecutionEnv", () => {
getOrThrow(await env.remove("missing", { force: true }));
});
it("returns aborted results for pre-aborted file operations", async () => {
it("returns aborted results for pre-aborted cancellable file operations", async () => {
const root = createTempDir();
const env = new NodeExecutionEnv({ cwd: root });
getOrThrow(await env.writeFile("file.txt", "hello"));
@@ -165,17 +173,10 @@ describe("NodeExecutionEnv", () => {
const results = await Promise.all([
env.readTextFile("file.txt", signal),
env.readTextLines("file.txt", { abortSignal: signal }),
env.readBinaryFile("file.txt", signal),
env.writeFile("other.txt", "hello", signal),
env.appendFile("other.txt", "hello", signal),
env.fileInfo("file.txt", signal),
env.listDir(".", signal),
env.canonicalPath("file.txt", signal),
env.exists("file.txt", signal),
env.createDir("dir", { abortSignal: signal }),
env.remove("file.txt", { abortSignal: signal }),
env.createTempDir("node-env-test-", signal),
env.createTempFile({ abortSignal: signal }),
]);
for (const result of results) {
expect(result.ok).toBe(false);
@@ -244,7 +245,7 @@ describe("NodeExecutionEnv", () => {
},
});
expect(result.ok).toBe(false);
if (!result.ok) expect(result.error).toMatchObject({ code: "unknown", message: "callback failed" });
if (!result.ok) expect(result.error).toMatchObject({ code: "callback_error", message: "callback failed" });
});
it("returns shell unavailable and spawn errors", async () => {

6
packages/agent/test/harness/session.test.ts
View File

@@ -140,10 +140,10 @@ runSessionSuite(
const header = JSON.parse(lines[0]!);
expect(header.type).toBe("session");
expect(header.version).toBe(3);
for (const line of lines.slice(1)) {
const entry = JSON.parse(line);
const entries = lines.slice(1).map((line) => JSON.parse(line));
expect(entries.some((entry) => entry.type === "leaf")).toBe(true);
for (const entry of entries) {
expect(entry.type).not.toBe("entry");
expect(entry.type).not.toBe("leaf");
expect(typeof entry.id).toBe("string");
}
},

1
packages/agent/test/harness/skills.test.ts
View File

@@ -93,6 +93,7 @@ Use this skill.
expect(diagnostics).toEqual([
{
type: "warning",
code: "invalid_metadata",
message: "description is required",
path: join(root, "user/broken/SKILL.md"),
source: { type: "user" },

34
packages/agent/test/harness/storage.test.ts
View File

@@ -4,7 +4,7 @@ import { describe, expect, it } from "vitest";
import { NodeExecutionEnv } from "../../src/harness/env/nodejs.js";
import { JsonlSessionStorage, loadJsonlSessionMetadata } from "../../src/harness/session/jsonl-storage.js";
import { InMemorySessionStorage } from "../../src/harness/session/memory-storage.js";
import type { MessageEntry, SessionMetadata } from "../../src/harness/types.js";
import { type MessageEntry, ok, type SessionMetadata } from "../../src/harness/types.js";
import { createAssistantMessage, createTempDir, createUserMessage } from "./session-test-utils.js";
describe("InMemorySessionStorage", () => {
@@ -14,7 +14,7 @@ describe("InMemorySessionStorage", () => {
expect(await storage.getMetadata()).toEqual(metadata);
});
it("copies initial entries and tracks leaf independently", async () => {
it("copies initial entries and persists leaf changes", async () => {
const entry: MessageEntry = {
type: "message",
id: "entry-1",
@@ -29,12 +29,12 @@ describe("InMemorySessionStorage", () => {
expect(await storage.getLeafId()).toBe("entry-1");
await storage.setLeafId(null);
expect(await storage.getLeafId()).toBeNull();
expect((await storage.getEntries()).at(-1)).toMatchObject({ type: "leaf", targetId: null });
});
it("rejects invalid leaf ids", async () => {
const storage = new InMemorySessionStorage();
await expect(storage.setLeafId("missing")).rejects.toThrow("Entry missing not found");
expect(() => new InMemorySessionStorage({ leafId: "missing" })).toThrow("Entry missing not found");
});
it("finds entries by type", async () => {
@@ -138,7 +138,7 @@ describe("JsonlSessionStorage", () => {
await expect(JsonlSessionStorage.open(env, filePath)).rejects.toThrow("first line is not a valid session header");
});
it("ignores malformed entry lines", async () => {
it("throws for malformed entry lines", async () => {
const dir = createTempDir();
const env = new NodeExecutionEnv({ cwd: dir });
const filePath = join(dir, "session.jsonl");
@@ -157,9 +157,7 @@ describe("JsonlSessionStorage", () => {
message: createUserMessage("one"),
};
writeFileSync(filePath, `${JSON.stringify(header)}\nnot json\n${JSON.stringify(entry)}\n`);
const storage = await JsonlSessionStorage.open(env, filePath);
expect((await storage.getEntries()).map((loadedEntry) => loadedEntry.id)).toEqual(["entry-1"]);
expect(await storage.getLeafId()).toBe("entry-1");
await expect(JsonlSessionStorage.open(env, filePath)).rejects.toMatchObject({ code: "invalid_entry" });
});
it("creates and reads session metadata from the header", async () => {
@@ -211,6 +209,10 @@ describe("JsonlSessionStorage", () => {
const loaded = await JsonlSessionStorage.open(env, filePath);
expect(await loaded.getLeafId()).toBe("child");
expect((await loaded.getEntries()).map((entry) => entry.id)).toEqual(["root", "child"]);
await loaded.setLeafId("root");
const reloaded = await JsonlSessionStorage.open(env, filePath);
expect(await reloaded.getLeafId()).toBe("root");
expect((await reloaded.getEntries()).at(-1)).toMatchObject({ type: "leaf", targetId: "root" });
expect((await loaded.getPathToRoot("child")).map((entry) => entry.id)).toEqual(["root", "child"]);
});
@@ -265,9 +267,8 @@ describe("JsonlSessionStorage", () => {
expect(await loaded.getLabel("entry-1")).toBeUndefined();
});
it("reads session metadata from only the first JSONL line", async () => {
it("reads session metadata through the line-reading filesystem operation", async () => {
const dir = createTempDir();
const env = new NodeExecutionEnv({ cwd: dir });
const filePath = join(dir, "session.jsonl");
const header = {
type: "session",
@@ -276,9 +277,18 @@ describe("JsonlSessionStorage", () => {
timestamp: "2026-01-01T00:00:00.000Z",
cwd: dir,
};
const malformedSecondLine = "{".repeat(10000);
writeFileSync(filePath, `${JSON.stringify(header)}\n${malformedSecondLine}\n`);
expect(await loadJsonlSessionMetadata(env, filePath)).toEqual({
const metadata = await loadJsonlSessionMetadata(
{
readTextLines: async () => ok([JSON.stringify(header)]),
readTextFile: async () => {
throw new Error("readTextFile should not be called for metadata");
},
writeFile: async () => ok(undefined),
appendFile: async () => ok(undefined),
},
filePath,
);
expect(metadata).toEqual({
id: "session-1",
createdAt: "2026-01-01T00:00:00.000Z",
cwd: dir,