refactor(core): dedupe command, hook, and cron fixtures

This commit is contained in:
Peter Steinberger
2026-03-02 21:30:58 +00:00
parent 5f0cbd0edc
commit 91dd89313a
16 changed files with 325 additions and 330 deletions

View File

@@ -1,17 +1,23 @@
import { describe, expect, it, vi } from "vitest";
import { createArmableStallWatchdog } from "./stall-watchdog.js";
function createTestWatchdog(
onTimeout: Parameters<typeof createArmableStallWatchdog>[0]["onTimeout"],
) {
return createArmableStallWatchdog({
label: "test-watchdog",
timeoutMs: 1_000,
checkIntervalMs: 100,
onTimeout,
});
}
describe("createArmableStallWatchdog", () => {
it("fires onTimeout once when armed and idle exceeds timeout", async () => {
vi.useFakeTimers();
try {
const onTimeout = vi.fn();
const watchdog = createArmableStallWatchdog({
label: "test-watchdog",
timeoutMs: 1_000,
checkIntervalMs: 100,
onTimeout,
});
const watchdog = createTestWatchdog(onTimeout);
watchdog.arm();
await vi.advanceTimersByTimeAsync(1_500);
@@ -28,12 +34,7 @@ describe("createArmableStallWatchdog", () => {
vi.useFakeTimers();
try {
const onTimeout = vi.fn();
const watchdog = createArmableStallWatchdog({
label: "test-watchdog",
timeoutMs: 1_000,
checkIntervalMs: 100,
onTimeout,
});
const watchdog = createTestWatchdog(onTimeout);
watchdog.arm();
await vi.advanceTimersByTimeAsync(500);
@@ -51,12 +52,7 @@ describe("createArmableStallWatchdog", () => {
vi.useFakeTimers();
try {
const onTimeout = vi.fn();
const watchdog = createArmableStallWatchdog({
label: "test-watchdog",
timeoutMs: 1_000,
checkIntervalMs: 100,
onTimeout,
});
const watchdog = createTestWatchdog(onTimeout);
watchdog.arm();
await vi.advanceTimersByTimeAsync(700);

View File

@@ -128,6 +128,28 @@ function emitJsonPayload(params: {
return true;
}
async function resolveConfigAndTargetAgentIdOrExit(params: {
runtime: RuntimeEnv;
agentInput: string | undefined;
}): Promise<{
cfg: NonNullable<Awaited<ReturnType<typeof requireValidConfig>>>;
agentId: string;
} | null> {
const cfg = await requireValidConfig(params.runtime);
if (!cfg) {
return null;
}
const agentId = resolveTargetAgentIdOrExit({
cfg,
runtime: params.runtime,
agentInput: params.agentInput,
});
if (!agentId) {
return null;
}
return { cfg, agentId };
}
export async function agentsBindingsCommand(
opts: AgentsBindingsListOptions,
runtime: RuntimeEnv = defaultRuntime,
@@ -186,15 +208,14 @@ export async function agentsBindCommand(
opts: AgentsBindOptions,
runtime: RuntimeEnv = defaultRuntime,
) {
const cfg = await requireValidConfig(runtime);
if (!cfg) {
return;
}
const agentId = resolveTargetAgentIdOrExit({ cfg, runtime, agentInput: opts.agent });
if (!agentId) {
const resolved = await resolveConfigAndTargetAgentIdOrExit({
runtime,
agentInput: opts.agent,
});
if (!resolved) {
return;
}
const { cfg, agentId } = resolved;
const parsed = resolveParsedBindingsOrExit({
runtime,
@@ -264,15 +285,14 @@ export async function agentsUnbindCommand(
opts: AgentsUnbindOptions,
runtime: RuntimeEnv = defaultRuntime,
) {
const cfg = await requireValidConfig(runtime);
if (!cfg) {
return;
}
const agentId = resolveTargetAgentIdOrExit({ cfg, runtime, agentInput: opts.agent });
if (!agentId) {
const resolved = await resolveConfigAndTargetAgentIdOrExit({
runtime,
agentInput: opts.agent,
});
if (!resolved) {
return;
}
const { cfg, agentId } = resolved;
if (opts.all && (opts.bind?.length ?? 0) > 0) {
runtime.error("Use either --all or --bind, not both.");
runtime.exit(1);

View File

@@ -44,6 +44,26 @@ function createPromptSpies(params?: { confirmResult?: boolean; textResult?: stri
return { confirm, note, text };
}
async function ensureMinimaxApiKey(params: {
confirm: WizardPrompter["confirm"];
text: WizardPrompter["text"];
setCredential: Parameters<typeof ensureApiKeyFromEnvOrPrompt>[0]["setCredential"];
config?: Parameters<typeof ensureApiKeyFromEnvOrPrompt>[0]["config"];
secretInputMode?: Parameters<typeof ensureApiKeyFromEnvOrPrompt>[0]["secretInputMode"];
}) {
return await ensureApiKeyFromEnvOrPrompt({
config: params.config ?? {},
provider: "minimax",
envLabel: "MINIMAX_API_KEY",
promptMessage: "Enter key",
normalize: (value) => value.trim(),
validate: () => undefined,
prompter: createPrompter({ confirm: params.confirm, text: params.text }),
secretInputMode: params.secretInputMode,
setCredential: params.setCredential,
});
}
async function runEnsureMinimaxApiKeyFlow(params: { confirmResult: boolean; textResult: string }) {
process.env.MINIMAX_API_KEY = "env-key";
delete process.env.MINIMAX_OAUTH_TOKEN;
@@ -53,15 +73,9 @@ async function runEnsureMinimaxApiKeyFlow(params: { confirmResult: boolean; text
textResult: params.textResult,
});
const setCredential = vi.fn(async () => undefined);
const result = await ensureApiKeyFromEnvOrPrompt({
config: {},
provider: "minimax",
envLabel: "MINIMAX_API_KEY",
promptMessage: "Enter key",
normalize: (value) => value.trim(),
validate: () => undefined,
prompter: createPrompter({ confirm, text }),
const result = await ensureMinimaxApiKey({
confirm,
text,
setCredential,
});
@@ -164,14 +178,9 @@ describe("ensureApiKeyFromEnvOrPrompt", () => {
});
const setCredential = vi.fn(async () => undefined);
const result = await ensureApiKeyFromEnvOrPrompt({
config: {},
provider: "minimax",
envLabel: "MINIMAX_API_KEY",
promptMessage: "Enter key",
normalize: (value) => value.trim(),
validate: () => undefined,
prompter: createPrompter({ confirm, text }),
const result = await ensureMinimaxApiKey({
confirm,
text,
secretInputMode: "ref",
setCredential,
});
@@ -195,14 +204,9 @@ describe("ensureApiKeyFromEnvOrPrompt", () => {
const setCredential = vi.fn(async () => undefined);
await expect(
ensureApiKeyFromEnvOrPrompt({
config: {},
provider: "minimax",
envLabel: "MINIMAX_API_KEY",
promptMessage: "Enter key",
normalize: (value) => value.trim(),
validate: () => undefined,
prompter: createPrompter({ confirm, text }),
ensureMinimaxApiKey({
confirm,
text,
secretInputMode: "ref",
setCredential,
}),

View File

@@ -29,6 +29,19 @@ function createHuggingfacePrompter(params: {
return createWizardPrompter(overrides, { defaultSelect: "" });
}
type ApplyHuggingfaceParams = Parameters<typeof applyAuthChoiceHuggingface>[0];
async function runHuggingfaceApply(
params: Omit<ApplyHuggingfaceParams, "authChoice" | "setDefaultModel"> &
Partial<Pick<ApplyHuggingfaceParams, "setDefaultModel">>,
) {
return await applyAuthChoiceHuggingface({
authChoice: "huggingface-api-key",
setDefaultModel: params.setDefaultModel ?? true,
...params,
});
}
describe("applyAuthChoiceHuggingface", () => {
const lifecycle = createAuthTestLifecycle([
"OPENCLAW_STATE_DIR",
@@ -75,12 +88,10 @@ describe("applyAuthChoiceHuggingface", () => {
const prompter = createHuggingfacePrompter({ text, select });
const runtime = createExitThrowingRuntime();
const result = await applyAuthChoiceHuggingface({
authChoice: "huggingface-api-key",
const result = await runHuggingfaceApply({
config: {},
prompter,
runtime,
setDefaultModel: true,
});
expect(result).not.toBeNull();
@@ -132,12 +143,10 @@ describe("applyAuthChoiceHuggingface", () => {
const prompter = createHuggingfacePrompter({ text, select, confirm });
const runtime = createExitThrowingRuntime();
const result = await applyAuthChoiceHuggingface({
authChoice: "huggingface-api-key",
const result = await runHuggingfaceApply({
config: {},
prompter,
runtime,
setDefaultModel: true,
opts: {
tokenProvider,
token,
@@ -167,12 +176,10 @@ describe("applyAuthChoiceHuggingface", () => {
const prompter = createHuggingfacePrompter({ text, select, note });
const runtime = createExitThrowingRuntime();
const result = await applyAuthChoiceHuggingface({
authChoice: "huggingface-api-key",
const result = await runHuggingfaceApply({
config: {},
prompter,
runtime,
setDefaultModel: true,
});
expect(result).not.toBeNull();

View File

@@ -1,16 +1,15 @@
import { describe, expect, it, vi } from "vitest";
import { withTempHomeConfig } from "../config/test-helpers.js";
const { noteSpy } = vi.hoisted(() => ({
noteSpy: vi.fn(),
}));
import { note } from "../terminal/note.js";
vi.mock("../terminal/note.js", () => ({
note: noteSpy,
note: vi.fn(),
}));
import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js";
const noteSpy = vi.mocked(note);
describe("doctor include warning", () => {
it("surfaces include confinement hint for escaped include paths", async () => {
await withTempHomeConfig({ $include: "/etc/passwd" }, async () => {

View File

@@ -1,13 +1,10 @@
import { describe, expect, it, vi } from "vitest";
import { note } from "../terminal/note.js";
import { withEnvAsync } from "../test-utils/env.js";
import { runDoctorConfigWithInput } from "./doctor-config-flow.test-utils.js";
const { noteSpy } = vi.hoisted(() => ({
noteSpy: vi.fn(),
}));
vi.mock("../terminal/note.js", () => ({
note: noteSpy,
note: vi.fn(),
}));
vi.mock("./doctor-legacy-config.js", async (importOriginal) => {
@@ -23,6 +20,8 @@ vi.mock("./doctor-legacy-config.js", async (importOriginal) => {
import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js";
const noteSpy = vi.mocked(note);
describe("doctor missing default account binding warning", () => {
it("emits a doctor warning when named accounts have no valid account-scoped bindings", async () => {
await withEnvAsync(

View File

@@ -2,20 +2,19 @@ import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { note } from "../terminal/note.js";
import { withEnvAsync } from "../test-utils/env.js";
import { runDoctorConfigWithInput } from "./doctor-config-flow.test-utils.js";
const { noteSpy } = vi.hoisted(() => ({
noteSpy: vi.fn(),
}));
vi.mock("../terminal/note.js", () => ({
note: noteSpy,
note: vi.fn(),
}));
import { loadAndMaybeMigrateDoctorConfig } from "./doctor-config-flow.js";
describe("doctor config flow safe bins", () => {
const noteSpy = vi.mocked(note);
beforeEach(() => {
noteSpy.mockClear();
});

View File

@@ -65,6 +65,20 @@ async function runStateIntegrity(cfg: OpenClawConfig) {
return confirmSkipInNonInteractive;
}
function writeSessionStore(
cfg: OpenClawConfig,
sessions: Record<string, { sessionId: string; updatedAt: number }>,
) {
setupSessionState(cfg, process.env, process.env.HOME ?? "");
const storePath = resolveStorePath(cfg.session?.store, { agentId: "main" });
fs.writeFileSync(storePath, JSON.stringify(sessions, null, 2));
}
async function runStateIntegrityText(cfg: OpenClawConfig): Promise<string> {
await noteStateIntegrity(cfg, { confirmSkipInNonInteractive: vi.fn(async () => false) });
return stateIntegrityText();
}
describe("doctor state integrity oauth dir checks", () => {
let envSnapshot: EnvSnapshot;
let tempHome = "";
@@ -146,25 +160,13 @@ describe("doctor state integrity oauth dir checks", () => {
it("prints openclaw-only verification hints when recent sessions are missing transcripts", async () => {
const cfg: OpenClawConfig = {};
setupSessionState(cfg, process.env, process.env.HOME ?? "");
const storePath = resolveStorePath(cfg.session?.store, { agentId: "main" });
fs.writeFileSync(
storePath,
JSON.stringify(
{
"agent:main:main": {
sessionId: "missing-transcript",
updatedAt: Date.now(),
},
},
null,
2,
),
);
await noteStateIntegrity(cfg, { confirmSkipInNonInteractive: vi.fn(async () => false) });
const text = stateIntegrityText();
writeSessionStore(cfg, {
"agent:main:main": {
sessionId: "missing-transcript",
updatedAt: Date.now(),
},
});
const text = await runStateIntegrityText(cfg);
expect(text).toContain("recent sessions are missing transcripts");
expect(text).toMatch(/openclaw sessions --store ".*sessions\.json"/);
expect(text).toMatch(/openclaw sessions cleanup --store ".*sessions\.json" --dry-run/);
@@ -177,25 +179,13 @@ describe("doctor state integrity oauth dir checks", () => {
it("ignores slash-routing sessions for recent missing transcript warnings", async () => {
const cfg: OpenClawConfig = {};
setupSessionState(cfg, process.env, process.env.HOME ?? "");
const storePath = resolveStorePath(cfg.session?.store, { agentId: "main" });
fs.writeFileSync(
storePath,
JSON.stringify(
{
"agent:main:telegram:slash:6790081233": {
sessionId: "missing-slash-transcript",
updatedAt: Date.now(),
},
},
null,
2,
),
);
await noteStateIntegrity(cfg, { confirmSkipInNonInteractive: vi.fn(async () => false) });
const text = stateIntegrityText();
writeSessionStore(cfg, {
"agent:main:telegram:slash:6790081233": {
sessionId: "missing-slash-transcript",
updatedAt: Date.now(),
},
});
const text = await runStateIntegrityText(cfg);
expect(text).not.toContain("recent sessions are missing transcripts");
});
});

View File

@@ -152,6 +152,19 @@ async function runTurnWithStoredModelOverride(
});
}
async function runStoredOverrideAndExpectModel(params: {
home: string;
deterministicCatalog: Array<{ id: string; name: string; provider: string }>;
jobPayload: CronJob["payload"];
expected: { provider: string; model: string };
}) {
vi.mocked(runEmbeddedPiAgent).mockClear();
vi.mocked(loadModelCatalog).mockResolvedValue(params.deterministicCatalog);
const res = (await runTurnWithStoredModelOverride(params.home, params.jobPayload)).res;
expect(res.status).toBe("ok");
expectEmbeddedProviderModel(params.expected);
}
describe("runCronIsolatedAgentTurn", () => {
beforeEach(() => {
vi.mocked(runEmbeddedPiAgent).mockClear();
@@ -352,30 +365,28 @@ describe("runCronIsolatedAgentTurn", () => {
expect(res.status).toBe("ok");
expectEmbeddedProviderModel({ provider: "openai", model: "gpt-4.1-mini" });
vi.mocked(runEmbeddedPiAgent).mockClear();
vi.mocked(loadModelCatalog).mockResolvedValue(deterministicCatalog);
res = (
await runTurnWithStoredModelOverride(home, {
await runStoredOverrideAndExpectModel({
home,
deterministicCatalog,
jobPayload: {
kind: "agentTurn",
message: DEFAULT_MESSAGE,
deliver: false,
})
).res;
expect(res.status).toBe("ok");
expectEmbeddedProviderModel({ provider: "openai", model: "gpt-4.1-mini" });
},
expected: { provider: "openai", model: "gpt-4.1-mini" },
});
vi.mocked(runEmbeddedPiAgent).mockClear();
vi.mocked(loadModelCatalog).mockResolvedValue(deterministicCatalog);
res = (
await runTurnWithStoredModelOverride(home, {
await runStoredOverrideAndExpectModel({
home,
deterministicCatalog,
jobPayload: {
kind: "agentTurn",
message: DEFAULT_MESSAGE,
model: "anthropic/claude-opus-4-5",
deliver: false,
})
).res;
expect(res.status).toBe("ok");
expectEmbeddedProviderModel({ provider: "anthropic", model: "claude-opus-4-5" });
},
expected: { provider: "anthropic", model: "claude-opus-4-5" },
});
});
});

View File

@@ -35,6 +35,17 @@ function makeCfg(overrides?: Partial<OpenClawConfig>): OpenClawConfig {
} as OpenClawConfig;
}
function makeTelegramBoundCfg(accountId = "account-b"): OpenClawConfig {
return makeCfg({
bindings: [
{
agentId: AGENT_ID,
match: { channel: "telegram", accountId },
},
],
});
}
const AGENT_ID = "agent-b";
const DEFAULT_TARGET = {
channel: "telegram" as const,
@@ -109,16 +120,7 @@ describe("resolveDeliveryTarget", () => {
it("falls back to bound accountId when session has no lastAccountId", async () => {
setMainSessionEntry(undefined);
const cfg = makeCfg({
bindings: [
{
agentId: "agent-b",
match: { channel: "telegram", accountId: "account-b" },
},
],
});
const cfg = makeTelegramBoundCfg();
const result = await resolveForAgent({ cfg });
expect(result.accountId).toBe("account-b");
@@ -133,15 +135,7 @@ describe("resolveDeliveryTarget", () => {
lastAccountId: "session-account",
});
const cfg = makeCfg({
bindings: [
{
agentId: "agent-b",
match: { channel: "telegram", accountId: "account-b" },
},
],
});
const cfg = makeTelegramBoundCfg();
const result = await resolveForAgent({ cfg });
// Session-derived accountId should take precedence over binding

View File

@@ -4,6 +4,8 @@ import path from "node:path";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { CronService } from "./service.js";
type CronServiceParams = ConstructorParameters<typeof CronService>[0];
const noopLogger = {
debug: vi.fn(),
info: vi.fn(),
@@ -21,6 +23,24 @@ async function makeStorePath() {
};
}
function createFailureAlertCron(params: {
storePath: string;
cronConfig?: CronServiceParams["cronConfig"];
runIsolatedAgentJob: NonNullable<CronServiceParams["runIsolatedAgentJob"]>;
sendCronFailureAlert: NonNullable<CronServiceParams["sendCronFailureAlert"]>;
}) {
return new CronService({
storePath: params.storePath,
cronEnabled: true,
cronConfig: params.cronConfig,
log: noopLogger,
enqueueSystemEvent: vi.fn(),
requestHeartbeatNow: vi.fn(),
runIsolatedAgentJob: params.runIsolatedAgentJob,
sendCronFailureAlert: params.sendCronFailureAlert,
});
}
describe("CronService failure alerts", () => {
beforeEach(() => {
vi.useFakeTimers();
@@ -43,9 +63,8 @@ describe("CronService failure alerts", () => {
error: "wrong model id",
}));
const cron = new CronService({
const cron = createFailureAlertCron({
storePath: store.storePath,
cronEnabled: true,
cronConfig: {
failureAlert: {
enabled: true,
@@ -53,9 +72,6 @@ describe("CronService failure alerts", () => {
cooldownMs: 60_000,
},
},
log: noopLogger,
enqueueSystemEvent: vi.fn(),
requestHeartbeatNow: vi.fn(),
runIsolatedAgentJob,
sendCronFailureAlert,
});
@@ -109,17 +125,13 @@ describe("CronService failure alerts", () => {
error: "timeout",
}));
const cron = new CronService({
const cron = createFailureAlertCron({
storePath: store.storePath,
cronEnabled: true,
cronConfig: {
failureAlert: {
enabled: false,
},
},
log: noopLogger,
enqueueSystemEvent: vi.fn(),
requestHeartbeatNow: vi.fn(),
runIsolatedAgentJob,
sendCronFailureAlert,
});
@@ -161,18 +173,14 @@ describe("CronService failure alerts", () => {
error: "auth error",
}));
const cron = new CronService({
const cron = createFailureAlertCron({
storePath: store.storePath,
cronEnabled: true,
cronConfig: {
failureAlert: {
enabled: true,
after: 1,
},
},
log: noopLogger,
enqueueSystemEvent: vi.fn(),
requestHeartbeatNow: vi.fn(),
runIsolatedAgentJob,
sendCronFailureAlert,
});
@@ -204,9 +212,8 @@ describe("CronService failure alerts", () => {
error: "temporary upstream error",
}));
const cron = new CronService({
const cron = createFailureAlertCron({
storePath: store.storePath,
cronEnabled: true,
cronConfig: {
failureAlert: {
enabled: true,
@@ -215,9 +222,6 @@ describe("CronService failure alerts", () => {
accountId: "global-account",
},
},
log: noopLogger,
enqueueSystemEvent: vi.fn(),
requestHeartbeatNow: vi.fn(),
runIsolatedAgentJob,
sendCronFailureAlert,
});

View File

@@ -6,52 +6,80 @@ import type { CronJob } from "./types.js";
const { logger, makeStorePath } = setupCronServiceSuite({
prefix: "cron-heartbeat-ok-suppressed",
});
type CronServiceParams = ConstructorParameters<typeof CronService>[0];
function createDueIsolatedAnnounceJob(params: {
id: string;
message: string;
now: number;
}): CronJob {
return {
id: params.id,
name: params.id,
enabled: true,
createdAtMs: params.now - 10_000,
updatedAtMs: params.now - 10_000,
schedule: { kind: "every", everyMs: 60_000 },
sessionTarget: "isolated",
wakeMode: "now",
payload: { kind: "agentTurn", message: params.message },
delivery: { mode: "announce" },
state: { nextRunAtMs: params.now - 1 },
};
}
function createCronServiceForSummary(params: {
storePath: string;
summary: string;
enqueueSystemEvent: CronServiceParams["enqueueSystemEvent"];
requestHeartbeatNow: CronServiceParams["requestHeartbeatNow"];
}) {
return new CronService({
storePath: params.storePath,
cronEnabled: true,
log: logger,
enqueueSystemEvent: params.enqueueSystemEvent,
requestHeartbeatNow: params.requestHeartbeatNow,
runHeartbeatOnce: vi.fn(),
runIsolatedAgentJob: vi.fn(async () => ({
status: "ok" as const,
summary: params.summary,
delivered: false,
deliveryAttempted: false,
})),
});
}
async function runScheduledCron(cron: CronService): Promise<void> {
await cron.start();
await vi.advanceTimersByTimeAsync(2_000);
await vi.advanceTimersByTimeAsync(1_000);
cron.stop();
}
describe("cron isolated job HEARTBEAT_OK summary suppression (#32013)", () => {
it("does not enqueue HEARTBEAT_OK as a system event to the main session", async () => {
const { storePath } = await makeStorePath();
const now = Date.now();
const job: CronJob = {
const job = createDueIsolatedAnnounceJob({
id: "heartbeat-only-job",
name: "heartbeat-only-job",
enabled: true,
createdAtMs: now - 10_000,
updatedAtMs: now - 10_000,
schedule: { kind: "every", everyMs: 60_000 },
sessionTarget: "isolated",
wakeMode: "now",
payload: { kind: "agentTurn", message: "Check if anything is new" },
delivery: { mode: "announce" },
state: { nextRunAtMs: now - 1 },
};
message: "Check if anything is new",
now,
});
await writeCronStoreSnapshot({ storePath, jobs: [job] });
const enqueueSystemEvent = vi.fn();
const requestHeartbeatNow = vi.fn();
const cron = new CronService({
const cron = createCronServiceForSummary({
storePath,
cronEnabled: true,
log: logger,
summary: "HEARTBEAT_OK",
enqueueSystemEvent,
requestHeartbeatNow,
runHeartbeatOnce: vi.fn(),
// Simulate the isolated agent returning HEARTBEAT_OK — nothing to
// announce. The delivery was intentionally skipped.
runIsolatedAgentJob: vi.fn(async () => ({
status: "ok" as const,
summary: "HEARTBEAT_OK",
delivered: false,
deliveryAttempted: false,
})),
});
await cron.start();
await vi.advanceTimersByTimeAsync(2_000);
await vi.advanceTimersByTimeAsync(1_000);
cron.stop();
await runScheduledCron(cron);
// HEARTBEAT_OK should NOT leak into the main session as a system event.
expect(enqueueSystemEvent).not.toHaveBeenCalled();
@@ -62,45 +90,24 @@ describe("cron isolated job HEARTBEAT_OK summary suppression (#32013)", () => {
const { storePath } = await makeStorePath();
const now = Date.now();
const job: CronJob = {
const job = createDueIsolatedAnnounceJob({
id: "real-summary-job",
name: "real-summary-job",
enabled: true,
createdAtMs: now - 10_000,
updatedAtMs: now - 10_000,
schedule: { kind: "every", everyMs: 60_000 },
sessionTarget: "isolated",
wakeMode: "now",
payload: { kind: "agentTurn", message: "Check weather" },
delivery: { mode: "announce" },
state: { nextRunAtMs: now - 1 },
};
message: "Check weather",
now,
});
await writeCronStoreSnapshot({ storePath, jobs: [job] });
const enqueueSystemEvent = vi.fn();
const requestHeartbeatNow = vi.fn();
const cron = new CronService({
const cron = createCronServiceForSummary({
storePath,
cronEnabled: true,
log: logger,
summary: "Weather update: sunny, 72°F",
enqueueSystemEvent,
requestHeartbeatNow,
runHeartbeatOnce: vi.fn(),
// Simulate real content that should be forwarded.
runIsolatedAgentJob: vi.fn(async () => ({
status: "ok" as const,
summary: "Weather update: sunny, 72°F",
delivered: false,
deliveryAttempted: false,
})),
});
await cron.start();
await vi.advanceTimersByTimeAsync(2_000);
await vi.advanceTimersByTimeAsync(1_000);
cron.stop();
await runScheduledCron(cron);
// Real summaries SHOULD be enqueued.
expect(enqueueSystemEvent).toHaveBeenCalledWith(

View File

@@ -27,6 +27,11 @@ function createStartedCron(storePath: string) {
};
}
async function listJobById(cron: CronService, jobId: string) {
const jobs = await cron.list({ includeDisabled: true });
return jobs.find((entry) => entry.id === jobId);
}
describe("CronService store migrations", () => {
it("migrates legacy top-level agentTurn fields and initializes missing state", async () => {
const store = await makeStorePath();
@@ -69,8 +74,7 @@ describe("CronService store migrations", () => {
const status = await cron.status();
expect(status.enabled).toBe(true);
const jobs = await cron.list({ includeDisabled: true });
const job = jobs.find((entry) => entry.id === "legacy-agentturn-job");
const job = await listJobById(cron, "legacy-agentturn-job");
expect(job).toBeDefined();
expect(job?.state).toBeDefined();
expect(job?.sessionTarget).toBe("isolated");
@@ -137,8 +141,7 @@ describe("CronService store migrations", () => {
const cron = await createStartedCron(store.storePath).start();
const jobs = await cron.list({ includeDisabled: true });
const job = jobs.find((entry) => entry.id === "legacy-agentturn-no-timeout");
const job = await listJobById(cron, "legacy-agentturn-no-timeout");
expect(job).toBeDefined();
expect(job?.payload.kind).toBe("agentTurn");
if (job?.payload.kind === "agentTurn") {
@@ -177,8 +180,7 @@ describe("CronService store migrations", () => {
);
const cron = await createStartedCron(store.storePath).start();
const jobs = await cron.list({ includeDisabled: true });
const job = jobs.find((entry) => entry.id === "legacy-cron-field-job");
const job = await listJobById(cron, "legacy-cron-field-job");
expect(job).toBeDefined();
expect(job?.wakeMode).toBe("now");
expect(job?.schedule.kind).toBe("cron");

View File

@@ -77,43 +77,34 @@ export type CronFailureAlert = {
accountId?: string;
};
export type CronPayload =
| { kind: "systemEvent"; text: string }
| {
kind: "agentTurn";
message: string;
/** Optional model override (provider/model or alias). */
model?: string;
/** Optional per-job fallback models; overrides agent/global fallbacks when defined. */
fallbacks?: string[];
thinking?: string;
timeoutSeconds?: number;
allowUnsafeExternalContent?: boolean;
/** If true, run with lightweight bootstrap context. */
lightContext?: boolean;
deliver?: boolean;
channel?: CronMessageChannel;
to?: string;
bestEffortDeliver?: boolean;
};
export type CronPayload = { kind: "systemEvent"; text: string } | CronAgentTurnPayload;
export type CronPayloadPatch =
| { kind: "systemEvent"; text?: string }
| {
kind: "agentTurn";
message?: string;
model?: string;
fallbacks?: string[];
thinking?: string;
timeoutSeconds?: number;
allowUnsafeExternalContent?: boolean;
/** If true, run with lightweight bootstrap context. */
lightContext?: boolean;
deliver?: boolean;
channel?: CronMessageChannel;
to?: string;
bestEffortDeliver?: boolean;
};
export type CronPayloadPatch = { kind: "systemEvent"; text?: string } | CronAgentTurnPayloadPatch;
type CronAgentTurnPayloadFields = {
message: string;
/** Optional model override (provider/model or alias). */
model?: string;
/** Optional per-job fallback models; overrides agent/global fallbacks when defined. */
fallbacks?: string[];
thinking?: string;
timeoutSeconds?: number;
allowUnsafeExternalContent?: boolean;
/** If true, run with lightweight bootstrap context. */
lightContext?: boolean;
deliver?: boolean;
channel?: CronMessageChannel;
to?: string;
bestEffortDeliver?: boolean;
};
type CronAgentTurnPayload = {
kind: "agentTurn";
} & CronAgentTurnPayloadFields;
type CronAgentTurnPayloadPatch = {
kind: "agentTurn";
} & Partial<CronAgentTurnPayloadFields>;
export type CronJobState = {
nextRunAtMs?: number;

View File

@@ -3,11 +3,15 @@ import path from "node:path";
import { MANIFEST_KEY } from "../compat/legacy-names.js";
import { fileExists, readJsonFile, resolveArchiveKind } from "../infra/archive.js";
import { resolveExistingInstallPath, withExtractedArchiveRoot } from "../infra/install-flow.js";
import { installFromValidatedNpmSpecArchive } from "../infra/install-from-npm-spec.js";
import {
resolveInstallModeOptions,
resolveTimedInstallModeOptions,
} from "../infra/install-mode-options.js";
import { installPackageDir } from "../infra/install-package-dir.js";
import {
installPackageDir,
installPackageDirWithManifestDeps,
} from "../infra/install-package-dir.js";
import { resolveSafeInstallDir, unscopedPackageName } from "../infra/install-safe-path.js";
import {
type NpmIntegrityDrift,
@@ -18,11 +22,6 @@ import {
ensureInstallTargetAvailable,
resolveCanonicalInstallTarget,
} from "../infra/install-target.js";
import {
finalizeNpmSpecArchiveInstall,
installFromNpmSpecArchiveWithInstaller,
} from "../infra/npm-pack-install.js";
import { validateRegistryNpmSpec } from "../infra/npm-registry-spec.js";
import { isPathInside, isPathInsideWithRealpath } from "../security/scan-paths.js";
import { CONFIG_DIR, resolveUserPath } from "../utils.js";
import { parseFrontmatter } from "./frontmatter.js";
@@ -231,17 +230,15 @@ async function installHookPackageFromDir(params: {
};
}
const deps = manifest.dependencies ?? {};
const hasDeps = Object.keys(deps).length > 0;
const installRes = await installPackageDir({
const installRes = await installPackageDirWithManifestDeps({
sourceDir: params.packageDir,
targetDir,
mode,
timeoutMs,
logger,
copyErrorPrefix: "failed to copy hook pack",
hasDeps,
depsLogMessage: "Installing hook pack dependencies…",
manifestDependencies: manifest.dependencies,
});
if (!installRes.ok) {
return installRes;
@@ -376,14 +373,10 @@ export async function installHooksFromNpmSpec(params: {
}): Promise<InstallHooksResult> {
const { logger, timeoutMs, mode, dryRun } = resolveTimedInstallModeOptions(params, defaultLogger);
const expectedHookPackId = params.expectedHookPackId;
const spec = params.spec.trim();
const specError = validateRegistryNpmSpec(spec);
if (specError) {
return { ok: false, error: specError };
}
const spec = params.spec;
logger.info?.(`Downloading ${spec}`);
const flowResult = await installFromNpmSpecArchiveWithInstaller({
logger.info?.(`Downloading ${spec.trim()}`);
return await installFromValidatedNpmSpecArchive({
tempDirPrefix: "openclaw-hook-pack-",
spec,
timeoutMs,
@@ -402,7 +395,6 @@ export async function installHooksFromNpmSpec(params: {
expectedHookPackId,
},
});
return finalizeNpmSpecArchiveInstall(flowResult);
}
export async function installHooksFromPath(params: {

View File

@@ -5,6 +5,22 @@ import { describe, expect, it } from "vitest";
import { MANIFEST_KEY } from "../compat/legacy-names.js";
import { loadHookEntriesFromDir } from "./workspace.js";
function writeHookPackageManifest(pkgDir: string, hooks: string[]): void {
fs.writeFileSync(
path.join(pkgDir, "package.json"),
JSON.stringify(
{
name: "pkg",
[MANIFEST_KEY]: {
hooks,
},
},
null,
2,
),
);
}
describe("hooks workspace", () => {
it("ignores package.json hook paths that traverse outside package directory", () => {
const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-hooks-workspace-"));
@@ -19,19 +35,7 @@ describe("hooks workspace", () => {
fs.writeFileSync(path.join(outsideHookDir, "HOOK.md"), "---\nname: outside\n---\n");
fs.writeFileSync(path.join(outsideHookDir, "handler.js"), "export default async () => {};\n");
fs.writeFileSync(
path.join(pkgDir, "package.json"),
JSON.stringify(
{
name: "pkg",
[MANIFEST_KEY]: {
hooks: ["../outside"],
},
},
null,
2,
),
);
writeHookPackageManifest(pkgDir, ["../outside"]);
const entries = loadHookEntriesFromDir({ dir: hooksRoot, source: "openclaw-workspace" });
expect(entries.some((e) => e.hook.name === "outside")).toBe(false);
@@ -49,19 +53,7 @@ describe("hooks workspace", () => {
fs.writeFileSync(path.join(nested, "HOOK.md"), "---\nname: nested\n---\n");
fs.writeFileSync(path.join(nested, "handler.js"), "export default async () => {};\n");
fs.writeFileSync(
path.join(pkgDir, "package.json"),
JSON.stringify(
{
name: "pkg",
[MANIFEST_KEY]: {
hooks: ["./nested"],
},
},
null,
2,
),
);
writeHookPackageManifest(pkgDir, ["./nested"]);
const entries = loadHookEntriesFromDir({ dir: hooksRoot, source: "openclaw-workspace" });
expect(entries.some((e) => e.hook.name === "nested")).toBe(true);
@@ -85,19 +77,7 @@ describe("hooks workspace", () => {
return;
}
fs.writeFileSync(
path.join(pkgDir, "package.json"),
JSON.stringify(
{
name: "pkg",
[MANIFEST_KEY]: {
hooks: ["./linked"],
},
},
null,
2,
),
);
writeHookPackageManifest(pkgDir, ["./linked"]);
const entries = loadHookEntriesFromDir({ dir: hooksRoot, source: "openclaw-workspace" });
expect(entries.some((e) => e.hook.name === "outside")).toBe(false);