From b146ef8a592521a419c59ea57cc7148dd6755df3 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 14 Jul 2026 09:02:18 -0700 Subject: [PATCH] refactor(cron): trim internal export surface (#107640) * refactor(cron): trim internal export surface * chore(deadcode): refresh cron export baseline --- scripts/deadcode-exports.baseline.mjs | 20 ---- src/cron/command-output-summary.test.ts | 2 - src/cron/command-output-summary.ts | 2 +- src/cron/delivery-context.test.ts | 22 ++-- src/cron/delivery-context.ts | 2 +- src/cron/delivery-preview.test.ts | 28 ++--- src/cron/delivery-preview.ts | 2 +- src/cron/isolated-agent.ts | 1 - src/cron/isolated-agent/helpers.test.ts | 108 +----------------- src/cron/isolated-agent/helpers.ts | 6 +- src/cron/isolated-agent/run-executor.ts | 2 +- .../run.issue-91613-preflight.test.ts | 92 --------------- .../run.message-tool-policy.test.ts | 104 +++++++++-------- .../run.source-delivery-guard.test.ts | 46 +++----- src/cron/isolated-agent/run.test-harness.ts | 2 - .../isolated-agent/run.tools-allow.test.ts | 4 +- src/cron/isolated-agent/run.ts | 6 +- src/cron/job-session-bindings.test.ts | 6 +- src/cron/job-session-bindings.ts | 5 +- src/cron/run-diagnostics.test.ts | 4 +- src/cron/run-diagnostics.ts | 2 +- .../service/active-run-cancellation.test.ts | 3 +- src/cron/service/active-run-cancellation.ts | 2 +- src/cron/service/timeout-policy.test.ts | 9 +- src/cron/service/timeout-policy.ts | 4 +- src/cron/service/timer.regression.test.ts | 3 +- src/cron/session-reaper.test.ts | 46 ++++---- src/cron/session-reaper.ts | 2 +- src/cron/stagger.test.ts | 38 +++--- src/cron/stagger.ts | 2 +- src/cron/store/row-codec.schedule.test.ts | 13 +-- src/cron/store/row-codec.ts | 4 +- src/cron/task-run-history.ts | 2 +- 33 files changed, 181 insertions(+), 413 deletions(-) delete mode 100644 src/cron/isolated-agent/run.issue-91613-preflight.test.ts diff --git a/scripts/deadcode-exports.baseline.mjs b/scripts/deadcode-exports.baseline.mjs index fc12c9a5b9d6..a1673d792cd1 100644 --- a/scripts/deadcode-exports.baseline.mjs +++ b/scripts/deadcode-exports.baseline.mjs @@ -514,37 +514,17 @@ export const KNIP_UNUSED_EXPORT_BASELINE = [ "src/crestodian/agent-turn.ts: CrestodianAgentTurnDeps", "src/crestodian/agent-turn.ts: CrestodianAgentTurnDirective", "src/crestodian/agent-turn.ts: runCrestodianAgentTurnWithDeps", - "src/cron/command-output-summary.ts: cronCommandSummaryNeedsExternalRedaction", - "src/cron/delivery-context.ts: cronDeliveryFromContext", - "src/cron/delivery-preview.ts: resolveCronDeliveryPreview", - "src/cron/isolated-agent.ts: RunCronAgentTurnResult", "src/cron/isolated-agent/delivery-dispatch.ts: getCompletedDirectCronDeliveriesCountForTests", "src/cron/isolated-agent/delivery-dispatch.ts: resetCompletedDirectCronDeliveriesForTests", - "src/cron/isolated-agent/helpers.ts: pickDeliverablePayloads", - "src/cron/isolated-agent/helpers.ts: pickLastDeliverablePayload", - "src/cron/isolated-agent/helpers.ts: pickSummaryFromPayloads", - "src/cron/isolated-agent/run-executor.ts: createCronPromptExecutor", - "src/cron/isolated-agent/run.ts: resolveCronDeliveryContext", - "src/cron/isolated-agent/run.ts: RunCronAgentTurnResult", - "src/cron/job-session-bindings.ts: CronJobSessionBinding", - "src/cron/run-diagnostics.ts: MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE", "src/cron/schedule.ts: clearCronScheduleCacheForTest", "src/cron/schedule.ts: getCronScheduleCacheMaxForTest", "src/cron/schedule.ts: getCronScheduleCacheSizeForTest", "src/cron/schedule.ts: hasCronInCacheForTest", "src/cron/service/active-run-cancellation.ts: cancelActiveCronTaskRun", - "src/cron/service/active-run-cancellation.ts: CRON_TASK_RUN_SETTLEMENT_TRACKING_MAX_MS", "src/cron/service/active-run-cancellation.ts: resetActiveCronTaskRunsForTests", - "src/cron/service/timeout-policy.ts: AGENT_TURN_SAFETY_TIMEOUT_MS", - "src/cron/service/timeout-policy.ts: DEFAULT_JOB_TIMEOUT_MS", "src/cron/service/timer.ts: executeJobCore", "src/cron/service/timer.ts: onTimer", "src/cron/session-reaper.ts: resetReaperThrottle", - "src/cron/session-reaper.ts: resolveRetentionMs", - "src/cron/stagger.ts: isRecurringTopOfHourCronExpr", - "src/cron/store/row-codec.ts: bindScheduleColumns", - "src/cron/store/row-codec.ts: scheduleFromRow", - "src/cron/task-run-history.ts: ReadCronTaskRunHistoryPageOptions", "src/entry.compile-cache.ts: buildOpenClawCompileCacheRespawnPlan", "src/entry.compile-cache.ts: isNodeVersionAffectedByCompileCacheDeadlock", "src/entry.compile-cache.ts: isSourceCheckoutInstallRoot", diff --git a/src/cron/command-output-summary.test.ts b/src/cron/command-output-summary.test.ts index 6c71b021a02f..7322a226a956 100644 --- a/src/cron/command-output-summary.test.ts +++ b/src/cron/command-output-summary.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it } from "vitest"; import { buildCronCommandSummary, - cronCommandSummaryNeedsExternalRedaction, redactCronCommandSummaryForExternalDelivery, } from "./command-output-summary.js"; @@ -22,7 +21,6 @@ describe("cron command output summaries", () => { const summary = "action-required output preserved:\nVisit https://example.com/device or www.example.com/device and enter code ABCD-EFGH\n\ncompleted"; - expect(cronCommandSummaryNeedsExternalRedaction(summary)).toBe(true); expect(redactCronCommandSummaryForExternalDelivery(summary)).toBe( "action-required output preserved:\nVisit [redacted-url] or [redacted-url] and enter code [redacted-code]\n\ncompleted", ); diff --git a/src/cron/command-output-summary.ts b/src/cron/command-output-summary.ts index 9e0f83ea0206..a17c50e8b54f 100644 --- a/src/cron/command-output-summary.ts +++ b/src/cron/command-output-summary.ts @@ -76,7 +76,7 @@ export function buildCronCommandSummary(params: { return tail ? `${actionBlock}\n\n${tail}` : actionBlock; } -export function cronCommandSummaryNeedsExternalRedaction(summary: string | undefined): boolean { +function cronCommandSummaryNeedsExternalRedaction(summary: string | undefined): boolean { if (!summary) { return false; } diff --git a/src/cron/delivery-context.test.ts b/src/cron/delivery-context.test.ts index 04ad923f3445..6789daeb884c 100644 --- a/src/cron/delivery-context.test.ts +++ b/src/cron/delivery-context.test.ts @@ -10,7 +10,7 @@ vi.mock("../config/sessions/delivery-info.js", () => ({ extractDeliveryInfo: extractDeliveryInfoMock, })); -import { cronDeliveryFromContext, resolveCronCreationDelivery } from "./delivery-context.js"; +import { resolveCronCreationDelivery } from "./delivery-context.js"; describe("cron delivery context", () => { const cfg = {} as OpenClawConfig; @@ -22,11 +22,14 @@ describe("cron delivery context", () => { it("builds announce delivery from deliveryContext without changing target casing", () => { expect( - cronDeliveryFromContext({ - channel: " Matrix ", - to: " !AbCdEf1234567890:Example.Org ", - accountId: " Bot-A ", - threadId: " $RootEvent:Example.Org ", + resolveCronCreationDelivery({ + cfg, + currentDeliveryContext: { + channel: " Matrix ", + to: " !AbCdEf1234567890:Example.Org ", + accountId: " Bot-A ", + threadId: " $RootEvent:Example.Org ", + }, }), ).toEqual({ mode: "announce", @@ -107,7 +110,12 @@ describe("cron delivery context", () => { }); it("does not create delivery without a concrete target", () => { - expect(cronDeliveryFromContext({ channel: "matrix", to: " " })).toBeNull(); + expect( + resolveCronCreationDelivery({ + cfg, + currentDeliveryContext: { channel: "matrix", to: " " }, + }), + ).toBeNull(); expect( resolveCronCreationDelivery({ cfg, diff --git a/src/cron/delivery-context.ts b/src/cron/delivery-context.ts index acb0357c319a..bcfb4512d3f5 100644 --- a/src/cron/delivery-context.ts +++ b/src/cron/delivery-context.ts @@ -8,7 +8,7 @@ import { import type { CronDelivery, CronMessageChannel } from "./types.js"; /** Converts an active delivery context into cron announce delivery config. */ -export function cronDeliveryFromContext(context?: DeliveryContext): CronDelivery | null { +function cronDeliveryFromContext(context?: DeliveryContext): CronDelivery | null { const normalized = normalizeDeliveryContext(context); if (!normalized?.to) { return null; diff --git a/src/cron/delivery-preview.test.ts b/src/cron/delivery-preview.test.ts index 0bcc10b4f440..a8ec98b6c310 100644 --- a/src/cron/delivery-preview.test.ts +++ b/src/cron/delivery-preview.test.ts @@ -1,6 +1,7 @@ // Delivery preview tests cover dry-run delivery plan output for cron jobs. import { beforeEach, describe, expect, it, vi } from "vitest"; import { makeCronJob } from "./delivery.test-helpers.js"; +import type { CronJob } from "./types.js"; const mocks = vi.hoisted(() => ({ resolveDeliveryTarget: vi.fn(), @@ -10,7 +11,12 @@ vi.mock("./isolated-agent/delivery-target.js", () => ({ resolveDeliveryTarget: mocks.resolveDeliveryTarget, })); -const { resolveCronDeliveryPreview } = await import("./delivery-preview.js"); +const { resolveCronDeliveryPreviews } = await import("./delivery-preview.js"); + +async function previewForJob(job: CronJob) { + const previews = await resolveCronDeliveryPreviews({ cfg: {} as never, jobs: [job] }); + return previews[job.id]!; +} describe("resolveCronDeliveryPreview", () => { beforeEach(() => { @@ -31,10 +37,7 @@ describe("resolveCronDeliveryPreview", () => { delivery: undefined, }); - const preview = await resolveCronDeliveryPreview({ - cfg: {} as never, - job, - }); + const preview = await previewForJob(job); expect(mocks.resolveDeliveryTarget).toHaveBeenCalledWith( {}, @@ -59,10 +62,7 @@ describe("resolveCronDeliveryPreview", () => { sessionTarget: "isolated", }); - const preview = await resolveCronDeliveryPreview({ - cfg: {} as never, - job, - }); + const preview = await previewForJob(job); expect(preview).toEqual({ label: "not requested", detail: "not requested" }); expect(mocks.resolveDeliveryTarget).not.toHaveBeenCalled(); @@ -81,10 +81,7 @@ describe("resolveCronDeliveryPreview", () => { sessionTarget: "isolated", }); - const preview = await resolveCronDeliveryPreview({ - cfg: {} as never, - job, - }); + const preview = await previewForJob(job); expect(mocks.resolveDeliveryTarget).toHaveBeenCalledWith( {}, @@ -119,10 +116,7 @@ describe("resolveCronDeliveryPreview", () => { sessionTarget: "isolated", }); - const preview = await resolveCronDeliveryPreview({ - cfg: {} as never, - job, - }); + const preview = await previewForJob(job); expect(mocks.resolveDeliveryTarget).toHaveBeenCalledWith( {}, diff --git a/src/cron/delivery-preview.ts b/src/cron/delivery-preview.ts index 7bbd20b92e68..01c6013cf992 100644 --- a/src/cron/delivery-preview.ts +++ b/src/cron/delivery-preview.ts @@ -36,7 +36,7 @@ function formatDeliveryDetail(params: { } /** Builds the user-visible cron delivery preview for one job without sending anything. */ -export async function resolveCronDeliveryPreview(params: { +async function resolveCronDeliveryPreview(params: { cfg: OpenClawConfig; defaultAgentId?: string; job: CronJob; diff --git a/src/cron/isolated-agent.ts b/src/cron/isolated-agent.ts index 5fb048adf42d..1dc39b7434e1 100644 --- a/src/cron/isolated-agent.ts +++ b/src/cron/isolated-agent.ts @@ -1,3 +1,2 @@ // Public facade for isolated cron agent turn execution. export { runCronIsolatedAgentTurn } from "./isolated-agent/run.js"; -export type { RunCronAgentTurnResult } from "./isolated-agent/run.types.js"; diff --git a/src/cron/isolated-agent/helpers.test.ts b/src/cron/isolated-agent/helpers.test.ts index c09f8d09eea0..12adfb9906a9 100644 --- a/src/cron/isolated-agent/helpers.test.ts +++ b/src/cron/isolated-agent/helpers.test.ts @@ -1,12 +1,6 @@ // Isolated agent helper tests cover low-level cron agent utilities. import { describe, expect, it } from "vitest"; -import { - isHeartbeatOnlyResponse, - pickDeliverablePayloads, - pickLastDeliverablePayload, - pickLastNonEmptyTextFromPayloads, - pickSummaryFromPayloads, -} from "./helpers.js"; +import { isHeartbeatOnlyResponse, pickLastNonEmptyTextFromPayloads } from "./helpers.js"; type TextPayload = { text?: string | undefined; isError?: boolean | undefined }; @@ -16,36 +10,6 @@ const textPayloadPickerCases: Array<{ payloads: TextPayload[]; expected: string | undefined; }> = [ - { - name: "summary picks real text over error payload", - pick: pickSummaryFromPayloads, - payloads: [ - { text: "Here is your summary" }, - { text: "Tool error: rate limited", isError: true }, - ], - expected: "Here is your summary", - }, - { - name: "summary falls back to error payload when no real text exists", - pick: pickSummaryFromPayloads, - payloads: [{ text: "Tool error: rate limited", isError: true }], - expected: "Tool error: rate limited", - }, - { - name: "summary returns undefined for empty payloads", - pick: pickSummaryFromPayloads, - payloads: [], - expected: undefined, - }, - { - name: "summary treats isError: undefined as non-error", - pick: pickSummaryFromPayloads, - payloads: [ - { text: "normal text", isError: undefined }, - { text: "error text", isError: true }, - ], - expected: "normal text", - }, { name: "last non-empty text picks real text over error payload", pick: pickLastNonEmptyTextFromPayloads, @@ -81,76 +45,6 @@ describe("text payload pickers", () => { }); }); -describe("pickLastDeliverablePayload", () => { - it("picks real payload over error payload", () => { - const real = { text: "Delivered content" }; - const error = { text: "Error warning", isError: true as const }; - expect(pickLastDeliverablePayload([real, error])).toBe(real); - }); - - it("falls back to error payload when no real payload exists", () => { - const error = { text: "Error warning", isError: true as const }; - expect(pickLastDeliverablePayload([error])).toBe(error); - }); - - it("returns undefined for empty payloads", () => { - expect(pickLastDeliverablePayload([])).toBeUndefined(); - }); - - it("picks media payload over error text payload", () => { - const media = { mediaUrl: "https://example.com/img.png" }; - const error = { text: "Error warning", isError: true as const }; - expect(pickLastDeliverablePayload([media, error])).toBe(media); - }); - - it("picks presentation-only payload over error text payload", () => { - const presentation = { - presentation: { - blocks: [{ type: "buttons" as const, buttons: [{ label: "Ack", value: "ack" }] }], - }, - }; - const error = { text: "Error warning", isError: true as const }; - expect(pickLastDeliverablePayload([presentation, error])).toBe(presentation); - }); - - it("treats isError: undefined as non-error", () => { - const normal = { text: "ok", isError: undefined }; - const error = { text: "bad", isError: true as const }; - expect(pickLastDeliverablePayload([normal, error])).toBe(normal); - }); -}); - -describe("pickDeliverablePayloads", () => { - it("preserves all successful deliverable payloads", () => { - const payloads = [ - { text: "line 1" }, - { text: "temporary error", isError: true as const }, - { text: "line 2" }, - ]; - - expect(pickDeliverablePayloads(payloads)).toEqual([{ text: "line 1" }, { text: "line 2" }]); - }); - - it("preserves rich-only successful deliverable payloads", () => { - const presentation = { - presentation: { - blocks: [{ type: "buttons" as const, buttons: [{ label: "Ack", value: "ack" }] }], - }, - }; - - expect(pickDeliverablePayloads([presentation])).toEqual([presentation]); - }); - - it("returns only the last error payload when all payloads are errors", () => { - const payloads = [ - { text: "first error", isError: true as const }, - { text: "last error", isError: true as const }, - ]; - - expect(pickDeliverablePayloads(payloads)).toEqual([{ text: "last error", isError: true }]); - }); -}); - describe("isHeartbeatOnlyResponse", () => { const ACK_MAX = 300; diff --git a/src/cron/isolated-agent/helpers.ts b/src/cron/isolated-agent/helpers.ts index f18601750c6e..99c8d6f7955a 100644 --- a/src/cron/isolated-agent/helpers.ts +++ b/src/cron/isolated-agent/helpers.ts @@ -92,7 +92,7 @@ export function pickSummaryFromOutput(text: string | undefined) { } /** Picks the last non-error payload text suitable for cron run summaries. */ -export function pickSummaryFromPayloads( +function pickSummaryFromPayloads( payloads: Array<{ text?: string | undefined; isError?: boolean }>, ) { for (let i = payloads.length - 1; i >= 0; i--) { @@ -162,7 +162,7 @@ function payloadHasStructuredDeliveryContent(payload: DeliveryPayload | null | u } /** Picks the last payload with deliverable outbound content, preferring non-error payloads. */ -export function pickLastDeliverablePayload(payloads: DeliveryPayload[]) { +function pickLastDeliverablePayload(payloads: DeliveryPayload[]) { for (let i = payloads.length - 1; i >= 0; i--) { if (payloads[i]?.isError) { continue; @@ -180,7 +180,7 @@ export function pickLastDeliverablePayload(payloads: DeliveryPayload[]) { } /** Selects deliverable cron payloads while preserving multi-payload successful responses. */ -export function pickDeliverablePayloads(payloads: DeliveryPayload[]): DeliveryPayload[] { +function pickDeliverablePayloads(payloads: DeliveryPayload[]): DeliveryPayload[] { const successfulDeliverablePayloads = payloads.filter( (payload) => payload != null && payload.isError !== true && isDeliverablePayload(payload), ); diff --git a/src/cron/isolated-agent/run-executor.ts b/src/cron/isolated-agent/run-executor.ts index 8aedf4c4fb40..24097a2b414a 100644 --- a/src/cron/isolated-agent/run-executor.ts +++ b/src/cron/isolated-agent/run-executor.ts @@ -193,7 +193,7 @@ export type CronExecutionResult = { }; /** Creates the model-fallback executor for one isolated cron prompt run. */ -export function createCronPromptExecutor(params: { +function createCronPromptExecutor(params: { cfg: OpenClawConfig; cfgWithAgentDefaults: OpenClawConfig; job: CronJob; diff --git a/src/cron/isolated-agent/run.issue-91613-preflight.test.ts b/src/cron/isolated-agent/run.issue-91613-preflight.test.ts deleted file mode 100644 index b17f8f31d71f..000000000000 --- a/src/cron/isolated-agent/run.issue-91613-preflight.test.ts +++ /dev/null @@ -1,92 +0,0 @@ -// Issue #91613 run-layer wiring: the keyless-inherited refusal now lives in resolveDeliveryTarget -// (returns ok:false), so resolveCronDeliveryContext simply passes it through — no flag the caller -// must remember to check. It also resolves the job's OWN session identity (sessionTarget takes -// precedence over sessionKey, like delivery preview) so a session-scoped cron is not misread as -// keyless. The resolver seam is mocked so this exercises the run-layer wiring in isolation. -import { describe, expect, it, vi } from "vitest"; -import type { OpenClawConfig } from "../../config/config.js"; -import type { CronJob } from "../types.js"; - -const resolveDeliveryTargetMock = vi.hoisted(() => vi.fn()); -vi.mock("./run-delivery.runtime.js", () => ({ - resolveDeliveryTarget: resolveDeliveryTargetMock, -})); - -import { resolveCronDeliveryContext } from "./run.js"; - -// An isolated cron defaults to announce/"last" delivery, so it reaches the resolver. -const ISOLATED_JOB = { - sessionTarget: "isolated", - payload: { kind: "agentTurn" }, -} as unknown as CronJob; -const CFG = {} as OpenClawConfig; - -describe("resolveCronDeliveryContext — issue #91613 keyless-inherited wiring", () => { - it("passes through the resolver's ok:false refusal for a keyless-inherited target", async () => { - resolveDeliveryTargetMock.mockResolvedValueOnce({ - ok: false, - channel: "alpha", - to: undefined, - mode: "implicit", - error: new Error( - "Refusing implicit isolated cron delivery: the target would be inherited from the shared " + - "agent-main session bucket's last recipient ... can deliver to the wrong room ...", - ), - }); - - const result = await resolveCronDeliveryContext({ - cfg: CFG, - job: ISOLATED_JOB, - agentId: "agent-x", - }); - - expect(result.resolvedDelivery.ok).toBe(false); - if (!result.resolvedDelivery.ok) { - expect(result.resolvedDelivery.error.message).toMatch( - /shared agent-main|inherited|wrong room/i, - ); - } - }); - - it("delivers normally when the resolver resolves an explicit target", async () => { - resolveDeliveryTargetMock.mockResolvedValueOnce({ - ok: true, - channel: "alpha", - to: "room:cron-own", - mode: "explicit", - }); - - const result = await resolveCronDeliveryContext({ - cfg: CFG, - job: ISOLATED_JOB, - agentId: "agent-x", - }); - - expect(result.resolvedDelivery.ok).toBe(true); - if (result.resolvedDelivery.ok) { - expect(result.resolvedDelivery.to).toBe("room:cron-own"); - } - }); - - it("resolves the job's sessionTarget into the resolver's sessionKey so a session-scoped cron is not misread as keyless", async () => { - resolveDeliveryTargetMock.mockResolvedValueOnce({ - ok: true, - channel: "alpha", - to: "room:session-scoped", - mode: "implicit", - }); - - const sessionScopedJob = { - sessionTarget: "session:my-session", - payload: { kind: "agentTurn" }, - } as unknown as CronJob; - - await resolveCronDeliveryContext({ cfg: CFG, job: sessionScopedJob, agentId: "agent-x" }); - - expect(resolveDeliveryTargetMock).toHaveBeenCalledWith( - CFG, - "agent-x", - expect.objectContaining({ sessionKey: "my-session" }), - ); - }); -}); diff --git a/src/cron/isolated-agent/run.message-tool-policy.test.ts b/src/cron/isolated-agent/run.message-tool-policy.test.ts index 893e7579f3ce..d99c018ffc28 100644 --- a/src/cron/isolated-agent/run.message-tool-policy.test.ts +++ b/src/cron/isolated-agent/run.message-tool-policy.test.ts @@ -29,7 +29,7 @@ import { } from "./run.test-harness.js"; const runCronIsolatedAgentTurn = await loadRunCronIsolatedAgentTurn(); -const { createCronPromptExecutor } = await import("./run-executor.js"); +const { executeCronRun } = await import("./run-executor.js"); function makeMessageToolPolicyJob( delivery: Record = { mode: "none" }, @@ -336,53 +336,61 @@ describe("runCronIsolatedAgentTurn message tool policy", () => { version: 1, }; - function createMessageToolExecutor( - overrides: Partial[0]>, - ) { - const resolvedDelivery = overrides.resolvedDelivery ?? {}; - - return createCronPromptExecutor({ - cfg: {}, - cfgWithAgentDefaults: {}, - job: makeMessageToolPolicyJob(), - agentId: "default", - agentDir: "/tmp/agent-dir", - agentSessionKey: "cron:message-tool-policy", - runSessionKey: "cron:message-tool-policy:run:test-session-id", - workspaceDir: "/tmp/workspace", - resolvedVerboseLevel: "off", - thinkLevel: undefined, - timeoutMs: 60_000, - suppressExecNotifyOnExit: true, - resolvedDeliveryOk: true, - messageToolPromptEnabled: true, - sourceDelivery: createSourceDeliveryPlan({ - owner: "direct_fallback", - reason: "cron_announce", - target: { - channel: resolvedDelivery.channel ?? "messagechat", - to: resolvedDelivery.to, - accountId: resolvedDelivery.accountId, - threadId: resolvedDelivery.threadId, - }, - messageToolEnabled: true, - messageToolForced: false, - requireExplicitMessageTarget: true, - requireExplicitMessageTargetEvidence: true, - directFallback: true, - }), - skillsSnapshot: emptySkillsSnapshot, - agentPayload: null, - useSubagentFallbacks: false, - liveSelection: { - provider: "openai", - model: "gpt-5.4", - }, - cronSession: makeCronSession() as MutableCronSession, - abortReason: () => "aborted", - ...overrides, - resolvedDelivery, - }); + function createMessageToolExecutor(overrides: Record) { + const resolvedDelivery = (overrides.resolvedDelivery ?? {}) as { + channel?: string; + to?: string; + accountId?: string; + threadId?: string | number; + }; + return { + runPrompt: async (commandBody: string) => + await executeCronRun({ + cfg: {}, + cfgWithAgentDefaults: {}, + job: makeMessageToolPolicyJob(), + agentId: "default", + agentDir: "/tmp/agent-dir", + agentSessionKey: "cron:message-tool-policy", + runSessionKey: "cron:message-tool-policy:run:test-session-id", + workspaceDir: "/tmp/workspace", + agentVerboseDefault: undefined, + thinkLevel: undefined, + timeoutMs: 60_000, + suppressExecNotifyOnExit: true, + resolvedDeliveryOk: true, + messageToolPromptEnabled: true, + sourceDelivery: createSourceDeliveryPlan({ + owner: "direct_fallback", + reason: "cron_announce", + target: { + channel: resolvedDelivery.channel ?? "messagechat", + to: resolvedDelivery.to, + accountId: resolvedDelivery.accountId, + threadId: resolvedDelivery.threadId, + }, + messageToolEnabled: true, + messageToolForced: false, + requireExplicitMessageTarget: true, + requireExplicitMessageTargetEvidence: true, + directFallback: true, + }), + skillsSnapshot: emptySkillsSnapshot, + agentPayload: null, + useSubagentFallbacks: false, + liveSelection: { + provider: "openai", + model: "gpt-5.4", + }, + cronSession: makeCronSession() as MutableCronSession, + commandBody, + persistSessionEntry: async () => undefined, + abortReason: () => "aborted", + isAborted: () => false, + ...overrides, + resolvedDelivery, + } as never), + }; } afterEach(() => { diff --git a/src/cron/isolated-agent/run.source-delivery-guard.test.ts b/src/cron/isolated-agent/run.source-delivery-guard.test.ts index d37f4295ce9f..62c040f056e7 100644 --- a/src/cron/isolated-agent/run.source-delivery-guard.test.ts +++ b/src/cron/isolated-agent/run.source-delivery-guard.test.ts @@ -15,7 +15,7 @@ import { const actualDeliveryPlanModule = await vi.importActual("../delivery-plan.js"); -const { createCronPromptExecutor, executeCronRun } = await import("./run-executor.js"); +const { executeCronRun } = await import("./run-executor.js"); const { resolveCronSourceDeliveryPlan, resolveFallbackCronSourceDeliveryPlan } = await import("./source-delivery-fallback.js"); @@ -44,36 +44,20 @@ function makeJob( } as CronJob; } -function makeExecutor(overrides: Partial[0]>) { +function makeExecutor(overrides: Record) { const resolvedDelivery = overrides.resolvedDelivery ?? {}; - - return createCronPromptExecutor({ - cfg: {}, - cfgWithAgentDefaults: {}, - job: makeJob(), - agentId: "default", - agentDir: "/tmp/agent-dir", - agentSessionKey: "cron:source-delivery-guard", - runSessionKey: "cron:source-delivery-guard:run:test-session-id", - workspaceDir: "/tmp/workspace", - resolvedVerboseLevel: "off", - thinkLevel: undefined, - timeoutMs: 60_000, - suppressExecNotifyOnExit: true, - skillsSnapshot: emptySkillsSnapshot, - agentPayload: null, - useSubagentFallbacks: false, - liveSelection: { - provider: "openai", - model: "gpt-5.4", - }, - cronSession: makeCronSession() as MutableCronSession, - abortReason: () => "aborted", - resolvedDeliveryOk: true, - messageToolPromptEnabled: true, - ...overrides, - resolvedDelivery, - }); + return { + runPrompt: async (commandBody: string) => + await executeCronRun( + makeExecuteCronRunParams({ + resolvedDeliveryOk: true, + messageToolPromptEnabled: true, + ...overrides, + resolvedDelivery, + commandBody, + }), + ), + }; } function getEmbeddedRunArg(): Record { @@ -256,7 +240,7 @@ describe("resolveCronSourceDeliveryPlan", () => { } }); -describe("createCronPromptExecutor sourceDelivery guard", () => { +describe("executeCronRun sourceDelivery mapping", () => { let previousFastTestEnv: string | undefined; beforeEach(() => { diff --git a/src/cron/isolated-agent/run.test-harness.ts b/src/cron/isolated-agent/run.test-harness.ts index 35d0c900c30a..de87983ac248 100644 --- a/src/cron/isolated-agent/run.test-harness.ts +++ b/src/cron/isolated-agent/run.test-harness.ts @@ -393,10 +393,8 @@ vi.mock("./model-preflight.runtime.js", () => ({ vi.mock("./helpers.js", () => ({ isHeartbeatOnlyResponse: isHeartbeatOnlyResponseMock, - pickLastDeliverablePayload: vi.fn().mockReturnValue(undefined), pickLastNonEmptyTextFromPayloads: pickLastNonEmptyTextFromPayloadsMock, pickSummaryFromOutput: vi.fn().mockReturnValue("summary"), - pickSummaryFromPayloads: vi.fn().mockReturnValue("summary"), resolveCronPayloadOutcome: resolveCronPayloadOutcomeMock, resolveHeartbeatAckMaxChars: resolveHeartbeatAckMaxCharsMock, })); diff --git a/src/cron/isolated-agent/run.tools-allow.test.ts b/src/cron/isolated-agent/run.tools-allow.test.ts index aa994adc1757..90a715f38cfb 100644 --- a/src/cron/isolated-agent/run.tools-allow.test.ts +++ b/src/cron/isolated-agent/run.tools-allow.test.ts @@ -1,6 +1,5 @@ // Tool allowlist tests cover tool availability for isolated cron runs. import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE } from "../run-diagnostics.js"; import "../../agents/test-helpers/fast-coding-tools.js"; import { listWebSearchProvidersMock, @@ -14,6 +13,9 @@ import { runWithModelFallbackMock, } from "./run.test-harness.js"; +const MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE = + "web_search tool requested in toolsAllow but no web search provider is selected. Configure one with: openclaw configure --section web, or set tools.web.search.provider."; + const RUN_TOOLS_ALLOW_TIMEOUT_MS = 300_000; const runCronIsolatedAgentTurn = await loadRunCronIsolatedAgentTurn(); diff --git a/src/cron/isolated-agent/run.ts b/src/cron/isolated-agent/run.ts index f54dc8575959..d2894993dc8e 100644 --- a/src/cron/isolated-agent/run.ts +++ b/src/cron/isolated-agent/run.ts @@ -235,8 +235,6 @@ async function retireRolledCronSessionMcpRuntime(params: { }); } -export type { RunCronAgentTurnResult } from "./run.types.js"; - type CronExecutionRuntime = typeof import("./run-executor.runtime.js"); type CronExecutionResult = Awaited>; type CronModelCatalogRuntime = typeof import("./run-model-catalog.runtime.js"); @@ -411,8 +409,8 @@ async function createCronToolsAllowPreflightDiagnostics(params: { } } -/** Exported for #91613 keyless-inherited delivery-context regression coverage. */ -export async function resolveCronDeliveryContext(params: { +/** Resolves the delivery plan and concrete target for one isolated cron run. */ +async function resolveCronDeliveryContext(params: { cfg: OpenClawConfig; job: CronJob; agentId: string; diff --git a/src/cron/job-session-bindings.test.ts b/src/cron/job-session-bindings.test.ts index 27e1ceacf5d9..c862e3c3d04e 100644 --- a/src/cron/job-session-bindings.test.ts +++ b/src/cron/job-session-bindings.test.ts @@ -3,13 +3,15 @@ import type { OpenClawConfig } from "../config/types.openclaw.js"; import { disableCronJobsBoundToSession, resolveCronJobBoundSessionKeys, - type CronJobSessionBinding, } from "./job-session-bindings.js"; import type { CronJob } from "./types.js"; const cfg = {} as OpenClawConfig; -function bindingKeys(job: CronJobSessionBinding, defaultAgentId?: string) { +function bindingKeys( + job: Parameters[0], + defaultAgentId?: string, +) { return resolveCronJobBoundSessionKeys(job, { cfg, defaultAgentId }); } diff --git a/src/cron/job-session-bindings.ts b/src/cron/job-session-bindings.ts index f5ade5666f5f..6314eb0302b4 100644 --- a/src/cron/job-session-bindings.ts +++ b/src/cron/job-session-bindings.ts @@ -6,10 +6,7 @@ import type { CronServiceContract } from "./service-contract.js"; import { resolveCronSessionTargetSessionKey } from "./session-target.js"; import type { CronJob } from "./types.js"; -export type CronJobSessionBinding = Pick< - CronJob, - "id" | "agentId" | "sessionKey" | "sessionTarget" ->; +type CronJobSessionBinding = Pick; /** * Resolves every canonical session key a job is bound to: the session the run diff --git a/src/cron/run-diagnostics.test.ts b/src/cron/run-diagnostics.test.ts index ee98f5e8a71e..6853659ec40f 100644 --- a/src/cron/run-diagnostics.test.ts +++ b/src/cron/run-diagnostics.test.ts @@ -5,12 +5,14 @@ import { createCronRunDiagnosticsFromMissingWebSearchProvider, createCronRunDiagnosticsFromAgentResult, createCronRunDiagnosticsFromError, - MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE, mergeCronRunDiagnostics, normalizeCronRunDiagnostics, summarizeCronRunDiagnostics, } from "./run-diagnostics.js"; +const MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE = + "web_search tool requested in toolsAllow but no web search provider is selected. Configure one with: openclaw configure --section web, or set tools.web.search.provider."; + describe("cron run diagnostics", () => { it("normalizes and bounds diagnostic entries", () => { const diagnostics = normalizeCronRunDiagnostics({ diff --git a/src/cron/run-diagnostics.ts b/src/cron/run-diagnostics.ts index 5db028102907..0e403a2e786a 100644 --- a/src/cron/run-diagnostics.ts +++ b/src/cron/run-diagnostics.ts @@ -24,7 +24,7 @@ const MAX_SUMMARY_CHARS = 2_000; const EXEC_DIAGNOSTIC_TAIL_CHARS = 2_000; const WEB_SEARCH_TOOL_NAME = "web_search"; -export const MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE = +const MISSING_WEB_SEARCH_PROVIDER_DIAGNOSTIC_MESSAGE = "web_search tool requested in toolsAllow but no web search provider is selected. Configure one with: openclaw configure --section web, or set tools.web.search.provider."; export function toolsAllowRequestsWebSearch(toolsAllow?: string[]): boolean { diff --git a/src/cron/service/active-run-cancellation.test.ts b/src/cron/service/active-run-cancellation.test.ts index 53db9c1f1c71..23c13fce0a1a 100644 --- a/src/cron/service/active-run-cancellation.test.ts +++ b/src/cron/service/active-run-cancellation.test.ts @@ -1,6 +1,5 @@ import { describe, expect, it, vi } from "vitest"; import { - CRON_TASK_RUN_SETTLEMENT_TRACKING_MAX_MS, getSuspensionVisibleCronTaskRunCount, resetActiveCronTaskRunsForTests, retireActiveCronTaskRunTracking, @@ -9,6 +8,8 @@ import { waitForActiveCronTaskRuns, } from "./active-run-cancellation.js"; +const CRON_TASK_RUN_SETTLEMENT_TRACKING_MAX_MS = 60_000; + describe("cron task cancellation tracking", () => { it("retires restart tracking while keeping an unsettled core suspension-visible", async () => { resetActiveCronTaskRunsForTests(); diff --git a/src/cron/service/active-run-cancellation.ts b/src/cron/service/active-run-cancellation.ts index f1af1995c1c2..3d939e3174a3 100644 --- a/src/cron/service/active-run-cancellation.ts +++ b/src/cron/service/active-run-cancellation.ts @@ -15,7 +15,7 @@ const settlingCronTaskRuns = new Map, SettlingCronTaskRun>(); // host snapshot must keep refusing readiness until that core actually settles. const suspensionVisibleCronTaskRuns = new Set>(); const DEFAULT_CRON_TASK_RUN_DRAIN_POLL_MS = 25; -export const CRON_TASK_RUN_SETTLEMENT_TRACKING_MAX_MS = 60_000; +const CRON_TASK_RUN_SETTLEMENT_TRACKING_MAX_MS = 60_000; export function startActiveCronTaskRunSettlementGrace(): void { for (const [promise, entry] of settlingCronTaskRuns) { diff --git a/src/cron/service/timeout-policy.test.ts b/src/cron/service/timeout-policy.test.ts index c256c190badb..f0e4bfedd476 100644 --- a/src/cron/service/timeout-policy.test.ts +++ b/src/cron/service/timeout-policy.test.ts @@ -2,11 +2,10 @@ import { MAX_TIMER_TIMEOUT_MS } from "@openclaw/normalization-core/number-coercion"; import { describe, expect, it } from "vitest"; import type { CronJob } from "../types.js"; -import { - AGENT_TURN_SAFETY_TIMEOUT_MS, - DEFAULT_JOB_TIMEOUT_MS, - resolveCronJobTimeoutMs, -} from "./timeout-policy.js"; +import { resolveCronJobTimeoutMs } from "./timeout-policy.js"; + +const DEFAULT_JOB_TIMEOUT_MS = 10 * 60_000; +const AGENT_TURN_SAFETY_TIMEOUT_MS = 60 * 60_000; function makeJob(payload: CronJob["payload"]): CronJob { const sessionTarget = payload.kind === "agentTurn" ? "isolated" : "main"; diff --git a/src/cron/service/timeout-policy.ts b/src/cron/service/timeout-policy.ts index a949e6356150..cb7245c7166f 100644 --- a/src/cron/service/timeout-policy.ts +++ b/src/cron/service/timeout-policy.ts @@ -7,13 +7,13 @@ import type { CronJob } from "../types.js"; * on top of per-provider/per-agent timeouts to prevent one stuck job from * wedging the entire cron lane. */ -export const DEFAULT_JOB_TIMEOUT_MS = 10 * 60_000; // 10 minutes +const DEFAULT_JOB_TIMEOUT_MS = 10 * 60_000; // 10 minutes /** * Agent turns can legitimately run much longer than generic cron jobs. * Use a larger safety ceiling when no explicit timeout is set. */ -export const AGENT_TURN_SAFETY_TIMEOUT_MS = 60 * 60_000; // 60 minutes +const AGENT_TURN_SAFETY_TIMEOUT_MS = 60 * 60_000; // 60 minutes /** Resolves the wall-clock timeout for a cron job, including explicit detached-run overrides. */ export function resolveCronJobTimeoutMs(job: CronJob): number | undefined { diff --git a/src/cron/service/timer.regression.test.ts b/src/cron/service/timer.regression.test.ts index da69aa393093..70106f8d6564 100644 --- a/src/cron/service/timer.regression.test.ts +++ b/src/cron/service/timer.regression.test.ts @@ -41,7 +41,6 @@ import { import { computeJobNextRunAtMs } from "./jobs.js"; import { run as runManualCronJob } from "./ops.js"; import { createCronServiceState, type CronEvent } from "./state.js"; -import { DEFAULT_JOB_TIMEOUT_MS } from "./timeout-policy.js"; import { applyJobResult, executeJobCore, @@ -958,7 +957,7 @@ describe("cron service timer regressions", () => { settled = true; }); - await vi.advanceTimersByTimeAsync(DEFAULT_JOB_TIMEOUT_MS + 1_000); + await vi.advanceTimersByTimeAsync(10 * 60_000 + 1_000); await Promise.resolve(); expect(settled).toBe(false); diff --git a/src/cron/session-reaper.test.ts b/src/cron/session-reaper.test.ts index f641495de613..172082fe5a82 100644 --- a/src/cron/session-reaper.test.ts +++ b/src/cron/session-reaper.test.ts @@ -9,7 +9,7 @@ import { isCronRunSessionKey } from "../sessions/session-key-utils.js"; import { beginSessionWorkAdmission } from "../sessions/session-lifecycle-admission.js"; import { createDeferred } from "../test-utils/deferred.js"; import type { Logger } from "./service/state.js"; -import { sweepCronRunSessions, resolveRetentionMs, resetReaperThrottle } from "./session-reaper.js"; +import { resetReaperThrottle, sweepCronRunSessions } from "./session-reaper.js"; const { listSessionEntries, patchSessionEntry, replaceSessionEntry } = sessionAccessor; @@ -43,30 +43,6 @@ function readSessionEntries(storePath: string): Record { ); } -describe("resolveRetentionMs", () => { - it("returns 24h default when no config", () => { - expect(resolveRetentionMs()).toBe(24 * 3_600_000); - }); - - it("returns 24h default when config is empty", () => { - expect(resolveRetentionMs({})).toBe(24 * 3_600_000); - }); - - it("parses duration string", () => { - expect(resolveRetentionMs({ sessionRetention: "1h" })).toBe(3_600_000); - expect(resolveRetentionMs({ sessionRetention: "7d" })).toBe(7 * 86_400_000); - expect(resolveRetentionMs({ sessionRetention: "30m" })).toBe(30 * 60_000); - }); - - it("returns null when disabled", () => { - expect(resolveRetentionMs({ sessionRetention: false })).toBeNull(); - }); - - it("falls back to default on invalid string", () => { - expect(resolveRetentionMs({ sessionRetention: "abc" })).toBe(24 * 3_600_000); - }); -}); - describe("isCronRunSessionKey", () => { it("matches cron run session keys", () => { expect(isCronRunSessionKey("agent:main:cron:abc-123:run:def-456")).toBe(true); @@ -169,6 +145,26 @@ describe("sweepCronRunSessions", () => { }); }); + it("falls back to the default retention when the configured duration is invalid", async () => { + const now = Date.now(); + await seedSessionEntries(storePath, { + "agent:main:cron:job1:run:old-run": { + sessionId: "old-run", + updatedAt: now - 25 * 3_600_000, + }, + }); + + const result = await sweepCronRunSessions({ + cronConfig: { sessionRetention: "not-a-duration" }, + sessionStorePath: storePath, + nowMs: now, + log, + force: true, + }); + + expect(result).toEqual({ swept: true, pruned: 1 }); + }); + it("preserves expired continuation rows while generated media is pending", async () => { const now = Date.now(); const sessionKey = "agent:main:cron:job1:run:pending-run"; diff --git a/src/cron/session-reaper.ts b/src/cron/session-reaper.ts index 62ee2349d19f..76fc1bc8148a 100644 --- a/src/cron/session-reaper.ts +++ b/src/cron/session-reaper.ts @@ -20,7 +20,7 @@ const MIN_SWEEP_INTERVAL_MS = 5 * 60_000; // 5 minutes const lastSweepAtMsByStore = new Map(); /** Resolves cron run-session retention; `false` disables pruning, bad strings fall back safely. */ -export function resolveRetentionMs(cronConfig?: CronConfig): number | null { +function resolveRetentionMs(cronConfig?: CronConfig): number | null { if (cronConfig?.sessionRetention === false) { return null; // pruning disabled } diff --git a/src/cron/stagger.test.ts b/src/cron/stagger.test.ts index e8bbe400edf0..1411e226c03f 100644 --- a/src/cron/stagger.test.ts +++ b/src/cron/stagger.test.ts @@ -1,29 +1,35 @@ // Cron stagger tests cover deterministic schedule spreading across jobs. import { describe, expect, it } from "vitest"; -import { - isRecurringTopOfHourCronExpr, - normalizeCronStaggerMs, - resolveCronStaggerMs, -} from "./stagger.js"; +import { normalizeCronStaggerMs, resolveCronStaggerMs } from "./stagger.js"; const DEFAULT_TOP_OF_HOUR_STAGGER_MS = 5 * 60 * 1000; describe("cron stagger helpers", () => { it("detects recurring top-of-hour cron expressions for 5-field and 6-field cron", () => { - expect(isRecurringTopOfHourCronExpr("0 * * * *")).toBe(true); - expect(isRecurringTopOfHourCronExpr("0 */2 * * *")).toBe(true); - expect(isRecurringTopOfHourCronExpr("0 0 */3 * * *")).toBe(true); - expect(isRecurringTopOfHourCronExpr("0 */2,3 * * *")).toBe(true); - expect(isRecurringTopOfHourCronExpr("0 */2,? * * *")).toBe(true); - expect(isRecurringTopOfHourCronExpr("0 7 * * *")).toBe(false); - expect(isRecurringTopOfHourCronExpr("15 * * * *")).toBe(false); + expect(resolveCronStaggerMs({ kind: "cron", expr: "0 * * * *" })).toBe( + DEFAULT_TOP_OF_HOUR_STAGGER_MS, + ); + expect(resolveCronStaggerMs({ kind: "cron", expr: "0 */2 * * *" })).toBe( + DEFAULT_TOP_OF_HOUR_STAGGER_MS, + ); + expect(resolveCronStaggerMs({ kind: "cron", expr: "0 0 */3 * * *" })).toBe( + DEFAULT_TOP_OF_HOUR_STAGGER_MS, + ); + expect(resolveCronStaggerMs({ kind: "cron", expr: "0 */2,3 * * *" })).toBe( + DEFAULT_TOP_OF_HOUR_STAGGER_MS, + ); + expect(resolveCronStaggerMs({ kind: "cron", expr: "0 */2,? * * *" })).toBe( + DEFAULT_TOP_OF_HOUR_STAGGER_MS, + ); + expect(resolveCronStaggerMs({ kind: "cron", expr: "0 7 * * *" })).toBe(0); + expect(resolveCronStaggerMs({ kind: "cron", expr: "15 * * * *" })).toBe(0); }); it("rejects malformed hour fields that merely contain a wildcard", () => { - expect(isRecurringTopOfHourCronExpr("0 5* * * *")).toBe(false); - expect(isRecurringTopOfHourCronExpr("0 *5 * * *")).toBe(false); - expect(isRecurringTopOfHourCronExpr("0 1-*/2 * * *")).toBe(false); - expect(isRecurringTopOfHourCronExpr("0 0 5* * * *")).toBe(false); + expect(resolveCronStaggerMs({ kind: "cron", expr: "0 5* * * *" })).toBe(0); + expect(resolveCronStaggerMs({ kind: "cron", expr: "0 *5 * * *" })).toBe(0); + expect(resolveCronStaggerMs({ kind: "cron", expr: "0 1-*/2 * * *" })).toBe(0); + expect(resolveCronStaggerMs({ kind: "cron", expr: "0 0 5* * * *" })).toBe(0); }); it("normalizes explicit stagger values", () => { diff --git a/src/cron/stagger.ts b/src/cron/stagger.ts index 720434b2b144..c7ed55d49614 100644 --- a/src/cron/stagger.ts +++ b/src/cron/stagger.ts @@ -21,7 +21,7 @@ function hasRecurringWildcardHour(field: string): boolean { } /** Returns whether a cron expression fires recurring jobs exactly at the top of an hour. */ -export function isRecurringTopOfHourCronExpr(expr: string) { +function isRecurringTopOfHourCronExpr(expr: string) { const fields = parseCronFields(expr); if (fields.length === 5) { const [minuteField, hourField] = fields; diff --git a/src/cron/store/row-codec.schedule.test.ts b/src/cron/store/row-codec.schedule.test.ts index 32f241597c8c..88da7a0b4dd9 100644 --- a/src/cron/store/row-codec.schedule.test.ts +++ b/src/cron/store/row-codec.schedule.test.ts @@ -2,15 +2,12 @@ // on-exit command/cwd persistence (v1 reuses schedule_expr/schedule_tz) is // covered alongside the existing kinds. import { describe, expect, it } from "vitest"; +import { makeCronJob } from "../delivery.test-helpers.js"; import type { CronSchedule } from "../types.js"; -import { bindScheduleColumns, scheduleFromRow } from "./row-codec.js"; -import type { CronJobRow } from "./schema.js"; +import { projectCronJobThroughStorageCodec } from "./row-codec.js"; function roundTrip(schedule: CronSchedule): CronSchedule | null { - const cols = bindScheduleColumns(schedule); - // scheduleFromRow only reads the schedule_* / at / every_ms / anchor_ms / - // stagger_ms columns; the rest of the row is irrelevant here. - return scheduleFromRow(cols as unknown as CronJobRow); + return projectCronJobThroughStorageCodec(makeCronJob({ schedule })).schedule; } describe("schedule column codec round-trip", () => { @@ -46,9 +43,7 @@ describe("schedule column codec round-trip", () => { }); it("an on-exit row is decoded as on-exit, not cron (schedule_kind disambiguates)", () => { - const cols = bindScheduleColumns({ kind: "on-exit", command: "sleep 5" }); - expect(cols.schedule_kind).toBe("on-exit"); - const decoded = scheduleFromRow(cols as unknown as CronJobRow); + const decoded = roundTrip({ kind: "on-exit", command: "sleep 5" }); expect(decoded?.kind).toBe("on-exit"); }); }); diff --git a/src/cron/store/row-codec.ts b/src/cron/store/row-codec.ts index fcbefbcc39b6..8ff15e95facf 100644 --- a/src/cron/store/row-codec.ts +++ b/src/cron/store/row-codec.ts @@ -22,7 +22,7 @@ import { bindStateColumns, stateFromRow } from "./state-codec.js"; import { bindTriggerColumns, triggerFromRow } from "./trigger-codec.js"; import type { LoadedCronStore } from "./types.js"; -export function bindScheduleColumns( +function bindScheduleColumns( schedule: CronSchedule, ): Pick< CronJobInsert, @@ -210,7 +210,7 @@ export function assertCronStoreCanPersist(store: CronStoreFile): void { } } -export function scheduleFromRow(row: CronJobRow): CronSchedule | null { +function scheduleFromRow(row: CronJobRow): CronSchedule | null { if (row.schedule_kind === "at" && row.at) { return { kind: "at", at: row.at }; } diff --git a/src/cron/task-run-history.ts b/src/cron/task-run-history.ts index 3071124aa31f..153b320e2eb0 100644 --- a/src/cron/task-run-history.ts +++ b/src/cron/task-run-history.ts @@ -13,7 +13,7 @@ import type { CronDeliveryStatus, CronRunStatus } from "./types.js"; type CronRunHistorySortDir = "asc" | "desc"; type CronRunHistoryStatusFilter = "all" | CronRunStatus; -export type ReadCronTaskRunHistoryPageOptions = { +type ReadCronTaskRunHistoryPageOptions = { storeKey: string; limit?: number; offset?: number;