From fa0329c340761b7315ac025121b2308aa75734ff Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 11 Mar 2026 00:01:45 +0000 Subject: [PATCH] test: cover cron nested lane selection --- src/agents/lanes.test.ts | 18 ++++ src/agents/lanes.ts | 10 +++ src/cron/isolated-agent.lane.test.ts | 84 +++++++++++++++++++ .../isolated-agent.model-formatting.test.ts | 14 +--- src/cron/isolated-agent/run.ts | 15 +--- 5 files changed, 117 insertions(+), 24 deletions(-) create mode 100644 src/agents/lanes.test.ts create mode 100644 src/cron/isolated-agent.lane.test.ts diff --git a/src/agents/lanes.test.ts b/src/agents/lanes.test.ts new file mode 100644 index 00000000000..9538de70d26 --- /dev/null +++ b/src/agents/lanes.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from "vitest"; +import { AGENT_LANE_NESTED, resolveNestedAgentLane } from "./lanes.js"; + +describe("resolveNestedAgentLane", () => { + it("defaults to the nested lane when no lane is provided", () => { + expect(resolveNestedAgentLane()).toBe(AGENT_LANE_NESTED); + }); + + it("moves cron lane callers onto the nested lane", () => { + expect(resolveNestedAgentLane("cron")).toBe(AGENT_LANE_NESTED); + expect(resolveNestedAgentLane(" cron ")).toBe(AGENT_LANE_NESTED); + }); + + it("preserves non-cron lanes", () => { + expect(resolveNestedAgentLane("subagent")).toBe("subagent"); + expect(resolveNestedAgentLane(" custom-lane ")).toBe("custom-lane"); + }); +}); diff --git a/src/agents/lanes.ts b/src/agents/lanes.ts index 1688a4b8b9a..e9fa2217cf7 100644 --- a/src/agents/lanes.ts +++ b/src/agents/lanes.ts @@ -2,3 +2,13 @@ import { CommandLane } from "../process/lanes.js"; export const AGENT_LANE_NESTED = CommandLane.Nested; export const AGENT_LANE_SUBAGENT = CommandLane.Subagent; + +export function resolveNestedAgentLane(lane?: string): string { + const trimmed = lane?.trim(); + // Nested agent runs should not inherit the cron execution lane. Cron jobs + // already occupy that lane while they dispatch inner work. + if (!trimmed || trimmed === "cron") { + return AGENT_LANE_NESTED; + } + return trimmed; +} diff --git a/src/cron/isolated-agent.lane.test.ts b/src/cron/isolated-agent.lane.test.ts new file mode 100644 index 00000000000..5d26faff327 --- /dev/null +++ b/src/cron/isolated-agent.lane.test.ts @@ -0,0 +1,84 @@ +import "./isolated-agent.mocks.js"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { runEmbeddedPiAgent } from "../agents/pi-embedded.js"; +import { runCronIsolatedAgentTurn } from "./isolated-agent.js"; +import { + makeCfg, + makeJob, + withTempCronHome, + writeSessionStoreEntries, +} from "./isolated-agent.test-harness.js"; + +function makeDeps() { + return { + sendMessageSlack: vi.fn(), + sendMessageWhatsApp: vi.fn(), + sendMessageTelegram: vi.fn(), + sendMessageDiscord: vi.fn(), + sendMessageSignal: vi.fn(), + sendMessageIMessage: vi.fn(), + }; +} + +function mockEmbeddedOk() { + vi.mocked(runEmbeddedPiAgent).mockResolvedValue({ + payloads: [{ text: "ok" }], + meta: { + durationMs: 5, + agentMeta: { sessionId: "s", provider: "p", model: "m" }, + }, + }); +} + +function lastEmbeddedLane(): string | undefined { + const calls = vi.mocked(runEmbeddedPiAgent).mock.calls; + expect(calls.length).toBeGreaterThan(0); + return (calls.at(-1)?.[0] as { lane?: string } | undefined)?.lane; +} + +async function runLaneCase(home: string, lane?: string) { + const storePath = await writeSessionStoreEntries(home, { + "agent:main:main": { + sessionId: "main-session", + updatedAt: Date.now(), + lastProvider: "webchat", + lastTo: "", + }, + }); + mockEmbeddedOk(); + + await runCronIsolatedAgentTurn({ + cfg: makeCfg(home, storePath), + deps: makeDeps(), + job: makeJob({ kind: "agentTurn", message: "do it", deliver: false }), + message: "do it", + sessionKey: "cron:job-1", + ...(lane === undefined ? {} : { lane }), + }); + + return lastEmbeddedLane(); +} + +describe("runCronIsolatedAgentTurn lane selection", () => { + beforeEach(() => { + vi.mocked(runEmbeddedPiAgent).mockClear(); + }); + + it("moves the cron lane to nested for embedded runs", async () => { + await withTempCronHome(async (home) => { + expect(await runLaneCase(home, "cron")).toBe("nested"); + }); + }); + + it("defaults missing lanes to nested for embedded runs", async () => { + await withTempCronHome(async (home) => { + expect(await runLaneCase(home)).toBe("nested"); + }); + }); + + it("preserves non-cron lanes for embedded runs", async () => { + await withTempCronHome(async (home) => { + expect(await runLaneCase(home, "subagent")).toBe("subagent"); + }); + }); +}); diff --git a/src/cron/isolated-agent.model-formatting.test.ts b/src/cron/isolated-agent.model-formatting.test.ts index 7083c61e179..e78f251dc8b 100644 --- a/src/cron/isolated-agent.model-formatting.test.ts +++ b/src/cron/isolated-agent.model-formatting.test.ts @@ -35,12 +35,12 @@ function mockEmbeddedOk() { } /** - * Extract select fields from the last runEmbeddedPiAgent call. + * Extract the provider and model from the last runEmbeddedPiAgent call. */ -function lastEmbeddedCall(): { provider?: string; model?: string; lane?: string } { +function lastEmbeddedCall(): { provider?: string; model?: string } { const calls = vi.mocked(runEmbeddedPiAgent).mock.calls; expect(calls.length).toBeGreaterThan(0); - return calls.at(-1)?.[0] as { provider?: string; model?: string; lane?: string }; + return calls.at(-1)?.[0] as { provider?: string; model?: string }; } const DEFAULT_MESSAGE = "do it"; @@ -106,14 +106,6 @@ describe("cron model formatting and precedence edge cases", () => { // ------ provider/model string splitting ------ describe("parseModelRef formatting", () => { - it("moves nested embedded runs off the cron lane to avoid self-deadlock", async () => { - await withTempHome(async (home) => { - const { res, call } = await runTurn(home); - expect(res.status).toBe("ok"); - expect(call.lane).toBe("nested"); - }); - }); - it("splits standard provider/model", async () => { await withTempHome(async (home) => { const { res, call } = await runTurn(home, { diff --git a/src/cron/isolated-agent/run.ts b/src/cron/isolated-agent/run.ts index bea43d5e2d1..4c7a5c87fe2 100644 --- a/src/cron/isolated-agent/run.ts +++ b/src/cron/isolated-agent/run.ts @@ -12,6 +12,7 @@ import { getCliSessionId, setCliSessionId } from "../../agents/cli-session.js"; import { lookupContextTokens } from "../../agents/context.js"; import { resolveCronStyleNow } from "../../agents/current-time.js"; import { DEFAULT_CONTEXT_TOKENS, DEFAULT_MODEL, DEFAULT_PROVIDER } from "../../agents/defaults.js"; +import { resolveNestedAgentLane } from "../../agents/lanes.js"; import { loadModelCatalog } from "../../agents/model-catalog.js"; import { runWithModelFallback } from "../../agents/model-fallback.js"; import { @@ -46,7 +47,6 @@ import { import type { AgentDefaultsConfig } from "../../config/types.js"; import { registerAgentRunContext } from "../../infra/agent-events.js"; import { logWarn } from "../../logger.js"; -import { CommandLane } from "../../process/lanes.js"; import { normalizeAgentId } from "../../routing/session-key.js"; import { buildSafeExternalPrompt, @@ -198,17 +198,6 @@ function appendCronDeliveryInstruction(params: { return `${params.commandBody}\n\nReturn your summary as plain text; it will be delivered automatically. If the task explicitly calls for messaging a specific external recipient, note who/where it should go instead of sending it yourself.`.trim(); } -function resolveCronEmbeddedAgentLane(lane?: string) { - const trimmed = lane?.trim(); - // Cron jobs already execute inside the cron command lane. Reusing that same - // lane for the nested embedded-agent run deadlocks: the outer cron task holds - // the lane while the inner run waits to reacquire it. - if (!trimmed || trimmed === "cron") { - return CommandLane.Nested; - } - return trimmed; -} - export async function runCronIsolatedAgentTurn(params: { cfg: OpenClawConfig; deps: CliDeps; @@ -622,7 +611,7 @@ export async function runCronIsolatedAgentTurn(params: { config: cfgWithAgentDefaults, skillsSnapshot, prompt: promptText, - lane: resolveCronEmbeddedAgentLane(params.lane), + lane: resolveNestedAgentLane(params.lane), provider: providerOverride, model: modelOverride, authProfileId,