refactor: add hook guards and test helpers

This commit is contained in:
Peter Steinberger
2026-01-18 06:07:20 +00:00
parent 32dd052260
commit 28f8b7bafa
12 changed files with 145 additions and 50 deletions

View File

@@ -1,10 +1,12 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path"; import path from "node:path";
import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { resolveBootstrapContextForRun, resolveBootstrapFilesForRun } from "./bootstrap-files.js"; import {
resolveBootstrapContextForRun,
resolveBootstrapFilesForRun,
} from "./bootstrap-files.js";
import { makeTempWorkspace } from "../test-helpers/workspace.js";
import { import {
clearInternalHooks, clearInternalHooks,
registerInternalHook, registerInternalHook,
@@ -29,7 +31,7 @@ describe("resolveBootstrapFilesForRun", () => {
]; ];
}); });
const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-bootstrap-")); const workspaceDir = await makeTempWorkspace("clawdbot-bootstrap-");
const files = await resolveBootstrapFilesForRun({ workspaceDir }); const files = await resolveBootstrapFilesForRun({ workspaceDir });
expect(files.some((file) => file.name === "EXTRA.md")).toBe(true); expect(files.some((file) => file.name === "EXTRA.md")).toBe(true);
@@ -54,7 +56,7 @@ describe("resolveBootstrapContextForRun", () => {
]; ];
}); });
const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-bootstrap-")); const workspaceDir = await makeTempWorkspace("clawdbot-bootstrap-");
const result = await resolveBootstrapContextForRun({ workspaceDir }); const result = await resolveBootstrapContextForRun({ workspaceDir });
const extra = result.contextFiles.find((file) => file.path === "EXTRA.md"); const extra = result.contextFiles.find((file) => file.path === "EXTRA.md");

View File

