diff --git a/src/cli/browser-cli-actions-input/register.element.ts b/src/cli/browser-cli-actions-input/register.element.ts index 270d59d6825..fc2070807de 100644 --- a/src/cli/browser-cli-actions-input/register.element.ts +++ b/src/cli/browser-cli-actions-input/register.element.ts @@ -2,7 +2,12 @@ import type { Command } from "commander"; import { danger } from "../../globals.js"; import { defaultRuntime } from "../../runtime.js"; import type { BrowserParentOpts } from "../browser-cli-shared.js"; -import { callBrowserAct, requireRef, resolveBrowserActionContext } from "./shared.js"; +import { + callBrowserAct, + logBrowserActionResult, + requireRef, + resolveBrowserActionContext, +} from "./shared.js"; export function registerBrowserElementCommands( browser: Command, @@ -41,12 +46,8 @@ export function registerBrowserElementCommands( modifiers, }, }); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } const suffix = result.url ? ` on ${result.url}` : ""; - defaultRuntime.log(`clicked ref ${refValue}${suffix}`); + logBrowserActionResult(parent, result, `clicked ref ${refValue}${suffix}`); } catch (err) { defaultRuntime.error(danger(String(err))); defaultRuntime.exit(1); @@ -80,11 +81,7 @@ export function registerBrowserElementCommands( targetId: opts.targetId?.trim() || undefined, }, }); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`typed into ref ${refValue}`); + logBrowserActionResult(parent, result, `typed into ref ${refValue}`); } catch (err) { defaultRuntime.error(danger(String(err))); defaultRuntime.exit(1); @@ -104,11 +101,7 @@ export function registerBrowserElementCommands( profile, body: { kind: "press", key, targetId: opts.targetId?.trim() || undefined }, }); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`pressed ${key}`); + logBrowserActionResult(parent, result, `pressed ${key}`); } catch (err) { defaultRuntime.error(danger(String(err))); defaultRuntime.exit(1); @@ -128,11 +121,7 @@ export function registerBrowserElementCommands( profile, body: { kind: "hover", ref, targetId: opts.targetId?.trim() || undefined }, }); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`hovered ref ${ref}`); + logBrowserActionResult(parent, result, `hovered ref ${ref}`); } catch (err) { defaultRuntime.error(danger(String(err))); defaultRuntime.exit(1); @@ -165,11 +154,7 @@ export function registerBrowserElementCommands( }, timeoutMs: Number.isFinite(opts.timeoutMs) ? opts.timeoutMs : undefined, }); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`scrolled into view: ${refValue}`); + logBrowserActionResult(parent, result, `scrolled into view: ${refValue}`); } catch (err) { defaultRuntime.error(danger(String(err))); defaultRuntime.exit(1); @@ -195,11 +180,7 @@ export function registerBrowserElementCommands( targetId: opts.targetId?.trim() || undefined, }, }); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`dragged ${startRef} → ${endRef}`); + logBrowserActionResult(parent, result, `dragged ${startRef} → ${endRef}`); } catch (err) { defaultRuntime.error(danger(String(err))); defaultRuntime.exit(1); @@ -225,11 +206,7 @@ export function registerBrowserElementCommands( targetId: opts.targetId?.trim() || undefined, }, }); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`selected ${values.join(", ")}`); + logBrowserActionResult(parent, result, `selected ${values.join(", ")}`); } catch (err) { defaultRuntime.error(danger(String(err))); defaultRuntime.exit(1); diff --git a/src/cli/browser-cli-actions-input/register.form-wait-eval.ts b/src/cli/browser-cli-actions-input/register.form-wait-eval.ts index f5e90c1321c..a49e768daf5 100644 --- a/src/cli/browser-cli-actions-input/register.form-wait-eval.ts +++ b/src/cli/browser-cli-actions-input/register.form-wait-eval.ts @@ -2,7 +2,12 @@ import type { Command } from "commander"; import { danger } from "../../globals.js"; import { defaultRuntime } from "../../runtime.js"; import type { BrowserParentOpts } from "../browser-cli-shared.js"; -import { callBrowserAct, readFields, resolveBrowserActionContext } from "./shared.js"; +import { + callBrowserAct, + logBrowserActionResult, + readFields, + resolveBrowserActionContext, +} from "./shared.js"; export function registerBrowserFormWaitEvalCommands( browser: Command, @@ -30,11 +35,7 @@ export function registerBrowserFormWaitEvalCommands( targetId: opts.targetId?.trim() || undefined, }, }); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log(`filled ${fields.length} field(s)`); + logBrowserActionResult(parent, result, `filled ${fields.length} field(s)`); } catch (err) { defaultRuntime.error(danger(String(err))); defaultRuntime.exit(1); @@ -83,11 +84,7 @@ export function registerBrowserFormWaitEvalCommands( }, timeoutMs, }); - if (parent?.json) { - defaultRuntime.log(JSON.stringify(result, null, 2)); - return; - } - defaultRuntime.log("wait complete"); + logBrowserActionResult(parent, result, "wait complete"); } catch (err) { defaultRuntime.error(danger(String(err))); defaultRuntime.exit(1); diff --git a/src/cli/browser-cli-actions-input/shared.ts b/src/cli/browser-cli-actions-input/shared.ts index 4d426e82304..8d9415b3a5f 100644 --- a/src/cli/browser-cli-actions-input/shared.ts +++ b/src/cli/browser-cli-actions-input/shared.ts @@ -40,6 +40,18 @@ export async function callBrowserAct(params: { ); } +export function logBrowserActionResult( + parent: BrowserParentOpts, + result: unknown, + successMessage: string, +) { + if (parent?.json) { + defaultRuntime.log(JSON.stringify(result, null, 2)); + return; + } + defaultRuntime.log(successMessage); +} + export function requireRef(ref: string | undefined) { const refValue = typeof ref === "string" ? ref.trim() : ""; if (!refValue) { diff --git a/src/cli/browser-cli-manage.timeout-option.test.ts b/src/cli/browser-cli-manage.timeout-option.test.ts index 87af6a24a79..9b6347fd90b 100644 --- a/src/cli/browser-cli-manage.timeout-option.test.ts +++ b/src/cli/browser-cli-manage.timeout-option.test.ts @@ -35,13 +35,7 @@ vi.mock("./cli-utils.js", () => ({ _runtime: unknown, action: () => Promise, onError: (err: unknown) => void, - ) => { - try { - await action(); - } catch (err) { - onError(err); - } - }, + ) => await action().catch(onError), })); vi.mock("../runtime.js", () => ({ diff --git a/src/cli/cron-cli/register.cron-add.ts b/src/cli/cron-cli/register.cron-add.ts index 59d1649af02..4316ec06c36 100644 --- a/src/cli/cron-cli/register.cron-add.ts +++ b/src/cli/cron-cli/register.cron-add.ts @@ -9,6 +9,7 @@ import { parsePositiveIntOrUndefined } from "../program/helpers.js"; import { getCronChannelOptions, parseAt, + parseCronStaggerMs, parseDurationMs, printCronList, warnIfCronSchedulerDisabled, @@ -129,19 +130,7 @@ export function registerCronAddCommand(cron: Command) { } return { kind: "every" as const, everyMs }; } - const staggerMs = (() => { - if (useExact) { - return 0; - } - if (!staggerRaw) { - return undefined; - } - const parsed = parseDurationMs(staggerRaw); - if (!parsed) { - throw new Error("Invalid --stagger; use e.g. 30s, 1m, 5m"); - } - return parsed; - })(); + const staggerMs = parseCronStaggerMs({ staggerRaw, useExact }); return { kind: "cron" as const, expr: cronExpr, diff --git a/src/cli/cron-cli/register.cron-edit.ts b/src/cli/cron-cli/register.cron-edit.ts index 9670c65cb29..35bf45907f9 100644 --- a/src/cli/cron-cli/register.cron-edit.ts +++ b/src/cli/cron-cli/register.cron-edit.ts @@ -7,6 +7,7 @@ import { addGatewayClientOptions, callGatewayFromCli } from "../gateway-rpc.js"; import { getCronChannelOptions, parseAt, + parseCronStaggerMs, parseDurationMs, warnIfCronSchedulerDisabled, } from "./shared.js"; @@ -98,19 +99,7 @@ export function registerCronEditCommand(cron: Command) { if (staggerRaw && useExact) { throw new Error("Choose either --stagger or --exact, not both"); } - const requestedStaggerMs = (() => { - if (useExact) { - return 0; - } - if (!staggerRaw) { - return undefined; - } - const parsed = parseDurationMs(staggerRaw); - if (!parsed) { - throw new Error("Invalid --stagger; use e.g. 30s, 1m, 5m"); - } - return parsed; - })(); + const requestedStaggerMs = parseCronStaggerMs({ staggerRaw, useExact }); const patch: Record = {}; if (typeof opts.name === "string") { diff --git a/src/cli/cron-cli/shared.ts b/src/cli/cron-cli/shared.ts index b9b1dda2a5e..5b9290fe858 100644 --- a/src/cli/cron-cli/shared.ts +++ b/src/cli/cron-cli/shared.ts @@ -62,6 +62,23 @@ export function parseDurationMs(input: string): number | null { return Math.floor(n * factor); } +export function parseCronStaggerMs(params: { + staggerRaw: string; + useExact: boolean; +}): number | undefined { + if (params.useExact) { + return 0; + } + if (!params.staggerRaw) { + return undefined; + } + const parsed = parseDurationMs(params.staggerRaw); + if (!parsed) { + throw new Error("Invalid --stagger; use e.g. 30s, 1m, 5m"); + } + return parsed; +} + export function parseAt(input: string): string | null { const raw = input.trim(); if (!raw) { diff --git a/src/cli/daemon-cli/restart-health.test.ts b/src/cli/daemon-cli/restart-health.test.ts index 647ca00fd9a..67fb5c0dd4f 100644 --- a/src/cli/daemon-cli/restart-health.test.ts +++ b/src/cli/daemon-cli/restart-health.test.ts @@ -15,6 +15,32 @@ vi.mock("../../infra/ports.js", () => ({ const originalPlatform = process.platform; +async function inspectUnknownListenerFallback(params: { + runtime: { status: "running"; pid: number } | { status: "stopped" }; + includeUnknownListenersAsStale: boolean; +}) { + Object.defineProperty(process, "platform", { value: "win32", configurable: true }); + classifyPortListener.mockReturnValue("unknown"); + + const service = { + readRuntime: vi.fn(async () => params.runtime), + } as unknown as GatewayService; + + inspectPortUsage.mockResolvedValue({ + port: 18789, + status: "busy", + listeners: [{ pid: 10920, command: "unknown" }], + hints: [], + }); + + const { inspectGatewayRestart } = await import("./restart-health.js"); + return inspectGatewayRestart({ + service, + port: 18789, + includeUnknownListenersAsStale: params.includeUnknownListenersAsStale, + }); +} + describe("inspectGatewayRestart", () => { beforeEach(() => { inspectPortUsage.mockReset(); @@ -71,24 +97,8 @@ describe("inspectGatewayRestart", () => { }); it("treats unknown listeners as stale on Windows when enabled", async () => { - Object.defineProperty(process, "platform", { value: "win32", configurable: true }); - classifyPortListener.mockReturnValue("unknown"); - - const service = { - readRuntime: vi.fn(async () => ({ status: "stopped" })), - } as unknown as GatewayService; - - inspectPortUsage.mockResolvedValue({ - port: 18789, - status: "busy", - listeners: [{ pid: 10920, command: "unknown" }], - hints: [], - }); - - const { inspectGatewayRestart } = await import("./restart-health.js"); - const snapshot = await inspectGatewayRestart({ - service, - port: 18789, + const snapshot = await inspectUnknownListenerFallback({ + runtime: { status: "stopped" }, includeUnknownListenersAsStale: true, }); @@ -96,24 +106,8 @@ describe("inspectGatewayRestart", () => { }); it("does not treat unknown listeners as stale when fallback is disabled", async () => { - Object.defineProperty(process, "platform", { value: "win32", configurable: true }); - classifyPortListener.mockReturnValue("unknown"); - - const service = { - readRuntime: vi.fn(async () => ({ status: "stopped" })), - } as unknown as GatewayService; - - inspectPortUsage.mockResolvedValue({ - port: 18789, - status: "busy", - listeners: [{ pid: 10920, command: "unknown" }], - hints: [], - }); - - const { inspectGatewayRestart } = await import("./restart-health.js"); - const snapshot = await inspectGatewayRestart({ - service, - port: 18789, + const snapshot = await inspectUnknownListenerFallback({ + runtime: { status: "stopped" }, includeUnknownListenersAsStale: false, }); @@ -121,24 +115,8 @@ describe("inspectGatewayRestart", () => { }); it("does not apply unknown-listener fallback while runtime is running", async () => { - Object.defineProperty(process, "platform", { value: "win32", configurable: true }); - classifyPortListener.mockReturnValue("unknown"); - - const service = { - readRuntime: vi.fn(async () => ({ status: "running", pid: 10920 })), - } as unknown as GatewayService; - - inspectPortUsage.mockResolvedValue({ - port: 18789, - status: "busy", - listeners: [{ pid: 10920, command: "unknown" }], - hints: [], - }); - - const { inspectGatewayRestart } = await import("./restart-health.js"); - const snapshot = await inspectGatewayRestart({ - service, - port: 18789, + const snapshot = await inspectUnknownListenerFallback({ + runtime: { status: "running", pid: 10920 }, includeUnknownListenersAsStale: true, }); diff --git a/src/cli/daemon-cli/status.gather.ts b/src/cli/daemon-cli/status.gather.ts index e603ea2c879..ee166ae31fc 100644 --- a/src/cli/daemon-cli/status.gather.ts +++ b/src/cli/daemon-cli/status.gather.ts @@ -10,6 +10,7 @@ import type { FindExtraGatewayServicesOptions } from "../../daemon/inspect.js"; import { findExtraGatewayServices } from "../../daemon/inspect.js"; import type { ServiceConfigAudit } from "../../daemon/service-audit.js"; import { auditGatewayServiceConfig } from "../../daemon/service-audit.js"; +import type { GatewayServiceRuntime } from "../../daemon/service-runtime.js"; import { resolveGatewayService } from "../../daemon/service.js"; import { resolveGatewayBindHost } from "../../gateway/net.js"; import { @@ -54,19 +55,7 @@ export type DaemonStatus = { environment?: Record; sourcePath?: string; } | null; - runtime?: { - status?: string; - state?: string; - subState?: string; - pid?: number; - lastExitStatus?: number; - lastExitReason?: string; - lastRunResult?: string; - lastRunTime?: string; - detail?: string; - cachedLabel?: boolean; - missingUnit?: boolean; - }; + runtime?: GatewayServiceRuntime; configAudit?: ServiceConfigAudit; }; config?: { diff --git a/src/cli/hooks-cli.ts b/src/cli/hooks-cli.ts index c53713cb31f..7ea0de030da 100644 --- a/src/cli/hooks-cli.ts +++ b/src/cli/hooks-cli.ts @@ -26,6 +26,7 @@ import { renderTable } from "../terminal/table.js"; import { theme } from "../terminal/theme.js"; import { resolveUserPath, shortenHomePath } from "../utils.js"; import { formatCliCommand } from "./command-format.js"; +import { looksLikeLocalInstallSpec } from "./install-spec.js"; import { buildNpmInstallRecordFields, resolvePinnedNpmInstallRecordForCli, @@ -660,15 +661,7 @@ export function registerHooksCli(program: Command): void { process.exit(1); } - const looksLikePath = - raw.startsWith(".") || - raw.startsWith("~") || - path.isAbsolute(raw) || - raw.endsWith(".zip") || - raw.endsWith(".tgz") || - raw.endsWith(".tar.gz") || - raw.endsWith(".tar"); - if (looksLikePath) { + if (looksLikeLocalInstallSpec(raw, [".zip", ".tgz", ".tar.gz", ".tar"])) { defaultRuntime.error(`Path not found: ${resolved}`); process.exit(1); } diff --git a/src/cli/install-spec.ts b/src/cli/install-spec.ts new file mode 100644 index 00000000000..b4d61a81100 --- /dev/null +++ b/src/cli/install-spec.ts @@ -0,0 +1,10 @@ +import path from "node:path"; + +export function looksLikeLocalInstallSpec(spec: string, knownSuffixes: readonly string[]): boolean { + return ( + spec.startsWith(".") || + spec.startsWith("~") || + path.isAbsolute(spec) || + knownSuffixes.some((suffix) => spec.endsWith(suffix)) + ); +} diff --git a/src/cli/npm-resolution.ts b/src/cli/npm-resolution.ts index 54776151899..7f549b66715 100644 --- a/src/cli/npm-resolution.ts +++ b/src/cli/npm-resolution.ts @@ -1,11 +1,7 @@ -export type NpmResolutionMetadata = { - name?: string; - version?: string; - resolvedSpec?: string; - integrity?: string; - shasum?: string; - resolvedAt?: string; -}; +import { + buildNpmResolutionFields, + type NpmSpecResolution as NpmResolutionMetadata, +} from "../infra/install-source-utils.js"; export function resolvePinnedNpmSpec(params: { rawSpec: string; @@ -36,14 +32,7 @@ export function mapNpmResolutionMetadata(resolution?: NpmResolutionMetadata): { shasum?: string; resolvedAt?: string; } { - return { - resolvedName: resolution?.name, - resolvedVersion: resolution?.version, - resolvedSpec: resolution?.resolvedSpec, - integrity: resolution?.integrity, - shasum: resolution?.shasum, - resolvedAt: resolution?.resolvedAt, - }; + return buildNpmResolutionFields(resolution); } export function buildNpmInstallRecordFields(params: { @@ -68,7 +57,7 @@ export function buildNpmInstallRecordFields(params: { spec: params.spec, installPath: params.installPath, version: params.version, - ...mapNpmResolutionMetadata(params.resolution), + ...buildNpmResolutionFields(params.resolution), }; } diff --git a/src/cli/plugins-cli.ts b/src/cli/plugins-cli.ts index 714550ab1ac..fa70fce794f 100644 --- a/src/cli/plugins-cli.ts +++ b/src/cli/plugins-cli.ts @@ -22,6 +22,7 @@ import { formatDocsLink } from "../terminal/links.js"; import { renderTable } from "../terminal/table.js"; import { theme } from "../terminal/theme.js"; import { resolveUserPath, shortenHomeInString, shortenHomePath } from "../utils.js"; +import { looksLikeLocalInstallSpec } from "./install-spec.js"; import { resolvePinnedNpmInstallRecordForCli } from "./npm-resolution.js"; import { setPluginEnabledInConfig } from "./plugins-config.js"; import { promptYesNo } from "./prompt.js"; @@ -603,19 +604,18 @@ export function registerPluginsCli(program: Command) { process.exit(1); } - const looksLikePath = - raw.startsWith(".") || - raw.startsWith("~") || - path.isAbsolute(raw) || - raw.endsWith(".ts") || - raw.endsWith(".js") || - raw.endsWith(".mjs") || - raw.endsWith(".cjs") || - raw.endsWith(".tgz") || - raw.endsWith(".tar.gz") || - raw.endsWith(".tar") || - raw.endsWith(".zip"); - if (looksLikePath) { + if ( + looksLikeLocalInstallSpec(raw, [ + ".ts", + ".js", + ".mjs", + ".cjs", + ".tgz", + ".tar.gz", + ".tar", + ".zip", + ]) + ) { defaultRuntime.error(`Path not found: ${resolved}`); process.exit(1); } diff --git a/src/cli/program.nodes-media.e2e.test.ts b/src/cli/program.nodes-media.e2e.test.ts index d4eb426d4ed..b47a931d4ee 100644 --- a/src/cli/program.nodes-media.e2e.test.ts +++ b/src/cli/program.nodes-media.e2e.test.ts @@ -65,6 +65,18 @@ describe("cli program (nodes media)", () => { await program.parseAsync(argv, { from: "user" }); } + async function expectCameraSnapParseFailure(args: string[], expectedError: RegExp) { + mockNodeGateway(); + + const parseProgram = new Command(); + parseProgram.exitOverride(); + registerNodesCli(parseProgram); + runtime.error.mockClear(); + + await expect(parseProgram.parseAsync(args, { from: "user" })).rejects.toThrow(/exit/i); + expect(runtime.error.mock.calls.some(([msg]) => expectedError.test(String(msg)))).toBe(true); + } + async function runAndExpectUrlPayloadMediaFile(params: { command: "camera.snap" | "camera.clip"; payload: Record; @@ -266,54 +278,27 @@ describe("cli program (nodes media)", () => { }); it("fails nodes camera snap on invalid facing", async () => { - mockNodeGateway(); - - const program = new Command(); - program.exitOverride(); - registerNodesCli(program); - runtime.error.mockClear(); - - await expect( - program.parseAsync(["nodes", "camera", "snap", "--node", "ios-node", "--facing", "nope"], { - from: "user", - }), - ).rejects.toThrow(/exit/i); - - expect(runtime.error.mock.calls.some(([msg]) => /invalid facing/i.test(String(msg)))).toBe( - true, + await expectCameraSnapParseFailure( + ["nodes", "camera", "snap", "--node", "ios-node", "--facing", "nope"], + /invalid facing/i, ); }); it("fails nodes camera snap when --facing both and --device-id are combined", async () => { - mockNodeGateway(); - - const program = new Command(); - program.exitOverride(); - registerNodesCli(program); - runtime.error.mockClear(); - - await expect( - program.parseAsync( - [ - "nodes", - "camera", - "snap", - "--node", - "ios-node", - "--facing", - "both", - "--device-id", - "cam-123", - ], - { from: "user" }, - ), - ).rejects.toThrow(/exit/i); - - expect( - runtime.error.mock.calls.some(([msg]) => - /facing=both is not allowed when --device-id is set/i.test(String(msg)), - ), - ).toBe(true); + await expectCameraSnapParseFailure( + [ + "nodes", + "camera", + "snap", + "--node", + "ios-node", + "--facing", + "both", + "--device-id", + "cam-123", + ], + /facing=both is not allowed when --device-id is set/i, + ); }); describe("URL-based payloads", () => { diff --git a/src/cli/update-cli/shared.ts b/src/cli/update-cli/shared.ts index 50e1fd09473..8e62301e79a 100644 --- a/src/cli/update-cli/shared.ts +++ b/src/cli/update-cli/shared.ts @@ -5,6 +5,7 @@ import path from "node:path"; import { resolveStateDir } from "../../config/paths.js"; import { resolveOpenClawPackageRoot } from "../../infra/openclaw-root.js"; import { readPackageName, readPackageVersion } from "../../infra/package-json.js"; +import { normalizePackageTagInput } from "../../infra/package-tag.js"; import { trimLogTail } from "../../infra/restart-sentinel.js"; import { parseSemver } from "../../infra/runtime-guard.js"; import { fetchNpmTagVersion } from "../../infra/update-check.js"; @@ -58,20 +59,7 @@ export const DEFAULT_PACKAGE_NAME = "openclaw"; const CORE_PACKAGE_NAMES = new Set([DEFAULT_PACKAGE_NAME]); export function normalizeTag(value?: string | null): string | null { - if (!value) { - return null; - } - const trimmed = value.trim(); - if (!trimmed) { - return null; - } - if (trimmed.startsWith("openclaw@")) { - return trimmed.slice("openclaw@".length); - } - if (trimmed.startsWith(`${DEFAULT_PACKAGE_NAME}@`)) { - return trimmed.slice(`${DEFAULT_PACKAGE_NAME}@`.length); - } - return trimmed; + return normalizePackageTagInput(value, ["openclaw", DEFAULT_PACKAGE_NAME]); } export function normalizeVersionTag(tag: string): string | null { diff --git a/src/commands/agent.acp.test.ts b/src/commands/agent.acp.test.ts index c2edd057478..e6c024f0a29 100644 --- a/src/commands/agent.acp.test.ts +++ b/src/commands/agent.acp.test.ts @@ -26,8 +26,8 @@ async function withTempHome(fn: (home: string) => Promise): Promise { return withTempHomeBase(fn, { prefix: "openclaw-agent-acp-" }); } -function mockConfig(home: string, storePath: string) { - loadConfigSpy.mockReturnValue({ +function createAcpEnabledConfig(home: string, storePath: string): OpenClawConfig { + return { acp: { enabled: true, backend: "acpx", @@ -42,7 +42,11 @@ function mockConfig(home: string, storePath: string) { }, }, session: { store: storePath, mainKey: "main" }, - } satisfies OpenClawConfig); + }; +} + +function mockConfig(home: string, storePath: string) { + loadConfigSpy.mockReturnValue(createAcpEnabledConfig(home, storePath)); } function mockConfigWithAcpOverrides( @@ -50,23 +54,12 @@ function mockConfigWithAcpOverrides( storePath: string, acpOverrides: Partial>, ) { - loadConfigSpy.mockReturnValue({ - acp: { - enabled: true, - backend: "acpx", - allowedAgents: ["codex"], - dispatch: { enabled: true }, - ...acpOverrides, - }, - agents: { - defaults: { - model: { primary: "openai/gpt-5.3-codex" }, - models: { "openai/gpt-5.3-codex": {} }, - workspace: path.join(home, "openclaw"), - }, - }, - session: { store: storePath, mainKey: "main" }, - } satisfies OpenClawConfig); + const cfg = createAcpEnabledConfig(home, storePath); + cfg.acp = { + ...cfg.acp, + ...acpOverrides, + }; + loadConfigSpy.mockReturnValue(cfg); } function writeAcpSessionStore(storePath: string) { diff --git a/src/commands/auth-choice.apply-helpers.ts b/src/commands/auth-choice.apply-helpers.ts index 52e019aae19..c15408b3d3a 100644 --- a/src/commands/auth-choice.apply-helpers.ts +++ b/src/commands/auth-choice.apply-helpers.ts @@ -304,6 +304,24 @@ export function createAuthChoiceDefaultModelApplier( }; } +export function createAuthChoiceDefaultModelApplierForMutableState( + params: ApplyAuthChoiceParams, + getConfig: () => ApplyAuthChoiceParams["config"], + setConfig: (config: ApplyAuthChoiceParams["config"]) => void, + getAgentModelOverride: () => string | undefined, + setAgentModelOverride: (model: string | undefined) => void, +): ReturnType { + return createAuthChoiceDefaultModelApplier( + params, + createAuthChoiceModelStateBridge({ + getConfig, + setConfig, + getAgentModelOverride, + setAgentModelOverride, + }), + ); +} + export function normalizeTokenProviderInput( tokenProvider: string | null | undefined, ): string | undefined { diff --git a/src/commands/auth-choice.apply.api-providers.ts b/src/commands/auth-choice.apply.api-providers.ts index 2be73ee14f2..370951e9f0d 100644 --- a/src/commands/auth-choice.apply.api-providers.ts +++ b/src/commands/auth-choice.apply.api-providers.ts @@ -4,8 +4,7 @@ import { normalizeApiKeyInput, validateApiKeyInput } from "./auth-choice.api-key import { normalizeSecretInputModeInput, createAuthChoiceAgentModelNoter, - createAuthChoiceDefaultModelApplier, - createAuthChoiceModelStateBridge, + createAuthChoiceDefaultModelApplierForMutableState, ensureApiKeyFromOptionEnvOrPrompt, normalizeTokenProviderInput, } from "./auth-choice.apply-helpers.js"; @@ -317,14 +316,12 @@ export async function applyAuthChoiceApiProviders( let nextConfig = params.config; let agentModelOverride: string | undefined; const noteAgentModel = createAuthChoiceAgentModelNoter(params); - const applyProviderDefaultModel = createAuthChoiceDefaultModelApplier( + const applyProviderDefaultModel = createAuthChoiceDefaultModelApplierForMutableState( params, - createAuthChoiceModelStateBridge({ - getConfig: () => nextConfig, - setConfig: (config) => (nextConfig = config), - getAgentModelOverride: () => agentModelOverride, - setAgentModelOverride: (model) => (agentModelOverride = model), - }), + () => nextConfig, + (config) => (nextConfig = config), + () => agentModelOverride, + (model) => (agentModelOverride = model), ); let authChoice = params.authChoice; diff --git a/src/commands/auth-choice.apply.minimax.ts b/src/commands/auth-choice.apply.minimax.ts index 9b6c83fc204..8f8b9cf818e 100644 --- a/src/commands/auth-choice.apply.minimax.ts +++ b/src/commands/auth-choice.apply.minimax.ts @@ -1,7 +1,6 @@ import { normalizeApiKeyInput, validateApiKeyInput } from "./auth-choice.api-key.js"; import { - createAuthChoiceDefaultModelApplier, - createAuthChoiceModelStateBridge, + createAuthChoiceDefaultModelApplierForMutableState, ensureApiKeyFromOptionEnvOrPrompt, normalizeSecretInputModeInput, } from "./auth-choice.apply-helpers.js"; @@ -23,14 +22,12 @@ export async function applyAuthChoiceMiniMax( ): Promise { let nextConfig = params.config; let agentModelOverride: string | undefined; - const applyProviderDefaultModel = createAuthChoiceDefaultModelApplier( + const applyProviderDefaultModel = createAuthChoiceDefaultModelApplierForMutableState( params, - createAuthChoiceModelStateBridge({ - getConfig: () => nextConfig, - setConfig: (config) => (nextConfig = config), - getAgentModelOverride: () => agentModelOverride, - setAgentModelOverride: (model) => (agentModelOverride = model), - }), + () => nextConfig, + (config) => (nextConfig = config), + () => agentModelOverride, + (model) => (agentModelOverride = model), ); const requestedSecretInputMode = normalizeSecretInputModeInput(params.opts?.secretInputMode); const ensureMinimaxApiKey = async (opts: { diff --git a/src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts b/src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts index 50217c5d8cb..41917d33e00 100644 --- a/src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts +++ b/src/commands/doctor-sandbox.warns-sandbox-enabled-without-docker.test.ts @@ -22,6 +22,8 @@ vi.mock("../terminal/note.js", () => ({ note, })); +const { maybeRepairSandboxImages } = await import("./doctor-sandbox.js"); + describe("maybeRepairSandboxImages", () => { const mockRuntime: RuntimeEnv = { log: vi.fn(), @@ -37,22 +39,32 @@ describe("maybeRepairSandboxImages", () => { vi.clearAllMocks(); }); - it("warns when sandbox mode is enabled but Docker is not available", async () => { - // Simulate Docker not available (command fails) - runExec.mockRejectedValue(new Error("Docker not installed")); - - const config: OpenClawConfig = { + function createSandboxConfig(mode: "off" | "all" | "non-main"): OpenClawConfig { + return { agents: { defaults: { sandbox: { - mode: "non-main", + mode, }, }, }, }; + } - const { maybeRepairSandboxImages } = await import("./doctor-sandbox.js"); - await maybeRepairSandboxImages(config, mockRuntime, mockPrompter); + async function runSandboxRepair(params: { + mode: "off" | "all" | "non-main"; + dockerAvailable: boolean; + }) { + if (params.dockerAvailable) { + runExec.mockResolvedValue({ stdout: "24.0.0", stderr: "" }); + } else { + runExec.mockRejectedValue(new Error("Docker not installed")); + } + await maybeRepairSandboxImages(createSandboxConfig(params.mode), mockRuntime, mockPrompter); + } + + it("warns when sandbox mode is enabled but Docker is not available", async () => { + await runSandboxRepair({ mode: "non-main", dockerAvailable: false }); // The warning should clearly indicate sandbox is enabled but won't work expect(note).toHaveBeenCalled(); @@ -66,20 +78,7 @@ describe("maybeRepairSandboxImages", () => { }); it("warns when sandbox mode is 'all' but Docker is not available", async () => { - runExec.mockRejectedValue(new Error("Docker not installed")); - - const config: OpenClawConfig = { - agents: { - defaults: { - sandbox: { - mode: "all", - }, - }, - }, - }; - - const { maybeRepairSandboxImages } = await import("./doctor-sandbox.js"); - await maybeRepairSandboxImages(config, mockRuntime, mockPrompter); + await runSandboxRepair({ mode: "all", dockerAvailable: false }); expect(note).toHaveBeenCalled(); const noteCall = note.mock.calls[0]; @@ -90,41 +89,14 @@ describe("maybeRepairSandboxImages", () => { }); it("does not warn when sandbox mode is off", async () => { - runExec.mockRejectedValue(new Error("Docker not installed")); - - const config: OpenClawConfig = { - agents: { - defaults: { - sandbox: { - mode: "off", - }, - }, - }, - }; - - const { maybeRepairSandboxImages } = await import("./doctor-sandbox.js"); - await maybeRepairSandboxImages(config, mockRuntime, mockPrompter); + await runSandboxRepair({ mode: "off", dockerAvailable: false }); // No warning needed when sandbox is off expect(note).not.toHaveBeenCalled(); }); it("does not warn when Docker is available", async () => { - // Simulate Docker available - runExec.mockResolvedValue({ stdout: "24.0.0", stderr: "" }); - - const config: OpenClawConfig = { - agents: { - defaults: { - sandbox: { - mode: "non-main", - }, - }, - }, - }; - - const { maybeRepairSandboxImages } = await import("./doctor-sandbox.js"); - await maybeRepairSandboxImages(config, mockRuntime, mockPrompter); + await runSandboxRepair({ mode: "non-main", dockerAvailable: true }); // May have other notes about images, but not the Docker unavailable warning const dockerUnavailableWarning = note.mock.calls.find( diff --git a/src/commands/onboard-channels.e2e.test.ts b/src/commands/onboard-channels.e2e.test.ts index 526087235e9..88606bcc3cc 100644 --- a/src/commands/onboard-channels.e2e.test.ts +++ b/src/commands/onboard-channels.e2e.test.ts @@ -95,6 +95,73 @@ function patchTelegramAdapter(overrides: Parameters { + throw new Error(message); + }); +} + +async function runConfiguredTelegramSetup(params: { + strictUnexpected?: boolean; + configureWhenConfigured: NonNullable< + Parameters[0]["configureWhenConfigured"] + >; + configureErrorMessage: string; +}) { + const select = createQuickstartTelegramSelect({ strictUnexpected: params.strictUnexpected }); + const selection = vi.fn(); + const onAccountId = vi.fn(); + const configure = createUnexpectedConfigureCall(params.configureErrorMessage); + const restore = patchTelegramAdapter({ + configureInteractive: undefined, + configureWhenConfigured: params.configureWhenConfigured, + configure, + }); + const { prompter } = createUnexpectedQuickstartPrompter( + select as unknown as WizardPrompter["select"], + ); + + try { + const cfg = await runSetupChannels(createTelegramCfg("old-token"), prompter, { + quickstartDefaults: true, + onSelection: selection, + onAccountId, + }); + return { cfg, selection, onAccountId, configure }; + } finally { + restore(); + } +} + +async function runQuickstartTelegramSetupWithInteractive(params: { + configureInteractive: NonNullable< + Parameters[0]["configureInteractive"] + >; + configure?: NonNullable[0]["configure"]>; +}) { + const select = createQuickstartTelegramSelect(); + const selection = vi.fn(); + const onAccountId = vi.fn(); + const restore = patchTelegramAdapter({ + configureInteractive: params.configureInteractive, + ...(params.configure ? { configure: params.configure } : {}), + }); + const { prompter } = createUnexpectedQuickstartPrompter( + select as unknown as WizardPrompter["select"], + ); + + try { + const cfg = await runSetupChannels({} as OpenClawConfig, prompter, { + quickstartDefaults: true, + onSelection: selection, + onAccountId, + }); + return { cfg, selection, onAccountId }; + } finally { + restore(); + } +} + vi.mock("node:fs/promises", () => ({ default: { access: vi.fn(async () => { @@ -269,39 +336,20 @@ describe("setupChannels", () => { }); it("uses configureInteractive skip without mutating selection/account state", async () => { - const select = createQuickstartTelegramSelect(); - const selection = vi.fn(); - const onAccountId = vi.fn(); const configureInteractive = vi.fn(async () => "skip" as const); - const restore = patchTelegramAdapter({ + const { cfg, selection, onAccountId } = await runQuickstartTelegramSetupWithInteractive({ configureInteractive, }); - const { prompter } = createUnexpectedQuickstartPrompter( - select as unknown as WizardPrompter["select"], + + expect(configureInteractive).toHaveBeenCalledWith( + expect.objectContaining({ configured: false, label: expect.any(String) }), ); - - try { - const cfg = await runSetupChannels({} as OpenClawConfig, prompter, { - quickstartDefaults: true, - onSelection: selection, - onAccountId, - }); - - expect(configureInteractive).toHaveBeenCalledWith( - expect.objectContaining({ configured: false, label: expect.any(String) }), - ); - expect(selection).toHaveBeenCalledWith([]); - expect(onAccountId).not.toHaveBeenCalled(); - expect(cfg.channels?.telegram?.botToken).toBeUndefined(); - } finally { - restore(); - } + expect(selection).toHaveBeenCalledWith([]); + expect(onAccountId).not.toHaveBeenCalled(); + expect(cfg.channels?.telegram?.botToken).toBeUndefined(); }); it("applies configureInteractive result cfg/account updates", async () => { - const select = createQuickstartTelegramSelect(); - const selection = vi.fn(); - const onAccountId = vi.fn(); const configureInteractive = vi.fn(async ({ cfg }: { cfg: OpenClawConfig }) => ({ cfg: { ...cfg, @@ -312,38 +360,22 @@ describe("setupChannels", () => { } as OpenClawConfig, accountId: "acct-1", })); - const configure = vi.fn(async () => { - throw new Error("configure should not be called when configureInteractive is present"); - }); - const restore = patchTelegramAdapter({ + const configure = createUnexpectedConfigureCall( + "configure should not be called when configureInteractive is present", + ); + const { cfg, selection, onAccountId } = await runQuickstartTelegramSetupWithInteractive({ configureInteractive, configure, }); - const { prompter } = createUnexpectedQuickstartPrompter( - select as unknown as WizardPrompter["select"], - ); - try { - const cfg = await runSetupChannels({} as OpenClawConfig, prompter, { - quickstartDefaults: true, - onSelection: selection, - onAccountId, - }); - - expect(configureInteractive).toHaveBeenCalledTimes(1); - expect(configure).not.toHaveBeenCalled(); - expect(selection).toHaveBeenCalledWith(["telegram"]); - expect(onAccountId).toHaveBeenCalledWith("telegram", "acct-1"); - expect(cfg.channels?.telegram?.botToken).toBe("new-token"); - } finally { - restore(); - } + expect(configureInteractive).toHaveBeenCalledTimes(1); + expect(configure).not.toHaveBeenCalled(); + expect(selection).toHaveBeenCalledWith(["telegram"]); + expect(onAccountId).toHaveBeenCalledWith("telegram", "acct-1"); + expect(cfg.channels?.telegram?.botToken).toBe("new-token"); }); it("uses configureWhenConfigured when channel is already configured", async () => { - const select = createQuickstartTelegramSelect(); - const selection = vi.fn(); - const onAccountId = vi.fn(); const configureWhenConfigured = vi.fn(async ({ cfg }: { cfg: OpenClawConfig }) => ({ cfg: { ...cfg, @@ -354,74 +386,37 @@ describe("setupChannels", () => { } as OpenClawConfig, accountId: "acct-2", })); - const configure = vi.fn(async () => { - throw new Error( - "configure should not be called when configureWhenConfigured handles updates", - ); - }); - const restore = patchTelegramAdapter({ - configureInteractive: undefined, + const { cfg, selection, onAccountId, configure } = await runConfiguredTelegramSetup({ configureWhenConfigured, - configure, + configureErrorMessage: + "configure should not be called when configureWhenConfigured handles updates", }); - const { prompter } = createUnexpectedQuickstartPrompter( - select as unknown as WizardPrompter["select"], + + expect(configureWhenConfigured).toHaveBeenCalledTimes(1); + expect(configureWhenConfigured).toHaveBeenCalledWith( + expect.objectContaining({ configured: true, label: expect.any(String) }), ); - - try { - const cfg = await runSetupChannels(createTelegramCfg("old-token"), prompter, { - quickstartDefaults: true, - onSelection: selection, - onAccountId, - }); - - expect(configureWhenConfigured).toHaveBeenCalledTimes(1); - expect(configureWhenConfigured).toHaveBeenCalledWith( - expect.objectContaining({ configured: true, label: expect.any(String) }), - ); - expect(configure).not.toHaveBeenCalled(); - expect(selection).toHaveBeenCalledWith(["telegram"]); - expect(onAccountId).toHaveBeenCalledWith("telegram", "acct-2"); - expect(cfg.channels?.telegram?.botToken).toBe("updated-token"); - } finally { - restore(); - } + expect(configure).not.toHaveBeenCalled(); + expect(selection).toHaveBeenCalledWith(["telegram"]); + expect(onAccountId).toHaveBeenCalledWith("telegram", "acct-2"); + expect(cfg.channels?.telegram?.botToken).toBe("updated-token"); }); it("respects configureWhenConfigured skip without mutating selection or account state", async () => { - const select = createQuickstartTelegramSelect({ strictUnexpected: true }); - const selection = vi.fn(); - const onAccountId = vi.fn(); const configureWhenConfigured = vi.fn(async () => "skip" as const); - const configure = vi.fn(async () => { - throw new Error("configure should not run when configureWhenConfigured handles skip"); - }); - const restore = patchTelegramAdapter({ - configureInteractive: undefined, + const { cfg, selection, onAccountId, configure } = await runConfiguredTelegramSetup({ + strictUnexpected: true, configureWhenConfigured, - configure, + configureErrorMessage: "configure should not run when configureWhenConfigured handles skip", }); - const { prompter } = createUnexpectedQuickstartPrompter( - select as unknown as WizardPrompter["select"], + + expect(configureWhenConfigured).toHaveBeenCalledWith( + expect.objectContaining({ configured: true, label: expect.any(String) }), ); - - try { - const cfg = await runSetupChannels(createTelegramCfg("old-token"), prompter, { - quickstartDefaults: true, - onSelection: selection, - onAccountId, - }); - - expect(configureWhenConfigured).toHaveBeenCalledWith( - expect.objectContaining({ configured: true, label: expect.any(String) }), - ); - expect(configure).not.toHaveBeenCalled(); - expect(selection).toHaveBeenCalledWith([]); - expect(onAccountId).not.toHaveBeenCalled(); - expect(cfg.channels?.telegram?.botToken).toBe("old-token"); - } finally { - restore(); - } + expect(configure).not.toHaveBeenCalled(); + expect(selection).toHaveBeenCalledWith([]); + expect(onAccountId).not.toHaveBeenCalled(); + expect(cfg.channels?.telegram?.botToken).toBe("old-token"); }); it("prefers configureInteractive over configureWhenConfigured when both hooks exist", async () => { diff --git a/src/commands/onboard-remote.test.ts b/src/commands/onboard-remote.test.ts index 509af82c221..d9977f5e32a 100644 --- a/src/commands/onboard-remote.test.ts +++ b/src/commands/onboard-remote.test.ts @@ -42,6 +42,21 @@ function createSelectPrompter( describe("promptRemoteGatewayConfig", () => { const envSnapshot = captureEnv(["OPENCLAW_ALLOW_INSECURE_PRIVATE_WS"]); + async function runRemotePrompt(params: { + text: WizardPrompter["text"]; + selectResponses: Partial>; + confirm: boolean; + }) { + const cfg = {} as OpenClawConfig; + const prompter = createPrompter({ + confirm: vi.fn(async () => params.confirm), + select: createSelectPrompter(params.selectResponses), + text: params.text, + }); + const next = await promptRemoteGatewayConfig(cfg, prompter); + return { next, prompter }; + } + beforeEach(() => { vi.clearAllMocks(); envSnapshot.restore(); @@ -61,12 +76,6 @@ describe("promptRemoteGatewayConfig", () => { }, ]); - const select = createSelectPrompter({ - "Select gateway": "0", - "Connection method": "direct", - "Gateway auth": "token", - }); - const text: WizardPrompter["text"] = vi.fn(async (params) => { if (params.message === "Gateway WebSocket URL") { expect(params.initialValue).toBe("wss://gateway.tailnet.ts.net:18789"); @@ -79,15 +88,16 @@ describe("promptRemoteGatewayConfig", () => { return ""; }) as WizardPrompter["text"]; - const cfg = {} as OpenClawConfig; - const prompter = createPrompter({ - confirm: vi.fn(async () => true), - select, + const { next, prompter } = await runRemotePrompt({ text, + confirm: true, + selectResponses: { + "Select gateway": "0", + "Connection method": "direct", + "Gateway auth": "token", + }, }); - const next = await promptRemoteGatewayConfig(cfg, prompter); - expect(next.gateway?.mode).toBe("remote"); expect(next.gateway?.remote?.url).toBe("wss://gateway.tailnet.ts.net:18789"); expect(next.gateway?.remote?.token).toBe("token-123"); @@ -111,17 +121,12 @@ describe("promptRemoteGatewayConfig", () => { return ""; }) as WizardPrompter["text"]; - const select = createSelectPrompter({ "Gateway auth": "off" }); - - const cfg = {} as OpenClawConfig; - const prompter = createPrompter({ - confirm: vi.fn(async () => false), - select, + const { next } = await runRemotePrompt({ text, + confirm: false, + selectResponses: { "Gateway auth": "off" }, }); - const next = await promptRemoteGatewayConfig(cfg, prompter); - expect(next.gateway?.mode).toBe("remote"); expect(next.gateway?.remote?.url).toBe("wss://remote.example.com:18789"); expect(next.gateway?.remote?.token).toBeUndefined(); @@ -138,17 +143,12 @@ describe("promptRemoteGatewayConfig", () => { return ""; }) as WizardPrompter["text"]; - const select = createSelectPrompter({ "Gateway auth": "off" }); - - const cfg = {} as OpenClawConfig; - const prompter = createPrompter({ - confirm: vi.fn(async () => false), - select, + const { next } = await runRemotePrompt({ text, + confirm: false, + selectResponses: { "Gateway auth": "off" }, }); - const next = await promptRemoteGatewayConfig(cfg, prompter); - expect(next.gateway?.remote?.url).toBe("ws://10.0.0.8:18789"); }); }); diff --git a/src/commands/status-all/channel-issues.ts b/src/commands/status-all/channel-issues.ts new file mode 100644 index 00000000000..1fbe2e688e0 --- /dev/null +++ b/src/commands/status-all/channel-issues.ts @@ -0,0 +1,15 @@ +export function groupChannelIssuesByChannel( + issues: readonly T[], +): Map { + const byChannel = new Map(); + for (const issue of issues) { + const key = issue.channel; + const list = byChannel.get(key); + if (list) { + list.push(issue); + } else { + byChannel.set(key, [issue]); + } + } + return byChannel; +} diff --git a/src/commands/status-all/channels.ts b/src/commands/status-all/channels.ts index 1a324c93207..c4b32ec46f2 100644 --- a/src/commands/status-all/channels.ts +++ b/src/commands/status-all/channels.ts @@ -2,6 +2,8 @@ import fs from "node:fs"; import { buildChannelAccountSnapshot, formatChannelAllowFrom, + resolveChannelAccountConfigured, + resolveChannelAccountEnabled, } from "../../channels/account-summary.js"; import { resolveChannelDefaultAccountId } from "../../channels/plugins/helpers.js"; import { listChannelPlugins } from "../../channels/plugins/index.js"; @@ -85,30 +87,6 @@ const formatAccountLabel = (params: { accountId: string; name?: string }) => { return base; }; -const resolveAccountEnabled = ( - plugin: ChannelPlugin, - account: unknown, - cfg: OpenClawConfig, -): boolean => { - if (plugin.config.isEnabled) { - return plugin.config.isEnabled(account, cfg); - } - const enabled = asRecord(account).enabled; - return enabled !== false; -}; - -const resolveAccountConfigured = async ( - plugin: ChannelPlugin, - account: unknown, - cfg: OpenClawConfig, -): Promise => { - if (plugin.config.isConfigured) { - return await plugin.config.isConfigured(account, cfg); - } - const configured = asRecord(account).configured; - return configured !== false; -}; - const buildAccountNotes = (params: { plugin: ChannelPlugin; cfg: OpenClawConfig; @@ -343,8 +321,13 @@ export async function buildChannelsTable( const accounts: ChannelAccountRow[] = []; for (const accountId of resolvedAccountIds) { const account = plugin.config.resolveAccount(cfg, accountId); - const enabled = resolveAccountEnabled(plugin, account, cfg); - const configured = await resolveAccountConfigured(plugin, account, cfg); + const enabled = resolveChannelAccountEnabled({ plugin, account, cfg }); + const configured = await resolveChannelAccountConfigured({ + plugin, + account, + cfg, + readAccountConfiguredField: true, + }); const snapshot = buildChannelAccountSnapshot({ plugin, cfg, diff --git a/src/commands/status-all/report-lines.ts b/src/commands/status-all/report-lines.ts index 0db503002bd..152918029b5 100644 --- a/src/commands/status-all/report-lines.ts +++ b/src/commands/status-all/report-lines.ts @@ -1,6 +1,7 @@ import type { ProgressReporter } from "../../cli/progress.js"; import { renderTable } from "../../terminal/table.js"; import { isRich, theme } from "../../terminal/theme.js"; +import { groupChannelIssuesByChannel } from "./channel-issues.js"; import { appendStatusAllDiagnosis } from "./diagnosis.js"; import { formatTimeAgo } from "./format.js"; @@ -81,19 +82,7 @@ export async function buildStatusAllReportLines(params: { : theme.accentDim("SETUP"), Detail: row.detail, })); - const channelIssuesByChannel = (() => { - const map = new Map(); - for (const issue of params.channelIssues) { - const key = issue.channel; - const list = map.get(key); - if (list) { - list.push(issue); - } else { - map.set(key, [issue]); - } - } - return map; - })(); + const channelIssuesByChannel = groupChannelIssuesByChannel(params.channelIssues); const channelRowsWithIssues = channelRows.map((row) => { const issues = channelIssuesByChannel.get(row.channelId) ?? []; if (issues.length === 0) { diff --git a/src/commands/status.command.ts b/src/commands/status.command.ts index 1fdb1ab8b4b..4fbb54f98c3 100644 --- a/src/commands/status.command.ts +++ b/src/commands/status.command.ts @@ -21,6 +21,7 @@ import { theme } from "../terminal/theme.js"; import { formatHealthChannelLines, type HealthSummary } from "./health.js"; import { resolveControlUiLinks } from "./onboard-helpers.js"; import { statusAllCommand } from "./status-all.js"; +import { groupChannelIssuesByChannel } from "./status-all/channel-issues.js"; import { formatGatewayAuthUsed } from "./status-all/format.js"; import { getDaemonStatusSummary, getNodeDaemonStatusSummary } from "./status.daemon.js"; import { @@ -500,19 +501,7 @@ export async function statusCommand( runtime.log(""); runtime.log(theme.heading("Channels")); - const channelIssuesByChannel = (() => { - const map = new Map(); - for (const issue of channelIssues) { - const key = issue.channel; - const list = map.get(key); - if (list) { - list.push(issue); - } else { - map.set(key, [issue]); - } - } - return map; - })(); + const channelIssuesByChannel = groupChannelIssuesByChannel(channelIssues); runtime.log( renderTable({ width: tableWidth, diff --git a/src/config/config-misc.test.ts b/src/config/config-misc.test.ts index 94daa1523b9..3dc55f981ac 100644 --- a/src/config/config-misc.test.ts +++ b/src/config/config-misc.test.ts @@ -1,5 +1,3 @@ -import fs from "node:fs/promises"; -import path from "node:path"; import { describe, expect, it } from "vitest"; import { getConfigValueAtPath, @@ -8,7 +6,7 @@ import { unsetConfigValueAtPath, } from "./config-paths.js"; import { readConfigFileSnapshot, validateConfigObject } from "./config.js"; -import { buildWebSearchProviderConfig, withTempHome } from "./test-helpers.js"; +import { buildWebSearchProviderConfig, withTempHome, writeOpenClawConfig } from "./test-helpers.js"; import { OpenClawSchema } from "./zod-schema.js"; describe("$schema key in config (#14998)", () => { @@ -304,16 +302,10 @@ describe("config strict validation", () => { it("flags legacy config entries without auto-migrating", async () => { await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify({ - agents: { list: [{ id: "pi" }] }, - routing: { allowFrom: ["+15555550123"] }, - }), - "utf-8", - ); + await writeOpenClawConfig(home, { + agents: { list: [{ id: "pi" }] }, + routing: { allowFrom: ["+15555550123"] }, + }); const snap = await readConfigFileSnapshot(); @@ -324,15 +316,9 @@ describe("config strict validation", () => { it("does not mark resolved-only gateway.bind aliases as auto-migratable legacy", async () => { await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify({ - gateway: { bind: "${OPENCLAW_BIND}" }, - }), - "utf-8", - ); + await writeOpenClawConfig(home, { + gateway: { bind: "${OPENCLAW_BIND}" }, + }); const prev = process.env.OPENCLAW_BIND; process.env.OPENCLAW_BIND = "0.0.0.0"; @@ -353,15 +339,9 @@ describe("config strict validation", () => { it("still marks literal gateway.bind host aliases as legacy", async () => { await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify({ - gateway: { bind: "0.0.0.0" }, - }), - "utf-8", - ); + await writeOpenClawConfig(home, { + gateway: { bind: "0.0.0.0" }, + }); const snap = await readConfigFileSnapshot(); expect(snap.valid).toBe(false); diff --git a/src/config/config.agent-concurrency-defaults.test.ts b/src/config/config.agent-concurrency-defaults.test.ts index d2fc3853914..aa707e75b1c 100644 --- a/src/config/config.agent-concurrency-defaults.test.ts +++ b/src/config/config.agent-concurrency-defaults.test.ts @@ -1,5 +1,3 @@ -import fs from "node:fs/promises"; -import path from "node:path"; import { describe, expect, it } from "vitest"; import { DEFAULT_AGENT_MAX_CONCURRENT, @@ -8,7 +6,7 @@ import { resolveSubagentMaxConcurrent, } from "./agent-limits.js"; import { loadConfig } from "./config.js"; -import { withTempHome } from "./test-helpers.js"; +import { withTempHome, writeOpenClawConfig } from "./test-helpers.js"; import { OpenClawSchema } from "./zod-schema.js"; describe("agent concurrency defaults", () => { @@ -48,13 +46,7 @@ describe("agent concurrency defaults", () => { it("injects defaults on load", async () => { await withTempHome(async (home) => { - const configDir = path.join(home, ".openclaw"); - await fs.mkdir(configDir, { recursive: true }); - await fs.writeFile( - path.join(configDir, "openclaw.json"), - JSON.stringify({}, null, 2), - "utf-8", - ); + await writeOpenClawConfig(home, {}); const cfg = loadConfig(); diff --git a/src/config/config.legacy-config-detection.rejects-routing-allowfrom.test.ts b/src/config/config.legacy-config-detection.rejects-routing-allowfrom.test.ts index f2b2405706e..f18f5ff72a7 100644 --- a/src/config/config.legacy-config-detection.rejects-routing-allowfrom.test.ts +++ b/src/config/config.legacy-config-detection.rejects-routing-allowfrom.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import type { OpenClawConfig } from "./config.js"; import { migrateLegacyConfig, validateConfigObject } from "./config.js"; +import { WHISPER_BASE_AUDIO_MODEL } from "./legacy-migrate.test-helpers.js"; function getLegacyRouting(config: unknown) { return (config as { routing?: Record } | undefined)?.routing; @@ -137,17 +138,7 @@ describe("legacy config detection", () => { mode: "queue", cap: 3, }); - expect(res.config?.tools?.media?.audio).toEqual({ - enabled: true, - models: [ - { - command: "whisper", - type: "cli", - args: ["--model", "base"], - timeoutSeconds: 2, - }, - ], - }); + expect(res.config?.tools?.media?.audio).toEqual(WHISPER_BASE_AUDIO_MODEL); expect(getLegacyRouting(res.config)).toBeUndefined(); }); it("migrates audio.transcription with custom script names", async () => { diff --git a/src/config/config.secrets-schema.test.ts b/src/config/config.secrets-schema.test.ts index 56b0f2e06e3..196bb50ace4 100644 --- a/src/config/config.secrets-schema.test.ts +++ b/src/config/config.secrets-schema.test.ts @@ -1,6 +1,20 @@ import { describe, expect, it } from "vitest"; import { validateConfigObjectRaw } from "./validation.js"; +function validateOpenAiApiKeyRef(apiKey: unknown) { + return validateConfigObjectRaw({ + models: { + providers: { + openai: { + baseUrl: "https://api.openai.com/v1", + apiKey, + models: [{ id: "gpt-5", name: "gpt-5" }], + }, + }, + }, + }); +} + describe("config secret refs schema", () => { it("accepts top-level secrets sources and model apiKey refs", () => { const result = validateConfigObjectRaw({ @@ -108,16 +122,10 @@ describe("config secret refs schema", () => { }); it("rejects invalid secret ref id", () => { - const result = validateConfigObjectRaw({ - models: { - providers: { - openai: { - baseUrl: "https://api.openai.com/v1", - apiKey: { source: "env", provider: "default", id: "bad id with spaces" }, - models: [{ id: "gpt-5", name: "gpt-5" }], - }, - }, - }, + const result = validateOpenAiApiKeyRef({ + source: "env", + provider: "default", + id: "bad id with spaces", }); expect(result.ok).toBe(false); @@ -129,16 +137,10 @@ describe("config secret refs schema", () => { }); it("rejects env refs that are not env var names", () => { - const result = validateConfigObjectRaw({ - models: { - providers: { - openai: { - baseUrl: "https://api.openai.com/v1", - apiKey: { source: "env", provider: "default", id: "/providers/openai/apiKey" }, - models: [{ id: "gpt-5", name: "gpt-5" }], - }, - }, - }, + const result = validateOpenAiApiKeyRef({ + source: "env", + provider: "default", + id: "/providers/openai/apiKey", }); expect(result.ok).toBe(false); @@ -154,16 +156,10 @@ describe("config secret refs schema", () => { }); it("rejects file refs that are not absolute JSON pointers", () => { - const result = validateConfigObjectRaw({ - models: { - providers: { - openai: { - baseUrl: "https://api.openai.com/v1", - apiKey: { source: "file", provider: "default", id: "providers/openai/apiKey" }, - models: [{ id: "gpt-5", name: "gpt-5" }], - }, - }, - }, + const result = validateOpenAiApiKeyRef({ + source: "file", + provider: "default", + id: "providers/openai/apiKey", }); expect(result.ok).toBe(false); diff --git a/src/config/env-preserve-io.test.ts b/src/config/env-preserve-io.test.ts index ce6a215f611..b072013ec4e 100644 --- a/src/config/env-preserve-io.test.ts +++ b/src/config/env-preserve-io.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { describe, it, expect } from "vitest"; +import { withEnvAsync } from "../test-utils/env.js"; import { createConfigIO, readConfigFileSnapshotForWrite, @@ -22,37 +23,8 @@ async function withTempConfig( } } -async function withEnvOverrides( - updates: Record, - run: () => Promise, -): Promise { - const previous = new Map(); - for (const key of Object.keys(updates)) { - previous.set(key, process.env[key]); - } - - try { - for (const [key, value] of Object.entries(updates)) { - if (value === undefined) { - delete process.env[key]; - } else { - process.env[key] = value; - } - } - await run(); - } finally { - for (const [key, value] of previous.entries()) { - if (value === undefined) { - delete process.env[key]; - } else { - process.env[key] = value; - } - } - } -} - async function withWrapperEnvContext(configPath: string, run: () => Promise): Promise { - await withEnvOverrides( + await withEnvAsync( { OPENCLAW_CONFIG_PATH: configPath, OPENCLAW_DISABLE_CONFIG_CACHE: "1", diff --git a/src/config/legacy-migrate.test-helpers.ts b/src/config/legacy-migrate.test-helpers.ts new file mode 100644 index 00000000000..c59b64ec309 --- /dev/null +++ b/src/config/legacy-migrate.test-helpers.ts @@ -0,0 +1,11 @@ +export const WHISPER_BASE_AUDIO_MODEL = { + enabled: true, + models: [ + { + command: "whisper", + type: "cli", + args: ["--model", "base"], + timeoutSeconds: 2, + }, + ], +}; diff --git a/src/config/legacy-migrate.test.ts b/src/config/legacy-migrate.test.ts index 89c1977e9cc..63d971af0d4 100644 --- a/src/config/legacy-migrate.test.ts +++ b/src/config/legacy-migrate.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "vitest"; import { migrateLegacyConfig } from "./legacy-migrate.js"; +import { WHISPER_BASE_AUDIO_MODEL } from "./legacy-migrate.test-helpers.js"; describe("legacy migrate audio transcription", () => { it("moves routing.transcribeAudio into tools.media.audio.models", () => { @@ -13,17 +14,7 @@ describe("legacy migrate audio transcription", () => { }); expect(res.changes).toContain("Moved routing.transcribeAudio → tools.media.audio.models."); - expect(res.config?.tools?.media?.audio).toEqual({ - enabled: true, - models: [ - { - command: "whisper", - type: "cli", - args: ["--model", "base"], - timeoutSeconds: 2, - }, - ], - }); + expect(res.config?.tools?.media?.audio).toEqual(WHISPER_BASE_AUDIO_MODEL); expect((res.config as { routing?: unknown } | null)?.routing).toBeUndefined(); }); diff --git a/src/config/sessions/store.pruning.integration.test.ts b/src/config/sessions/store.pruning.integration.test.ts index 75cf27e20a2..d5cf106c520 100644 --- a/src/config/sessions/store.pruning.integration.test.ts +++ b/src/config/sessions/store.pruning.integration.test.ts @@ -37,6 +37,19 @@ function applyEnforcedMaintenanceConfig(mockLoadConfig: ReturnType }); } +function applyCappedMaintenanceConfig(mockLoadConfig: ReturnType) { + mockLoadConfig.mockReturnValue({ + session: { + maintenance: { + mode: "enforce", + pruneAfter: "365d", + maxEntries: 1, + rotateBytes: 10_485_760, + }, + }, + }); +} + async function createCaseDir(prefix: string): Promise { const dir = path.join(fixtureRoot, `${prefix}-${fixtureCount++}`); await fs.mkdir(dir, { recursive: true }); @@ -216,16 +229,7 @@ describe("Integration: saveSessionStore with pruning", () => { }); it("archives transcript files for entries evicted by maxEntries capping", async () => { - mockLoadConfig.mockReturnValue({ - session: { - maintenance: { - mode: "enforce", - pruneAfter: "365d", - maxEntries: 1, - rotateBytes: 10_485_760, - }, - }, - }); + applyCappedMaintenanceConfig(mockLoadConfig); const now = Date.now(); const oldestSessionId = "oldest-session"; @@ -251,16 +255,7 @@ describe("Integration: saveSessionStore with pruning", () => { }); it("does not archive external transcript paths when capping entries", async () => { - mockLoadConfig.mockReturnValue({ - session: { - maintenance: { - mode: "enforce", - pruneAfter: "365d", - maxEntries: 1, - rotateBytes: 10_485_760, - }, - }, - }); + applyCappedMaintenanceConfig(mockLoadConfig); const now = Date.now(); const externalDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-external-cap-")); diff --git a/src/config/talk.normalize.test.ts b/src/config/talk.normalize.test.ts index a61af099bf3..67bcc3a6b23 100644 --- a/src/config/talk.normalize.test.ts +++ b/src/config/talk.normalize.test.ts @@ -2,6 +2,7 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; +import { withEnvAsync } from "../test-utils/env.js"; import { createConfigIO } from "./io.js"; import { normalizeTalkSection } from "./talk.js"; @@ -19,33 +20,6 @@ async function withTempConfig( } } -async function withEnv( - updates: Record, - run: () => Promise, -): Promise { - const previous = new Map(); - for (const [key, value] of Object.entries(updates)) { - previous.set(key, process.env[key]); - if (value === undefined) { - delete process.env[key]; - } else { - process.env[key] = value; - } - } - - try { - await run(); - } finally { - for (const [key, value] of previous.entries()) { - if (value === undefined) { - delete process.env[key]; - } else { - process.env[key] = value; - } - } - } -} - describe("talk normalization", () => { it("maps legacy ElevenLabs fields into provider/providers", () => { const normalized = normalizeTalkSection({ @@ -104,7 +78,7 @@ describe("talk normalization", () => { }); it("merges ELEVENLABS_API_KEY into normalized defaults for legacy configs", async () => { - await withEnv({ ELEVENLABS_API_KEY: "env-eleven-key" }, async () => { + await withEnvAsync({ ELEVENLABS_API_KEY: "env-eleven-key" }, async () => { await withTempConfig( { talk: { @@ -124,7 +98,7 @@ describe("talk normalization", () => { }); it("does not apply ELEVENLABS_API_KEY when active provider is not elevenlabs", async () => { - await withEnv({ ELEVENLABS_API_KEY: "env-eleven-key" }, async () => { + await withEnvAsync({ ELEVENLABS_API_KEY: "env-eleven-key" }, async () => { await withTempConfig( { talk: { diff --git a/src/cron/isolated-agent.auth-profile-propagation.test.ts b/src/cron/isolated-agent.auth-profile-propagation.test.ts index 4e4539f6316..382a0dd23ab 100644 --- a/src/cron/isolated-agent.auth-profile-propagation.test.ts +++ b/src/cron/isolated-agent.auth-profile-propagation.test.ts @@ -3,8 +3,14 @@ import fs from "node:fs/promises"; import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { runEmbeddedPiAgent } from "../agents/pi-embedded.js"; +import { createCliDeps } from "./isolated-agent.delivery.test-helpers.js"; import { runCronIsolatedAgentTurn } from "./isolated-agent.js"; -import { makeCfg, makeJob, withTempCronHome } from "./isolated-agent.test-harness.js"; +import { + makeCfg, + makeJob, + withTempCronHome, + writeSessionStore, +} from "./isolated-agent.test-harness.js"; import { setupIsolatedAgentTurnMocks } from "./isolated-agent.test-setup.js"; describe("runCronIsolatedAgentTurn auth profile propagation (#20624)", () => { @@ -14,26 +20,7 @@ describe("runCronIsolatedAgentTurn auth profile propagation (#20624)", () => { it("passes authProfileId to runEmbeddedPiAgent when auth profiles exist", async () => { await withTempCronHome(async (home) => { - // 1. Write session store - const sessionsDir = path.join(home, ".openclaw", "sessions"); - await fs.mkdir(sessionsDir, { recursive: true }); - const storePath = path.join(sessionsDir, "sessions.json"); - await fs.writeFile( - storePath, - JSON.stringify( - { - "agent:main:main": { - sessionId: "main-session", - updatedAt: Date.now(), - lastProvider: "webchat", - lastTo: "", - }, - }, - null, - 2, - ), - "utf-8", - ); + const storePath = await writeSessionStore(home, { lastProvider: "webchat", lastTo: "" }); // 2. Write auth-profiles.json in the agent directory // resolveAgentDir returns /agents/main/agent @@ -79,14 +66,7 @@ describe("runCronIsolatedAgentTurn auth profile propagation (#20624)", () => { const res = await runCronIsolatedAgentTurn({ cfg, - deps: { - sendMessageSlack: vi.fn(), - sendMessageWhatsApp: vi.fn(), - sendMessageTelegram: vi.fn(), - sendMessageDiscord: vi.fn(), - sendMessageSignal: vi.fn(), - sendMessageIMessage: vi.fn(), - }, + deps: createCliDeps(), job: makeJob({ kind: "agentTurn", message: "check status", deliver: false }), message: "check status", sessionKey: "cron:job-1", diff --git a/src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts b/src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts index 61e8aed9b4a..e0afc020d27 100644 --- a/src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts +++ b/src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts @@ -1,8 +1,6 @@ import "./isolated-agent.mocks.js"; -import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; -import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { withTempHome as withTempHomeBase } from "../../test/helpers/temp-home.js"; import { runEmbeddedPiAgent } from "../agents/pi-embedded.js"; import { runSubagentAnnounceFlow } from "../agents/subagent-announce.js"; import type { CliDeps } from "../cli/deps.js"; @@ -10,56 +8,8 @@ import { runCronIsolatedAgentTurn } from "./isolated-agent.js"; import { makeCfg, makeJob, writeSessionStore } from "./isolated-agent.test-harness.js"; import { setupIsolatedAgentTurnMocks } from "./isolated-agent.test-setup.js"; -let tempRoot = ""; -let tempHomeId = 0; - async function withTempHome(fn: (home: string) => Promise): Promise { - if (!tempRoot) { - throw new Error("temp root not initialized"); - } - const home = path.join(tempRoot, `case-${tempHomeId++}`); - await fs.mkdir(path.join(home, ".openclaw", "agents", "main", "sessions"), { - recursive: true, - }); - const snapshot = { - HOME: process.env.HOME, - USERPROFILE: process.env.USERPROFILE, - HOMEDRIVE: process.env.HOMEDRIVE, - HOMEPATH: process.env.HOMEPATH, - OPENCLAW_HOME: process.env.OPENCLAW_HOME, - OPENCLAW_STATE_DIR: process.env.OPENCLAW_STATE_DIR, - }; - process.env.HOME = home; - process.env.USERPROFILE = home; - delete process.env.OPENCLAW_HOME; - process.env.OPENCLAW_STATE_DIR = path.join(home, ".openclaw"); - - if (process.platform === "win32") { - const driveMatch = home.match(/^([A-Za-z]:)(.*)$/); - if (driveMatch) { - process.env.HOMEDRIVE = driveMatch[1]; - process.env.HOMEPATH = driveMatch[2] || "\\"; - } - } - - try { - return await fn(home); - } finally { - const restoreKey = (key: keyof typeof snapshot) => { - const value = snapshot[key]; - if (value === undefined) { - delete process.env[key]; - } else { - process.env[key] = value; - } - }; - restoreKey("HOME"); - restoreKey("USERPROFILE"); - restoreKey("HOMEDRIVE"); - restoreKey("HOMEPATH"); - restoreKey("OPENCLAW_HOME"); - restoreKey("OPENCLAW_STATE_DIR"); - } + return withTempHomeBase(fn, { prefix: "openclaw-cron-heartbeat-suite-" }); } async function createTelegramDeliveryFixture(home: string): Promise<{ @@ -120,17 +70,6 @@ async function runTelegramAnnounceTurn(params: { } describe("runCronIsolatedAgentTurn", () => { - beforeAll(async () => { - tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-cron-heartbeat-suite-")); - }); - - afterAll(async () => { - if (!tempRoot) { - return; - } - await fs.rm(tempRoot, { recursive: true, force: true }); - }); - beforeEach(() => { setupIsolatedAgentTurnMocks({ fast: true }); }); diff --git a/src/cron/isolated-agent.mocks.ts b/src/cron/isolated-agent.mocks.ts index 3e5ab1ae2a7..913f5ab74d4 100644 --- a/src/cron/isolated-agent.mocks.ts +++ b/src/cron/isolated-agent.mocks.ts @@ -1,4 +1,8 @@ import { vi } from "vitest"; +import { + makeIsolatedAgentJobFixture, + makeIsolatedAgentParamsFixture, +} from "./isolated-agent/job-fixtures.js"; vi.mock("../agents/pi-embedded.js", () => ({ abortEmbeddedPiRun: vi.fn().mockReturnValue(false), @@ -22,28 +26,5 @@ vi.mock("../agents/subagent-announce.js", () => ({ runSubagentAnnounceFlow: vi.fn(), })); -type LooseRecord = Record; - -export function makeIsolatedAgentJob(overrides?: LooseRecord) { - return { - id: "test-job", - name: "Test Job", - schedule: { kind: "cron", expr: "0 9 * * *", tz: "UTC" }, - sessionTarget: "isolated", - payload: { kind: "agentTurn", message: "test" }, - ...overrides, - } as never; -} - -export function makeIsolatedAgentParams(overrides?: LooseRecord) { - const jobOverrides = - overrides && "job" in overrides ? (overrides.job as LooseRecord | undefined) : undefined; - return { - cfg: {}, - deps: {} as never, - job: makeIsolatedAgentJob(jobOverrides), - message: "test", - sessionKey: "cron:test", - ...overrides, - }; -} +export const makeIsolatedAgentJob = makeIsolatedAgentJobFixture; +export const makeIsolatedAgentParams = makeIsolatedAgentParamsFixture; diff --git a/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts b/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts index 2a9d3abe6e6..883c197bd95 100644 --- a/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts +++ b/src/cron/isolated-agent.skips-delivery-without-whatsapp-recipient-besteffortdeliver-true.test.ts @@ -1,8 +1,7 @@ import "./isolated-agent.mocks.js"; import fs from "node:fs/promises"; -import os from "node:os"; -import path from "node:path"; -import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { withTempHome as withTempHomeBase } from "../../test/helpers/temp-home.js"; import { runSubagentAnnounceFlow } from "../agents/subagent-announce.js"; import type { CliDeps } from "../cli/deps.js"; import { @@ -14,56 +13,8 @@ import { runCronIsolatedAgentTurn } from "./isolated-agent.js"; import { makeCfg, makeJob, writeSessionStore } from "./isolated-agent.test-harness.js"; import { setupIsolatedAgentTurnMocks } from "./isolated-agent.test-setup.js"; -let tempRoot = ""; -let tempHomeId = 0; - async function withTempHome(fn: (home: string) => Promise): Promise { - if (!tempRoot) { - throw new Error("temp root not initialized"); - } - const home = path.join(tempRoot, `case-${tempHomeId++}`); - await fs.mkdir(path.join(home, ".openclaw", "agents", "main", "sessions"), { - recursive: true, - }); - const snapshot = { - HOME: process.env.HOME, - USERPROFILE: process.env.USERPROFILE, - HOMEDRIVE: process.env.HOMEDRIVE, - HOMEPATH: process.env.HOMEPATH, - OPENCLAW_HOME: process.env.OPENCLAW_HOME, - OPENCLAW_STATE_DIR: process.env.OPENCLAW_STATE_DIR, - }; - process.env.HOME = home; - process.env.USERPROFILE = home; - delete process.env.OPENCLAW_HOME; - process.env.OPENCLAW_STATE_DIR = path.join(home, ".openclaw"); - - if (process.platform === "win32") { - const driveMatch = home.match(/^([A-Za-z]:)(.*)$/); - if (driveMatch) { - process.env.HOMEDRIVE = driveMatch[1]; - process.env.HOMEPATH = driveMatch[2] || "\\"; - } - } - - try { - return await fn(home); - } finally { - const restoreKey = (key: keyof typeof snapshot) => { - const value = snapshot[key]; - if (value === undefined) { - delete process.env[key]; - } else { - process.env[key] = value; - } - }; - restoreKey("HOME"); - restoreKey("USERPROFILE"); - restoreKey("HOMEDRIVE"); - restoreKey("HOMEPATH"); - restoreKey("OPENCLAW_HOME"); - restoreKey("OPENCLAW_STATE_DIR"); - } + return withTempHomeBase(fn, { prefix: "openclaw-cron-delivery-suite-" }); } async function runExplicitTelegramAnnounceTurn(params: { @@ -216,17 +167,6 @@ async function assertExplicitTelegramTargetAnnounce(params: { } describe("runCronIsolatedAgentTurn", () => { - beforeAll(async () => { - tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-cron-delivery-suite-")); - }); - - afterAll(async () => { - if (!tempRoot) { - return; - } - await fs.rm(tempRoot, { recursive: true, force: true }); - }); - beforeEach(() => { setupIsolatedAgentTurnMocks(); }); diff --git a/src/cron/isolated-agent.uses-last-non-empty-agent-text-as.test.ts b/src/cron/isolated-agent.uses-last-non-empty-agent-text-as.test.ts index 02e1e054fca..90e6de7e3ac 100644 --- a/src/cron/isolated-agent.uses-last-non-empty-agent-text-as.test.ts +++ b/src/cron/isolated-agent.uses-last-non-empty-agent-text-as.test.ts @@ -1,8 +1,8 @@ import "./isolated-agent.mocks.js"; import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; -import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { withTempHome as withTempHomeBase } from "../../test/helpers/temp-home.js"; import { loadModelCatalog } from "../agents/model-catalog.js"; import { runEmbeddedPiAgent } from "../agents/pi-embedded.js"; import type { CliDeps } from "../cli/deps.js"; @@ -15,56 +15,8 @@ import { } from "./isolated-agent.test-harness.js"; import type { CronJob } from "./types.js"; -let tempRoot = ""; -let tempHomeId = 0; - async function withTempHome(fn: (home: string) => Promise): Promise { - if (!tempRoot) { - throw new Error("temp root not initialized"); - } - const home = path.join(tempRoot, `case-${tempHomeId++}`); - await fs.mkdir(path.join(home, ".openclaw", "agents", "main", "sessions"), { - recursive: true, - }); - const snapshot = { - HOME: process.env.HOME, - USERPROFILE: process.env.USERPROFILE, - HOMEDRIVE: process.env.HOMEDRIVE, - HOMEPATH: process.env.HOMEPATH, - OPENCLAW_HOME: process.env.OPENCLAW_HOME, - OPENCLAW_STATE_DIR: process.env.OPENCLAW_STATE_DIR, - }; - process.env.HOME = home; - process.env.USERPROFILE = home; - delete process.env.OPENCLAW_HOME; - process.env.OPENCLAW_STATE_DIR = path.join(home, ".openclaw"); - - if (process.platform === "win32") { - const driveMatch = home.match(/^([A-Za-z]:)(.*)$/); - if (driveMatch) { - process.env.HOMEDRIVE = driveMatch[1]; - process.env.HOMEPATH = driveMatch[2] || "\\"; - } - } - - try { - return await fn(home); - } finally { - const restoreKey = (key: keyof typeof snapshot) => { - const value = snapshot[key]; - if (value === undefined) { - delete process.env[key]; - } else { - process.env[key] = value; - } - }; - restoreKey("HOME"); - restoreKey("USERPROFILE"); - restoreKey("HOMEDRIVE"); - restoreKey("HOMEPATH"); - restoreKey("OPENCLAW_HOME"); - restoreKey("OPENCLAW_STATE_DIR"); - } + return withTempHomeBase(fn, { prefix: "openclaw-cron-turn-suite-" }); } function makeDeps(): CliDeps { @@ -201,17 +153,6 @@ async function runTurnWithStoredModelOverride( } describe("runCronIsolatedAgentTurn", () => { - beforeAll(async () => { - tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-cron-turn-suite-")); - }); - - afterAll(async () => { - if (!tempRoot) { - return; - } - await fs.rm(tempRoot, { recursive: true, force: true }); - }); - beforeEach(() => { vi.mocked(runEmbeddedPiAgent).mockClear(); vi.mocked(loadModelCatalog).mockResolvedValue([]); diff --git a/src/cron/isolated-agent/job-fixtures.ts b/src/cron/isolated-agent/job-fixtures.ts new file mode 100644 index 00000000000..3456e7e948d --- /dev/null +++ b/src/cron/isolated-agent/job-fixtures.ts @@ -0,0 +1,25 @@ +type LooseRecord = Record; + +export function makeIsolatedAgentJobFixture(overrides?: LooseRecord) { + return { + id: "test-job", + name: "Test Job", + schedule: { kind: "cron", expr: "0 9 * * *", tz: "UTC" }, + sessionTarget: "isolated", + payload: { kind: "agentTurn", message: "test" }, + ...overrides, + } as never; +} + +export function makeIsolatedAgentParamsFixture(overrides?: LooseRecord) { + const jobOverrides = + overrides && "job" in overrides ? (overrides.job as LooseRecord | undefined) : undefined; + return { + cfg: {}, + deps: {} as never, + job: makeIsolatedAgentJobFixture(jobOverrides), + message: "test", + sessionKey: "cron:test", + ...overrides, + }; +} diff --git a/src/cron/isolated-agent/run.payload-fallbacks.test.ts b/src/cron/isolated-agent/run.payload-fallbacks.test.ts index c1fe0fd73bf..dd1b672636f 100644 --- a/src/cron/isolated-agent/run.payload-fallbacks.test.ts +++ b/src/cron/isolated-agent/run.payload-fallbacks.test.ts @@ -1,53 +1,21 @@ -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { describe, expect, it } from "vitest"; +import { + makeIsolatedAgentTurnJob, + makeIsolatedAgentTurnParams, + setupRunCronIsolatedAgentTurnSuite, +} from "./run.suite-helpers.js"; import { - clearFastTestEnv, loadRunCronIsolatedAgentTurn, - makeCronSession, resolveAgentModelFallbacksOverrideMock, - resolveCronSessionMock, - resetRunCronIsolatedAgentTurnHarness, - restoreFastTestEnv, runWithModelFallbackMock, } from "./run.test-harness.js"; const runCronIsolatedAgentTurn = await loadRunCronIsolatedAgentTurn(); -function makePayloadJob(overrides?: Record) { - return { - id: "test-job", - name: "Test Job", - schedule: { kind: "cron", expr: "0 9 * * *", tz: "UTC" }, - sessionTarget: "isolated", - payload: { kind: "agentTurn", message: "test" }, - ...overrides, - } as never; -} - -function makePayloadParams(overrides?: Record) { - return { - cfg: {}, - deps: {} as never, - job: makePayloadJob(overrides?.job as Record | undefined), - message: "test", - sessionKey: "cron:test", - ...overrides, - }; -} - // ---------- tests ---------- describe("runCronIsolatedAgentTurn — payload.fallbacks", () => { - let previousFastTestEnv: string | undefined; - - beforeEach(() => { - previousFastTestEnv = clearFastTestEnv(); - resetRunCronIsolatedAgentTurnHarness(); - resolveCronSessionMock.mockReturnValue(makeCronSession()); - }); - - afterEach(() => { - restoreFastTestEnv(previousFastTestEnv); - }); + setupRunCronIsolatedAgentTurnSuite(); it.each([ { @@ -77,8 +45,8 @@ describe("runCronIsolatedAgentTurn — payload.fallbacks", () => { } const result = await runCronIsolatedAgentTurn( - makePayloadParams({ - job: makePayloadJob({ payload }), + makeIsolatedAgentTurnParams({ + job: makeIsolatedAgentTurnJob({ payload }), }), ); diff --git a/src/cron/isolated-agent/run.skill-filter.test.ts b/src/cron/isolated-agent/run.skill-filter.test.ts index 67b6bfedb63..b0d34ad2f40 100644 --- a/src/cron/isolated-agent/run.skill-filter.test.ts +++ b/src/cron/isolated-agent/run.skill-filter.test.ts @@ -1,62 +1,34 @@ -import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { describe, expect, it } from "vitest"; +import { + makeIsolatedAgentTurnJob, + makeIsolatedAgentTurnParams, + setupRunCronIsolatedAgentTurnSuite, +} from "./run.suite-helpers.js"; import { buildWorkspaceSkillSnapshotMock, - clearFastTestEnv, getCliSessionIdMock, isCliProviderMock, loadRunCronIsolatedAgentTurn, logWarnMock, - makeCronSession, resolveAgentConfigMock, resolveAgentSkillsFilterMock, resolveAllowedModelRefMock, resolveCronSessionMock, - resetRunCronIsolatedAgentTurnHarness, - restoreFastTestEnv, runCliAgentMock, runWithModelFallbackMock, } from "./run.test-harness.js"; const runCronIsolatedAgentTurn = await loadRunCronIsolatedAgentTurn(); - -function makeSkillJob(overrides?: Record) { - return { - id: "test-job", - name: "Test Job", - schedule: { kind: "cron", expr: "0 9 * * *", tz: "UTC" }, - sessionTarget: "isolated", - payload: { kind: "agentTurn", message: "test" }, - ...overrides, - } as never; -} - -function makeSkillParams(overrides?: Record) { - return { - cfg: {}, - deps: {} as never, - job: makeSkillJob(overrides?.job as Record | undefined), - message: "test", - sessionKey: "cron:test", - ...overrides, - }; -} +const makeSkillJob = makeIsolatedAgentTurnJob; +const makeSkillParams = makeIsolatedAgentTurnParams; // ---------- tests ---------- describe("runCronIsolatedAgentTurn — skill filter", () => { - let previousFastTestEnv: string | undefined; - beforeEach(() => { - previousFastTestEnv = clearFastTestEnv(); - resetRunCronIsolatedAgentTurnHarness(); - resolveCronSessionMock.mockReturnValue(makeCronSession()); - }); - - afterEach(() => { - restoreFastTestEnv(previousFastTestEnv); - }); + setupRunCronIsolatedAgentTurnSuite(); async function runSkillFilterCase(overrides?: Record) { - const result = await runCronIsolatedAgentTurn(makeSkillParams(overrides)); + const result = await runCronIsolatedAgentTurn(makeIsolatedAgentTurnParams(overrides)); expect(result.status).toBe("ok"); return result; } diff --git a/src/cron/isolated-agent/run.suite-helpers.ts b/src/cron/isolated-agent/run.suite-helpers.ts new file mode 100644 index 00000000000..291029d6f99 --- /dev/null +++ b/src/cron/isolated-agent/run.suite-helpers.ts @@ -0,0 +1,24 @@ +import { afterEach, beforeEach } from "vitest"; +import { makeIsolatedAgentJobFixture, makeIsolatedAgentParamsFixture } from "./job-fixtures.js"; +import { + clearFastTestEnv, + makeCronSession, + resolveCronSessionMock, + resetRunCronIsolatedAgentTurnHarness, + restoreFastTestEnv, +} from "./run.test-harness.js"; + +export function setupRunCronIsolatedAgentTurnSuite() { + let previousFastTestEnv: string | undefined; + beforeEach(() => { + previousFastTestEnv = clearFastTestEnv(); + resetRunCronIsolatedAgentTurnHarness(); + resolveCronSessionMock.mockReturnValue(makeCronSession()); + }); + afterEach(() => { + restoreFastTestEnv(previousFastTestEnv); + }); +} + +export const makeIsolatedAgentTurnJob = makeIsolatedAgentJobFixture; +export const makeIsolatedAgentTurnParams = makeIsolatedAgentParamsFixture; diff --git a/src/cron/service.armtimer-tight-loop.test.ts b/src/cron/service.armtimer-tight-loop.test.ts index a82aa36fbb2..b725adc78d6 100644 --- a/src/cron/service.armtimer-tight-loop.test.ts +++ b/src/cron/service.armtimer-tight-loop.test.ts @@ -39,6 +39,30 @@ function createStuckPastDueJob(params: { id: string; nowMs: number; pastDueMs: n } describe("CronService - armTimer tight loop prevention", () => { + function extractTimeoutDelays(timeoutSpy: ReturnType) { + const calls = timeoutSpy.mock.calls as Array<[unknown, unknown, ...unknown[]]>; + return calls + .map(([, delay]: [unknown, unknown, ...unknown[]]) => delay) + .filter((d: unknown): d is number => typeof d === "number"); + } + + function createTimerState(params: { + storePath: string; + now: number; + runIsolatedAgentJob?: () => Promise<{ status: "ok" }>; + }) { + return createCronServiceState({ + storePath: params.storePath, + cronEnabled: true, + log: noopLogger, + nowMs: () => params.now, + enqueueSystemEvent: vi.fn(), + requestHeartbeatNow: vi.fn(), + runIsolatedAgentJob: + params.runIsolatedAgentJob ?? vi.fn().mockResolvedValue({ status: "ok" }), + }); + } + beforeEach(() => { noopLogger.debug.mockClear(); noopLogger.info.mockClear(); @@ -55,14 +79,9 @@ describe("CronService - armTimer tight loop prevention", () => { const now = Date.parse("2026-02-28T12:32:00.000Z"); const pastDueMs = 17 * 60 * 1000; // 17 minutes past due - const state = createCronServiceState({ + const state = createTimerState({ storePath: "/tmp/test-cron/jobs.json", - cronEnabled: true, - log: noopLogger, - nowMs: () => now, - enqueueSystemEvent: vi.fn(), - requestHeartbeatNow: vi.fn(), - runIsolatedAgentJob: vi.fn().mockResolvedValue({ status: "ok" }), + now, }); state.store = { version: 1, @@ -72,9 +91,7 @@ describe("CronService - armTimer tight loop prevention", () => { armTimer(state); expect(state.timer).not.toBeNull(); - const delays = timeoutSpy.mock.calls - .map(([, delay]) => delay) - .filter((d): d is number => typeof d === "number"); + const delays = extractTimeoutDelays(timeoutSpy); // Before the fix, delay would be 0 (tight loop). // After the fix, delay must be >= MIN_REFIRE_GAP_MS (2000 ms). @@ -90,14 +107,9 @@ describe("CronService - armTimer tight loop prevention", () => { const timeoutSpy = vi.spyOn(globalThis, "setTimeout"); const now = Date.parse("2026-02-28T12:32:00.000Z"); - const state = createCronServiceState({ + const state = createTimerState({ storePath: "/tmp/test-cron/jobs.json", - cronEnabled: true, - log: noopLogger, - nowMs: () => now, - enqueueSystemEvent: vi.fn(), - requestHeartbeatNow: vi.fn(), - runIsolatedAgentJob: vi.fn().mockResolvedValue({ status: "ok" }), + now, }); state.store = { version: 1, @@ -121,9 +133,7 @@ describe("CronService - armTimer tight loop prevention", () => { armTimer(state); - const delays = timeoutSpy.mock.calls - .map(([, delay]) => delay) - .filter((d): d is number => typeof d === "number"); + const delays = extractTimeoutDelays(timeoutSpy); // The natural delay (10 s) should be used, not the floor. expect(delays).toContain(10_000); @@ -151,14 +161,9 @@ describe("CronService - armTimer tight loop prevention", () => { "utf-8", ); - const state = createCronServiceState({ + const state = createTimerState({ storePath: store.storePath, - cronEnabled: true, - log: noopLogger, - nowMs: () => now, - enqueueSystemEvent: vi.fn(), - requestHeartbeatNow: vi.fn(), - runIsolatedAgentJob: vi.fn().mockResolvedValue({ status: "ok" }), + now, }); // Simulate the onTimer path: it will find no runnable jobs (blocked by @@ -170,9 +175,7 @@ describe("CronService - armTimer tight loop prevention", () => { // The re-armed timer must NOT use delay=0. It should use at least // MIN_REFIRE_GAP_MS to prevent the hot-loop. - const allDelays = timeoutSpy.mock.calls - .map(([, delay]) => delay) - .filter((d): d is number => typeof d === "number"); + const allDelays = extractTimeoutDelays(timeoutSpy); // The last setTimeout call is from the finally→armTimer path. const lastDelay = allDelays[allDelays.length - 1]; diff --git a/src/cron/service.main-job-passes-heartbeat-target-last.test.ts b/src/cron/service.main-job-passes-heartbeat-target-last.test.ts index 03a8eb214dd..39959f63207 100644 --- a/src/cron/service.main-job-passes-heartbeat-target-last.test.ts +++ b/src/cron/service.main-job-passes-heartbeat-target-last.test.ts @@ -1,5 +1,4 @@ import { describe, expect, it, vi } from "vitest"; -import type { HeartbeatRunResult } from "../infra/heartbeat-wake.js"; import { CronService } from "./service.js"; import { setupCronServiceSuite, writeCronStoreSnapshot } from "./service.test-harness.js"; import type { CronJob } from "./types.js"; @@ -8,59 +7,75 @@ const { logger, makeStorePath } = setupCronServiceSuite({ prefix: "cron-main-heartbeat-target", }); -describe("cron main job passes heartbeat target=last", () => { - it("should pass heartbeat.target=last to runHeartbeatOnce for wakeMode=now main jobs", async () => { - const { storePath } = await makeStorePath(); - const now = Date.now(); +type RunHeartbeatOnce = NonNullable< + ConstructorParameters[0]["runHeartbeatOnce"] +>; - const job: CronJob = { - id: "test-main-delivery", - name: "test-main-delivery", +describe("cron main job passes heartbeat target=last", () => { + function createMainCronJob(params: { + now: number; + id: string; + wakeMode: CronJob["wakeMode"]; + }): CronJob { + return { + id: params.id, + name: params.id, enabled: true, - createdAtMs: now - 10_000, - updatedAtMs: now - 10_000, + createdAtMs: params.now - 10_000, + updatedAtMs: params.now - 10_000, schedule: { kind: "every", everyMs: 60_000 }, sessionTarget: "main", - wakeMode: "now", + wakeMode: params.wakeMode, payload: { kind: "systemEvent", text: "Check in" }, - state: { nextRunAtMs: now - 1 }, + state: { nextRunAtMs: params.now - 1 }, }; + } - await writeCronStoreSnapshot({ storePath, jobs: [job] }); - + function createCronWithSpies(params: { storePath: string; runHeartbeatOnce: RunHeartbeatOnce }) { const enqueueSystemEvent = vi.fn(); const requestHeartbeatNow = vi.fn(); - const runHeartbeatOnce = vi.fn< - (opts?: { - reason?: string; - agentId?: string; - sessionKey?: string; - heartbeat?: { target?: string }; - }) => Promise - >(async () => ({ - status: "ran" as const, - durationMs: 50, - })); - const cron = new CronService({ - storePath, + storePath: params.storePath, cronEnabled: true, log: logger, enqueueSystemEvent, requestHeartbeatNow, - runHeartbeatOnce, + runHeartbeatOnce: params.runHeartbeatOnce, runIsolatedAgentJob: vi.fn(async () => ({ status: "ok" as const })), }); + return { cron, requestHeartbeatNow }; + } + async function runSingleTick(cron: CronService) { await cron.start(); - - // Wait for the timer to fire await vi.advanceTimersByTimeAsync(2_000); - - // Give the async run a chance to complete await vi.advanceTimersByTimeAsync(1_000); - cron.stop(); + } + + it("should pass heartbeat.target=last to runHeartbeatOnce for wakeMode=now main jobs", async () => { + const { storePath } = await makeStorePath(); + const now = Date.now(); + + const job = createMainCronJob({ + now, + id: "test-main-delivery", + wakeMode: "now", + }); + + await writeCronStoreSnapshot({ storePath, jobs: [job] }); + + const runHeartbeatOnce = vi.fn(async () => ({ + status: "ran" as const, + durationMs: 50, + })); + + const { cron } = createCronWithSpies({ + storePath, + runHeartbeatOnce, + }); + + await runSingleTick(cron); // runHeartbeatOnce should have been called expect(runHeartbeatOnce).toHaveBeenCalled(); @@ -77,42 +92,25 @@ describe("cron main job passes heartbeat target=last", () => { const { storePath } = await makeStorePath(); const now = Date.now(); - const job: CronJob = { + const job = createMainCronJob({ + now, id: "test-next-heartbeat", - name: "test-next-heartbeat", - enabled: true, - createdAtMs: now - 10_000, - updatedAtMs: now - 10_000, - schedule: { kind: "every", everyMs: 60_000 }, - sessionTarget: "main", wakeMode: "next-heartbeat", - payload: { kind: "systemEvent", text: "Check in" }, - state: { nextRunAtMs: now - 1 }, - }; + }); await writeCronStoreSnapshot({ storePath, jobs: [job] }); - const enqueueSystemEvent = vi.fn(); - const requestHeartbeatNow = vi.fn(); - const runHeartbeatOnce = vi.fn(async () => ({ + const runHeartbeatOnce = vi.fn(async () => ({ status: "ran" as const, durationMs: 50, })); - const cron = new CronService({ + const { cron, requestHeartbeatNow } = createCronWithSpies({ storePath, - cronEnabled: true, - log: logger, - enqueueSystemEvent, - requestHeartbeatNow, runHeartbeatOnce, - runIsolatedAgentJob: vi.fn(async () => ({ status: "ok" as const })), }); - await cron.start(); - await vi.advanceTimersByTimeAsync(2_000); - await vi.advanceTimersByTimeAsync(1_000); - cron.stop(); + await runSingleTick(cron); // wakeMode=next-heartbeat uses requestHeartbeatNow, not runHeartbeatOnce expect(requestHeartbeatNow).toHaveBeenCalled(); diff --git a/src/cron/store.test.ts b/src/cron/store.test.ts index 02f7a11b7a1..1d318671437 100644 --- a/src/cron/store.test.ts +++ b/src/cron/store.test.ts @@ -1,32 +1,11 @@ import fs from "node:fs/promises"; -import os from "node:os"; import path from "node:path"; -import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { createCronStoreHarness } from "./service.test-harness.js"; import { loadCronStore, resolveCronStorePath, saveCronStore } from "./store.js"; import type { CronStoreFile } from "./types.js"; -let fixtureRoot = ""; -let fixtureCount = 0; - -beforeAll(async () => { - fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-cron-store-")); -}); - -afterAll(async () => { - if (!fixtureRoot) { - return; - } - await fs.rm(fixtureRoot, { recursive: true, force: true }); -}); - -async function makeStorePath() { - const dir = path.join(fixtureRoot, `case-${fixtureCount++}`); - await fs.mkdir(dir, { recursive: true }); - return { - dir, - storePath: path.join(dir, "jobs.json"), - }; -} +const { makeStorePath } = createCronStoreHarness({ prefix: "openclaw-cron-store-" }); function makeStore(jobId: string, enabled: boolean): CronStoreFile { const now = Date.now(); @@ -72,6 +51,7 @@ describe("cron store", () => { it("throws when store contains invalid JSON", async () => { const store = await makeStorePath(); + await fs.mkdir(path.dirname(store.storePath), { recursive: true }); await fs.writeFile(store.storePath, "{ not json", "utf-8"); await expect(loadCronStore(store.storePath)).rejects.toThrow(/Failed to parse cron store/i); }); diff --git a/src/cron/types-shared.ts b/src/cron/types-shared.ts new file mode 100644 index 00000000000..68c7f0c97a3 --- /dev/null +++ b/src/cron/types-shared.ts @@ -0,0 +1,18 @@ +export type CronJobBase = + { + id: string; + agentId?: string; + sessionKey?: string; + name: string; + description?: string; + enabled: boolean; + deleteAfterRun?: boolean; + createdAtMs: number; + updatedAtMs: number; + schedule: TSchedule; + sessionTarget: TSessionTarget; + wakeMode: TWakeMode; + payload: TPayload; + delivery?: TDelivery; + failureAlert?: TFailureAlert; + }; diff --git a/src/cron/types.ts b/src/cron/types.ts index 1010f4b7682..7e88e76ecb3 100644 --- a/src/cron/types.ts +++ b/src/cron/types.ts @@ -1,4 +1,5 @@ import type { ChannelId } from "../channels/plugins/types.js"; +import type { CronJobBase } from "./types-shared.js"; export type CronSchedule = | { kind: "at"; at: string } @@ -138,23 +139,14 @@ export type CronJobState = { lastDelivered?: boolean; }; -export type CronJob = { - id: string; - agentId?: string; - /** Origin session namespace for reminder delivery and wake routing. */ - sessionKey?: string; - name: string; - description?: string; - enabled: boolean; - deleteAfterRun?: boolean; - createdAtMs: number; - updatedAtMs: number; - schedule: CronSchedule; - sessionTarget: CronSessionTarget; - wakeMode: CronWakeMode; - payload: CronPayload; - delivery?: CronDelivery; - failureAlert?: CronFailureAlert | false; +export type CronJob = CronJobBase< + CronSchedule, + CronSessionTarget, + CronWakeMode, + CronPayload, + CronDelivery, + CronFailureAlert | false +> & { state: CronJobState; }; diff --git a/src/daemon/service-runtime.ts b/src/daemon/service-runtime.ts index 8589af4bc80..08fe12cfc3d 100644 --- a/src/daemon/service-runtime.ts +++ b/src/daemon/service-runtime.ts @@ -1,5 +1,5 @@ export type GatewayServiceRuntime = { - status?: "running" | "stopped" | "unknown"; + status?: string; state?: string; subState?: string; pid?: number; diff --git a/src/hooks/loader.test.ts b/src/hooks/loader.test.ts index d9107d2e390..a6618ab70c1 100644 --- a/src/hooks/loader.test.ts +++ b/src/hooks/loader.test.ts @@ -65,6 +65,20 @@ describe("loader", () => { }); describe("loadInternalHooks", () => { + const createLegacyHandlerConfig = () => + createEnabledHooksConfig([ + { + event: "command:new", + module: "legacy-handler.js", + }, + ]); + + const expectNoCommandHookRegistration = async (cfg: OpenClawConfig) => { + const count = await loadInternalHooks(cfg, tmpDir); + expect(count).toBe(0); + expect(getRegisteredEventKeys()).not.toContain("command:new"); + }; + it("should return 0 when hooks are not enabled", async () => { const cfg: OpenClawConfig = { hooks: { @@ -252,11 +266,7 @@ describe("loader", () => { return; } - const cfg = createEnabledHooksConfig(); - - const count = await loadInternalHooks(cfg, tmpDir); - expect(count).toBe(0); - expect(getRegisteredEventKeys()).not.toContain("command:new"); + await expectNoCommandHookRegistration(createEnabledHooksConfig()); }); it("rejects legacy handler modules that escape workspace via symlink", async () => { @@ -270,16 +280,7 @@ describe("loader", () => { return; } - const cfg = createEnabledHooksConfig([ - { - event: "command:new", - module: "legacy-handler.js", - }, - ]); - - const count = await loadInternalHooks(cfg, tmpDir); - expect(count).toBe(0); - expect(getRegisteredEventKeys()).not.toContain("command:new"); + await expectNoCommandHookRegistration(createLegacyHandlerConfig()); }); it("rejects directory hook handlers that escape hook dir via hardlink", async () => { @@ -313,10 +314,7 @@ describe("loader", () => { throw err; } - const cfg = createEnabledHooksConfig(); - const count = await loadInternalHooks(cfg, tmpDir); - expect(count).toBe(0); - expect(getRegisteredEventKeys()).not.toContain("command:new"); + await expectNoCommandHookRegistration(createEnabledHooksConfig()); }); it("rejects legacy handler modules that escape workspace via hardlink", async () => { @@ -336,16 +334,7 @@ describe("loader", () => { throw err; } - const cfg = createEnabledHooksConfig([ - { - event: "command:new", - module: "legacy-handler.js", - }, - ]); - - const count = await loadInternalHooks(cfg, tmpDir); - expect(count).toBe(0); - expect(getRegisteredEventKeys()).not.toContain("command:new"); + await expectNoCommandHookRegistration(createLegacyHandlerConfig()); }); }); }); diff --git a/src/hooks/workspace.ts b/src/hooks/workspace.ts index ab6375cd8ea..56e2fc05339 100644 --- a/src/hooks/workspace.ts +++ b/src/hooks/workspace.ts @@ -339,6 +339,23 @@ function readBoundaryFileUtf8(params: { rootPath: string; boundaryLabel: string; }): string | null { + return withOpenedBoundaryFileSync(params, (opened) => { + try { + return fs.readFileSync(opened.fd, "utf-8"); + } catch { + return null; + } + }); +} + +function withOpenedBoundaryFileSync( + params: { + absolutePath: string; + rootPath: string; + boundaryLabel: string; + }, + read: (opened: { fd: number; path: string }) => T, +): T | null { const opened = openBoundaryFileSync({ absolutePath: params.absolutePath, rootPath: params.rootPath, @@ -348,9 +365,7 @@ function readBoundaryFileUtf8(params: { return null; } try { - return fs.readFileSync(opened.fd, "utf-8"); - } catch { - return null; + return read({ fd: opened.fd, path: opened.path }); } finally { fs.closeSync(opened.fd); } @@ -361,15 +376,5 @@ function resolveBoundaryFilePath(params: { rootPath: string; boundaryLabel: string; }): string | null { - const opened = openBoundaryFileSync({ - absolutePath: params.absolutePath, - rootPath: params.rootPath, - boundaryLabel: params.boundaryLabel, - }); - if (!opened.ok) { - return null; - } - const safePath = opened.path; - fs.closeSync(opened.fd); - return safePath; + return withOpenedBoundaryFileSync(params, (opened) => opened.path); } diff --git a/src/infra/boundary-file-read.ts b/src/infra/boundary-file-read.ts index eea0cc66cb3..93ffef6deeb 100644 --- a/src/infra/boundary-file-read.ts +++ b/src/infra/boundary-file-read.ts @@ -80,13 +80,8 @@ export function openBoundaryFileSync(params: OpenBoundaryFileSyncParams): Bounda if (resolved instanceof Promise) { return toBoundaryValidationError(new Error("Unexpected async boundary resolution")); } - if ("ok" in resolved) { - return resolved; - } - return openBoundaryFileResolved({ - absolutePath: resolved.absolutePath, - resolvedPath: resolved.resolvedPath, - rootRealPath: resolved.rootRealPath, + return finalizeBoundaryFileOpen({ + resolved, maxBytes: params.maxBytes, rejectHardlinks: params.rejectHardlinks, allowedType: params.allowedType, @@ -123,6 +118,27 @@ function openBoundaryFileResolved(params: { }; } +function finalizeBoundaryFileOpen(params: { + resolved: ResolvedBoundaryFilePath | BoundaryFileOpenResult; + maxBytes?: number; + rejectHardlinks?: boolean; + allowedType?: SafeOpenSyncAllowedType; + ioFs: BoundaryReadFs; +}): BoundaryFileOpenResult { + if ("ok" in params.resolved) { + return params.resolved; + } + return openBoundaryFileResolved({ + absolutePath: params.resolved.absolutePath, + resolvedPath: params.resolved.resolvedPath, + rootRealPath: params.resolved.rootRealPath, + maxBytes: params.maxBytes, + rejectHardlinks: params.rejectHardlinks, + allowedType: params.allowedType, + ioFs: params.ioFs, + }); +} + export async function openBoundaryFile( params: OpenBoundaryFileParams, ): Promise { @@ -140,13 +156,8 @@ export async function openBoundaryFile( }), }); const resolved = maybeResolved instanceof Promise ? await maybeResolved : maybeResolved; - if ("ok" in resolved) { - return resolved; - } - return openBoundaryFileResolved({ - absolutePath: resolved.absolutePath, - resolvedPath: resolved.resolvedPath, - rootRealPath: resolved.rootRealPath, + return finalizeBoundaryFileOpen({ + resolved, maxBytes: params.maxBytes, rejectHardlinks: params.rejectHardlinks, allowedType: params.allowedType, diff --git a/src/infra/channel-summary.ts b/src/infra/channel-summary.ts index 095f717c418..19114a367e8 100644 --- a/src/infra/channel-summary.ts +++ b/src/infra/channel-summary.ts @@ -1,6 +1,8 @@ import { buildChannelAccountSnapshot, formatChannelAllowFrom, + resolveChannelAccountConfigured, + resolveChannelAccountEnabled, } from "../channels/account-summary.js"; import { listChannelPlugins } from "../channels/plugins/index.js"; import type { ChannelAccountSnapshot, ChannelPlugin } from "../channels/plugins/types.js"; @@ -38,32 +40,6 @@ const formatAccountLabel = (params: { accountId: string; name?: string }) => { const accountLine = (label: string, details: string[]) => ` - ${label}${details.length ? ` (${details.join(", ")})` : ""}`; -const resolveAccountEnabled = ( - plugin: ChannelPlugin, - account: unknown, - cfg: OpenClawConfig, -): boolean => { - if (plugin.config.isEnabled) { - return plugin.config.isEnabled(account, cfg); - } - if (!account || typeof account !== "object") { - return true; - } - const enabled = (account as { enabled?: boolean }).enabled; - return enabled !== false; -}; - -const resolveAccountConfigured = async ( - plugin: ChannelPlugin, - account: unknown, - cfg: OpenClawConfig, -): Promise => { - if (plugin.config.isConfigured) { - return await plugin.config.isConfigured(account, cfg); - } - return true; -}; - const buildAccountDetails = (params: { entry: ChannelAccountEntry; plugin: ChannelPlugin; @@ -133,8 +109,12 @@ export async function buildChannelSummary( for (const accountId of resolvedAccountIds) { const account = plugin.config.resolveAccount(effective, accountId); - const enabled = resolveAccountEnabled(plugin, account, effective); - const configured = await resolveAccountConfigured(plugin, account, effective); + const enabled = resolveChannelAccountEnabled({ plugin, account, cfg: effective }); + const configured = await resolveChannelAccountConfigured({ + plugin, + account, + cfg: effective, + }); const snapshot = buildChannelAccountSnapshot({ plugin, account, diff --git a/src/infra/errors.ts b/src/infra/errors.ts index e64881d1d65..bff922c4235 100644 --- a/src/infra/errors.ts +++ b/src/infra/errors.ts @@ -14,6 +14,43 @@ export function extractErrorCode(err: unknown): string | undefined { return undefined; } +export function readErrorName(err: unknown): string { + if (!err || typeof err !== "object") { + return ""; + } + const name = (err as { name?: unknown }).name; + return typeof name === "string" ? name : ""; +} + +export function collectErrorGraphCandidates( + err: unknown, + resolveNested?: (current: Record) => Iterable, +): unknown[] { + const queue: unknown[] = [err]; + const seen = new Set(); + const candidates: unknown[] = []; + + while (queue.length > 0) { + const current = queue.shift(); + if (current == null || seen.has(current)) { + continue; + } + seen.add(current); + candidates.push(current); + + if (!current || typeof current !== "object" || !resolveNested) { + continue; + } + for (const nested of resolveNested(current as Record)) { + if (nested != null && !seen.has(nested)) { + queue.push(nested); + } + } + } + + return candidates; +} + /** * Type guard for NodeJS.ErrnoException (any error with a `code` property). */ diff --git a/src/infra/exec-approvals-allow-always.test.ts b/src/infra/exec-approvals-allow-always.test.ts index 640ea8706d6..4a3c53c7614 100644 --- a/src/infra/exec-approvals-allow-always.test.ts +++ b/src/infra/exec-approvals-allow-always.test.ts @@ -18,6 +18,49 @@ describe("resolveAllowAlwaysPatterns", () => { return exe; } + function expectAllowAlwaysBypassBlocked(params: { + dir: string; + firstCommand: string; + secondCommand: string; + env: Record; + persistedPattern: string; + }) { + const safeBins = resolveSafeBins(undefined); + const first = evaluateShellAllowlist({ + command: params.firstCommand, + allowlist: [], + safeBins, + cwd: params.dir, + env: params.env, + platform: process.platform, + }); + const persisted = resolveAllowAlwaysPatterns({ + segments: first.segments, + cwd: params.dir, + env: params.env, + platform: process.platform, + }); + expect(persisted).toEqual([params.persistedPattern]); + + const second = evaluateShellAllowlist({ + command: params.secondCommand, + allowlist: [{ pattern: params.persistedPattern }], + safeBins, + cwd: params.dir, + env: params.env, + platform: process.platform, + }); + expect(second.allowlistSatisfied).toBe(false); + expect( + requiresExecApproval({ + ask: "on-miss", + security: "allowlist", + analysisOk: second.analysisOk, + allowlistSatisfied: second.allowlistSatisfied, + }), + ).toBe(true); + } + it("returns direct executable paths for non-shell segments", () => { const exe = path.join("/tmp", "openclaw-tool"); const patterns = resolveAllowAlwaysPatterns({ @@ -233,42 +276,14 @@ describe("resolveAllowAlwaysPatterns", () => { const busybox = makeExecutable(dir, "busybox"); const echo = makeExecutable(dir, "echo"); makeExecutable(dir, "id"); - const safeBins = resolveSafeBins(undefined); const env = { PATH: `${dir}${path.delimiter}${process.env.PATH ?? ""}` }; - - const first = evaluateShellAllowlist({ - command: `${busybox} sh -c 'echo warmup-ok'`, - allowlist: [], - safeBins, - cwd: dir, + expectAllowAlwaysBypassBlocked({ + dir, + firstCommand: `${busybox} sh -c 'echo warmup-ok'`, + secondCommand: `${busybox} sh -c 'id > marker'`, env, - platform: process.platform, + persistedPattern: echo, }); - const persisted = resolveAllowAlwaysPatterns({ - segments: first.segments, - cwd: dir, - env, - platform: process.platform, - }); - expect(persisted).toEqual([echo]); - - const second = evaluateShellAllowlist({ - command: `${busybox} sh -c 'id > marker'`, - allowlist: [{ pattern: echo }], - safeBins, - cwd: dir, - env, - platform: process.platform, - }); - expect(second.allowlistSatisfied).toBe(false); - expect( - requiresExecApproval({ - ask: "on-miss", - security: "allowlist", - analysisOk: second.analysisOk, - allowlistSatisfied: second.allowlistSatisfied, - }), - ).toBe(true); }); it("prevents allow-always bypass for dispatch-wrapper + shell-wrapper chains", () => { @@ -278,41 +293,13 @@ describe("resolveAllowAlwaysPatterns", () => { const dir = makeTempDir(); const echo = makeExecutable(dir, "echo"); makeExecutable(dir, "id"); - const safeBins = resolveSafeBins(undefined); const env = makePathEnv(dir); - - const first = evaluateShellAllowlist({ - command: "/usr/bin/nice /bin/zsh -lc 'echo warmup-ok'", - allowlist: [], - safeBins, - cwd: dir, + expectAllowAlwaysBypassBlocked({ + dir, + firstCommand: "/usr/bin/nice /bin/zsh -lc 'echo warmup-ok'", + secondCommand: "/usr/bin/nice /bin/zsh -lc 'id > marker'", env, - platform: process.platform, + persistedPattern: echo, }); - const persisted = resolveAllowAlwaysPatterns({ - segments: first.segments, - cwd: dir, - env, - platform: process.platform, - }); - expect(persisted).toEqual([echo]); - - const second = evaluateShellAllowlist({ - command: "/usr/bin/nice /bin/zsh -lc 'id > marker'", - allowlist: [{ pattern: echo }], - safeBins, - cwd: dir, - env, - platform: process.platform, - }); - expect(second.allowlistSatisfied).toBe(false); - expect( - requiresExecApproval({ - ask: "on-miss", - security: "allowlist", - analysisOk: second.analysisOk, - allowlistSatisfied: second.allowlistSatisfied, - }), - ).toBe(true); }); }); diff --git a/src/infra/exec-approvals-analysis.ts b/src/infra/exec-approvals-analysis.ts index e28e0e5c673..d67256e891c 100644 --- a/src/infra/exec-approvals-analysis.ts +++ b/src/infra/exec-approvals-analysis.ts @@ -616,16 +616,26 @@ export function buildSafeShellCommand(params: { command: string; platform?: stri return { ok: true, rendered: argv.map((token) => shellEscapeSingleArg(token)).join(" ") }; }, }); - if (!rebuilt.ok) { - return { ok: false, reason: rebuilt.reason }; - } - return { ok: true, command: rebuilt.command }; + return finalizeRebuiltShellCommand(rebuilt); } function renderQuotedArgv(argv: string[]): string { return argv.map((token) => shellEscapeSingleArg(token)).join(" "); } +function finalizeRebuiltShellCommand( + rebuilt: ReturnType, + expectedSegmentCount?: number, +): { ok: boolean; command?: string; reason?: string } { + if (!rebuilt.ok) { + return { ok: false, reason: rebuilt.reason }; + } + if (typeof expectedSegmentCount === "number" && rebuilt.segmentCount !== expectedSegmentCount) { + return { ok: false, reason: "segment count mismatch" }; + } + return { ok: true, command: rebuilt.command }; +} + export function resolvePlannedSegmentArgv(segment: ExecCommandSegment): string[] | null { if (segment.resolution?.policyBlocked === true) { return null; @@ -688,13 +698,7 @@ export function buildSafeBinsShellCommand(params: { return { ok: true, rendered }; }, }); - if (!rebuilt.ok) { - return { ok: false, reason: rebuilt.reason }; - } - if (rebuilt.segmentCount !== params.segments.length) { - return { ok: false, reason: "segment count mismatch" }; - } - return { ok: true, command: rebuilt.command }; + return finalizeRebuiltShellCommand(rebuilt, params.segments.length); } export function buildEnforcedShellCommand(params: { @@ -717,13 +721,7 @@ export function buildEnforcedShellCommand(params: { return { ok: true, rendered: renderQuotedArgv(argv) }; }, }); - if (!rebuilt.ok) { - return { ok: false, reason: rebuilt.reason }; - } - if (rebuilt.segmentCount !== params.segments.length) { - return { ok: false, reason: "segment count mismatch" }; - } - return { ok: true, command: rebuilt.command }; + return finalizeRebuiltShellCommand(rebuilt, params.segments.length); } /** diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index bd61cc8eb5f..1a7a2a7935b 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -625,6 +625,36 @@ describe("exec approvals shell allowlist (chained commands)", () => { }); describe("exec approvals allowlist evaluation", () => { + function evaluateAutoAllowSkills(params: { + analysis: { + ok: boolean; + segments: Array<{ + raw: string; + argv: string[]; + resolution: { + rawExecutable: string; + executableName: string; + resolvedPath?: string; + }; + }>; + }; + resolvedPath: string; + }) { + return evaluateExecAllowlist({ + analysis: params.analysis, + allowlist: [], + safeBins: new Set(), + skillBins: [{ name: "skill-bin", resolvedPath: params.resolvedPath }], + autoAllowSkills: true, + cwd: "/tmp", + }); + } + + function expectAutoAllowSkillsMiss(result: ReturnType): void { + expect(result.allowlistSatisfied).toBe(false); + expect(result.segmentSatisfiedBy).toEqual([null]); + } + it("satisfies allowlist on exact match", () => { const analysis = { ok: true, @@ -696,13 +726,9 @@ describe("exec approvals allowlist evaluation", () => { }, ], }; - const result = evaluateExecAllowlist({ + const result = evaluateAutoAllowSkills({ analysis, - allowlist: [], - safeBins: new Set(), - skillBins: [{ name: "skill-bin", resolvedPath: "/opt/skills/skill-bin" }], - autoAllowSkills: true, - cwd: "/tmp", + resolvedPath: "/opt/skills/skill-bin", }); expect(result.allowlistSatisfied).toBe(true); }); @@ -722,16 +748,11 @@ describe("exec approvals allowlist evaluation", () => { }, ], }; - const result = evaluateExecAllowlist({ + const result = evaluateAutoAllowSkills({ analysis, - allowlist: [], - safeBins: new Set(), - skillBins: [{ name: "skill-bin", resolvedPath: "/tmp/skill-bin" }], - autoAllowSkills: true, - cwd: "/tmp", + resolvedPath: "/tmp/skill-bin", }); - expect(result.allowlistSatisfied).toBe(false); - expect(result.segmentSatisfiedBy).toEqual([null]); + expectAutoAllowSkillsMiss(result); }); it("does not satisfy auto-allow skills when command resolution is missing", () => { @@ -748,16 +769,11 @@ describe("exec approvals allowlist evaluation", () => { }, ], }; - const result = evaluateExecAllowlist({ + const result = evaluateAutoAllowSkills({ analysis, - allowlist: [], - safeBins: new Set(), - skillBins: [{ name: "skill-bin", resolvedPath: "/opt/skills/skill-bin" }], - autoAllowSkills: true, - cwd: "/tmp", + resolvedPath: "/opt/skills/skill-bin", }); - expect(result.allowlistSatisfied).toBe(false); - expect(result.segmentSatisfiedBy).toEqual([null]); + expectAutoAllowSkillsMiss(result); }); it("returns empty segment details for chain misses", () => { diff --git a/src/infra/exec-wrapper-resolution.ts b/src/infra/exec-wrapper-resolution.ts index 1f91c3b4a1f..95489abe84a 100644 --- a/src/infra/exec-wrapper-resolution.ts +++ b/src/infra/exec-wrapper-resolution.ts @@ -1,4 +1,9 @@ import path from "node:path"; +import { + POSIX_INLINE_COMMAND_FLAGS, + POWERSHELL_INLINE_COMMAND_FLAGS, + resolveInlineCommandMatch, +} from "./shell-inline-command.js"; export const MAX_DISPATCH_WRAPPER_DEPTH = 4; @@ -51,9 +56,6 @@ const SHELL_WRAPPER_CANONICAL = new Set([ ...POWERSHELL_WRAPPER_NAMES, ]); -const POSIX_INLINE_COMMAND_FLAGS = new Set(["-lc", "-c", "--command"]); -const POWERSHELL_INLINE_COMMAND_FLAGS = new Set(["-c", "-command", "--command"]); - const ENV_OPTIONS_WITH_VALUE = new Set([ "-u", "--unset", @@ -586,30 +588,7 @@ function extractInlineCommandByFlags( flags: ReadonlySet, options: { allowCombinedC?: boolean } = {}, ): string | null { - for (let i = 1; i < argv.length; i += 1) { - const token = argv[i]?.trim(); - if (!token) { - continue; - } - const lower = token.toLowerCase(); - if (lower === "--") { - break; - } - if (flags.has(lower)) { - const cmd = argv[i + 1]?.trim(); - return cmd ? cmd : null; - } - if (options.allowCombinedC && /^-[^-]*c[^-]*$/i.test(token)) { - const commandIndex = lower.indexOf("c"); - const inline = token.slice(commandIndex + 1).trim(); - if (inline) { - return inline; - } - const cmd = argv[i + 1]?.trim(); - return cmd ? cmd : null; - } - } - return null; + return resolveInlineCommandMatch(argv, flags, options).command; } function extractShellWrapperPayload(argv: string[], spec: ShellWrapperSpec): string | null { diff --git a/src/infra/install-from-npm-spec.ts b/src/infra/install-from-npm-spec.ts new file mode 100644 index 00000000000..76877fa0525 --- /dev/null +++ b/src/infra/install-from-npm-spec.ts @@ -0,0 +1,38 @@ +import type { NpmIntegrityDriftPayload } from "./npm-integrity.js"; +import { + finalizeNpmSpecArchiveInstall, + installFromNpmSpecArchiveWithInstaller, + type NpmSpecArchiveFinalInstallResult, +} from "./npm-pack-install.js"; +import { validateRegistryNpmSpec } from "./npm-registry-spec.js"; + +export async function installFromValidatedNpmSpecArchive< + TResult extends { ok: boolean }, + TArchiveInstallParams extends { archivePath: string }, +>(params: { + spec: string; + timeoutMs: number; + tempDirPrefix: string; + expectedIntegrity?: string; + onIntegrityDrift?: (payload: NpmIntegrityDriftPayload) => boolean | Promise; + warn?: (message: string) => void; + installFromArchive: (params: TArchiveInstallParams) => Promise; + archiveInstallParams: Omit; +}): Promise> { + const spec = params.spec.trim(); + const specError = validateRegistryNpmSpec(spec); + if (specError) { + return { ok: false, error: specError }; + } + const flowResult = await installFromNpmSpecArchiveWithInstaller({ + tempDirPrefix: params.tempDirPrefix, + spec, + timeoutMs: params.timeoutMs, + expectedIntegrity: params.expectedIntegrity, + onIntegrityDrift: params.onIntegrityDrift, + warn: params.warn, + installFromArchive: params.installFromArchive, + archiveInstallParams: params.archiveInstallParams, + }); + return finalizeNpmSpecArchiveInstall(flowResult); +} diff --git a/src/infra/install-package-dir.ts b/src/infra/install-package-dir.ts index 22b2293f465..8cf6388f6ca 100644 --- a/src/infra/install-package-dir.ts +++ b/src/infra/install-package-dir.ts @@ -147,3 +147,20 @@ export async function installPackageDir(params: { return { ok: true }; } + +export async function installPackageDirWithManifestDeps(params: { + sourceDir: string; + targetDir: string; + mode: "install" | "update"; + timeoutMs: number; + logger?: { info?: (message: string) => void }; + copyErrorPrefix: string; + depsLogMessage: string; + manifestDependencies?: Record; + afterCopy?: () => void | Promise; +}): Promise<{ ok: true } | { ok: false; error: string }> { + return installPackageDir({ + ...params, + hasDeps: Object.keys(params.manifestDependencies ?? {}).length > 0, + }); +} diff --git a/src/infra/install-source-utils.test.ts b/src/infra/install-source-utils.test.ts index 64cb804210f..bbcc17cb968 100644 --- a/src/infra/install-source-utils.test.ts +++ b/src/infra/install-source-utils.test.ts @@ -56,6 +56,31 @@ async function runPack(spec: string, cwd: string, timeoutMs = 1000) { }); } +async function expectPackFallsBackToDetectedArchive(params: { stdout: string }) { + const cwd = await createTempDir("openclaw-install-source-utils-"); + const archivePath = path.join(cwd, "openclaw-plugin-1.2.3.tgz"); + await fs.writeFile(archivePath, "", "utf-8"); + runCommandWithTimeoutMock.mockResolvedValue({ + stdout: params.stdout, + stderr: "", + code: 0, + signal: null, + killed: false, + }); + + const result = await packNpmSpecToArchive({ + spec: "openclaw-plugin@1.2.3", + timeoutMs: 5000, + cwd, + }); + + expect(result).toEqual({ + ok: true, + archivePath, + metadata: {}, + }); +} + beforeEach(() => { runCommandWithTimeoutMock.mockClear(); }); @@ -195,53 +220,11 @@ describe("packNpmSpecToArchive", () => { }); it("falls back to archive detected in cwd when npm pack stdout is empty", async () => { - const cwd = await createTempDir("openclaw-install-source-utils-"); - const archivePath = path.join(cwd, "openclaw-plugin-1.2.3.tgz"); - await fs.writeFile(archivePath, "", "utf-8"); - runCommandWithTimeoutMock.mockResolvedValue({ - stdout: " \n\n", - stderr: "", - code: 0, - signal: null, - killed: false, - }); - - const result = await packNpmSpecToArchive({ - spec: "openclaw-plugin@1.2.3", - timeoutMs: 5000, - cwd, - }); - - expect(result).toEqual({ - ok: true, - archivePath, - metadata: {}, - }); + await expectPackFallsBackToDetectedArchive({ stdout: " \n\n" }); }); it("falls back to archive detected in cwd when stdout does not contain a tgz", async () => { - const cwd = await createTempDir("openclaw-install-source-utils-"); - const archivePath = path.join(cwd, "openclaw-plugin-1.2.3.tgz"); - await fs.writeFile(archivePath, "", "utf-8"); - runCommandWithTimeoutMock.mockResolvedValue({ - stdout: "npm pack completed successfully\n", - stderr: "", - code: 0, - signal: null, - killed: false, - }); - - const result = await packNpmSpecToArchive({ - spec: "openclaw-plugin@1.2.3", - timeoutMs: 5000, - cwd, - }); - - expect(result).toEqual({ - ok: true, - archivePath, - metadata: {}, - }); + await expectPackFallsBackToDetectedArchive({ stdout: "npm pack completed successfully\n" }); }); it("returns friendly error for 404 (package not on npm)", async () => { diff --git a/src/infra/install-source-utils.ts b/src/infra/install-source-utils.ts index fce33b61979..9fba1924a15 100644 --- a/src/infra/install-source-utils.ts +++ b/src/infra/install-source-utils.ts @@ -14,6 +14,26 @@ export type NpmSpecResolution = { resolvedAt?: string; }; +export type NpmResolutionFields = { + resolvedName?: string; + resolvedVersion?: string; + resolvedSpec?: string; + integrity?: string; + shasum?: string; + resolvedAt?: string; +}; + +export function buildNpmResolutionFields(resolution?: NpmSpecResolution): NpmResolutionFields { + return { + resolvedName: resolution?.name, + resolvedVersion: resolution?.version, + resolvedSpec: resolution?.resolvedSpec, + integrity: resolution?.integrity, + shasum: resolution?.shasum, + resolvedAt: resolution?.resolvedAt, + }; +} + export type NpmIntegrityDrift = { expectedIntegrity: string; actualIntegrity: string; diff --git a/src/infra/outbound/message.channels.test.ts b/src/infra/outbound/message.channels.test.ts index 12b9b120f66..af10cb9faf3 100644 --- a/src/infra/outbound/message.channels.test.ts +++ b/src/infra/outbound/message.channels.test.ts @@ -155,20 +155,24 @@ describe("sendPoll channel normalization", () => { }); }); +const setMattermostGatewayRegistry = () => { + setRegistry( + createTestRegistry([ + { + pluginId: "mattermost", + source: "test", + plugin: { + ...createMattermostLikePlugin({ onSendText: () => {} }), + outbound: { deliveryMode: "gateway" }, + }, + }, + ]), + ); +}; + describe("gateway url override hardening", () => { it("drops gateway url overrides in backend mode (SSRF hardening)", async () => { - setRegistry( - createTestRegistry([ - { - pluginId: "mattermost", - source: "test", - plugin: { - ...createMattermostLikePlugin({ onSendText: () => {} }), - outbound: { deliveryMode: "gateway" }, - }, - }, - ]), - ); + setMattermostGatewayRegistry(); callGatewayMock.mockResolvedValueOnce({ messageId: "m1" }); await sendMessage({ @@ -196,18 +200,7 @@ describe("gateway url override hardening", () => { }); it("forwards explicit agentId in gateway send params", async () => { - setRegistry( - createTestRegistry([ - { - pluginId: "mattermost", - source: "test", - plugin: { - ...createMattermostLikePlugin({ onSendText: () => {} }), - outbound: { deliveryMode: "gateway" }, - }, - }, - ]), - ); + setMattermostGatewayRegistry(); callGatewayMock.mockResolvedValueOnce({ messageId: "m-agent" }); await sendMessage({ diff --git a/src/infra/outbound/targets.test.ts b/src/infra/outbound/targets.test.ts index cbad502cdde..2e2f0cbfa00 100644 --- a/src/infra/outbound/targets.test.ts +++ b/src/infra/outbound/targets.test.ts @@ -301,43 +301,44 @@ describe("resolveSessionDeliveryTarget", () => { expect(resolved.to).toBe("63448508"); }); - it("allows heartbeat delivery to Slack DMs and avoids inherited threadId by default", () => { - const cfg: OpenClawConfig = {}; - const resolved = resolveHeartbeatDeliveryTarget({ - cfg, - entry: { - sessionId: "sess-heartbeat-outbound", - updatedAt: 1, - lastChannel: "slack", - lastTo: "user:U123", - lastThreadId: "1739142736.000100", - }, + const resolveHeartbeatTarget = ( + entry: Parameters[0]["entry"], + directPolicy?: "allow" | "block", + ) => + resolveHeartbeatDeliveryTarget({ + cfg: {}, + entry, heartbeat: { target: "last", + ...(directPolicy ? { directPolicy } : {}), }, }); + it("allows heartbeat delivery to Slack DMs and avoids inherited threadId by default", () => { + const resolved = resolveHeartbeatTarget({ + sessionId: "sess-heartbeat-outbound", + updatedAt: 1, + lastChannel: "slack", + lastTo: "user:U123", + lastThreadId: "1739142736.000100", + }); + expect(resolved.channel).toBe("slack"); expect(resolved.to).toBe("user:U123"); expect(resolved.threadId).toBeUndefined(); }); it("blocks heartbeat delivery to Slack DMs when directPolicy is block", () => { - const cfg: OpenClawConfig = {}; - const resolved = resolveHeartbeatDeliveryTarget({ - cfg, - entry: { + const resolved = resolveHeartbeatTarget( + { sessionId: "sess-heartbeat-outbound", updatedAt: 1, lastChannel: "slack", lastTo: "user:U123", lastThreadId: "1739142736.000100", }, - heartbeat: { - target: "last", - directPolicy: "block", - }, - }); + "block", + ); expect(resolved.channel).toBe("none"); expect(resolved.reason).toBe("dm-blocked"); @@ -460,19 +461,12 @@ describe("resolveSessionDeliveryTarget", () => { }); it("uses session chatType hint when target parser cannot classify and allows direct by default", () => { - const cfg: OpenClawConfig = {}; - const resolved = resolveHeartbeatDeliveryTarget({ - cfg, - entry: { - sessionId: "sess-heartbeat-imessage-direct", - updatedAt: 1, - lastChannel: "imessage", - lastTo: "chat-guid-unknown-shape", - chatType: "direct", - }, - heartbeat: { - target: "last", - }, + const resolved = resolveHeartbeatTarget({ + sessionId: "sess-heartbeat-imessage-direct", + updatedAt: 1, + lastChannel: "imessage", + lastTo: "chat-guid-unknown-shape", + chatType: "direct", }); expect(resolved.channel).toBe("imessage"); @@ -480,21 +474,16 @@ describe("resolveSessionDeliveryTarget", () => { }); it("blocks session chatType direct hints when directPolicy is block", () => { - const cfg: OpenClawConfig = {}; - const resolved = resolveHeartbeatDeliveryTarget({ - cfg, - entry: { + const resolved = resolveHeartbeatTarget( + { sessionId: "sess-heartbeat-imessage-direct", updatedAt: 1, lastChannel: "imessage", lastTo: "chat-guid-unknown-shape", chatType: "direct", }, - heartbeat: { - target: "last", - directPolicy: "block", - }, - }); + "block", + ); expect(resolved.channel).toBe("none"); expect(resolved.reason).toBe("dm-blocked"); diff --git a/src/infra/package-tag.ts b/src/infra/package-tag.ts new file mode 100644 index 00000000000..105afeb769c --- /dev/null +++ b/src/infra/package-tag.ts @@ -0,0 +1,18 @@ +export function normalizePackageTagInput( + value: string | undefined | null, + packageNames: readonly string[], +): string | null { + const trimmed = value?.trim(); + if (!trimmed) { + return null; + } + + for (const packageName of packageNames) { + const prefix = `${packageName}@`; + if (trimmed.startsWith(prefix)) { + return trimmed.slice(prefix.length); + } + } + + return trimmed; +} diff --git a/src/infra/shell-inline-command.ts b/src/infra/shell-inline-command.ts new file mode 100644 index 00000000000..2d6f8ae772e --- /dev/null +++ b/src/infra/shell-inline-command.ts @@ -0,0 +1,35 @@ +export const POSIX_INLINE_COMMAND_FLAGS = new Set(["-lc", "-c", "--command"]); +export const POWERSHELL_INLINE_COMMAND_FLAGS = new Set(["-c", "-command", "--command"]); + +export function resolveInlineCommandMatch( + argv: string[], + flags: ReadonlySet, + options: { allowCombinedC?: boolean } = {}, +): { command: string | null; valueTokenIndex: number | null } { + for (let i = 1; i < argv.length; i += 1) { + const token = argv[i]?.trim(); + if (!token) { + continue; + } + const lower = token.toLowerCase(); + if (lower === "--") { + break; + } + if (flags.has(lower)) { + const valueTokenIndex = i + 1 < argv.length ? i + 1 : null; + const command = argv[i + 1]?.trim(); + return { command: command ? command : null, valueTokenIndex }; + } + if (options.allowCombinedC && /^-[^-]*c[^-]*$/i.test(token)) { + const commandIndex = lower.indexOf("c"); + const inline = token.slice(commandIndex + 1).trim(); + if (inline) { + return { command: inline, valueTokenIndex: i }; + } + const valueTokenIndex = i + 1 < argv.length ? i + 1 : null; + const command = argv[i + 1]?.trim(); + return { command: command ? command : null, valueTokenIndex }; + } + } + return { command: null, valueTokenIndex: null }; +} diff --git a/src/infra/system-run-command.ts b/src/infra/system-run-command.ts index dc54bf7b561..e23b798f442 100644 --- a/src/infra/system-run-command.ts +++ b/src/infra/system-run-command.ts @@ -5,6 +5,11 @@ import { unwrapDispatchWrappersForResolution, unwrapKnownShellMultiplexerInvocation, } from "./exec-wrapper-resolution.js"; +import { + POSIX_INLINE_COMMAND_FLAGS, + POWERSHELL_INLINE_COMMAND_FLAGS, + resolveInlineCommandMatch, +} from "./shell-inline-command.js"; export type SystemRunCommandValidation = | { @@ -63,41 +68,12 @@ const POSIX_OR_POWERSHELL_INLINE_WRAPPER_NAMES = new Set([ "zsh", ]); -const POSIX_INLINE_COMMAND_FLAGS = new Set(["-lc", "-c", "--command"]); -const POWERSHELL_INLINE_COMMAND_FLAGS = new Set(["-c", "-command", "--command"]); - function unwrapShellWrapperArgv(argv: string[]): string[] { const dispatchUnwrapped = unwrapDispatchWrappersForResolution(argv); const shellMultiplexer = unwrapKnownShellMultiplexerInvocation(dispatchUnwrapped); return shellMultiplexer.kind === "unwrapped" ? shellMultiplexer.argv : dispatchUnwrapped; } -function resolveInlineCommandTokenIndex( - argv: string[], - flags: ReadonlySet, - options: { allowCombinedC?: boolean } = {}, -): number | null { - for (let i = 1; i < argv.length; i += 1) { - const token = argv[i]?.trim(); - if (!token) { - continue; - } - const lower = token.toLowerCase(); - if (lower === "--") { - break; - } - if (flags.has(lower)) { - return i + 1 < argv.length ? i + 1 : null; - } - if (options.allowCombinedC && /^-[^-]*c[^-]*$/i.test(token)) { - const commandIndex = lower.indexOf("c"); - const inline = token.slice(commandIndex + 1).trim(); - return inline ? i : i + 1 < argv.length ? i + 1 : null; - } - } - return null; -} - function hasTrailingPositionalArgvAfterInlineCommand(argv: string[]): boolean { const wrapperArgv = unwrapShellWrapperArgv(argv); const token0 = wrapperArgv[0]?.trim(); @@ -112,10 +88,10 @@ function hasTrailingPositionalArgvAfterInlineCommand(argv: string[]): boolean { const inlineCommandIndex = wrapper === "powershell" || wrapper === "pwsh" - ? resolveInlineCommandTokenIndex(wrapperArgv, POWERSHELL_INLINE_COMMAND_FLAGS) - : resolveInlineCommandTokenIndex(wrapperArgv, POSIX_INLINE_COMMAND_FLAGS, { + ? resolveInlineCommandMatch(wrapperArgv, POWERSHELL_INLINE_COMMAND_FLAGS).valueTokenIndex + : resolveInlineCommandMatch(wrapperArgv, POSIX_INLINE_COMMAND_FLAGS, { allowCombinedC: true, - }); + }).valueTokenIndex; if (inlineCommandIndex === null) { return false; } diff --git a/src/infra/unhandled-rejections.ts b/src/infra/unhandled-rejections.ts index 03bbb003af6..67f60d3f389 100644 --- a/src/infra/unhandled-rejections.ts +++ b/src/infra/unhandled-rejections.ts @@ -1,5 +1,10 @@ import process from "node:process"; -import { extractErrorCode, formatUncaughtError } from "./errors.js"; +import { + collectErrorGraphCandidates, + extractErrorCode, + formatUncaughtError, + readErrorName, +} from "./errors.js"; type UnhandledRejectionHandler = (reason: unknown) => boolean; @@ -62,14 +67,6 @@ function getErrorCause(err: unknown): unknown { return (err as { cause?: unknown }).cause; } -function getErrorName(err: unknown): string { - if (!err || typeof err !== "object") { - return ""; - } - const name = (err as { name?: unknown }).name; - return typeof name === "string" ? name : ""; -} - function extractErrorCodeOrErrno(err: unknown): string | undefined { const code = extractErrorCode(err); if (code) { @@ -96,44 +93,6 @@ function extractErrorCodeWithCause(err: unknown): string | undefined { return extractErrorCode(getErrorCause(err)); } -function collectErrorCandidates(err: unknown): unknown[] { - const queue: unknown[] = [err]; - const seen = new Set(); - const candidates: unknown[] = []; - - while (queue.length > 0) { - const current = queue.shift(); - if (current == null || seen.has(current)) { - continue; - } - seen.add(current); - candidates.push(current); - - if (!current || typeof current !== "object") { - continue; - } - - const maybeNested: Array = [ - (current as { cause?: unknown }).cause, - (current as { reason?: unknown }).reason, - (current as { original?: unknown }).original, - (current as { error?: unknown }).error, - (current as { data?: unknown }).data, - ]; - const errors = (current as { errors?: unknown }).errors; - if (Array.isArray(errors)) { - maybeNested.push(...errors); - } - for (const nested of maybeNested) { - if (nested != null && !seen.has(nested)) { - queue.push(nested); - } - } - } - - return candidates; -} - /** * Checks if an error is an AbortError. * These are typically intentional cancellations (e.g., during shutdown) and shouldn't crash. @@ -172,13 +131,25 @@ export function isTransientNetworkError(err: unknown): boolean { if (!err) { return false; } - for (const candidate of collectErrorCandidates(err)) { + for (const candidate of collectErrorGraphCandidates(err, (current) => { + const nested: Array = [ + current.cause, + current.reason, + current.original, + current.error, + current.data, + ]; + if (Array.isArray(current.errors)) { + nested.push(...current.errors); + } + return nested; + })) { const code = extractErrorCodeOrErrno(candidate); if (code && TRANSIENT_NETWORK_CODES.has(code)) { return true; } - const name = getErrorName(candidate); + const name = readErrorName(candidate); if (name && TRANSIENT_NETWORK_ERROR_NAMES.has(name)) { return true; } diff --git a/src/infra/update-runner.ts b/src/infra/update-runner.ts index 8a9d56158b8..7fde9322f99 100644 --- a/src/infra/update-runner.ts +++ b/src/infra/update-runner.ts @@ -8,6 +8,7 @@ import { } from "./control-ui-assets.js"; import { detectPackageManager as detectPackageManagerImpl } from "./detect-package-manager.js"; import { readPackageName, readPackageVersion } from "./package-json.js"; +import { normalizePackageTagInput } from "./package-tag.js"; import { trimLogTail } from "./restart-sentinel.js"; import { channelToNpmTag, @@ -312,17 +313,7 @@ function managerInstallArgs(manager: "pnpm" | "bun" | "npm") { } function normalizeTag(tag?: string) { - const trimmed = tag?.trim(); - if (!trimmed) { - return "latest"; - } - if (trimmed.startsWith("openclaw@")) { - return trimmed.slice("openclaw@".length); - } - if (trimmed.startsWith(`${DEFAULT_PACKAGE_NAME}@`)) { - return trimmed.slice(`${DEFAULT_PACKAGE_NAME}@`.length); - } - return trimmed; + return normalizePackageTagInput(tag, ["openclaw", DEFAULT_PACKAGE_NAME]) ?? "latest"; } export async function runGatewayUpdate(opts: UpdateRunnerOptions = {}): Promise { diff --git a/src/infra/update-startup.test.ts b/src/infra/update-startup.test.ts index 845b8f0f2e4..1b382dededc 100644 --- a/src/infra/update-startup.test.ts +++ b/src/infra/update-startup.test.ts @@ -147,6 +147,32 @@ describe("update-startup", () => { }); } + function createBetaAutoUpdateConfig(params?: { checkOnStart?: boolean }) { + return { + update: { + ...(params?.checkOnStart === false ? { checkOnStart: false } : {}), + channel: "beta" as const, + auto: { + enabled: true, + betaCheckIntervalHours: 1, + }, + }, + }; + } + + async function runAutoUpdateCheckWithDefaults(params: { + cfg: { update?: Record }; + runAutoUpdate?: ReturnType; + }) { + await runGatewayUpdateCheck({ + cfg: params.cfg, + log: { info: vi.fn() }, + isNixMode: false, + allowInTests: true, + ...(params.runAutoUpdate ? { runAutoUpdate: params.runAutoUpdate } : {}), + }); + } + it.each([ { name: "stable channel", @@ -310,19 +336,8 @@ describe("update-startup", () => { mockPackageUpdateStatus("beta", "2.0.0-beta.1"); const runAutoUpdate = createAutoUpdateSuccessMock(); - await runGatewayUpdateCheck({ - cfg: { - update: { - channel: "beta", - auto: { - enabled: true, - betaCheckIntervalHours: 1, - }, - }, - }, - log: { info: vi.fn() }, - isNixMode: false, - allowInTests: true, + await runAutoUpdateCheckWithDefaults({ + cfg: createBetaAutoUpdateConfig(), runAutoUpdate, }); @@ -338,20 +353,8 @@ describe("update-startup", () => { mockPackageUpdateStatus("beta", "2.0.0-beta.1"); const runAutoUpdate = createAutoUpdateSuccessMock(); - await runGatewayUpdateCheck({ - cfg: { - update: { - checkOnStart: false, - channel: "beta", - auto: { - enabled: true, - betaCheckIntervalHours: 1, - }, - }, - }, - log: { info: vi.fn() }, - isNixMode: false, - allowInTests: true, + await runAutoUpdateCheckWithDefaults({ + cfg: createBetaAutoUpdateConfig({ checkOnStart: false }), runAutoUpdate, }); @@ -381,19 +384,8 @@ describe("update-startup", () => { const originalArgv = process.argv.slice(); process.argv = [process.execPath, "/opt/openclaw/dist/entry.js"]; try { - await runGatewayUpdateCheck({ - cfg: { - update: { - channel: "beta", - auto: { - enabled: true, - betaCheckIntervalHours: 1, - }, - }, - }, - log: { info: vi.fn() }, - isNixMode: false, - allowInTests: true, + await runAutoUpdateCheckWithDefaults({ + cfg: createBetaAutoUpdateConfig(), }); } finally { process.argv = originalArgv; diff --git a/src/plugins/config-state.test.ts b/src/plugins/config-state.test.ts index 01beb51b8d7..ccebd313198 100644 --- a/src/plugins/config-state.test.ts +++ b/src/plugins/config-state.test.ts @@ -50,11 +50,9 @@ describe("normalizePluginsConfig", () => { }); describe("resolveEffectiveEnableState", () => { - it("enables bundled channels when channels..enabled=true", () => { - const normalized = normalizePluginsConfig({ - enabled: true, - }); - const state = resolveEffectiveEnableState({ + function resolveBundledTelegramState(config: Parameters[0]) { + const normalized = normalizePluginsConfig(config); + return resolveEffectiveEnableState({ id: "telegram", origin: "bundled", config: normalized, @@ -66,11 +64,17 @@ describe("resolveEffectiveEnableState", () => { }, }, }); + } + + it("enables bundled channels when channels..enabled=true", () => { + const state = resolveBundledTelegramState({ + enabled: true, + }); expect(state).toEqual({ enabled: true }); }); it("keeps explicit plugin-level disable authoritative", () => { - const normalized = normalizePluginsConfig({ + const state = resolveBundledTelegramState({ enabled: true, entries: { telegram: { @@ -78,18 +82,6 @@ describe("resolveEffectiveEnableState", () => { }, }, }); - const state = resolveEffectiveEnableState({ - id: "telegram", - origin: "bundled", - config: normalized, - rootConfig: { - channels: { - telegram: { - enabled: true, - }, - }, - }, - }); expect(state).toEqual({ enabled: false, reason: "disabled in config" }); }); }); diff --git a/src/plugins/hooks.before-agent-start.test.ts b/src/plugins/hooks.before-agent-start.test.ts index 7a0785823c9..89072c10be7 100644 --- a/src/plugins/hooks.before-agent-start.test.ts +++ b/src/plugins/hooks.before-agent-start.test.ts @@ -7,6 +7,7 @@ */ import { beforeEach, describe, expect, it } from "vitest"; import { createHookRunner } from "./hooks.js"; +import { addTestHook, TEST_PLUGIN_AGENT_CTX } from "./hooks.test-helpers.js"; import { createEmptyPluginRegistry, type PluginRegistry } from "./registry.js"; import type { PluginHookBeforeAgentStartResult, PluginHookRegistration } from "./types.js"; @@ -16,21 +17,16 @@ function addBeforeAgentStartHook( handler: () => PluginHookBeforeAgentStartResult | Promise, priority?: number, ) { - registry.typedHooks.push({ + addTestHook({ + registry, pluginId, hookName: "before_agent_start", - handler, + handler: handler as PluginHookRegistration["handler"], priority, - source: "test", - } as PluginHookRegistration); + }); } -const stubCtx = { - agentId: "test-agent", - sessionKey: "sk", - sessionId: "sid", - workspaceDir: "/tmp", -}; +const stubCtx = TEST_PLUGIN_AGENT_CTX; describe("before_agent_start hook merger", () => { let registry: PluginRegistry; diff --git a/src/plugins/hooks.model-override-wiring.test.ts b/src/plugins/hooks.model-override-wiring.test.ts index feb3b0a8afa..74ca09fe39d 100644 --- a/src/plugins/hooks.model-override-wiring.test.ts +++ b/src/plugins/hooks.model-override-wiring.test.ts @@ -8,10 +8,10 @@ */ import { beforeEach, describe, expect, it, vi } from "vitest"; import { createHookRunner } from "./hooks.js"; +import { addTestHook, TEST_PLUGIN_AGENT_CTX } from "./hooks.test-helpers.js"; import { createEmptyPluginRegistry, type PluginRegistry } from "./registry.js"; import type { PluginHookAgentContext, - PluginHookBeforeAgentStartResult, PluginHookBeforeModelResolveEvent, PluginHookBeforeModelResolveResult, PluginHookBeforePromptBuildEvent, @@ -28,13 +28,13 @@ function addBeforeModelResolveHook( ) => PluginHookBeforeModelResolveResult | Promise, priority?: number, ) { - registry.typedHooks.push({ + addTestHook({ + registry, pluginId, hookName: "before_model_resolve", - handler, + handler: handler as PluginHookRegistration["handler"], priority, - source: "test", - } as PluginHookRegistration); + }); } function addBeforePromptBuildHook( @@ -46,36 +46,16 @@ function addBeforePromptBuildHook( ) => PluginHookBeforePromptBuildResult | Promise, priority?: number, ) { - registry.typedHooks.push({ + addTestHook({ + registry, pluginId, hookName: "before_prompt_build", - handler, + handler: handler as PluginHookRegistration["handler"], priority, - source: "test", - } as PluginHookRegistration); + }); } -function addLegacyBeforeAgentStartHook( - registry: PluginRegistry, - pluginId: string, - handler: () => PluginHookBeforeAgentStartResult | Promise, - priority?: number, -) { - registry.typedHooks.push({ - pluginId, - hookName: "before_agent_start", - handler, - priority, - source: "test", - } as PluginHookRegistration); -} - -const stubCtx: PluginHookAgentContext = { - agentId: "test-agent", - sessionKey: "sk", - sessionId: "sid", - workspaceDir: "/tmp", -}; +const stubCtx: PluginHookAgentContext = TEST_PLUGIN_AGENT_CTX; describe("model override pipeline wiring", () => { let registry: PluginRegistry; @@ -109,10 +89,15 @@ describe("model override pipeline wiring", () => { modelOverride: "llama3.3:8b", providerOverride: "ollama", })); - addLegacyBeforeAgentStartHook(registry, "legacy-hook", () => ({ - modelOverride: "gpt-4o", - providerOverride: "openai", - })); + addTestHook({ + registry, + pluginId: "legacy-hook", + hookName: "before_agent_start", + handler: (() => ({ + modelOverride: "gpt-4o", + providerOverride: "openai", + })) as PluginHookRegistration["handler"], + }); const runner = createHookRunner(registry); const explicit = await runner.runBeforeModelResolve({ prompt: "sensitive" }, stubCtx); @@ -151,9 +136,14 @@ describe("model override pipeline wiring", () => { addBeforePromptBuildHook(registry, "new-hook", () => ({ prependContext: "new context", })); - addLegacyBeforeAgentStartHook(registry, "legacy-hook", () => ({ - prependContext: "legacy context", - })); + addTestHook({ + registry, + pluginId: "legacy-hook", + hookName: "before_agent_start", + handler: (() => ({ + prependContext: "legacy context", + })) as PluginHookRegistration["handler"], + }); const runner = createHookRunner(registry); const promptBuild = await runner.runBeforePromptBuild( @@ -207,7 +197,12 @@ describe("model override pipeline wiring", () => { addBeforeModelResolveHook(registry, "plugin-a", () => ({})); addBeforePromptBuildHook(registry, "plugin-b", () => ({})); - addLegacyBeforeAgentStartHook(registry, "plugin-c", () => ({})); + addTestHook({ + registry, + pluginId: "plugin-c", + hookName: "before_agent_start", + handler: (() => ({})) as PluginHookRegistration["handler"], + }); const runner2 = createHookRunner(registry); expect(runner2.hasHooks("before_model_resolve")).toBe(true); diff --git a/src/plugins/installs.ts b/src/plugins/installs.ts index aa58e529fea..ef19a2b63f2 100644 --- a/src/plugins/installs.ts +++ b/src/plugins/installs.ts @@ -1,6 +1,6 @@ import type { OpenClawConfig } from "../config/config.js"; import type { PluginInstallRecord } from "../config/types.plugins.js"; -import type { NpmSpecResolution } from "../infra/install-source-utils.js"; +import { buildNpmResolutionFields, type NpmSpecResolution } from "../infra/install-source-utils.js"; export type PluginInstallUpdate = PluginInstallRecord & { pluginId: string }; @@ -10,14 +10,7 @@ export function buildNpmResolutionInstallFields( PluginInstallRecord, "resolvedName" | "resolvedVersion" | "resolvedSpec" | "integrity" | "shasum" | "resolvedAt" > { - return { - resolvedName: resolution?.name, - resolvedVersion: resolution?.version, - resolvedSpec: resolution?.resolvedSpec, - integrity: resolution?.integrity, - shasum: resolution?.shasum, - resolvedAt: resolution?.resolvedAt, - }; + return buildNpmResolutionFields(resolution); } export function recordPluginInstall( diff --git a/src/plugins/types.ts b/src/plugins/types.ts index e664327a373..50ad451fd5e 100644 --- a/src/plugins/types.ts +++ b/src/plugins/types.ts @@ -570,8 +570,7 @@ export type PluginHookSubagentContext = { export type PluginHookSubagentTargetKind = "subagent" | "acp"; -// subagent_spawning hook -export type PluginHookSubagentSpawningEvent = { +type PluginHookSubagentSpawnBase = { childSessionKey: string; agentId: string; label?: string; @@ -585,6 +584,9 @@ export type PluginHookSubagentSpawningEvent = { threadRequested: boolean; }; +// subagent_spawning hook +export type PluginHookSubagentSpawningEvent = PluginHookSubagentSpawnBase; + export type PluginHookSubagentSpawningResult = | { status: "ok"; @@ -620,19 +622,8 @@ export type PluginHookSubagentDeliveryTargetResult = { }; // subagent_spawned hook -export type PluginHookSubagentSpawnedEvent = { +export type PluginHookSubagentSpawnedEvent = PluginHookSubagentSpawnBase & { runId: string; - childSessionKey: string; - agentId: string; - label?: string; - mode: "run" | "session"; - requester?: { - channel?: string; - accountId?: string; - to?: string; - threadId?: string | number; - }; - threadRequested: boolean; }; // subagent_ended hook diff --git a/src/process/exec.windows.test.ts b/src/process/exec.windows.test.ts index 10405a735ed..85600755dac 100644 --- a/src/process/exec.windows.test.ts +++ b/src/process/exec.windows.test.ts @@ -41,6 +41,28 @@ function createMockChild(params?: { code?: number; signal?: NodeJS.Signals | nul return child; } +type SpawnCall = [string, string[], Record]; + +type ExecCall = [ + string, + string[], + Record, + (err: Error | null, stdout: string, stderr: string) => void, +]; + +function expectCmdWrappedInvocation(params: { + captured: SpawnCall | ExecCall | undefined; + expectedComSpec: string; +}) { + if (!params.captured) { + throw new Error("expected command wrapper to be called"); + } + expect(params.captured[0]).toBe(params.expectedComSpec); + expect(params.captured[1].slice(0, 3)).toEqual(["/d", "/s", "/c"]); + expect(params.captured[1][3]).toContain("pnpm.cmd --version"); + expect(params.captured[2].windowsVerbatimArguments).toBe(true); +} + describe("windows command wrapper behavior", () => { afterEach(() => { spawnMock.mockReset(); @@ -59,16 +81,8 @@ describe("windows command wrapper behavior", () => { try { const result = await runCommandWithTimeout(["pnpm", "--version"], { timeoutMs: 1000 }); expect(result.code).toBe(0); - const captured = spawnMock.mock.calls[0] as - | [string, string[], Record] - | undefined; - if (!captured) { - throw new Error("spawn mock was not called"); - } - expect(captured[0]).toBe(expectedComSpec); - expect(captured[1].slice(0, 3)).toEqual(["/d", "/s", "/c"]); - expect(captured[1][3]).toContain("pnpm.cmd --version"); - expect(captured[2].windowsVerbatimArguments).toBe(true); + const captured = spawnMock.mock.calls[0] as SpawnCall | undefined; + expectCmdWrappedInvocation({ captured, expectedComSpec }); } finally { platformSpy.mockRestore(); } @@ -91,21 +105,8 @@ describe("windows command wrapper behavior", () => { try { await runExec("pnpm", ["--version"], 1000); - const captured = execFileMock.mock.calls[0] as - | [ - string, - string[], - Record, - (err: Error | null, stdout: string, stderr: string) => void, - ] - | undefined; - if (!captured) { - throw new Error("execFile mock was not called"); - } - expect(captured[0]).toBe(expectedComSpec); - expect(captured[1].slice(0, 3)).toEqual(["/d", "/s", "/c"]); - expect(captured[1][3]).toContain("pnpm.cmd --version"); - expect(captured[2].windowsVerbatimArguments).toBe(true); + const captured = execFileMock.mock.calls[0] as ExecCall | undefined; + expectCmdWrappedInvocation({ captured, expectedComSpec }); } finally { platformSpy.mockRestore(); } diff --git a/src/secrets/configure.ts b/src/secrets/configure.ts index a8e6e9b2f32..cee8d3952b5 100644 --- a/src/secrets/configure.ts +++ b/src/secrets/configure.ts @@ -210,6 +210,31 @@ function assertNoCancel(value: T | symbol, message: string): T { return value; } +function validateEnvNameCsv(value: string): string | undefined { + const entries = parseCsv(value); + for (const entry of entries) { + if (!ENV_NAME_PATTERN.test(entry)) { + return `Invalid env name: ${entry}`; + } + } + return undefined; +} + +async function promptEnvNameCsv(params: { + message: string; + initialValue: string; +}): Promise { + const raw = assertNoCancel( + await text({ + message: params.message, + initialValue: params.initialValue, + validate: (value) => validateEnvNameCsv(String(value ?? "")), + }), + "Secrets configure cancelled.", + ); + return parseCsv(String(raw ?? "")); +} + async function promptOptionalPositiveInt(params: { message: string; initialValue?: number; @@ -275,23 +300,10 @@ async function promptProviderSource(initial?: SecretRefSource): Promise, ): Promise> { - const allowlistRaw = assertNoCancel( - await text({ - message: "Env allowlist (comma-separated, blank for unrestricted)", - initialValue: base?.allowlist?.join(",") ?? "", - validate: (value) => { - const entries = parseCsv(String(value ?? "")); - for (const entry of entries) { - if (!ENV_NAME_PATTERN.test(entry)) { - return `Invalid env name: ${entry}`; - } - } - return undefined; - }, - }), - "Secrets configure cancelled.", - ); - const allowlist = parseCsv(String(allowlistRaw ?? "")); + const allowlist = await promptEnvNameCsv({ + message: "Env allowlist (comma-separated, blank for unrestricted)", + initialValue: base?.allowlist?.join(",") ?? "", + }); return { source: "env", ...(allowlist.length > 0 ? { allowlist } : {}), @@ -436,22 +448,10 @@ async function promptExecProvider( "Secrets configure cancelled.", ); - const passEnvRaw = assertNoCancel( - await text({ - message: "Pass-through env vars (comma-separated, blank for none)", - initialValue: base?.passEnv?.join(",") ?? "", - validate: (value) => { - const entries = parseCsv(String(value ?? "")); - for (const entry of entries) { - if (!ENV_NAME_PATTERN.test(entry)) { - return `Invalid env name: ${entry}`; - } - } - return undefined; - }, - }), - "Secrets configure cancelled.", - ); + const passEnv = await promptEnvNameCsv({ + message: "Pass-through env vars (comma-separated, blank for none)", + initialValue: base?.passEnv?.join(",") ?? "", + }); const trustedDirsRaw = assertNoCancel( await text({ @@ -486,7 +486,6 @@ async function promptExecProvider( ); const args = await parseArgsInput(String(argsRaw ?? "")); - const passEnv = parseCsv(String(passEnvRaw ?? "")); const trustedDirs = parseCsv(String(trustedDirsRaw ?? "")); return { diff --git a/src/secrets/resolve.test.ts b/src/secrets/resolve.test.ts index eb1622c884b..64018ca8f7f 100644 --- a/src/secrets/resolve.test.ts +++ b/src/secrets/resolve.test.ts @@ -27,6 +27,46 @@ describe("secret ref resolver", () => { return dir; }; + type ExecProviderConfig = { + source: "exec"; + command: string; + passEnv?: string[]; + jsonOnly?: boolean; + allowSymlinkCommand?: boolean; + trustedDirs?: string[]; + args?: string[]; + }; + + function createExecProviderConfig( + command: string, + overrides: Partial = {}, + ): ExecProviderConfig { + return { + source: "exec", + command, + passEnv: ["PATH"], + ...overrides, + }; + } + + async function resolveExecSecret( + command: string, + overrides: Partial = {}, + ): Promise { + return resolveSecretRefString( + { source: "exec", provider: "execmain", id: "openai/api-key" }, + { + config: { + secrets: { + providers: { + execmain: createExecProviderConfig(command, overrides), + }, + }, + }, + }, + ); + } + beforeAll(async () => { fixtureRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-secrets-resolve-")); const sharedExecDir = path.join(fixtureRoot, "shared-exec"); @@ -134,22 +174,7 @@ describe("secret ref resolver", () => { return; } - const value = await resolveSecretRefString( - { source: "exec", provider: "execmain", id: "openai/api-key" }, - { - config: { - secrets: { - providers: { - execmain: { - source: "exec", - command: execProtocolV1ScriptPath, - passEnv: ["PATH"], - }, - }, - }, - }, - }, - ); + const value = await resolveExecSecret(execProtocolV1ScriptPath); expect(value).toBe("value:openai/api-key"); }); @@ -158,23 +183,7 @@ describe("secret ref resolver", () => { return; } - const value = await resolveSecretRefString( - { source: "exec", provider: "execmain", id: "openai/api-key" }, - { - config: { - secrets: { - providers: { - execmain: { - source: "exec", - command: execPlainScriptPath, - passEnv: ["PATH"], - jsonOnly: false, - }, - }, - }, - }, - }, - ); + const value = await resolveExecSecret(execPlainScriptPath, { jsonOnly: false }); expect(value).toBe("plain-secret"); }); @@ -210,25 +219,9 @@ describe("secret ref resolver", () => { const symlinkPath = path.join(root, "resolver-link.mjs"); await fs.symlink(execPlainScriptPath, symlinkPath); - await expect( - resolveSecretRefString( - { source: "exec", provider: "execmain", id: "openai/api-key" }, - { - config: { - secrets: { - providers: { - execmain: { - source: "exec", - command: symlinkPath, - passEnv: ["PATH"], - jsonOnly: false, - }, - }, - }, - }, - }, - ), - ).rejects.toThrow("must not be a symlink"); + await expect(resolveExecSecret(symlinkPath, { jsonOnly: false })).rejects.toThrow( + "must not be a symlink", + ); }); it("allows symlink command paths when allowSymlinkCommand is enabled", async () => { @@ -240,25 +233,11 @@ describe("secret ref resolver", () => { await fs.symlink(execPlainScriptPath, symlinkPath); const trustedRoot = await fs.realpath(fixtureRoot); - const value = await resolveSecretRefString( - { source: "exec", provider: "execmain", id: "openai/api-key" }, - { - config: { - secrets: { - providers: { - execmain: { - source: "exec", - command: symlinkPath, - passEnv: ["PATH"], - jsonOnly: false, - allowSymlinkCommand: true, - trustedDirs: [trustedRoot], - }, - }, - }, - }, - }, - ); + const value = await resolveExecSecret(symlinkPath, { + jsonOnly: false, + allowSymlinkCommand: true, + trustedDirs: [trustedRoot], + }); expect(value).toBe("plain-secret"); }); @@ -287,44 +266,15 @@ describe("secret ref resolver", () => { await fs.symlink(targetCommand, symlinkCommand); const trustedRoot = await fs.realpath(root); - await expect( - resolveSecretRefString( - { source: "exec", provider: "execmain", id: "openai/api-key" }, - { - config: { - secrets: { - providers: { - execmain: { - source: "exec", - command: symlinkCommand, - args: ["brew"], - passEnv: ["PATH"], - }, - }, - }, - }, - }, - ), - ).rejects.toThrow("must not be a symlink"); - - const value = await resolveSecretRefString( - { source: "exec", provider: "execmain", id: "openai/api-key" }, - { - config: { - secrets: { - providers: { - execmain: { - source: "exec", - command: symlinkCommand, - args: ["brew"], - allowSymlinkCommand: true, - trustedDirs: [trustedRoot], - }, - }, - }, - }, - }, + await expect(resolveExecSecret(symlinkCommand, { args: ["brew"] })).rejects.toThrow( + "must not be a symlink", ); + + const value = await resolveExecSecret(symlinkCommand, { + args: ["brew"], + allowSymlinkCommand: true, + trustedDirs: [trustedRoot], + }); expect(value).toBe("brew:openai/api-key"); }); @@ -337,25 +287,11 @@ describe("secret ref resolver", () => { await fs.symlink(execPlainScriptPath, symlinkPath); await expect( - resolveSecretRefString( - { source: "exec", provider: "execmain", id: "openai/api-key" }, - { - config: { - secrets: { - providers: { - execmain: { - source: "exec", - command: symlinkPath, - passEnv: ["PATH"], - jsonOnly: false, - allowSymlinkCommand: true, - trustedDirs: [root], - }, - }, - }, - }, - }, - ), + resolveExecSecret(symlinkPath, { + jsonOnly: false, + allowSymlinkCommand: true, + trustedDirs: [root], + }), ).rejects.toThrow("outside trustedDirs"); }); @@ -363,73 +299,27 @@ describe("secret ref resolver", () => { if (process.platform === "win32") { return; } - await expect( - resolveSecretRefString( - { source: "exec", provider: "execmain", id: "openai/api-key" }, - { - config: { - secrets: { - providers: { - execmain: { - source: "exec", - command: execProtocolV2ScriptPath, - passEnv: ["PATH"], - }, - }, - }, - }, - }, - ), - ).rejects.toThrow("protocolVersion must be 1"); + await expect(resolveExecSecret(execProtocolV2ScriptPath)).rejects.toThrow( + "protocolVersion must be 1", + ); }); it("rejects exec refs when response omits requested id", async () => { if (process.platform === "win32") { return; } - await expect( - resolveSecretRefString( - { source: "exec", provider: "execmain", id: "openai/api-key" }, - { - config: { - secrets: { - providers: { - execmain: { - source: "exec", - command: execMissingIdScriptPath, - passEnv: ["PATH"], - }, - }, - }, - }, - }, - ), - ).rejects.toThrow('response missing id "openai/api-key"'); + await expect(resolveExecSecret(execMissingIdScriptPath)).rejects.toThrow( + 'response missing id "openai/api-key"', + ); }); it("rejects exec refs with invalid JSON when jsonOnly is true", async () => { if (process.platform === "win32") { return; } - await expect( - resolveSecretRefString( - { source: "exec", provider: "execmain", id: "openai/api-key" }, - { - config: { - secrets: { - providers: { - execmain: { - source: "exec", - command: execInvalidJsonScriptPath, - passEnv: ["PATH"], - jsonOnly: true, - }, - }, - }, - }, - }, - ), - ).rejects.toThrow("returned invalid JSON"); + await expect(resolveExecSecret(execInvalidJsonScriptPath, { jsonOnly: true })).rejects.toThrow( + "returned invalid JSON", + ); }); it("supports file singleValue mode with id=value", async () => { diff --git a/src/shared/pid-alive.test.ts b/src/shared/pid-alive.test.ts index 1edafa77cab..c0d714fb21a 100644 --- a/src/shared/pid-alive.test.ts +++ b/src/shared/pid-alive.test.ts @@ -2,6 +2,35 @@ import fsSync from "node:fs"; import { describe, expect, it, vi } from "vitest"; import { getProcessStartTime, isPidAlive } from "./pid-alive.js"; +function mockProcReads(entries: Record) { + const originalReadFileSync = fsSync.readFileSync; + vi.spyOn(fsSync, "readFileSync").mockImplementation((filePath, encoding) => { + const key = String(filePath); + if (Object.hasOwn(entries, key)) { + return entries[key] as never; + } + return originalReadFileSync(filePath as never, encoding as never) as never; + }); +} + +async function withLinuxProcessPlatform(run: () => Promise): Promise { + const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); + if (!originalPlatformDescriptor) { + throw new Error("missing process.platform descriptor"); + } + Object.defineProperty(process, "platform", { + ...originalPlatformDescriptor, + value: "linux", + }); + try { + vi.resetModules(); + return await run(); + } finally { + Object.defineProperty(process, "platform", originalPlatformDescriptor); + vi.restoreAllMocks(); + } +} + describe("isPidAlive", () => { it("returns true for the current running process", () => { expect(isPidAlive(process.pid)).toBe(true); @@ -22,68 +51,29 @@ describe("isPidAlive", () => { it("returns false for zombie processes on Linux", async () => { const zombiePid = process.pid; - // Mock readFileSync to return zombie state for /proc//status - const originalReadFileSync = fsSync.readFileSync; - vi.spyOn(fsSync, "readFileSync").mockImplementation((filePath, encoding) => { - if (filePath === `/proc/${zombiePid}/status`) { - return `Name:\tnode\nUmask:\t0022\nState:\tZ (zombie)\nTgid:\t${zombiePid}\nPid:\t${zombiePid}\n`; - } - return originalReadFileSync(filePath as never, encoding as never) as never; + mockProcReads({ + [`/proc/${zombiePid}/status`]: `Name:\tnode\nUmask:\t0022\nState:\tZ (zombie)\nTgid:\t${zombiePid}\nPid:\t${zombiePid}\n`, }); - - // Override platform to linux so the zombie check runs - const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); - if (!originalPlatformDescriptor) { - throw new Error("missing process.platform descriptor"); - } - Object.defineProperty(process, "platform", { - ...originalPlatformDescriptor, - value: "linux", - }); - - try { - // Re-import the module so it picks up the mocked platform and fs - vi.resetModules(); + await withLinuxProcessPlatform(async () => { const { isPidAlive: freshIsPidAlive } = await import("./pid-alive.js"); expect(freshIsPidAlive(zombiePid)).toBe(false); - } finally { - Object.defineProperty(process, "platform", originalPlatformDescriptor); - vi.restoreAllMocks(); - } + }); }); }); describe("getProcessStartTime", () => { it("returns a number on Linux for the current process", async () => { - const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); - if (!originalPlatformDescriptor) { - throw new Error("missing process.platform descriptor"); - } - - const originalReadFileSync = fsSync.readFileSync; // Simulate a realistic /proc//stat line const fakeStat = `${process.pid} (node) S 1 ${process.pid} ${process.pid} 0 -1 4194304 12345 0 0 0 100 50 0 0 20 0 8 0 98765 123456789 5000 18446744073709551615 0 0 0 0 0 0 0 0 0 0 0 0 17 0 0 0 0 0 0`; - vi.spyOn(fsSync, "readFileSync").mockImplementation((filePath, encoding) => { - if (filePath === `/proc/${process.pid}/stat`) { - return fakeStat; - } - return originalReadFileSync(filePath as never, encoding as never) as never; + mockProcReads({ + [`/proc/${process.pid}/stat`]: fakeStat, }); - Object.defineProperty(process, "platform", { - ...originalPlatformDescriptor, - value: "linux", - }); - - try { - vi.resetModules(); + await withLinuxProcessPlatform(async () => { const { getProcessStartTime: fresh } = await import("./pid-alive.js"); const starttime = fresh(process.pid); expect(starttime).toBe(98765); - } finally { - Object.defineProperty(process, "platform", originalPlatformDescriptor); - vi.restoreAllMocks(); - } + }); }); it("returns null on non-Linux platforms", () => { @@ -104,62 +94,24 @@ describe("getProcessStartTime", () => { }); it("returns null for malformed /proc stat content", async () => { - const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); - if (!originalPlatformDescriptor) { - throw new Error("missing process.platform descriptor"); - } - - const originalReadFileSync = fsSync.readFileSync; - vi.spyOn(fsSync, "readFileSync").mockImplementation((filePath, encoding) => { - if (filePath === "/proc/42/stat") { - return "42 node S malformed"; - } - return originalReadFileSync(filePath as never, encoding as never) as never; + mockProcReads({ + "/proc/42/stat": "42 node S malformed", }); - - Object.defineProperty(process, "platform", { - ...originalPlatformDescriptor, - value: "linux", - }); - - try { - vi.resetModules(); + await withLinuxProcessPlatform(async () => { const { getProcessStartTime: fresh } = await import("./pid-alive.js"); expect(fresh(42)).toBeNull(); - } finally { - Object.defineProperty(process, "platform", originalPlatformDescriptor); - vi.restoreAllMocks(); - } + }); }); it("handles comm fields containing spaces and parentheses", async () => { - const originalPlatformDescriptor = Object.getOwnPropertyDescriptor(process, "platform"); - if (!originalPlatformDescriptor) { - throw new Error("missing process.platform descriptor"); - } - - const originalReadFileSync = fsSync.readFileSync; // comm field with spaces and nested parens: "(My App (v2))" const fakeStat = `42 (My App (v2)) S 1 42 42 0 -1 4194304 0 0 0 0 0 0 0 0 20 0 1 0 55555 0 0 0 0 0 0 0 0 0 0 0 0 0 17 0 0 0 0 0 0`; - vi.spyOn(fsSync, "readFileSync").mockImplementation((filePath, encoding) => { - if (filePath === "/proc/42/stat") { - return fakeStat; - } - return originalReadFileSync(filePath as never, encoding as never) as never; + mockProcReads({ + "/proc/42/stat": fakeStat, }); - - Object.defineProperty(process, "platform", { - ...originalPlatformDescriptor, - value: "linux", - }); - - try { - vi.resetModules(); + await withLinuxProcessPlatform(async () => { const { getProcessStartTime: fresh } = await import("./pid-alive.js"); expect(fresh(42)).toBe(55555); - } finally { - Object.defineProperty(process, "platform", originalPlatformDescriptor); - vi.restoreAllMocks(); - } + }); }); });