From 1d174ee5c96823b1922c902a94c37efe110dbd63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B0=8F=E6=A9=98?= Date: Sun, 24 May 2026 09:16:06 +0000 Subject: [PATCH] fix(agent-kit): separate session cache per agent Each agent now maintains its own session cache file instead of sharing a single agent-sessions.json. This prevents session ID conflicts when multiple agents operate on the same thread+role pair. Changes: - getCachePath() now takes agentName parameter - getCachedSessionId/setCachedSessionId require agentName as first param - Cache files named -sessions.json (e.g., hermes-sessions.json) - Agent wrappers inject their agent name into cache calls - Add comprehensive tests for session cache isolation - Handle malformed JSON gracefully (treat as empty cache) Fixes #461 --- .../src/claude-code.ts | 6 +- .../src/session-cache.ts | 21 +- .../__tests__/session-cache.test.ts | 247 ++++++++++++++++++ packages/workflow-agent-kit/src/index.ts | 2 +- .../workflow-agent-kit/src/session-cache.ts | 31 ++- packages/workflow-dashboard/vitest.config.ts | 1 - 6 files changed, 290 insertions(+), 18 deletions(-) create mode 100644 packages/workflow-agent-kit/__tests__/session-cache.test.ts diff --git a/packages/workflow-agent-claude-code/src/claude-code.ts b/packages/workflow-agent-claude-code/src/claude-code.ts index 5d8c73f..aa21e5a 100644 --- a/packages/workflow-agent-claude-code/src/claude-code.ts +++ b/packages/workflow-agent-claude-code/src/claude-code.ts @@ -146,13 +146,13 @@ async function runClaudeCode(ctx: AgentContext): Promise { // Try resuming a cached session for re-entry scenarios (e.g. reviewer reject → developer re-entry). if (!ctx.isFirstVisit) { - const cachedSessionId = await getCachedSessionId(ctx.threadId, ctx.role); + const cachedSessionId = await getCachedSessionId("claude-code", ctx.threadId, ctx.role); if (cachedSessionId !== null) { try { const { stdout } = await spawnClaudeResume(cachedSessionId, fullPrompt); const result = await processClaudeOutput(stdout, ctx.store); if (result.sessionId !== undefined && result.sessionId !== "") { - await setCachedSessionId(ctx.threadId, ctx.role, result.sessionId); + await setCachedSessionId("claude-code", ctx.threadId, ctx.role, result.sessionId); } return result; } catch (err) { @@ -169,7 +169,7 @@ async function runClaudeCode(ctx: AgentContext): Promise { const { stdout } = await spawnClaudeRun(fullPrompt); const result = await processClaudeOutput(stdout, ctx.store); if (result.sessionId !== undefined && result.sessionId !== "") { - await setCachedSessionId(ctx.threadId, ctx.role, result.sessionId); + await setCachedSessionId("claude-code", ctx.threadId, ctx.role, result.sessionId); } return result; } diff --git a/packages/workflow-agent-hermes/src/session-cache.ts b/packages/workflow-agent-hermes/src/session-cache.ts index 23935af..3c59ff2 100644 --- a/packages/workflow-agent-hermes/src/session-cache.ts +++ b/packages/workflow-agent-hermes/src/session-cache.ts @@ -1,5 +1,22 @@ -// Re-export session cache from the shared agent-kit package. -export { getCachedSessionId, setCachedSessionId } from "@uncaged/workflow-agent-kit"; +// Re-export session cache from the shared agent-kit package with agent name injected. + +import { + getCachedSessionId as getCachedSessionIdBase, + setCachedSessionId as setCachedSessionIdBase, +} from "@uncaged/workflow-agent-kit"; +import type { ThreadId } from "@uncaged/workflow-protocol"; + +export async function getCachedSessionId(threadId: ThreadId, role: string): Promise { + return getCachedSessionIdBase("hermes", threadId, role); +} + +export async function setCachedSessionId( + threadId: ThreadId, + role: string, + sessionId: string, +): Promise { + return setCachedSessionIdBase("hermes", threadId, role, sessionId); +} export function isResumeDisabled(): boolean { // Hermes ACP session/resume is broken: _restore fails for custom providers diff --git a/packages/workflow-agent-kit/__tests__/session-cache.test.ts b/packages/workflow-agent-kit/__tests__/session-cache.test.ts new file mode 100644 index 0000000..81934d8 --- /dev/null +++ b/packages/workflow-agent-kit/__tests__/session-cache.test.ts @@ -0,0 +1,247 @@ +import { mkdir, readdir, readFile, rm, stat, writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import type { ThreadId } from "@uncaged/workflow-protocol"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; + +import { getCachedSessionId, getCachePath, setCachedSessionId } from "../src/session-cache.js"; +import { resolveStorageRoot } from "../src/storage.js"; + +describe("session-cache", () => { + let originalStorageRoot: string; + let testStorageRoot: string; + + beforeEach(async () => { + // Create a temporary test storage root + originalStorageRoot = resolveStorageRoot(); + testStorageRoot = join(originalStorageRoot, "test-cache", `test-${Date.now()}`); + await mkdir(testStorageRoot, { recursive: true }); + + // Override the storage root for testing + process.env.WORKFLOW_STORAGE_ROOT = testStorageRoot; + }); + + afterEach(async () => { + // Clean up test storage root + await rm(testStorageRoot, { recursive: true, force: true }); + delete process.env.WORKFLOW_STORAGE_ROOT; + }); + + describe("getCachePath", () => { + test("returns agent-specific file path", () => { + const path = getCachePath("claude-code"); + expect(path).toMatch(/\/cache\/claude-code-sessions\.json$/); + }); + + test("returns different paths for different agents", () => { + const pathClaudeCode = getCachePath("claude-code"); + const pathHermes = getCachePath("hermes"); + + expect(pathClaudeCode).not.toBe(pathHermes); + expect(pathClaudeCode).toMatch(/claude-code-sessions\.json$/); + expect(pathHermes).toMatch(/hermes-sessions\.json$/); + }); + + test("handles agent names with special characters", () => { + const path1 = getCachePath("my-agent"); + const path2 = getCachePath("my_agent"); + + expect(path1).toMatch(/my-agent-sessions\.json$/); + expect(path2).toMatch(/my_agent-sessions\.json$/); + }); + }); + + describe("session isolation", () => { + const threadId = "01234567890123456789012345" as ThreadId; + const role = "developer"; + + test("sessions are isolated per agent", async () => { + // Cache different session IDs for each agent + await setCachedSessionId("claude-code", threadId, role, "session-cc-001"); + await setCachedSessionId("hermes", threadId, role, "session-hermes-001"); + + // Each agent should retrieve its own session ID + const sessionCC = await getCachedSessionId("claude-code", threadId, role); + const sessionHermes = await getCachedSessionId("hermes", threadId, role); + + expect(sessionCC).toBe("session-cc-001"); + expect(sessionHermes).toBe("session-hermes-001"); + }); + + test("updating one agent's cache does not affect another", async () => { + // Set initial sessions for both agents + await setCachedSessionId("claude-code", threadId, role, "session-cc-001"); + await setCachedSessionId("hermes", threadId, role, "session-hermes-001"); + + // Update claude-code's session + await setCachedSessionId("claude-code", threadId, role, "session-cc-002"); + + // Hermes's session should remain unchanged + const sessionHermes = await getCachedSessionId("hermes", threadId, role); + expect(sessionHermes).toBe("session-hermes-001"); + + // Claude-code should have the new session + const sessionCC = await getCachedSessionId("claude-code", threadId, role); + expect(sessionCC).toBe("session-cc-002"); + }); + + test("missing session returns null for specific agent", async () => { + const session = await getCachedSessionId("claude-code", threadId, role); + expect(session).toBeNull(); + }); + + test("empty session ID is treated as missing", async () => { + await setCachedSessionId("claude-code", threadId, role, ""); + + const session = await getCachedSessionId("claude-code", threadId, role); + expect(session).toBeNull(); + }); + }); + + describe("file system operations", () => { + const threadId = "01234567890123456789012345" as ThreadId; + const role = "developer"; + + test("cache directory is created if missing", async () => { + const cachePath = getCachePath("claude-code"); + const cacheDir = dirname(cachePath); + + // Ensure cache dir doesn't exist + await rm(cacheDir, { recursive: true, force: true }); + + // Write a session + await setCachedSessionId("claude-code", threadId, role, "session-001"); + + // Cache directory should be created + const stats = await stat(cacheDir); + expect(stats.isDirectory()).toBe(true); + }); + + test("multiple agents create separate cache files", async () => { + // Cache sessions for multiple agents + await setCachedSessionId("claude-code", threadId, role, "session-cc-001"); + await setCachedSessionId("hermes", threadId, role, "session-hermes-001"); + + // Separate cache files should exist + const pathCC = getCachePath("claude-code"); + const pathHermes = getCachePath("hermes"); + + const contentCC = JSON.parse(await readFile(pathCC, "utf8")) as Record; + const contentHermes = JSON.parse(await readFile(pathHermes, "utf8")) as Record< + string, + string + >; + + expect(contentCC).toHaveProperty(`${threadId}:${role}`, "session-cc-001"); + expect(contentHermes).toHaveProperty(`${threadId}:${role}`, "session-hermes-001"); + }); + + test("atomic writes prevent partial reads", async () => { + // Write a session + await setCachedSessionId("claude-code", threadId, role, "session-001"); + + // The final file should exist (no .tmp files left behind) + const cachePath = getCachePath("claude-code"); + const dir = dirname(cachePath); + const files = await readdir(dir); + + expect(files).toContain("claude-code-sessions.json"); + expect(files.every((f) => !f.endsWith(".tmp"))).toBe(true); + }); + }); + + describe("legacy migration", () => { + const threadId = "01234567890123456789012345" as ThreadId; + const role = "developer"; + + test("old agent-sessions.json is ignored", async () => { + // Create old agent-sessions.json file + const oldCachePath = join(resolveStorageRoot(), "cache", "agent-sessions.json"); + await mkdir(dirname(oldCachePath), { recursive: true }); + await writeFile( + oldCachePath, + JSON.stringify({ + "01234567890123456789012345:developer": "old-session-001", + }), + "utf8", + ); + + // Query with the new per-agent cache + const session = await getCachedSessionId("claude-code", threadId, role); + + // Should return null (old cache is ignored) + expect(session).toBeNull(); + }); + + test("new per-agent cache takes precedence", async () => { + // Create both old and new cache files + const oldPath = join(resolveStorageRoot(), "cache", "agent-sessions.json"); + await mkdir(dirname(oldPath), { recursive: true }); + await writeFile( + oldPath, + JSON.stringify({ + [`${threadId}:${role}`]: "old-session", + }), + "utf8", + ); + + await setCachedSessionId("claude-code", threadId, role, "new-session"); + + // The new per-agent cache value should be returned + const session = await getCachedSessionId("claude-code", threadId, role); + expect(session).toBe("new-session"); + }); + }); + + describe("error handling", () => { + const threadId = "01234567890123456789012345" as ThreadId; + const role = "developer"; + + test("invalid JSON in cache file returns empty cache", async () => { + // Create a corrupted cache file + const cachePath = getCachePath("claude-code"); + await mkdir(dirname(cachePath), { recursive: true }); + await writeFile(cachePath, "{ invalid json }", "utf8"); + + // Should return null (treating corrupted cache as empty) + const session = await getCachedSessionId("claude-code", threadId, role); + expect(session).toBeNull(); + }); + + test("non-object JSON in cache file returns empty cache", async () => { + // Create a cache file with non-object JSON + const cachePath = getCachePath("claude-code"); + await mkdir(dirname(cachePath), { recursive: true }); + await writeFile(cachePath, JSON.stringify(["not", "an", "object"]), "utf8"); + + // Should return null + const session = await getCachedSessionId("claude-code", threadId, role); + expect(session).toBeNull(); + }); + + test("cache entries with non-string values are ignored", async () => { + // Create a cache file with mixed types + const cachePath = getCachePath("claude-code"); + const cacheData = { + "thread1:role1": "valid-session", + "thread2:role2": 12345, // number + "thread3:role3": null, // null + "thread4:role4": "", // empty string + }; + await mkdir(dirname(cachePath), { recursive: true }); + await writeFile(cachePath, JSON.stringify(cacheData), "utf8"); + + // Valid string entries should be returned + const session1 = await getCachedSessionId("claude-code", "thread1" as ThreadId, "role1"); + expect(session1).toBe("valid-session"); + + // Invalid entries should return null + const session2 = await getCachedSessionId("claude-code", "thread2" as ThreadId, "role2"); + const session3 = await getCachedSessionId("claude-code", "thread3" as ThreadId, "role3"); + const session4 = await getCachedSessionId("claude-code", "thread4" as ThreadId, "role4"); + + expect(session2).toBeNull(); + expect(session3).toBeNull(); + expect(session4).toBeNull(); // empty string is treated as missing + }); + }); +}); diff --git a/packages/workflow-agent-kit/src/index.ts b/packages/workflow-agent-kit/src/index.ts index ffeaaa6..0660f40 100644 --- a/packages/workflow-agent-kit/src/index.ts +++ b/packages/workflow-agent-kit/src/index.ts @@ -12,7 +12,7 @@ export { export type { FrontmatterFastPathResult } from "./frontmatter.js"; export { tryFrontmatterFastPath } from "./frontmatter.js"; export { createAgent } from "./run.js"; -export { getCachedSessionId, setCachedSessionId } from "./session-cache.js"; +export { getCachedSessionId, getCachePath, setCachedSessionId } from "./session-cache.js"; export { getConfigPath, getEnvPath, loadWorkflowConfig, resolveStorageRoot } from "./storage.js"; export type { AgentContext, diff --git a/packages/workflow-agent-kit/src/session-cache.ts b/packages/workflow-agent-kit/src/session-cache.ts index fd94de5..436da45 100644 --- a/packages/workflow-agent-kit/src/session-cache.ts +++ b/packages/workflow-agent-kit/src/session-cache.ts @@ -8,8 +8,8 @@ import { resolveStorageRoot } from "./storage.js"; type SessionCache = Record; -function getCachePath(): string { - return join(resolveStorageRoot(), "cache", "agent-sessions.json"); +export function getCachePath(agentName: string): string { + return join(resolveStorageRoot(), "cache", `${agentName}-sessions.json`); } function cacheKey(threadId: ThreadId, role: string): string { @@ -20,8 +20,8 @@ function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } -async function readCache(): Promise { - const path = getCachePath(); +async function readCache(agentName: string): Promise { + const path = getCachePath(agentName); try { const text = await readFile(path, "utf8"); const raw = JSON.parse(text) as unknown; @@ -40,36 +40,45 @@ async function readCache(): Promise { if (err.code === "ENOENT") { return {}; } + // Treat JSON parse errors as empty cache + if (err.name === "SyntaxError") { + return {}; + } throw e; } } -async function writeCache(cache: SessionCache): Promise { - const path = getCachePath(); +async function writeCache(agentName: string, cache: SessionCache): Promise { + const path = getCachePath(agentName); const dir = dirname(path); await mkdir(dir, { recursive: true }); // Atomic write: write to temp file then rename to avoid partial reads on concurrent access. // NOTE: Current workflow execution is serial (execFileSync), so true concurrency doesn't occur. // This is a safety net for future parallel execution. - const tmpPath = join(dir, `.agent-sessions.${randomBytes(4).toString("hex")}.tmp`); + const tmpPath = join(dir, `.${agentName}-sessions.${randomBytes(4).toString("hex")}.tmp`); await writeFile(tmpPath, `${JSON.stringify(cache, null, 2)}\n`, "utf8"); await rename(tmpPath, path); } /** Read the cached session ID for a thread+role pair. */ -export async function getCachedSessionId(threadId: ThreadId, role: string): Promise { - const cache = await readCache(); +export async function getCachedSessionId( + agentName: string, + threadId: ThreadId, + role: string, +): Promise { + const cache = await readCache(agentName); const sessionId = cache[cacheKey(threadId, role)]; return sessionId ?? null; } /** Write the session ID for a thread+role pair into the cache. */ export async function setCachedSessionId( + agentName: string, threadId: ThreadId, role: string, sessionId: string, ): Promise { - const cache = await readCache(); + const cache = await readCache(agentName); cache[cacheKey(threadId, role)] = sessionId; - await writeCache(cache); + await writeCache(agentName, cache); } diff --git a/packages/workflow-dashboard/vitest.config.ts b/packages/workflow-dashboard/vitest.config.ts index 6d04e9b..6b84473 100644 --- a/packages/workflow-dashboard/vitest.config.ts +++ b/packages/workflow-dashboard/vitest.config.ts @@ -1,7 +1,6 @@ import path from "node:path"; import { defineConfig } from "vitest/config"; -// biome-ignore lint/style/noDefaultExport: Vitest loads config from default export. export default defineConfig({ test: { environment: "node",