@@ -8,6 +8,14 @@ import {
import { buildBootstrapContextFiles, resolveBootstrapMaxChars } from "./pi-embedded-helpers.js"; import { buildBootstrapContextFiles, resolveBootstrapMaxChars } from "./pi-embedded-helpers.js";
import type { EmbeddedContextFile } from "./pi-embedded-helpers.js"; import type { EmbeddedContextFile } from "./pi-embedded-helpers.js";
export function makeBootstrapWarn(params: {
sessionLabel: string;
warn?: (message: string) => void;
}): ((message: string) => void) | undefined {
if (!params.warn) return undefined;
return (message: string) => params.warn?.(`${message} (sessionKey=${params.sessionLabel})`);
}
export async function resolveBootstrapFilesForRun(params: { export async function resolveBootstrapFilesForRun(params: {
workspaceDir: string; workspaceDir: string;
config?: ClawdbotConfig; config?: ClawdbotConfig;

View File

@@ -7,7 +7,7 @@ import { createSubsystemLogger } from "../logging.js";
import { runCommandWithTimeout } from "../process/exec.js"; import { runCommandWithTimeout } from "../process/exec.js";
import { resolveUserPath } from "../utils.js"; import { resolveUserPath } from "../utils.js";
import { resolveSessionAgentIds } from "./agent-scope.js"; import { resolveSessionAgentIds } from "./agent-scope.js";
import { resolveBootstrapContextForRun } from "./bootstrap-files.js"; import { makeBootstrapWarn, resolveBootstrapContextForRun } from "./bootstrap-files.js";
import { resolveCliBackendConfig } from "./cli-backends.js"; import { resolveCliBackendConfig } from "./cli-backends.js";
import { import {
appendImagePathsToPrompt, appendImagePathsToPrompt,
@@ -73,7 +73,7 @@ export async function runCliAgent(params: {
config: params.config, config: params.config,
sessionKey: params.sessionKey, sessionKey: params.sessionKey,
sessionId: params.sessionId, sessionId: params.sessionId,
warn: (message) => log.warn(`${message} (sessionKey=${sessionLabel})`), warn: makeBootstrapWarn({ sessionLabel, warn: (message) => log.warn(message) }),
}); });
const { defaultAgentId, sessionAgentId } = resolveSessionAgentIds({ const { defaultAgentId, sessionAgentId } = resolveSessionAgentIds({
sessionKey: params.sessionKey, sessionKey: params.sessionKey,

View File

@@ -16,7 +16,7 @@ import { isReasoningTagProvider } from "../../utils/provider-utils.js";
import { resolveUserPath } from "../../utils.js"; import { resolveUserPath } from "../../utils.js";
import { resolveClawdbotAgentDir } from "../agent-paths.js"; import { resolveClawdbotAgentDir } from "../agent-paths.js";
import { resolveSessionAgentIds } from "../agent-scope.js"; import { resolveSessionAgentIds } from "../agent-scope.js";
import { resolveBootstrapContextForRun } from "../bootstrap-files.js"; import { makeBootstrapWarn, resolveBootstrapContextForRun } from "../bootstrap-files.js";
import type { ExecElevatedDefaults } from "../bash-tools.js"; import type { ExecElevatedDefaults } from "../bash-tools.js";
import { DEFAULT_MODEL, DEFAULT_PROVIDER } from "../defaults.js"; import { DEFAULT_MODEL, DEFAULT_PROVIDER } from "../defaults.js";
import { getApiKeyForModel, resolveModelAuthMode } from "../model-auth.js"; import { getApiKeyForModel, resolveModelAuthMode } from "../model-auth.js";
@@ -181,7 +181,7 @@ export async function compactEmbeddedPiSession(params: {
config: params.config, config: params.config,
sessionKey: params.sessionKey, sessionKey: params.sessionKey,
sessionId: params.sessionId, sessionId: params.sessionId,
warn: (message) => log.warn(`${message} (sessionKey=${sessionLabel})`), warn: makeBootstrapWarn({ sessionLabel, warn: (message) => log.warn(message) }),
}); });
const runAbortController = new AbortController(); const runAbortController = new AbortController();
const toolsRaw = createClawdbotCodingTools({ const toolsRaw = createClawdbotCodingTools({

View File

@@ -17,7 +17,7 @@ import { isSubagentSessionKey } from "../../../routing/session-key.js";
import { resolveUserPath } from "../../../utils.js"; import { resolveUserPath } from "../../../utils.js";
import { resolveClawdbotAgentDir } from "../../agent-paths.js"; import { resolveClawdbotAgentDir } from "../../agent-paths.js";
import { resolveSessionAgentIds } from "../../agent-scope.js"; import { resolveSessionAgentIds } from "../../agent-scope.js";
import { resolveBootstrapContextForRun } from "../../bootstrap-files.js"; import { makeBootstrapWarn, resolveBootstrapContextForRun } from "../../bootstrap-files.js";
import { resolveModelAuthMode } from "../../model-auth.js"; import { resolveModelAuthMode } from "../../model-auth.js";
import { import {
isCloudCodeAssistFormatError, isCloudCodeAssistFormatError,
@@ -126,7 +126,7 @@ export async function runEmbeddedAttempt(
config: params.config, config: params.config,
sessionKey: params.sessionKey, sessionKey: params.sessionKey,
sessionId: params.sessionId, sessionId: params.sessionId,
warn: (message) => log.warn(`${message} (sessionKey=${sessionLabel})`), warn: makeBootstrapWarn({ sessionLabel, warn: (message) => log.warn(message) }),
}); });
const agentDir = params.agentDir ?? resolveClawdbotAgentDir(); const agentDir = params.agentDir ?? resolveClawdbotAgentDir();

View File

@@ -1,5 +1,3 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path"; import path from "node:path";
import { describe, expect, it } from "vitest"; import { describe, expect, it } from "vitest";
@@ -8,11 +6,16 @@ import handler from "./handler.js";
import { createHookEvent } from "../../hooks.js"; import { createHookEvent } from "../../hooks.js";
import type { AgentBootstrapHookContext } from "../../hooks.js"; import type { AgentBootstrapHookContext } from "../../hooks.js";
import type { ClawdbotConfig } from "../../../config/config.js"; import type { ClawdbotConfig } from "../../../config/config.js";
import { makeTempWorkspace, writeWorkspaceFile } from "../../../test-helpers/workspace.js";
describe("soul-evil hook", () => { describe("soul-evil hook", () => {
it("skips subagent sessions", async () => { it("skips subagent sessions", async () => {
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-soul-")); const tempDir = await makeTempWorkspace("clawdbot-soul-");
await fs.writeFile(path.join(tempDir, "SOUL_EVIL.md"), "chaotic", "utf-8"); await writeWorkspaceFile({
dir: tempDir,
name: "SOUL_EVIL.md",
content: "chaotic",
});
const cfg: ClawdbotConfig = { const cfg: ClawdbotConfig = {
hooks: { hooks: {

View File

@@ -1,42 +1,26 @@
import type { ClawdbotConfig } from "../../../config/config.js"; import type { ClawdbotConfig } from "../../../config/config.js";
import { isSubagentSessionKey } from "../../../routing/session-key.js"; import { isSubagentSessionKey } from "../../../routing/session-key.js";
import { resolveHookConfig } from "../../config.js"; import { resolveHookConfig } from "../../config.js";
import type { AgentBootstrapHookContext, HookHandler } from "../../hooks.js"; import { isAgentBootstrapEvent, type HookHandler } from "../../hooks.js";
import { applySoulEvilOverride, type SoulEvilConfig } from "../../soul-evil.js"; import {
applySoulEvilOverride,
resolveSoulEvilConfigFromHook,
} from "../../soul-evil.js";
const HOOK_KEY = "soul-evil"; const HOOK_KEY = "soul-evil";
function resolveSoulEvilConfig(entry: Record<string, unknown> | undefined): SoulEvilConfig | null {
if (!entry) return null;
const file = typeof entry.file === "string" ? entry.file : undefined;
const chance = typeof entry.chance === "number" ? entry.chance : undefined;
const purge =
entry.purge && typeof entry.purge === "object"
? {
at:
typeof (entry.purge as { at?: unknown }).at === "string"
? (entry.purge as { at?: string }).at
: undefined,
duration:
typeof (entry.purge as { duration?: unknown }).duration === "string"
? (entry.purge as { duration?: string }).duration
: undefined,
}
: undefined;
if (!file && chance === undefined && !purge) return null;
return { file, chance, purge };
}
const soulEvilHook: HookHandler = async (event) => { const soulEvilHook: HookHandler = async (event) => {
if (event.type !== "agent" || event.action !== "bootstrap") return; if (!isAgentBootstrapEvent(event)) return;
const context = event.context as AgentBootstrapHookContext; const context = event.context;
if (context.sessionKey && isSubagentSessionKey(context.sessionKey)) return; if (context.sessionKey && isSubagentSessionKey(context.sessionKey)) return;
const cfg = context.cfg as ClawdbotConfig | undefined; const cfg = context.cfg as ClawdbotConfig | undefined;
const hookConfig = resolveHookConfig(cfg, HOOK_KEY); const hookConfig = resolveHookConfig(cfg, HOOK_KEY);
if (!hookConfig || hookConfig.enabled === false) return; if (!hookConfig || hookConfig.enabled === false) return;
const soulConfig = resolveSoulEvilConfig(hookConfig as Record<string, unknown>); const soulConfig = resolveSoulEvilConfigFromHook(hookConfig as Record<string, unknown>, {
warn: (message) => console.warn(`[soul-evil] ${message}`),
});
if (!soulConfig) return; if (!soulConfig) return;
const workspaceDir = context.workspaceDir; const workspaceDir = context.workspaceDir;

View File

@@ -3,9 +3,11 @@ import {
clearInternalHooks, clearInternalHooks,
createInternalHookEvent, createInternalHookEvent,
getRegisteredEventKeys, getRegisteredEventKeys,
isAgentBootstrapEvent,
registerInternalHook, registerInternalHook,
triggerInternalHook, triggerInternalHook,
unregisterInternalHook, unregisterInternalHook,
type AgentBootstrapHookContext,
type InternalHookEvent, type InternalHookEvent,
} from "./internal-hooks.js"; } from "./internal-hooks.js";
@@ -164,6 +166,22 @@ describe("hooks", () => {
}); });
}); });
describe("isAgentBootstrapEvent", () => {
it("returns true for agent:bootstrap events with expected context", () => {
const context: AgentBootstrapHookContext = {
workspaceDir: "/tmp",
bootstrapFiles: [],
};
const event = createInternalHookEvent("agent", "bootstrap", "test-session", context);
expect(isAgentBootstrapEvent(event)).toBe(true);
});
it("returns false for non-bootstrap events", () => {
const event = createInternalHookEvent("command", "new", "test-session");
expect(isAgentBootstrapEvent(event)).toBe(false);
});
});
describe("getRegisteredEventKeys", () => { describe("getRegisteredEventKeys", () => {
it("should return all registered event keys", () => { it("should return all registered event keys", () => {
registerInternalHook("command:new", vi.fn()); registerInternalHook("command:new", vi.fn());

View File

@@ -19,6 +19,12 @@ export type AgentBootstrapHookContext = {
agentId?: string; agentId?: string;
}; };
export type AgentBootstrapHookEvent = InternalHookEvent & {
type: "agent";
action: "bootstrap";
context: AgentBootstrapHookContext;
};
export interface InternalHookEvent { export interface InternalHookEvent {
/** The type of event (command, session, agent, etc.) */ /** The type of event (command, session, agent, etc.) */
type: InternalHookEventType; type: InternalHookEventType;
@@ -159,3 +165,11 @@ export function createInternalHookEvent(
messages: [], messages: [],
}; };
} }
export function isAgentBootstrapEvent(event: InternalHookEvent): event is AgentBootstrapHookEvent {
if (event.type !== "agent" || event.action !== "bootstrap") return false;
const context = event.context as Partial<AgentBootstrapHookContext> | null;
if (!context || typeof context !== "object") return false;
if (typeof context.workspaceDir !== "string") return false;
return Array.isArray(context.bootstrapFiles);
}

View File

@@ -1,11 +1,10 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path"; import path from "node:path";
import { describe, expect, it } from "vitest"; import { describe, expect, it } from "vitest";
import { applySoulEvilOverride, decideSoulEvil, DEFAULT_SOUL_EVIL_FILENAME } from "./soul-evil.js"; import { applySoulEvilOverride, decideSoulEvil, DEFAULT_SOUL_EVIL_FILENAME } from "./soul-evil.js";
import { DEFAULT_SOUL_FILENAME, type WorkspaceBootstrapFile } from "../agents/workspace.js"; import { DEFAULT_SOUL_FILENAME, type WorkspaceBootstrapFile } from "../agents/workspace.js";
import { makeTempWorkspace, writeWorkspaceFile } from "../test-helpers/workspace.js";
const makeFiles = (overrides?: Partial<WorkspaceBootstrapFile>) => [ const makeFiles = (overrides?: Partial<WorkspaceBootstrapFile>) => [
{ {
@@ -91,9 +90,12 @@ describe("decideSoulEvil", () => {
describe("applySoulEvilOverride", () => { describe("applySoulEvilOverride", () => {
it("replaces SOUL content when evil is active and file exists", async () => { it("replaces SOUL content when evil is active and file exists", async () => {
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-soul-")); const tempDir = await makeTempWorkspace("clawdbot-soul-");
const evilPath = path.join(tempDir, DEFAULT_SOUL_EVIL_FILENAME); await writeWorkspaceFile({
await fs.writeFile(evilPath, "chaotic", "utf-8"); dir: tempDir,
name: DEFAULT_SOUL_EVIL_FILENAME,
content: "chaotic",
});
const files = makeFiles({ const files = makeFiles({
path: path.join(tempDir, DEFAULT_SOUL_FILENAME), path: path.join(tempDir, DEFAULT_SOUL_FILENAME),
@@ -112,7 +114,7 @@ describe("applySoulEvilOverride", () => {
}); });
it("leaves SOUL content when evil file is missing", async () => { it("leaves SOUL content when evil file is missing", async () => {
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-soul-")); const tempDir = await makeTempWorkspace("clawdbot-soul-");
const files = makeFiles({ const files = makeFiles({
path: path.join(tempDir, DEFAULT_SOUL_FILENAME), path: path.join(tempDir, DEFAULT_SOUL_FILENAME),
}); });
@@ -130,9 +132,12 @@ describe("applySoulEvilOverride", () => {
}); });
it("leaves files untouched when SOUL.md is not in bootstrap files", async () => { it("leaves files untouched when SOUL.md is not in bootstrap files", async () => {
const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-soul-")); const tempDir = await makeTempWorkspace("clawdbot-soul-");
const evilPath = path.join(tempDir, DEFAULT_SOUL_EVIL_FILENAME); await writeWorkspaceFile({
await fs.writeFile(evilPath, "chaotic", "utf-8"); dir: tempDir,
name: DEFAULT_SOUL_EVIL_FILENAME,
content: "chaotic",
});
const files: WorkspaceBootstrapFile[] = [ const files: WorkspaceBootstrapFile[] = [
{ {

View File

@@ -40,6 +40,50 @@ type SoulEvilLog = {
warn?: (message: string) => void; warn?: (message: string) => void;
}; };
export function resolveSoulEvilConfigFromHook(
entry: Record<string, unknown> | undefined,
log?: SoulEvilLog,
): SoulEvilConfig | null {
if (!entry) return null;
const file = typeof entry.file === "string" ? entry.file : undefined;
if (entry.file !== undefined && !file) {
log?.warn?.("soul-evil config: file must be a string");
}
let chance: number | undefined;
if (entry.chance !== undefined) {
if (typeof entry.chance === "number" && Number.isFinite(entry.chance)) {
chance = entry.chance;
} else {
log?.warn?.("soul-evil config: chance must be a number");
}
}
let purge: SoulEvilConfig["purge"];
if (entry.purge && typeof entry.purge === "object") {
const at =
typeof (entry.purge as { at?: unknown }).at === "string"
? (entry.purge as { at?: string }).at
: undefined;
const duration =
typeof (entry.purge as { duration?: unknown }).duration === "string"
? (entry.purge as { duration?: string }).duration
: undefined;
if ((entry.purge as { at?: unknown }).at !== undefined && !at) {
log?.warn?.("soul-evil config: purge.at must be a string");
}
if ((entry.purge as { duration?: unknown }).duration !== undefined && !duration) {
log?.warn?.("soul-evil config: purge.duration must be a string");
}
purge = { at, duration };
} else if (entry.purge !== undefined) {
log?.warn?.("soul-evil config: purge must be an object");
}
if (!file && chance === undefined && !purge) return null;
return { file, chance, purge };
}
function clampChance(value?: number): number { function clampChance(value?: number): number {
if (typeof value !== "number" || !Number.isFinite(value)) return 0; if (typeof value !== "number" || !Number.isFinite(value)) return 0;
return Math.min(1, Math.max(0, value)); return Math.min(1, Math.max(0, value));

View File

@@ -0,0 +1,17 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
export async function makeTempWorkspace(prefix = "clawdbot-workspace-"): Promise<string> {
return fs.mkdtemp(path.join(os.tmpdir(), prefix));
}
export async function writeWorkspaceFile(params: {
dir: string;
name: string;
content: string;
}): Promise<string> {
const filePath = path.join(params.dir, params.name);
await fs.writeFile(filePath, params.content, "utf-8");
return filePath;
}