refactor(hooks): split session events into individual typed events

Major changes:
- Replace monolithic SessionEvent with reason discriminator with individual
  event types: session_start, session_before_switch, session_switch,
  session_before_new, session_new, session_before_branch, session_branch,
  session_before_compact, session_compact, session_shutdown
- Each event has dedicated result type (SessionBeforeSwitchResult, etc.)
- HookHandler type now allows bare return statements (void in return type)
- HookAPI.on() has proper overloads for each event with correct typing

Additional fixes:
- AgentSession now always subscribes to agent in constructor (was only
  subscribing when external subscribe() called, breaking internal handlers)
- Standardize on undefined over null throughout codebase
- HookUIContext methods return undefined instead of null
- SessionManager methods return undefined instead of null
- Simplify hook exports to 'export type * from types.js'
- Add detailed JSDoc for skipConversationRestore vs cancel
- Fix createBranchedSession to rebuild index in persist mode
- newSession() now returns the session file path

Updated all example hooks, tests, and emission sites to use new event types.
This commit is contained in:
Mario Zechner 2025-12-28 20:06:20 +01:00
parent 38d65dfe59
commit d6283f99dc
43 changed files with 2129 additions and 640 deletions

View file

@ -100,7 +100,7 @@ describe.skipIf(!API_KEY)("AgentSession branching", () => {
createSession(true);
// Verify sessions are disabled
expect(session.sessionFile).toBeNull();
expect(session.sessionFile).toBeUndefined();
// Send one message
await session.prompt("Say hi");
@ -121,8 +121,8 @@ describe.skipIf(!API_KEY)("AgentSession branching", () => {
// After branching, conversation should be empty
expect(session.messages.length).toBe(0);
// Session file should still be null (no file created)
expect(session.sessionFile).toBeNull();
// Session file should still be undefined (no file created)
expect(session.sessionFile).toBeUndefined();
});
it("should branch from middle of conversation", async () => {

View file

@ -102,7 +102,7 @@ describe.skipIf(!API_KEY)("AgentSession compaction e2e", () => {
// First message should be the summary (a user message with summary content)
const firstMsg = messages[0];
expect(firstMsg.role).toBe("user");
expect(firstMsg.role).toBe("compactionSummary");
}, 120000);
it("should maintain valid session state after compaction", async () => {

View file

@ -3,18 +3,16 @@
*/
import { describe, expect, it } from "vitest";
import type { HookAPI } from "../src/core/hooks/index.js";
import type { HookAPI, SessionBeforeCompactEvent, SessionCompactEvent } from "../src/core/hooks/index.js";
describe("Documentation example", () => {
it("custom compaction example should type-check correctly", () => {
// This is the example from hooks.md - verify it compiles
const exampleHook = (pi: HookAPI) => {
pi.on("session", async (event, ctx) => {
if (event.reason !== "before_compact") return;
// After narrowing, these should all be accessible
// sessionManager and modelRegistry come from ctx, not event
pi.on("session_before_compact", async (event: SessionBeforeCompactEvent, ctx) => {
// All these should be accessible on the event
const { preparation, previousCompactions, model } = event;
// sessionManager and modelRegistry come from ctx, not event
const { sessionManager, modelRegistry } = ctx;
const { messagesToSummarize, messagesToKeep, tokensBefore, firstKeptEntryId, cutPoint } = preparation;
@ -51,12 +49,10 @@ describe("Documentation example", () => {
expect(typeof exampleHook).toBe("function");
});
it("compact event should have correct fields after narrowing", () => {
it("compact event should have correct fields", () => {
const checkCompactEvent = (pi: HookAPI) => {
pi.on("session", async (event, _ctx) => {
if (event.reason !== "compact") return;
// After narrowing, these should all be accessible
pi.on("session_compact", async (event: SessionCompactEvent) => {
// These should all be accessible
const entry = event.compactionEntry;
const fromHook = event.fromHook;

View file

@ -10,7 +10,13 @@ import { getModel } from "@mariozechner/pi-ai";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { AgentSession } from "../src/core/agent-session.js";
import { AuthStorage } from "../src/core/auth-storage.js";
import { HookRunner, type LoadedHook, type SessionEvent } from "../src/core/hooks/index.js";
import {
HookRunner,
type LoadedHook,
type SessionBeforeCompactEvent,
type SessionCompactEvent,
type SessionEvent,
} from "../src/core/hooks/index.js";
import { ModelRegistry } from "../src/core/model-registry.js";
import { SessionManager } from "../src/core/session-manager.js";
import { SettingsManager } from "../src/core/settings-manager.js";
@ -40,19 +46,25 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
});
function createHook(
onBeforeCompact?: (event: SessionEvent) => { cancel?: boolean; compaction?: any } | undefined,
onCompact?: (event: SessionEvent) => void,
onBeforeCompact?: (event: SessionBeforeCompactEvent) => { cancel?: boolean; compaction?: any } | undefined,
onCompact?: (event: SessionCompactEvent) => void,
): LoadedHook {
const handlers = new Map<string, ((event: any, ctx: any) => Promise<any>)[]>();
handlers.set("session", [
async (event: SessionEvent) => {
handlers.set("session_before_compact", [
async (event: SessionBeforeCompactEvent) => {
capturedEvents.push(event);
if (event.reason === "before_compact" && onBeforeCompact) {
if (onBeforeCompact) {
return onBeforeCompact(event);
}
if (event.reason === "compact" && onCompact) {
return undefined;
},
]);
handlers.set("session_compact", [
async (event: SessionCompactEvent) => {
capturedEvents.push(event);
if (onCompact) {
onCompact(event);
}
return undefined;
@ -89,9 +101,9 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
hookRunner = new HookRunner(hooks, tempDir, sessionManager, modelRegistry);
hookRunner.setUIContext(
{
select: async () => null,
select: async () => undefined,
confirm: async () => false,
input: async () => null,
input: async () => undefined,
notify: () => {},
custom: () => ({ close: () => {}, requestRender: () => {} }),
},
@ -121,30 +133,28 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
await session.compact();
const beforeCompactEvents = capturedEvents.filter((e) => e.reason === "before_compact");
const compactEvents = capturedEvents.filter((e) => e.reason === "compact");
const beforeCompactEvents = capturedEvents.filter(
(e): e is SessionBeforeCompactEvent => e.type === "session_before_compact",
);
const compactEvents = capturedEvents.filter((e): e is SessionCompactEvent => e.type === "session_compact");
expect(beforeCompactEvents.length).toBe(1);
expect(compactEvents.length).toBe(1);
const beforeEvent = beforeCompactEvents[0];
if (beforeEvent.reason === "before_compact") {
expect(beforeEvent.preparation).toBeDefined();
expect(beforeEvent.preparation.cutPoint.firstKeptEntryIndex).toBeGreaterThanOrEqual(0);
expect(beforeEvent.preparation.messagesToSummarize).toBeDefined();
expect(beforeEvent.preparation.messagesToKeep).toBeDefined();
expect(beforeEvent.preparation.tokensBefore).toBeGreaterThanOrEqual(0);
expect(beforeEvent.model).toBeDefined();
// sessionManager and modelRegistry are now on ctx, not event
}
expect(beforeEvent.preparation).toBeDefined();
expect(beforeEvent.preparation.cutPoint.firstKeptEntryIndex).toBeGreaterThanOrEqual(0);
expect(beforeEvent.preparation.messagesToSummarize).toBeDefined();
expect(beforeEvent.preparation.messagesToKeep).toBeDefined();
expect(beforeEvent.preparation.tokensBefore).toBeGreaterThanOrEqual(0);
expect(beforeEvent.model).toBeDefined();
// sessionManager and modelRegistry are now on ctx, not event
const afterEvent = compactEvents[0];
if (afterEvent.reason === "compact") {
expect(afterEvent.compactionEntry).toBeDefined();
expect(afterEvent.compactionEntry.summary.length).toBeGreaterThan(0);
expect(afterEvent.compactionEntry.tokensBefore).toBeGreaterThanOrEqual(0);
expect(afterEvent.fromHook).toBe(false);
}
expect(afterEvent.compactionEntry).toBeDefined();
expect(afterEvent.compactionEntry.summary.length).toBeGreaterThan(0);
expect(afterEvent.compactionEntry.tokensBefore).toBeGreaterThanOrEqual(0);
expect(afterEvent.fromHook).toBe(false);
}, 120000);
it("should allow hooks to cancel compaction", async () => {
@ -156,7 +166,7 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
await expect(session.compact()).rejects.toThrow("Compaction cancelled");
const compactEvents = capturedEvents.filter((e) => e.reason === "compact");
const compactEvents = capturedEvents.filter((e) => e.type === "session_compact");
expect(compactEvents.length).toBe(0);
}, 120000);
@ -164,7 +174,7 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
const customSummary = "Custom summary from hook";
const hook = createHook((event) => {
if (event.reason === "before_compact") {
if (event.type === "session_before_compact") {
return {
compaction: {
summary: customSummary,
@ -187,11 +197,11 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
expect(result.summary).toBe(customSummary);
const compactEvents = capturedEvents.filter((e) => e.reason === "compact");
const compactEvents = capturedEvents.filter((e) => e.type === "session_compact");
expect(compactEvents.length).toBe(1);
const afterEvent = compactEvents[0];
if (afterEvent.reason === "compact") {
if (afterEvent.type === "session_compact") {
expect(afterEvent.compactionEntry.summary).toBe(customSummary);
expect(afterEvent.fromHook).toBe(true);
}
@ -206,11 +216,11 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
await session.compact();
const compactEvents = capturedEvents.filter((e) => e.reason === "compact");
const compactEvents = capturedEvents.filter((e) => e.type === "session_compact");
expect(compactEvents.length).toBe(1);
const afterEvent = compactEvents[0];
if (afterEvent.reason === "compact") {
if (afterEvent.type === "session_compact") {
// sessionManager is now on ctx, use session.sessionManager directly
const entries = session.sessionManager.getEntries();
const hasCompactionEntry = entries.some((e: { type: string }) => e.type === "compaction");
@ -224,13 +234,19 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
resolvedPath: "/test/throwing-hook.ts",
handlers: new Map<string, ((event: any, ctx: any) => Promise<any>)[]>([
[
"session",
"session_before_compact",
[
async (event: SessionEvent) => {
async (event: SessionBeforeCompactEvent) => {
capturedEvents.push(event);
throw new Error("Hook intentionally throws");
},
],
],
[
"session_compact",
[
async (event: SessionCompactEvent) => {
capturedEvents.push(event);
if (event.reason === "before_compact") {
throw new Error("Hook intentionally failed");
}
return undefined;
},
],
@ -252,12 +268,9 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
expect(result.summary).toBeDefined();
expect(result.summary.length).toBeGreaterThan(0);
const compactEvents = capturedEvents.filter((e) => e.reason === "compact");
const compactEvents = capturedEvents.filter((e): e is SessionCompactEvent => e.type === "session_compact");
expect(compactEvents.length).toBe(1);
if (compactEvents[0].reason === "compact") {
expect(compactEvents[0].fromHook).toBe(false);
}
expect(compactEvents[0].fromHook).toBe(false);
}, 120000);
it("should call multiple hooks in order", async () => {
@ -268,15 +281,19 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
resolvedPath: "/test/hook1.ts",
handlers: new Map<string, ((event: any, ctx: any) => Promise<any>)[]>([
[
"session",
"session_before_compact",
[
async (event: SessionEvent) => {
if (event.reason === "before_compact") {
callOrder.push("hook1-before");
}
if (event.reason === "compact") {
callOrder.push("hook1-after");
}
async () => {
callOrder.push("hook1-before");
return undefined;
},
],
],
[
"session_compact",
[
async () => {
callOrder.push("hook1-after");
return undefined;
},
],
@ -293,15 +310,19 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
resolvedPath: "/test/hook2.ts",
handlers: new Map<string, ((event: any, ctx: any) => Promise<any>)[]>([
[
"session",
"session_before_compact",
[
async (event: SessionEvent) => {
if (event.reason === "before_compact") {
callOrder.push("hook2-before");
}
if (event.reason === "compact") {
callOrder.push("hook2-after");
}
async () => {
callOrder.push("hook2-before");
return undefined;
},
],
],
[
"session_compact",
[
async () => {
callOrder.push("hook2-after");
return undefined;
},
],
@ -324,12 +345,10 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
}, 120000);
it("should pass correct data in before_compact event", async () => {
let capturedBeforeEvent: (SessionEvent & { reason: "before_compact" }) | null = null;
let capturedBeforeEvent: SessionBeforeCompactEvent | null = null;
const hook = createHook((event) => {
if (event.reason === "before_compact") {
capturedBeforeEvent = event;
}
capturedBeforeEvent = event;
return undefined;
});
createSession([hook]);
@ -370,7 +389,7 @@ describe.skipIf(!API_KEY)("Compaction hooks", () => {
const customSummary = "Custom summary with modified values";
const hook = createHook((event) => {
if (event.reason === "before_compact") {
if (event.type === "session_before_compact") {
return {
compaction: {
summary: customSummary,

View file

@ -180,9 +180,9 @@ describe("getLastAssistantUsage", () => {
expect(usage!.input).toBe(100);
});
it("should return null if no assistant messages", () => {
it("should return undefined if no assistant messages", () => {
const entries: SessionEntry[] = [createMessageEntry(createUserMessage("Hello"))];
expect(getLastAssistantUsage(entries)).toBeNull();
expect(getLastAssistantUsage(entries)).toBeUndefined();
});
});
@ -301,8 +301,8 @@ describe("buildSessionContext", () => {
const loaded = buildSessionContext(entries);
// summary + kept (u2, a2) + after (u3, a3) = 5
expect(loaded.messages.length).toBe(5);
expect(loaded.messages[0].role).toBe("user");
expect((loaded.messages[0] as any).content).toContain("Summary of 1,a,2,b");
expect(loaded.messages[0].role).toBe("compactionSummary");
expect((loaded.messages[0] as any).summary).toContain("Summary of 1,a,2,b");
});
it("should handle multiple compactions (only latest matters)", () => {
@ -325,7 +325,7 @@ describe("buildSessionContext", () => {
const loaded = buildSessionContext(entries);
// summary + kept from u3 (u3, c) + after (u4, d) = 5
expect(loaded.messages.length).toBe(5);
expect((loaded.messages[0] as any).content).toContain("Second summary");
expect((loaded.messages[0] as any).summary).toContain("Second summary");
});
it("should keep all messages when firstKeptEntryId is first entry", () => {
@ -443,8 +443,8 @@ describe.skipIf(!process.env.ANTHROPIC_OAUTH_TOKEN)("LLM summarization", () => {
// Should have summary + kept messages
expect(reloaded.messages.length).toBeLessThan(loaded.messages.length);
expect(reloaded.messages[0].role).toBe("user");
expect((reloaded.messages[0] as any).content).toContain(compactionResult.summary);
expect(reloaded.messages[0].role).toBe("compactionSummary");
expect((reloaded.messages[0] as any).summary).toContain(compactionResult.summary);
console.log("Original messages:", loaded.messages.length);
console.log("After compaction:", reloaded.messages.length);

View file

@ -66,21 +66,21 @@ describe("parseModelPattern", () => {
const result = parseModelPattern("claude-sonnet-4-5", allModels);
expect(result.model?.id).toBe("claude-sonnet-4-5");
expect(result.thinkingLevel).toBe("off");
expect(result.warning).toBeNull();
expect(result.warning).toBeUndefined();
});
test("partial match returns best model", () => {
const result = parseModelPattern("sonnet", allModels);
expect(result.model?.id).toBe("claude-sonnet-4-5");
expect(result.thinkingLevel).toBe("off");
expect(result.warning).toBeNull();
expect(result.warning).toBeUndefined();
});
test("no match returns null model", () => {
const result = parseModelPattern("nonexistent", allModels);
expect(result.model).toBeNull();
expect(result.model).toBeUndefined();
expect(result.thinkingLevel).toBe("off");
expect(result.warning).toBeNull();
expect(result.warning).toBeUndefined();
});
});
@ -89,14 +89,14 @@ describe("parseModelPattern", () => {
const result = parseModelPattern("sonnet:high", allModels);
expect(result.model?.id).toBe("claude-sonnet-4-5");
expect(result.thinkingLevel).toBe("high");
expect(result.warning).toBeNull();
expect(result.warning).toBeUndefined();
});
test("gpt-4o:medium returns gpt-4o with medium thinking level", () => {
const result = parseModelPattern("gpt-4o:medium", allModels);
expect(result.model?.id).toBe("gpt-4o");
expect(result.thinkingLevel).toBe("medium");
expect(result.warning).toBeNull();
expect(result.warning).toBeUndefined();
});
test("all valid thinking levels work", () => {
@ -104,7 +104,7 @@ describe("parseModelPattern", () => {
const result = parseModelPattern(`sonnet:${level}`, allModels);
expect(result.model?.id).toBe("claude-sonnet-4-5");
expect(result.thinkingLevel).toBe(level);
expect(result.warning).toBeNull();
expect(result.warning).toBeUndefined();
}
});
});
@ -131,7 +131,7 @@ describe("parseModelPattern", () => {
const result = parseModelPattern("qwen/qwen3-coder:exacto", allModels);
expect(result.model?.id).toBe("qwen/qwen3-coder:exacto");
expect(result.thinkingLevel).toBe("off");
expect(result.warning).toBeNull();
expect(result.warning).toBeUndefined();
});
test("openrouter/qwen/qwen3-coder:exacto matches with provider prefix", () => {
@ -139,14 +139,14 @@ describe("parseModelPattern", () => {
expect(result.model?.id).toBe("qwen/qwen3-coder:exacto");
expect(result.model?.provider).toBe("openrouter");
expect(result.thinkingLevel).toBe("off");
expect(result.warning).toBeNull();
expect(result.warning).toBeUndefined();
});
test("qwen3-coder:exacto:high matches model with high thinking level", () => {
const result = parseModelPattern("qwen/qwen3-coder:exacto:high", allModels);
expect(result.model?.id).toBe("qwen/qwen3-coder:exacto");
expect(result.thinkingLevel).toBe("high");
expect(result.warning).toBeNull();
expect(result.warning).toBeUndefined();
});
test("openrouter/qwen/qwen3-coder:exacto:high matches with provider and thinking level", () => {
@ -154,14 +154,14 @@ describe("parseModelPattern", () => {
expect(result.model?.id).toBe("qwen/qwen3-coder:exacto");
expect(result.model?.provider).toBe("openrouter");
expect(result.thinkingLevel).toBe("high");
expect(result.warning).toBeNull();
expect(result.warning).toBeUndefined();
});
test("gpt-4o:extended matches the extended model", () => {
const result = parseModelPattern("openai/gpt-4o:extended", allModels);
expect(result.model?.id).toBe("openai/gpt-4o:extended");
expect(result.thinkingLevel).toBe("off");
expect(result.warning).toBeNull();
expect(result.warning).toBeUndefined();
});
});

View file

@ -273,7 +273,7 @@ describe.skipIf(!process.env.ANTHROPIC_API_KEY && !process.env.ANTHROPIC_OAUTH_T
// Initially null
let text = await client.getLastAssistantText();
expect(text).toBeNull();
expect(text).toBeUndefined();
// Send prompt
await client.promptAndWait("Reply with just: test123");

View file

@ -48,7 +48,7 @@ function compaction(id: string, parentId: string | null, summary: string, firstK
};
}
function branchSummary(id: string, parentId: string | null, fromId: string, summary: string): BranchSummaryEntry {
function branchSummary(id: string, parentId: string | null, summary: string, fromId: string): BranchSummaryEntry {
return { type: "branch_summary", id, parentId, timestamp: "2025-01-01T00:00:00Z", summary, fromId };
}
@ -132,7 +132,7 @@ describe("buildSessionContext", () => {
// Should have: summary + kept (3,4) + after (6,7) = 5 messages
expect(ctx.messages).toHaveLength(5);
expect((ctx.messages[0] as any).content).toContain("Summary of first two turns");
expect((ctx.messages[0] as any).summary).toContain("Summary of first two turns");
expect((ctx.messages[1] as any).content).toBe("second");
expect((ctx.messages[2] as any).content[0].text).toBe("response2");
expect((ctx.messages[3] as any).content).toBe("third");
@ -150,7 +150,7 @@ describe("buildSessionContext", () => {
// Summary + all messages (1,2,4)
expect(ctx.messages).toHaveLength(4);
expect((ctx.messages[0] as any).content).toContain("Empty summary");
expect((ctx.messages[0] as any).summary).toContain("Empty summary");
});
it("multiple compactions uses latest", () => {
@ -167,7 +167,7 @@ describe("buildSessionContext", () => {
// Should use second summary, keep from 4
expect(ctx.messages).toHaveLength(4);
expect((ctx.messages[0] as any).content).toContain("Second summary");
expect((ctx.messages[0] as any).summary).toContain("Second summary");
});
});
@ -203,7 +203,7 @@ describe("buildSessionContext", () => {
const ctx = buildSessionContext(entries, "5");
expect(ctx.messages).toHaveLength(4);
expect((ctx.messages[2] as any).content).toContain("Summary of abandoned work");
expect((ctx.messages[2] as any).summary).toContain("Summary of abandoned work");
expect((ctx.messages[3] as any).content).toBe("new direction");
});
@ -231,7 +231,7 @@ describe("buildSessionContext", () => {
// Main path to 7: summary + kept(3,4) + after(6,7)
const ctxMain = buildSessionContext(entries, "7");
expect(ctxMain.messages).toHaveLength(5);
expect((ctxMain.messages[0] as any).content).toContain("Compacted history");
expect((ctxMain.messages[0] as any).summary).toContain("Compacted history");
expect((ctxMain.messages[1] as any).content).toBe("q2");
expect((ctxMain.messages[2] as any).content[0].text).toBe("r2");
expect((ctxMain.messages[3] as any).content).toBe("q3");
@ -243,7 +243,7 @@ describe("buildSessionContext", () => {
expect((ctxBranch.messages[0] as any).content).toBe("start");
expect((ctxBranch.messages[1] as any).content[0].text).toBe("r1");
expect((ctxBranch.messages[2] as any).content).toBe("q2");
expect((ctxBranch.messages[3] as any).content).toContain("Tried wrong approach");
expect((ctxBranch.messages[3] as any).summary).toContain("Tried wrong approach");
expect((ctxBranch.messages[4] as any).content).toBe("better approach");
});
});

View file

@ -451,7 +451,7 @@ describe("createBranchedSession", () => {
// Create branched session from id2 (should only have 1 -> 2)
const result = session.createBranchedSession(id2);
expect(result).toBeNull(); // in-memory returns null
expect(result).toBeUndefined(); // in-memory returns null
// Session should now only have entries 1 and 2
const entries = session.getEntries();