feat(hooks): add postHookActions for order-independent hook coordination
Adds a generic postHookActions mechanism to InternalHookEvent: handlers push deferred callbacks that triggerInternalHook drains after all handlers complete. This eliminates FIFO registration-order dependencies — any hook can set context flags regardless of when it registered. Session-memory handler updated to use fail-safe pattern: - Writes file inline (preserves data if postHookActions never runs) - Pushes post-hook action for retraction (blockSessionSave) and content replacement (sessionSaveContent) - Pre-set flags still work inline; late-set flags work via post-hook New tests: - 5 tests for postHookActions mechanism (ordering, error isolation, late-context visibility) - 2 tests for late-set blockSessionSave/sessionSaveContent - 1 fail-safe test (file preserved when postHookActions not drained)
This commit is contained in:
parent
6b998ff746
commit
1ea1aa75df
@ -573,7 +573,16 @@ describe("session-memory hook", () => {
|
||||
expect(memoryContent).toContain("assistant: Only message 2");
|
||||
});
|
||||
|
||||
it("blockSessionSave prevents memory file creation", async () => {
|
||||
// Helper to drain postHookActions (simulates what triggerInternalHook does)
|
||||
async function drainPostHookActions(event: {
|
||||
postHookActions: Array<() => Promise<void> | void>;
|
||||
}) {
|
||||
for (const action of event.postHookActions) {
|
||||
await action();
|
||||
}
|
||||
}
|
||||
|
||||
it("blockSessionSave (pre-set) prevents memory file creation", async () => {
|
||||
const tempDir = await createCaseWorkspace("block-save");
|
||||
const sessionsDir = path.join(tempDir, "sessions");
|
||||
await fs.mkdir(sessionsDir, { recursive: true });
|
||||
@ -590,13 +599,46 @@ describe("session-memory hook", () => {
|
||||
event.context.blockSessionSave = true;
|
||||
|
||||
await handler(event);
|
||||
await drainPostHookActions(event);
|
||||
|
||||
const memoryDir = path.join(tempDir, "memory");
|
||||
const memoryFiles = await fs.readdir(memoryDir).catch(() => [] as string[]);
|
||||
expect(memoryFiles.filter((f) => f.endsWith(".md"))).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("sessionSaveContent overrides saved content", async () => {
|
||||
it("blockSessionSave (late-set) retracts memory file via postHookActions", async () => {
|
||||
const tempDir = await createCaseWorkspace("block-save-late");
|
||||
const sessionsDir = path.join(tempDir, "sessions");
|
||||
await fs.mkdir(sessionsDir, { recursive: true });
|
||||
const sessionFile = await writeWorkspaceFile({
|
||||
dir: sessionsDir,
|
||||
name: "test-session.jsonl",
|
||||
content: createMockSessionContent([{ role: "user", content: "secret" }]),
|
||||
});
|
||||
|
||||
const event = createHookEvent("command", "new", "agent:main:main", {
|
||||
cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig,
|
||||
previousSessionEntry: { sessionId: "s1", sessionFile },
|
||||
});
|
||||
|
||||
// Handler writes the file inline (fail-safe)
|
||||
await handler(event);
|
||||
|
||||
const memoryDir = path.join(tempDir, "memory");
|
||||
let memoryFiles = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md"));
|
||||
expect(memoryFiles.length).toBeGreaterThan(0); // file exists after inline write
|
||||
|
||||
// A later hook sets blockSessionSave
|
||||
event.context.blockSessionSave = true;
|
||||
|
||||
// Post-hook action retracts the file
|
||||
await drainPostHookActions(event);
|
||||
|
||||
memoryFiles = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md"));
|
||||
expect(memoryFiles).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("sessionSaveContent (pre-set) overrides saved content", async () => {
|
||||
const tempDir = await createCaseWorkspace("custom-content");
|
||||
const sessionsDir = path.join(tempDir, "sessions");
|
||||
await fs.mkdir(sessionsDir, { recursive: true });
|
||||
@ -613,6 +655,7 @@ describe("session-memory hook", () => {
|
||||
event.context.sessionSaveContent = "Custom summary from upstream hook";
|
||||
|
||||
await handler(event);
|
||||
await drainPostHookActions(event);
|
||||
|
||||
const memoryDir = path.join(tempDir, "memory");
|
||||
const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md"));
|
||||
@ -622,6 +665,37 @@ describe("session-memory hook", () => {
|
||||
expect(content).not.toContain("original");
|
||||
});
|
||||
|
||||
it("sessionSaveContent (late-set) overwrites file via postHookActions", async () => {
|
||||
const tempDir = await createCaseWorkspace("late-custom-content");
|
||||
const sessionsDir = path.join(tempDir, "sessions");
|
||||
await fs.mkdir(sessionsDir, { recursive: true });
|
||||
const sessionFile = await writeWorkspaceFile({
|
||||
dir: sessionsDir,
|
||||
name: "test-session.jsonl",
|
||||
content: createMockSessionContent([{ role: "user", content: "original" }]),
|
||||
});
|
||||
|
||||
const event = createHookEvent("command", "new", "agent:main:main", {
|
||||
cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig,
|
||||
previousSessionEntry: { sessionId: "s1", sessionFile },
|
||||
});
|
||||
|
||||
// Handler writes default content inline
|
||||
await handler(event);
|
||||
|
||||
// A later hook sets custom content
|
||||
event.context.sessionSaveContent = "Redacted by policy";
|
||||
|
||||
// Post-hook action overwrites
|
||||
await drainPostHookActions(event);
|
||||
|
||||
const memoryDir = path.join(tempDir, "memory");
|
||||
const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md"));
|
||||
expect(files.length).toBeGreaterThan(0);
|
||||
const content = await fs.readFile(path.join(memoryDir, files[0]), "utf-8");
|
||||
expect(content).toBe("Redacted by policy");
|
||||
});
|
||||
|
||||
it("sessionSaveContent empty string writes blank marker file", async () => {
|
||||
const tempDir = await createCaseWorkspace("empty-content");
|
||||
const sessionsDir = path.join(tempDir, "sessions");
|
||||
@ -636,17 +710,40 @@ describe("session-memory hook", () => {
|
||||
cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig,
|
||||
previousSessionEntry: { sessionId: "s1", sessionFile },
|
||||
});
|
||||
// Empty string is a valid override — persists a blank marker without
|
||||
// loading the transcript or generating an LLM slug.
|
||||
event.context.sessionSaveContent = "";
|
||||
|
||||
await handler(event);
|
||||
await drainPostHookActions(event);
|
||||
|
||||
const memoryDir = path.join(tempDir, "memory");
|
||||
const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md"));
|
||||
expect(files.length).toBeGreaterThan(0);
|
||||
const content = await fs.readFile(path.join(memoryDir, files[0]), "utf-8");
|
||||
// Should be truly empty — blank marker file
|
||||
expect(content).toBe("");
|
||||
});
|
||||
|
||||
it("fail-safe: file is preserved if postHookActions never drain", async () => {
|
||||
const tempDir = await createCaseWorkspace("fail-safe");
|
||||
const sessionsDir = path.join(tempDir, "sessions");
|
||||
await fs.mkdir(sessionsDir, { recursive: true });
|
||||
const sessionFile = await writeWorkspaceFile({
|
||||
dir: sessionsDir,
|
||||
name: "test-session.jsonl",
|
||||
content: createMockSessionContent([{ role: "user", content: "important data" }]),
|
||||
});
|
||||
|
||||
const event = createHookEvent("command", "new", "agent:main:main", {
|
||||
cfg: { agents: { defaults: { workspace: tempDir } } } satisfies OpenClawConfig,
|
||||
previousSessionEntry: { sessionId: "s1", sessionFile },
|
||||
});
|
||||
|
||||
await handler(event);
|
||||
// Deliberately do NOT drain postHookActions — simulates a system failure
|
||||
|
||||
const memoryDir = path.join(tempDir, "memory");
|
||||
const files = (await fs.readdir(memoryDir)).filter((f) => f.endsWith(".md"));
|
||||
expect(files.length).toBeGreaterThan(0);
|
||||
const content = await fs.readFile(path.join(memoryDir, files[0]), "utf-8");
|
||||
expect(content).toContain("important data");
|
||||
});
|
||||
});
|
||||
|
||||
@ -208,18 +208,12 @@ const saveSessionToMemory: HookHandler = async (event) => {
|
||||
|
||||
const context = event.context || {};
|
||||
|
||||
// Check if another hook (e.g., security plugin) blocked the save.
|
||||
// NOTE: Internal hooks execute sequentially in FIFO registration order.
|
||||
// Bundled hooks register before managed/workspace hooks (see workspace.ts
|
||||
// loadHookEntries merge order), so this check only works when the policy
|
||||
// decision is set by a typed plugin hook (registerTypedHook) that fires
|
||||
// earlier in the agent lifecycle, or by a hook loaded via extraDirs that
|
||||
// registers before bundled hooks. Managed/workspace internal hooks that
|
||||
// set blockSessionSave on the same event will run AFTER this handler.
|
||||
if (context.blockSessionSave === true) {
|
||||
log.debug("Session save blocked by upstream hook");
|
||||
return;
|
||||
}
|
||||
// NOTE: blockSessionSave and sessionSaveContent are checked in a
|
||||
// postHookActions callback (see bottom of this handler) so that hooks
|
||||
// registered after this bundled handler can still set them. The file
|
||||
// is written inline (fail-safe: if postHookActions never runs, data is
|
||||
// preserved on disk). The post-hook callback handles retraction
|
||||
// (blockSessionSave) and content replacement (sessionSaveContent).
|
||||
|
||||
const cfg = context.cfg as OpenClawConfig | undefined;
|
||||
const contextWorkspaceDir =
|
||||
@ -238,7 +232,6 @@ const saveSessionToMemory: HookHandler = async (event) => {
|
||||
sessionKey: event.sessionKey,
|
||||
});
|
||||
const memoryDir = path.join(workspaceDir, "memory");
|
||||
await fs.mkdir(memoryDir, { recursive: true });
|
||||
|
||||
// Get today's date for filename
|
||||
const now = new Date(event.timestamp);
|
||||
@ -368,18 +361,63 @@ const saveSessionToMemory: HookHandler = async (event) => {
|
||||
entry = entryParts.join("\n");
|
||||
}
|
||||
|
||||
// Write under memory root with alias-safe file validation.
|
||||
await writeFileWithinRoot({
|
||||
rootDir: memoryDir,
|
||||
relativePath: filename,
|
||||
data: entry,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
log.debug("Memory file written successfully");
|
||||
// Write inline (fail-safe: if postHookActions never drains, the file
|
||||
// is preserved on disk with the best content available at this point).
|
||||
// If blockSessionSave was already set by an upstream hook, skip the write.
|
||||
if (context.blockSessionSave === true) {
|
||||
log.debug("Session save blocked by upstream hook (inline check)");
|
||||
} else {
|
||||
await fs.mkdir(memoryDir, { recursive: true });
|
||||
await writeFileWithinRoot({
|
||||
rootDir: memoryDir,
|
||||
relativePath: filename,
|
||||
data: entry,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
log.debug("Memory file written successfully");
|
||||
|
||||
// Log completion (but don't send user-visible confirmation - it's internal housekeeping)
|
||||
const relPath = memoryFilePath.replace(os.homedir(), "~");
|
||||
log.info(`Session context saved to ${relPath}`);
|
||||
const relPath = memoryFilePath.replace(os.homedir(), "~");
|
||||
log.info(`Session context saved to ${relPath}`);
|
||||
}
|
||||
|
||||
// Defer retraction/replacement to post-hook phase so that hooks
|
||||
// registered after this handler can set blockSessionSave or
|
||||
// sessionSaveContent and still have them honored.
|
||||
const writtenEntry = context.blockSessionSave === true ? null : entry;
|
||||
event.postHookActions.push(async () => {
|
||||
try {
|
||||
// If a later hook blocked the save, retract the file we just wrote.
|
||||
if (event.context.blockSessionSave === true && writtenEntry !== null) {
|
||||
try {
|
||||
await fs.unlink(memoryFilePath);
|
||||
log.debug("Session save retracted by post-hook (blockSessionSave)");
|
||||
} catch (err) {
|
||||
// File may not exist if inline write also didn't happen — that's fine.
|
||||
if ((err as NodeJS.ErrnoException).code !== "ENOENT") {
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
// If a later hook set sessionSaveContent, overwrite with new content.
|
||||
const postContent = event.context.sessionSaveContent;
|
||||
if (typeof postContent === "string" && postContent !== writtenEntry) {
|
||||
await writeFileWithinRoot({
|
||||
rootDir: memoryDir,
|
||||
relativePath: filename,
|
||||
data: postContent,
|
||||
encoding: "utf-8",
|
||||
});
|
||||
log.debug("Session save content replaced by post-hook (sessionSaveContent)", {
|
||||
length: postContent.length,
|
||||
});
|
||||
}
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
log.error(`Post-hook session-memory action failed: ${message}`);
|
||||
}
|
||||
});
|
||||
} catch (err) {
|
||||
if (err instanceof Error) {
|
||||
log.error("Failed to save session memory", {
|
||||
|
||||
@ -445,6 +445,100 @@ describe("hooks", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("postHookActions", () => {
|
||||
it("executes post-hook actions after all handlers complete", async () => {
|
||||
const order: string[] = [];
|
||||
|
||||
registerInternalHook("command:new", async (event) => {
|
||||
order.push("handler-1");
|
||||
event.postHookActions.push(async () => {
|
||||
order.push("post-action-1");
|
||||
});
|
||||
});
|
||||
|
||||
registerInternalHook("command:new", async () => {
|
||||
order.push("handler-2");
|
||||
});
|
||||
|
||||
const event = createInternalHookEvent("command", "new", "test-session");
|
||||
await triggerInternalHook(event);
|
||||
|
||||
expect(order).toEqual(["handler-1", "handler-2", "post-action-1"]);
|
||||
});
|
||||
|
||||
it("post-hook actions see context set by later handlers", async () => {
|
||||
let sawFlag = false;
|
||||
|
||||
registerInternalHook("command:new", async (event) => {
|
||||
event.postHookActions.push(async () => {
|
||||
sawFlag = event.context.myFlag === true;
|
||||
});
|
||||
});
|
||||
|
||||
registerInternalHook("command:new", async (event) => {
|
||||
event.context.myFlag = true;
|
||||
});
|
||||
|
||||
const event = createInternalHookEvent("command", "new", "test-session");
|
||||
await triggerInternalHook(event);
|
||||
|
||||
expect(sawFlag).toBe(true);
|
||||
});
|
||||
|
||||
it("post-hook action errors don't prevent other post-hook actions", async () => {
|
||||
const executed: string[] = [];
|
||||
|
||||
registerInternalHook("command:new", async (event) => {
|
||||
event.postHookActions.push(async () => {
|
||||
throw new Error("boom");
|
||||
});
|
||||
event.postHookActions.push(async () => {
|
||||
executed.push("second-action");
|
||||
});
|
||||
});
|
||||
|
||||
const event = createInternalHookEvent("command", "new", "test-session");
|
||||
await triggerInternalHook(event);
|
||||
|
||||
expect(executed).toEqual(["second-action"]);
|
||||
});
|
||||
|
||||
it("multiple handlers can push post-hook actions; all execute in order", async () => {
|
||||
const order: string[] = [];
|
||||
|
||||
registerInternalHook("command:new", async (event) => {
|
||||
event.postHookActions.push(() => {
|
||||
order.push("from-handler-1");
|
||||
});
|
||||
});
|
||||
|
||||
registerInternalHook("command:new", async (event) => {
|
||||
event.postHookActions.push(() => {
|
||||
order.push("from-handler-2");
|
||||
});
|
||||
});
|
||||
|
||||
const event = createInternalHookEvent("command", "new", "test-session");
|
||||
await triggerInternalHook(event);
|
||||
|
||||
expect(order).toEqual(["from-handler-1", "from-handler-2"]);
|
||||
});
|
||||
|
||||
it("initializes postHookActions as empty array", () => {
|
||||
const event = createInternalHookEvent("command", "new", "test-session");
|
||||
expect(event.postHookActions).toEqual([]);
|
||||
});
|
||||
|
||||
it("does not run post-hook actions when no handlers are registered", async () => {
|
||||
const event = createInternalHookEvent("command", "new", "test-session");
|
||||
event.postHookActions.push(() => {
|
||||
throw new Error("should not run");
|
||||
});
|
||||
// triggerInternalHook returns early when no handlers — post-hooks don't run
|
||||
await expect(triggerInternalHook(event)).resolves.not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("getRegisteredEventKeys", () => {
|
||||
it("should return all registered event keys", () => {
|
||||
registerInternalHook("command:new", vi.fn());
|
||||
|
||||
@ -169,6 +169,12 @@ export interface InternalHookEvent {
|
||||
timestamp: Date;
|
||||
/** Messages to send back to the user (hooks can push to this array) */
|
||||
messages: string[];
|
||||
/** Deferred actions to run after all handlers complete.
|
||||
* Handlers push async callbacks here; triggerInternalHook drains them
|
||||
* sequentially after the main handler loop. This eliminates FIFO
|
||||
* registration-order dependencies: a handler that runs early can defer
|
||||
* work that depends on context set by later handlers. */
|
||||
postHookActions: Array<() => Promise<void> | void>;
|
||||
}
|
||||
|
||||
export type InternalHookHandler = (event: InternalHookEvent) => Promise<void> | void;
|
||||
@ -285,6 +291,21 @@ export async function triggerInternalHook(event: InternalHookEvent): Promise<voi
|
||||
log.error(`Hook error [${event.type}:${event.action}]: ${message}`);
|
||||
}
|
||||
}
|
||||
|
||||
// Drain post-hook actions — these run after all handlers have had
|
||||
// a chance to mutate event.context, eliminating FIFO ordering issues.
|
||||
// Actions execute in push order; errors are caught per-action so one
|
||||
// failure doesn't block others.
|
||||
if (event.postHookActions.length > 0) {
|
||||
for (const action of event.postHookActions) {
|
||||
try {
|
||||
await action();
|
||||
} catch (err) {
|
||||
const message = err instanceof Error ? err.message : String(err);
|
||||
log.error(`Post-hook action error [${event.type}:${event.action}]: ${message}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@ -308,6 +329,7 @@ export function createInternalHookEvent(
|
||||
context,
|
||||
timestamp: new Date(),
|
||||
messages: [],
|
||||
postHookActions: [],
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user