From d666516ce689ecc4f97e88662a49178c0e6a9141 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B0=8F=E6=A9=98?= Date: Sun, 7 Jun 2026 08:14:09 +0000 Subject: [PATCH] feat(util-agent): extend AgentOptions with fork / cleanup (Phase 2a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add AgentForkFn and AgentCleanupFn type aliases. Extend AgentOptions with fork: AgentForkFn | null and cleanup: AgentCleanupFn | null fields. Add getAskSessionId / setAskSessionId session-cache helpers using :ask key shape (coexists with exec sessions in the same per-agent cache file). All four adapters pass fork: null, cleanup: null — real wiring lands in Phase 2b. Resolves #145. Co-Authored-By: Claude Opus 4.6 --- .changeset/agent-fork-cleanup.md | 18 +++ packages/agent-builtin/src/agent.ts | 2 + packages/agent-claude-code/src/claude-code.ts | 2 + packages/agent-hermes/src/hermes.ts | 2 + packages/agent-mock/src/mock-agent.ts | 2 + .../__tests__/adapter-fork-null.test.ts | 60 ++++++++ .../__tests__/agent-options-type.test.ts | 78 +++++++++++ .../__tests__/session-cache-ask.test.ts | 131 ++++++++++++++++++ packages/util-agent/src/index.ts | 10 +- packages/util-agent/src/session-cache.ts | 34 +++++ packages/util-agent/src/types.ts | 25 ++++ 11 files changed, 363 insertions(+), 1 deletion(-) create mode 100644 .changeset/agent-fork-cleanup.md create mode 100644 packages/util-agent/__tests__/adapter-fork-null.test.ts create mode 100644 packages/util-agent/__tests__/agent-options-type.test.ts create mode 100644 packages/util-agent/__tests__/session-cache-ask.test.ts diff --git a/.changeset/agent-fork-cleanup.md b/.changeset/agent-fork-cleanup.md new file mode 100644 index 0000000..7f4f04e --- /dev/null +++ b/.changeset/agent-fork-cleanup.md @@ -0,0 +1,18 @@ +--- +"@united-workforce/util-agent": minor +"@united-workforce/agent-mock": patch +"@united-workforce/agent-builtin": patch +"@united-workforce/agent-hermes": patch +"@united-workforce/agent-claude-code": patch +--- + +feat(util-agent): extend AgentOptions with `fork` / `cleanup` and add ask-session cache + +Phase 2a infrastructure for `step ask`. Extends `AgentOptions` with +`fork: AgentForkFn | null` and `cleanup: AgentCleanupFn | null` fields, exporting +the new `AgentForkFn` and `AgentCleanupFn` type aliases. Adds `getAskSessionId` / +`setAskSessionId` to the per-agent session cache, using `:ask` keys +that share the cache file with exec sessions (`:` keys) without +collision. All four adapters (mock, builtin, hermes, claude-code) now pass +`fork: null, cleanup: null` — real implementations land in Phase 2b. Resolves +issue #145. diff --git a/packages/agent-builtin/src/agent.ts b/packages/agent-builtin/src/agent.ts index 2854669..dd6a9c7 100644 --- a/packages/agent-builtin/src/agent.ts +++ b/packages/agent-builtin/src/agent.ts @@ -167,5 +167,7 @@ export function createBuiltinAgent(): () => Promise { name: "builtin", run: runBuiltin, continue: continueBuiltin, + fork: null, + cleanup: null, }); } diff --git a/packages/agent-claude-code/src/claude-code.ts b/packages/agent-claude-code/src/claude-code.ts index f370471..ed8049f 100644 --- a/packages/agent-claude-code/src/claude-code.ts +++ b/packages/agent-claude-code/src/claude-code.ts @@ -253,5 +253,7 @@ export function createClaudeCodeAgent(model: string | null): () => Promise name: "claude-code", run: (ctx) => runClaudeCode(ctx, model), continue: (sessionId, message, store) => continueClaudeCode(sessionId, message, store, model), + fork: null, + cleanup: null, }); } diff --git a/packages/agent-hermes/src/hermes.ts b/packages/agent-hermes/src/hermes.ts index ad9e061..ade9410 100644 --- a/packages/agent-hermes/src/hermes.ts +++ b/packages/agent-hermes/src/hermes.ts @@ -246,6 +246,8 @@ export function createHermesAgent(resumeDisabled: boolean): () => Promise name: "hermes", run: runHermes, continue: continueHermes, + fork: null, + cleanup: null, }); // Wrap to ensure ACP client is closed after agent completes, diff --git a/packages/agent-mock/src/mock-agent.ts b/packages/agent-mock/src/mock-agent.ts index 0f9da4d..7205543 100644 --- a/packages/agent-mock/src/mock-agent.ts +++ b/packages/agent-mock/src/mock-agent.ts @@ -125,5 +125,7 @@ export function createMockAgent(mockDataPath: string): () => Promise { name: "mock", run, continue: continueRun, + fork: null, + cleanup: null, }); } diff --git a/packages/util-agent/__tests__/adapter-fork-null.test.ts b/packages/util-agent/__tests__/adapter-fork-null.test.ts new file mode 100644 index 0000000..f235f50 --- /dev/null +++ b/packages/util-agent/__tests__/adapter-fork-null.test.ts @@ -0,0 +1,60 @@ +import { readFile } from "node:fs/promises"; +import { join } from "node:path"; +import { describe, expect, test } from "vitest"; + +/** + * Source-level verification that each adapter's `createAgent({...})` call + * includes the new `fork: null` and `cleanup: null` fields. + * + * Adapters are CLI binaries that spawn external processes — runtime testing + * requires real LLM environments — so we use static source inspection here. + * Type-level correctness is enforced separately by `tsc --build`. + */ + +const REPO_ROOT = join(__dirname, "..", "..", ".."); + +const ADAPTERS: Array<{ name: string; path: string }> = [ + { name: "agent-mock", path: "packages/agent-mock/src/mock-agent.ts" }, + { name: "agent-builtin", path: "packages/agent-builtin/src/agent.ts" }, + { name: "agent-hermes", path: "packages/agent-hermes/src/hermes.ts" }, + { name: "agent-claude-code", path: "packages/agent-claude-code/src/claude-code.ts" }, +]; + +/** Find the matching `}` for the `{` at `openIdx` in `source`. */ +function findMatchingBrace(source: string, openIdx: number): number { + let depth = 0; + for (let i = openIdx; i < source.length; i++) { + const ch = source[i]; + if (ch === "{") { + depth++; + } else if (ch === "}") { + depth--; + if (depth === 0) { + return i; + } + } + } + return -1; +} + +/** Extract the `createAgent({...})` block from adapter source. */ +function extractCreateAgentBlock(source: string): string { + const startIdx = source.indexOf("createAgent({"); + expect(startIdx).toBeGreaterThanOrEqual(0); + const openIdx = source.indexOf("{", startIdx); + const endIdx = findMatchingBrace(source, openIdx); + expect(endIdx).toBeGreaterThan(openIdx); + return source.slice(openIdx, endIdx + 1); +} + +describe("adapter createAgent calls include fork: null and cleanup: null", () => { + for (const adapter of ADAPTERS) { + test(`${adapter.name} createAgent call includes fork: null and cleanup: null`, async () => { + const source = await readFile(join(REPO_ROOT, adapter.path), "utf8"); + expect(source).toMatch(/createAgent\s*\(\s*\{/); + const block = extractCreateAgentBlock(source); + expect(block).toMatch(/fork:\s*null/); + expect(block).toMatch(/cleanup:\s*null/); + }); + } +}); diff --git a/packages/util-agent/__tests__/agent-options-type.test.ts b/packages/util-agent/__tests__/agent-options-type.test.ts new file mode 100644 index 0000000..9f84553 --- /dev/null +++ b/packages/util-agent/__tests__/agent-options-type.test.ts @@ -0,0 +1,78 @@ +import type { Store } from "@ocas/core"; +import { describe, expect, test } from "vitest"; + +import type { + AgentCleanupFn, + AgentContext, + AgentContinueFn, + AgentForkFn, + AgentOptions, + AgentRunFn, +} from "../src/types.js"; + +const makeRun: AgentRunFn = async (_ctx: AgentContext) => ({ + output: "", + detailHash: "", + sessionId: "", + assembledPrompt: "", + usage: null, +}); + +const makeContinue: AgentContinueFn = async (_sessionId, _message, _store) => ({ + output: "", + detailHash: "", + sessionId: "", + assembledPrompt: "", + usage: null, +}); + +describe("AgentOptions fork/cleanup", () => { + test("AgentOptions accepts fork and cleanup as null", () => { + const opts: AgentOptions = { + name: "test", + run: makeRun, + continue: makeContinue, + fork: null, + cleanup: null, + }; + expect(opts.name).toBe("test"); + expect(opts.run).toBe(makeRun); + expect(opts.continue).toBe(makeContinue); + expect(opts.fork).toBeNull(); + expect(opts.cleanup).toBeNull(); + }); + + test("AgentOptions accepts real fork and cleanup functions", () => { + const fork: AgentForkFn = async (sessionId, _store) => `${sessionId}-forked`; + const cleanup: AgentCleanupFn = async () => { + /* no-op */ + }; + const opts: AgentOptions = { + name: "test", + run: makeRun, + continue: makeContinue, + fork, + cleanup, + }; + expect(typeof opts.fork).toBe("function"); + expect(typeof opts.cleanup).toBe("function"); + }); + + test("AgentForkFn signature accepts (sessionId: string, store: Store) and returns Promise", async () => { + const fork: AgentForkFn = async (sessionId, _store) => `${sessionId}-child`; + // Cast a placeholder Store — only the signature shape matters for this test. + const fakeStore = {} as Store; + const result = await fork("session-abc", fakeStore); + expect(result).toBe("session-abc-child"); + }); + + test("AgentCleanupFn signature accepts no args and returns Promise", async () => { + let called = false; + const cleanup: AgentCleanupFn = async () => { + called = true; + }; + const result = await cleanup(); + expect(result).toBeUndefined(); + expect(called).toBe(true); + }); +}); diff --git a/packages/util-agent/__tests__/session-cache-ask.test.ts b/packages/util-agent/__tests__/session-cache-ask.test.ts new file mode 100644 index 0000000..de08cc4 --- /dev/null +++ b/packages/util-agent/__tests__/session-cache-ask.test.ts @@ -0,0 +1,131 @@ +import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { dirname, join } from "node:path"; +import type { ThreadId } from "@united-workforce/protocol"; +import { afterEach, beforeEach, describe, expect, test } from "vitest"; + +import { + getAskSessionId, + getCachedSessionId, + getCachePath, + setAskSessionId, + setCachedSessionId, +} from "../src/session-cache.js"; +import { getDefaultStorageRoot } from "../src/storage.js"; + +describe("session-cache ask sessions", () => { + let testStorageRoot: string; + + beforeEach(async () => { + testStorageRoot = join( + getDefaultStorageRoot(), + "test-cache", + `ask-${Date.now()}-${Math.random()}`, + ); + await mkdir(testStorageRoot, { recursive: true }); + }); + + afterEach(async () => { + await rm(testStorageRoot, { recursive: true, force: true }); + }); + + const stepHash = "ABCDEFG1234567"; + + test("getAskSessionId returns null when no ask session cached", async () => { + const session = await getAskSessionId("claude-code", stepHash, testStorageRoot); + expect(session).toBeNull(); + }); + + test("setAskSessionId + getAskSessionId round-trip", async () => { + await setAskSessionId("claude-code", stepHash, "ask-session-123", testStorageRoot); + const session = await getAskSessionId("claude-code", stepHash, testStorageRoot); + expect(session).toBe("ask-session-123"); + }); + + test("ask cache keys use stepHash:ask format", async () => { + await setAskSessionId("claude-code", stepHash, "ask-session-456", testStorageRoot); + + const cachePath = getCachePath("claude-code", testStorageRoot); + const content = JSON.parse(await readFile(cachePath, "utf8")) as Record; + + expect(content).toHaveProperty(`${stepHash}:ask`, "ask-session-456"); + }); + + test("exec cache and ask cache coexist in same file", async () => { + const threadId = "01234567890123456789012345" as ThreadId; + const role = "developer"; + + await setCachedSessionId("claude-code", threadId, role, "exec-session", testStorageRoot); + await setAskSessionId("claude-code", stepHash, "ask-session", testStorageRoot); + + const cachePath = getCachePath("claude-code", testStorageRoot); + const content = JSON.parse(await readFile(cachePath, "utf8")) as Record; + + expect(content).toHaveProperty(`${threadId}:${role}`, "exec-session"); + expect(content).toHaveProperty(`${stepHash}:ask`, "ask-session"); + + expect(await getCachedSessionId("claude-code", threadId, role, testStorageRoot)).toBe( + "exec-session", + ); + expect(await getAskSessionId("claude-code", stepHash, testStorageRoot)).toBe("ask-session"); + }); + + test("updating ask session does not affect exec session", async () => { + const threadId = "01234567890123456789012345" as ThreadId; + const role = "developer"; + + await setCachedSessionId("claude-code", threadId, role, "exec-original", testStorageRoot); + await setAskSessionId("claude-code", stepHash, "ask-original", testStorageRoot); + + await setAskSessionId("claude-code", stepHash, "ask-updated", testStorageRoot); + + expect(await getCachedSessionId("claude-code", threadId, role, testStorageRoot)).toBe( + "exec-original", + ); + expect(await getAskSessionId("claude-code", stepHash, testStorageRoot)).toBe("ask-updated"); + }); + + test("updating exec session does not affect ask session", async () => { + const threadId = "01234567890123456789012345" as ThreadId; + const role = "developer"; + + await setAskSessionId("claude-code", stepHash, "ask-original", testStorageRoot); + await setCachedSessionId("claude-code", threadId, role, "exec-original", testStorageRoot); + + await setCachedSessionId("claude-code", threadId, role, "exec-updated", testStorageRoot); + + expect(await getAskSessionId("claude-code", stepHash, testStorageRoot)).toBe("ask-original"); + expect(await getCachedSessionId("claude-code", threadId, role, testStorageRoot)).toBe( + "exec-updated", + ); + }); + + test("different stepHashes have independent ask sessions", async () => { + const stepHashA = "AAAAAAA1234567"; + const stepHashB = "BBBBBBB1234567"; + + await setAskSessionId("claude-code", stepHashA, "session-A", testStorageRoot); + await setAskSessionId("claude-code", stepHashB, "session-B", testStorageRoot); + + expect(await getAskSessionId("claude-code", stepHashA, testStorageRoot)).toBe("session-A"); + expect(await getAskSessionId("claude-code", stepHashB, testStorageRoot)).toBe("session-B"); + }); + + test("ask session for one agent does not leak to another", async () => { + await setAskSessionId("claude-code", stepHash, "cc-ask-session", testStorageRoot); + + const ccSession = await getAskSessionId("claude-code", stepHash, testStorageRoot); + const hermesSession = await getAskSessionId("hermes", stepHash, testStorageRoot); + + expect(ccSession).toBe("cc-ask-session"); + expect(hermesSession).toBeNull(); + }); + + test("empty string ask session treated as missing", async () => { + const cachePath = getCachePath("claude-code", testStorageRoot); + await mkdir(dirname(cachePath), { recursive: true }); + await writeFile(cachePath, JSON.stringify({ [`${stepHash}:ask`]: "" }), "utf8"); + + const session = await getAskSessionId("claude-code", stepHash, testStorageRoot); + expect(session).toBeNull(); + }); +}); diff --git a/packages/util-agent/src/index.ts b/packages/util-agent/src/index.ts index 6e2c77f..6de911d 100644 --- a/packages/util-agent/src/index.ts +++ b/packages/util-agent/src/index.ts @@ -14,12 +14,20 @@ export type { FrontmatterFastPathResult } from "./frontmatter.js"; export { tryFrontmatterFastPath } from "./frontmatter.js"; export { buildFrontmatterRetryPrompt } from "./frontmatter-retry-prompt.js"; export { createAgent, parseArgv } from "./run.js"; -export { getCachedSessionId, getCachePath, setCachedSessionId } from "./session-cache.js"; +export { + getAskSessionId, + getCachedSessionId, + getCachePath, + setAskSessionId, + setCachedSessionId, +} from "./session-cache.js"; export { getConfigPath, getEnvPath, loadWorkflowConfig, resolveStorageRoot } from "./storage.js"; export type { AdapterOutput, + AgentCleanupFn, AgentContext, AgentContinueFn, + AgentForkFn, AgentOptions, AgentRunFn, AgentRunResult, diff --git a/packages/util-agent/src/session-cache.ts b/packages/util-agent/src/session-cache.ts index 855e8c9..3b00f79 100644 --- a/packages/util-agent/src/session-cache.ts +++ b/packages/util-agent/src/session-cache.ts @@ -14,6 +14,10 @@ function cacheKey(threadId: ThreadId, role: string): string { return `${threadId}:${role}`; } +function askCacheKey(stepHash: string): string { + return `${stepHash}:ask`; +} + function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } @@ -86,3 +90,33 @@ export async function setCachedSessionId( cache[cacheKey(threadId, role)] = sessionId; await writeCache(agentName, storageRoot, cache); } + +/** + * Read the cached ask-session ID for a stepHash. + * + * Ask sessions are forked side conversations spawned by `step ask` from a + * specific completed step. They share the per-agent cache file with exec + * sessions but use the `:ask` key shape so the two namespaces + * never collide. + */ +export async function getAskSessionId( + agentName: string, + stepHash: string, + storageRoot: string, +): Promise { + const cache = await readCache(agentName, storageRoot); + const sessionId = cache[askCacheKey(stepHash)]; + return sessionId ?? null; +} + +/** Write the ask-session ID for a stepHash into the cache. */ +export async function setAskSessionId( + agentName: string, + stepHash: string, + sessionId: string, + storageRoot: string, +): Promise { + const cache = await readCache(agentName, storageRoot); + cache[askCacheKey(stepHash)] = sessionId; + await writeCache(agentName, storageRoot, cache); +} diff --git a/packages/util-agent/src/types.ts b/packages/util-agent/src/types.ts index bd3e823..8e16edc 100644 --- a/packages/util-agent/src/types.ts +++ b/packages/util-agent/src/types.ts @@ -50,6 +50,21 @@ export type AgentContinueFn = ( export type AgentRunFn = (ctx: AgentContext) => Promise; +/** + * Fork an existing agent session, returning a new session ID that branches + * from the source session's state. Used by `step ask` (Phase 2a infrastructure) + * to spawn a side conversation from a completed step's session without + * polluting the original session's history. + */ +export type AgentForkFn = (sessionId: string, store: AgentContext["store"]) => Promise; + +/** + * Clean up adapter-level resources (e.g. close ACP client, kill subprocesses). + * Invoked by the agent CLI factory after the run completes — regardless of + * success or failure — so adapters can release I/O handles deterministically. + */ +export type AgentCleanupFn = () => Promise; + export type AdapterOutput = { stepHash: string; detailHash: string; @@ -65,4 +80,14 @@ export type AgentOptions = { name: string; run: AgentRunFn; continue: AgentContinueFn; + /** + * Optional session-fork hook. null means the adapter does not yet support + * `step ask` (Phase 2a placeholder — wired up in Phase 2b). + */ + fork: AgentForkFn | null; + /** + * Optional cleanup hook invoked after the agent CLI completes. null means + * the adapter has no resources to release. + */ + cleanup: AgentCleanupFn | null; }; -- 2.43.0