mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
fix(automations): isolate the scheduler tick and refuse oversized cron steps (#20152)
Two defects that change nothing about when an existing schedule fires. #16303: evaluateDueRuns awaited each row with no catch, so one unreadable schedule skipped every later due automation in that tick. Each row is isolated now; a poison record writes one folded skipped_unavailable run explaining itself and the tick continues. A renderer send that throws is closed out as dispatch_failed rather than mislabelled as an unreadable schedule. #15895: step validation only checked integer >= 1, so a step wider than its field degraded silently to a single value and still passed validation. Oversized steps are refused at input time only, bounded by the count of distinct values a field holds, so day of week rejects */8 while */7 stays legal. Runtime parsing stays lenient so rows saved before the gate keep running the cadence they have. isValidAutomationSchedule now answers only 'acceptable as new input'; a new isRunnableAutomationSchedule answers 'can Orca still run this', and the editor uses it so a legacy row opens intact and can be renamed without re-authoring a schedule that is still firing. Verified: 34/34 corpus expressions fire identically to main. Fixes #16303 Fixes #15895
This commit is contained in:
@@ -0,0 +1,113 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { Automation, AutomationRun } from '../../shared/automations-types'
|
||||
import type { AutomationRunWriter } from './automation-run-writer'
|
||||
import { UNEVALUABLE_SCHEDULE, recordUnevaluableAutomation } from './dispatch-refusal'
|
||||
|
||||
const brokenAutomation: Automation = {
|
||||
id: 'a1',
|
||||
name: 'Broken schedule',
|
||||
prompt: 'Check the repo',
|
||||
precheck: null,
|
||||
agentId: 'claude',
|
||||
projectId: 'r1',
|
||||
executionTargetType: 'local',
|
||||
executionTargetId: 'local',
|
||||
schedulerOwner: 'local_host_service',
|
||||
workspaceMode: 'existing',
|
||||
workspaceId: 'wt1',
|
||||
baseBranch: null,
|
||||
reuseSession: false,
|
||||
timezone: 'UTC',
|
||||
rrule: '0 9 32 * *',
|
||||
dtstart: 0,
|
||||
enabled: true,
|
||||
nextRunAt: 1000,
|
||||
missedRunPolicy: 'run_once_within_grace',
|
||||
missedRunGraceMinutes: 720,
|
||||
createdAt: 0,
|
||||
updatedAt: 0
|
||||
}
|
||||
|
||||
const makeRun = (id: string): AutomationRun => ({
|
||||
id,
|
||||
automationId: brokenAutomation.id,
|
||||
title: 'Broken schedule run',
|
||||
scheduledFor: brokenAutomation.nextRunAt,
|
||||
status: 'pending',
|
||||
trigger: 'scheduled',
|
||||
workspaceId: brokenAutomation.workspaceId,
|
||||
sessionKind: 'terminal',
|
||||
chatSessionId: null,
|
||||
terminalSessionId: null,
|
||||
terminalPaneKey: null,
|
||||
terminalPtyId: null,
|
||||
outputSnapshot: null,
|
||||
precheckResult: null,
|
||||
usage: null,
|
||||
error: null,
|
||||
startedAt: null,
|
||||
dispatchedAt: null,
|
||||
createdAt: 0
|
||||
})
|
||||
|
||||
/** Records what the writer was asked to do, with `repeatSkip` standing in for a fold or not. */
|
||||
function makeRunWriter(foldsRepeat: boolean): {
|
||||
writer: AutomationRunWriter
|
||||
created: string[]
|
||||
updated: { status: string; error?: string | null }[]
|
||||
} {
|
||||
const created: string[] = []
|
||||
const updated: { status: string; error?: string | null }[] = []
|
||||
const writer: AutomationRunWriter = {
|
||||
repeatSkip: () => (foldsRepeat ? makeRun('folded') : null),
|
||||
createRun: () => {
|
||||
const run = makeRun(`run-${created.length + 1}`)
|
||||
created.push(run.id)
|
||||
return run
|
||||
},
|
||||
updateRun: (args) => {
|
||||
updated.push({ status: args.status, error: args.error })
|
||||
return { ...makeRun(args.runId), status: args.status, error: args.error ?? null }
|
||||
}
|
||||
}
|
||||
return { writer, created, updated }
|
||||
}
|
||||
|
||||
describe('recordUnevaluableAutomation', () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks()
|
||||
})
|
||||
|
||||
it('writes one run and logs once when the record is newly broken', () => {
|
||||
const logged = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const { writer, created, updated } = makeRunWriter(false)
|
||||
|
||||
recordUnevaluableAutomation({
|
||||
runs: writer,
|
||||
automation: brokenAutomation,
|
||||
error: new Error('Invalid cron day of month.')
|
||||
})
|
||||
|
||||
expect(created).toEqual(['run-1'])
|
||||
expect(updated).toEqual([{ status: 'skipped_unavailable', error: UNEVALUABLE_SCHEDULE }])
|
||||
expect(logged).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
// The record is retried every tick on purpose, so a repaired schedule resumes on its own.
|
||||
// The fold is what keeps that from writing a row, and logging, once per tick forever.
|
||||
it('stays silent on a record it has already reported', () => {
|
||||
const logged = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
const { writer, created } = makeRunWriter(true)
|
||||
|
||||
for (let tick = 0; tick < 5; tick += 1) {
|
||||
recordUnevaluableAutomation({
|
||||
runs: writer,
|
||||
automation: brokenAutomation,
|
||||
error: new Error('Invalid cron day of month.')
|
||||
})
|
||||
}
|
||||
|
||||
expect(created).toEqual([])
|
||||
expect(logged).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
@@ -5,13 +5,22 @@
|
||||
* coalescing folds repeats only on byte-identical text, so a reason that varied
|
||||
* per occurrence would write a row each.
|
||||
*/
|
||||
import type { WebContents } from 'electron'
|
||||
import type { Store } from '../persistence'
|
||||
import type { Automation } from '../../shared/automations-types'
|
||||
import type {
|
||||
Automation,
|
||||
AutomationDispatchRequest,
|
||||
AutomationRun
|
||||
} from '../../shared/automations-types'
|
||||
import { resolveAutomationRunTarget, type AutomationRunTargetResult } from './run-target-resolution'
|
||||
import type { AutomationRunWriter } from './automation-run-writer'
|
||||
|
||||
export const NO_DISPATCH_HOST = 'No Orca window was available to launch the automation.'
|
||||
|
||||
/** A record the tick could not evaluate at all — its schedule no longer resolves (#16303). */
|
||||
export const UNEVALUABLE_SCHEDULE =
|
||||
'Orca could not evaluate this automation and skipped the occurrence.'
|
||||
|
||||
/** A record the authority refuses to execute at all, with no target diagnosis of its own. */
|
||||
export const NO_RUNNABLE_HOST = 'This automation has no host to run on.'
|
||||
|
||||
@@ -51,3 +60,62 @@ export function recordRefusedAutomationRun(input: {
|
||||
error: target.ok ? NO_RUNNABLE_HOST : target.error
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Marks the poison record the scheduler tick just stepped over, so the user sees why it
|
||||
* stalled. Folds on the fixed sentence and the unchanged nextRunAt, so a record that stays
|
||||
* broken writes one row rather than one per tick, and never throws back into the tick.
|
||||
*/
|
||||
export function recordUnevaluableAutomation(input: {
|
||||
runs: AutomationRunWriter
|
||||
automation: Automation
|
||||
error: unknown
|
||||
}): void {
|
||||
const { automation } = input
|
||||
try {
|
||||
// nextRunAt deliberately stays put: the record is retried so a repaired schedule resumes
|
||||
// on its own. The fold is what keeps that from writing a row — and logging — every tick.
|
||||
if (input.runs.repeatSkip(automation.id, UNEVALUABLE_SCHEDULE, automation.nextRunAt)) {
|
||||
return
|
||||
}
|
||||
console.error('[automations] failed to evaluate automation:', automation.id, input.error)
|
||||
const run = input.runs.createRun(automation, automation.nextRunAt)
|
||||
input.runs.updateRun({
|
||||
runId: run.id,
|
||||
status: 'skipped_unavailable',
|
||||
workspaceId: automation.workspaceId,
|
||||
error: UNEVALUABLE_SCHEDULE
|
||||
})
|
||||
} catch (writeError) {
|
||||
// The original failure has not been reported yet on this path, so carry it too.
|
||||
console.error(
|
||||
'[automations] failed to record unevaluable automation:',
|
||||
automation.id,
|
||||
input.error,
|
||||
writeError
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sends the dispatch request through the renderer channel, closing the run out as
|
||||
* `dispatch_failed` when the send throws — a failed send is not an unreadable schedule.
|
||||
*/
|
||||
export function sendRendererDispatch(
|
||||
channel: Pick<WebContents, 'send'> | null,
|
||||
payload: AutomationDispatchRequest,
|
||||
runs: AutomationRunWriter,
|
||||
run: AutomationRun
|
||||
): AutomationRun {
|
||||
try {
|
||||
channel?.send('automations:dispatchRequested', payload)
|
||||
return run
|
||||
} catch (error) {
|
||||
return runs.updateRun({
|
||||
runId: run.id,
|
||||
status: 'dispatch_failed',
|
||||
workspaceId: run.workspaceId,
|
||||
error: error instanceof Error ? error.message : String(error)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import type { Automation, AutomationRun, AutomationRunUsage } from '../../shared/automations-types'
|
||||
import type { Store } from '../persistence'
|
||||
import type { AutomationRunWriter } from './automation-run-writer'
|
||||
import type { ClaudeUsageStore } from '../claude-usage/store'
|
||||
import type { CodexUsageStore } from '../codex-usage/store'
|
||||
|
||||
@@ -97,3 +99,33 @@ export async function collectAutomationRunUsage({
|
||||
}
|
||||
return unavailable(null, 'provider_unsupported', 'This agent does not report usage to Orca yet.')
|
||||
}
|
||||
|
||||
/** Collects and writes the usage a just-finalized run earned, returning the row to answer with. */
|
||||
export async function writeAutomationRunUsage(input: {
|
||||
store: Store
|
||||
runs: AutomationRunWriter
|
||||
run: AutomationRun
|
||||
claudeUsage: ClaudeUsageStore | null
|
||||
codexUsage: CodexUsageStore | null
|
||||
}): Promise<AutomationRun> {
|
||||
const { store, run } = input
|
||||
const usage = await collectAutomationRunUsage({
|
||||
automation: store.listAutomations().find((entry) => entry.id === run.automationId),
|
||||
run,
|
||||
claudeUsage: input.claudeUsage,
|
||||
codexUsage: input.codexUsage
|
||||
})
|
||||
// Why: the run is final during the await above, so a concurrent create-time
|
||||
// retention prune may have evicted it — the usage write must not throw then.
|
||||
if (!store.listAutomationRuns(run.automationId).some((entry) => entry.id === run.id)) {
|
||||
return run
|
||||
}
|
||||
return input.runs.updateRun({
|
||||
runId: run.id,
|
||||
status: run.status,
|
||||
workspaceId: run.workspaceId,
|
||||
terminalSessionId: run.terminalSessionId,
|
||||
usage,
|
||||
error: run.error
|
||||
})
|
||||
}
|
||||
|
||||
@@ -647,4 +647,158 @@ describe('AutomationService', () => {
|
||||
expect(updated.usage?.status).toBe('unavailable')
|
||||
expect(updated.usage?.unavailableReason).toBe('provider_unsupported')
|
||||
})
|
||||
|
||||
// #16303: listAutomations sorts by name, so 'A ...' is evaluated before 'B ...'.
|
||||
it('keeps evaluating later due automations after an unreadable schedule throws', async () => {
|
||||
vi.setSystemTime(new Date('2026-05-13T08:59:00'))
|
||||
const store = await createStore()
|
||||
store.addRepo(makeRepo())
|
||||
const poison = store.createAutomation({
|
||||
name: 'A poison schedule',
|
||||
prompt: 'Check the repo',
|
||||
agentId: 'claude',
|
||||
projectId: 'r1',
|
||||
workspaceMode: 'existing',
|
||||
workspaceId: 'wt1',
|
||||
timezone: 'UTC',
|
||||
rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0',
|
||||
dtstart: new Date('2026-05-12T00:00:00').getTime()
|
||||
})
|
||||
const healthy = store.createAutomation({
|
||||
name: 'B healthy schedule',
|
||||
prompt: 'Check the repo',
|
||||
agentId: 'claude',
|
||||
projectId: 'r1',
|
||||
workspaceMode: 'existing',
|
||||
workspaceId: 'wt1',
|
||||
timezone: 'UTC',
|
||||
rrule: 'FREQ=DAILY;BYHOUR=9;BYMINUTE=0',
|
||||
dtstart: new Date('2026-05-12T00:00:00').getTime()
|
||||
})
|
||||
// Persisted by an older build, or hand-edited: WEEKLY with no BYDAY cannot resolve a day.
|
||||
mutateDataFile((state) => {
|
||||
const entry = state.automations.find((automation) => automation.id === poison.id)!
|
||||
entry.rrule = 'FREQ=WEEKLY;BYHOUR=9;BYMINUTE=0'
|
||||
})
|
||||
const reloaded = await createStore()
|
||||
const logged = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
|
||||
vi.setSystemTime(new Date('2026-05-13T09:01:00'))
|
||||
const send = vi.fn()
|
||||
const service = new AutomationService(reloaded, { tickMs: 60_000 })
|
||||
service.setWebContents({ isDestroyed: () => false, send })
|
||||
|
||||
service.start()
|
||||
service.setRendererReady()
|
||||
await vi.waitFor(() =>
|
||||
expect(send).toHaveBeenCalledWith('automations:dispatchRequested', expect.any(Object))
|
||||
)
|
||||
service.stop()
|
||||
|
||||
const [, payload] = send.mock.calls[0]
|
||||
expect(payload.automation.id).toBe(healthy.id)
|
||||
expect(reloaded.listAutomationRuns(healthy.id)[0]?.status).toBe('dispatching')
|
||||
const poisonRun = reloaded.listAutomationRuns(poison.id)[0]
|
||||
expect(poisonRun?.status).toBe('skipped_unavailable')
|
||||
expect(poisonRun?.error).toBe(
|
||||
'Orca could not evaluate this automation and skipped the occurrence.'
|
||||
)
|
||||
expect(logged).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// Same isolation, reached through the cron parser rather than the RRULE one, because that
|
||||
// is the path all four cron repairs run on.
|
||||
it('keeps evaluating later due automations after an unreadable cron schedule throws', async () => {
|
||||
vi.setSystemTime(new Date('2026-05-13T08:59:00'))
|
||||
const store = await createStore()
|
||||
store.addRepo(makeRepo())
|
||||
const poison = store.createAutomation({
|
||||
name: 'A poison cron',
|
||||
prompt: 'Check the repo',
|
||||
agentId: 'claude',
|
||||
projectId: 'r1',
|
||||
workspaceMode: 'existing',
|
||||
workspaceId: 'wt1',
|
||||
timezone: 'UTC',
|
||||
rrule: '0 9 * * *',
|
||||
dtstart: new Date('2026-05-12T00:00:00').getTime()
|
||||
})
|
||||
const healthy = store.createAutomation({
|
||||
name: 'B healthy cron',
|
||||
prompt: 'Check the repo',
|
||||
agentId: 'claude',
|
||||
projectId: 'r1',
|
||||
workspaceMode: 'existing',
|
||||
workspaceId: 'wt1',
|
||||
timezone: 'UTC',
|
||||
rrule: '0 9 * * *',
|
||||
dtstart: new Date('2026-05-12T00:00:00').getTime()
|
||||
})
|
||||
// Day of month 32 never validates at input; only a hand-edited or older-build row has it.
|
||||
mutateDataFile((state) => {
|
||||
const entry = state.automations.find((automation) => automation.id === poison.id)!
|
||||
entry.rrule = '0 9 32 * *'
|
||||
})
|
||||
const reloaded = await createStore()
|
||||
const logged = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
|
||||
vi.setSystemTime(new Date('2026-05-13T09:01:00'))
|
||||
const send = vi.fn()
|
||||
const service = new AutomationService(reloaded, { tickMs: 60_000 })
|
||||
service.setWebContents({ isDestroyed: () => false, send })
|
||||
|
||||
service.start()
|
||||
service.setRendererReady()
|
||||
await vi.waitFor(() =>
|
||||
expect(send).toHaveBeenCalledWith('automations:dispatchRequested', expect.any(Object))
|
||||
)
|
||||
service.stop()
|
||||
|
||||
const [, payload] = send.mock.calls[0]
|
||||
expect(payload.automation.id).toBe(healthy.id)
|
||||
expect(reloaded.listAutomationRuns(healthy.id)[0]?.status).toBe('dispatching')
|
||||
expect(reloaded.listAutomationRuns(poison.id)[0]?.error).toBe(
|
||||
'Orca could not evaluate this automation and skipped the occurrence.'
|
||||
)
|
||||
expect(logged).toHaveBeenCalled()
|
||||
})
|
||||
|
||||
// A send that throws is a dispatch failure, not an unreadable schedule: the run must land on
|
||||
// dispatch_failed rather than being left 'dispatching' beside a bogus skipped_unavailable row.
|
||||
it('marks the run dispatch_failed when the renderer send throws', async () => {
|
||||
vi.setSystemTime(new Date('2026-05-13T08:59:00'))
|
||||
const store = await createStore()
|
||||
store.addRepo(makeRepo())
|
||||
const automation = store.createAutomation({
|
||||
name: 'Renderer gone',
|
||||
prompt: 'Check the repo',
|
||||
agentId: 'claude',
|
||||
projectId: 'r1',
|
||||
workspaceMode: 'existing',
|
||||
workspaceId: 'wt1',
|
||||
timezone: 'UTC',
|
||||
rrule: '0 9 * * *',
|
||||
dtstart: new Date('2026-05-12T00:00:00').getTime()
|
||||
})
|
||||
const logged = vi.spyOn(console, 'error').mockImplementation(() => {})
|
||||
logged.mockClear()
|
||||
|
||||
vi.setSystemTime(new Date('2026-05-13T09:01:00'))
|
||||
const send = vi.fn(() => {
|
||||
throw new Error('renderer is gone')
|
||||
})
|
||||
const service = new AutomationService(store, { tickMs: 60_000 })
|
||||
service.setWebContents({ isDestroyed: () => false, send })
|
||||
|
||||
service.start()
|
||||
service.setRendererReady()
|
||||
await vi.waitFor(() => expect(send).toHaveBeenCalled())
|
||||
service.stop()
|
||||
|
||||
const runs = store.listAutomationRuns(automation.id)
|
||||
expect(runs).toHaveLength(1)
|
||||
expect(runs[0]?.status).toBe('dispatch_failed')
|
||||
expect(runs[0]?.error).toBe('renderer is gone')
|
||||
expect(logged).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,4 +1,8 @@
|
||||
import type { WebContents } from 'electron'
|
||||
|
||||
/** All the service asks of the renderer: is it still there, and take this message. Narrower
|
||||
* than WebContents so a test can supply the real shape instead of casting one. */
|
||||
export type AutomationRendererChannel = Pick<WebContents, 'isDestroyed' | 'send'>
|
||||
import type { Store } from '../persistence'
|
||||
import {
|
||||
isFinalAutomationRunStatus,
|
||||
@@ -12,7 +16,7 @@ import type { ClaudeUsageStore } from '../claude-usage/store'
|
||||
import type { CodexUsageStore } from '../codex-usage/store'
|
||||
import { runAutomationPrecheck } from './precheck-runner'
|
||||
import { resolveAutomationRunTarget, type AutomationRunTargetResult } from './run-target-resolution'
|
||||
import { collectAutomationRunUsage } from './run-usage-collection'
|
||||
import { writeAutomationRunUsage } from './run-usage-collection'
|
||||
import type { HeadlessAutomationDispatcher } from './headless-dispatch'
|
||||
import { clearAutomationDispatchTokens, createAutomationDispatchToken } from './dispatch-tokens'
|
||||
import { runHeadlessAutomationDispatch } from './headless-dispatch-runner'
|
||||
@@ -24,6 +28,8 @@ import { createAutomationRunWriter, type AutomationRunWriter } from './automatio
|
||||
import {
|
||||
describeScheduledRefusal,
|
||||
recordRefusedAutomationRun,
|
||||
recordUnevaluableAutomation,
|
||||
sendRendererDispatch,
|
||||
NO_DISPATCH_HOST
|
||||
} from './dispatch-refusal'
|
||||
import type {
|
||||
@@ -37,7 +43,7 @@ export class AutomationService {
|
||||
private readonly store: Store
|
||||
private readonly tickMs: number
|
||||
private timer: ReturnType<typeof setInterval> | null = null
|
||||
private webContents: WebContents | null = null
|
||||
private webContents: AutomationRendererChannel | null = null
|
||||
private rendererReady = false
|
||||
private evaluating = false
|
||||
private readonly claudeUsage: ClaudeUsageStore | null
|
||||
@@ -88,7 +94,7 @@ export class AutomationService {
|
||||
this.publish?.(payload)
|
||||
}
|
||||
|
||||
setWebContents(webContents: WebContents | null): void {
|
||||
setWebContents(webContents: AutomationRendererChannel | null): void {
|
||||
this.webContents = webContents
|
||||
this.rendererReady = false
|
||||
}
|
||||
@@ -204,25 +210,13 @@ export class AutomationService {
|
||||
if (run.usage) {
|
||||
return run
|
||||
}
|
||||
const usage = await collectAutomationRunUsage({
|
||||
automation: this.store.listAutomations().find((entry) => entry.id === run.automationId),
|
||||
return await writeAutomationRunUsage({
|
||||
store: this.store,
|
||||
runs: this.runs,
|
||||
run,
|
||||
claudeUsage: this.claudeUsage,
|
||||
codexUsage: this.codexUsage
|
||||
})
|
||||
// Why: the run is final during the await above, so a concurrent create-time
|
||||
// retention prune may have evicted it — the usage write must not throw then.
|
||||
if (!this.store.listAutomationRuns(run.automationId).some((entry) => entry.id === run.id)) {
|
||||
return run
|
||||
}
|
||||
return this.runs.updateRun({
|
||||
runId: run.id,
|
||||
status: run.status,
|
||||
workspaceId: run.workspaceId,
|
||||
terminalSessionId: run.terminalSessionId,
|
||||
usage,
|
||||
error: run.error
|
||||
})
|
||||
}
|
||||
|
||||
private async evaluateDueRuns(): Promise<void> {
|
||||
@@ -236,7 +230,13 @@ export class AutomationService {
|
||||
if (!automation.enabled || automation.nextRunAt > now) {
|
||||
continue
|
||||
}
|
||||
await this.evaluateAutomation(automation, now)
|
||||
// Isolated per record (#16303): an unreadable schedule throws out of the
|
||||
// occurrence math, and an uncaught throw here skipped every later due row.
|
||||
try {
|
||||
await this.evaluateAutomation(automation, now)
|
||||
} catch (error) {
|
||||
recordUnevaluableAutomation({ runs: this.runs, automation, error })
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
this.evaluating = false
|
||||
@@ -336,7 +336,6 @@ export class AutomationService {
|
||||
run: updated,
|
||||
dispatchToken: createAutomationDispatchToken(automation.id, updated.id)
|
||||
}
|
||||
this.webContents?.send('automations:dispatchRequested', payload)
|
||||
return updated
|
||||
return sendRendererDispatch(this.webContents, payload, this.runs, updated)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -50,6 +50,9 @@ export type AutomationDraft = {
|
||||
time: string
|
||||
dayOfWeek: string
|
||||
customSchedule: string
|
||||
// The cadence this record was opened with, or null for a new one. The strict schedule gate
|
||||
// judges new input; a saved cadence that still runs is not re-judged against it.
|
||||
savedSchedule: string | null
|
||||
missedRunGraceMinutes: string
|
||||
scheduleWarning: string | null
|
||||
}
|
||||
|
||||
@@ -49,6 +49,7 @@ const BASE_DRAFT: AutomationDraft = {
|
||||
dayOfWeek: '5',
|
||||
customSchedule: '',
|
||||
missedRunGraceMinutes: '720',
|
||||
savedSchedule: null,
|
||||
scheduleWarning: null
|
||||
}
|
||||
|
||||
|
||||
@@ -34,6 +34,7 @@ const BASE_DRAFT: AutomationDraft = {
|
||||
dayOfWeek: '1',
|
||||
customSchedule: '',
|
||||
missedRunGraceMinutes: '720',
|
||||
savedSchedule: null,
|
||||
scheduleWarning: null
|
||||
}
|
||||
|
||||
|
||||
@@ -11,6 +11,7 @@ import { buildAutomationCronSchedule } from '../../../../shared/automation-sched
|
||||
import { isValidAutomationSchedule } from '../../../../shared/automation-schedule-parsing'
|
||||
import type { AutomationDraft } from './AutomationEditorDialog'
|
||||
import { AutomationCustomCronPanel } from './AutomationCustomCronPanel'
|
||||
import { acceptsAutomationDraftSchedule } from './automation-schedule-input-gate'
|
||||
import { AutomationTimeField, parseAutomationTime } from './AutomationTimeField'
|
||||
import { Field } from './automation-page-parts'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
@@ -74,10 +75,16 @@ export function AutomationSchedulePicker({
|
||||
}): React.JSX.Element {
|
||||
const customSchedule = draft.customSchedule.trim()
|
||||
const weekdayNames = getUiWeekdayNames()
|
||||
// Same gate the save path uses, so an untouched legacy cadence is not flagged red for a
|
||||
// rule it only has to satisfy as new input.
|
||||
const acceptsSchedule = (schedule: string): boolean =>
|
||||
acceptsAutomationDraftSchedule({
|
||||
customSchedule: schedule,
|
||||
savedRrule: draft.savedSchedule,
|
||||
validate: validateAdvancedSchedule
|
||||
})
|
||||
const customScheduleInvalid =
|
||||
draft.preset === 'custom' &&
|
||||
customSchedule.length > 0 &&
|
||||
!validateAdvancedSchedule(customSchedule)
|
||||
draft.preset === 'custom' && customSchedule.length > 0 && !acceptsSchedule(customSchedule)
|
||||
|
||||
const setTime = (time: string): void => {
|
||||
onDraftChange((current) => ({
|
||||
@@ -119,7 +126,7 @@ export function AutomationSchedulePicker({
|
||||
<AutomationCustomCronPanel
|
||||
draft={draft}
|
||||
customScheduleInvalid={customScheduleInvalid}
|
||||
validateAdvancedSchedule={validateAdvancedSchedule}
|
||||
validateAdvancedSchedule={acceptsSchedule}
|
||||
onDraftChange={onDraftChange}
|
||||
/>
|
||||
) : (
|
||||
|
||||
@@ -14,7 +14,7 @@ import {
|
||||
import type { LucideIcon } from 'lucide-react'
|
||||
import CommentMarkdown from '@/components/sidebar/CommentMarkdown'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { isValidAutomationSchedule } from '../../../../shared/automation-schedule-parsing'
|
||||
import { isRunnableAutomationSchedule } from '../../../../shared/automation-schedule-parsing'
|
||||
import { formatUiAutomationSchedule } from './automation-schedule-label'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { parseHermesOutput, type ParsedHermesSection } from './hermes-cron-output-parse'
|
||||
@@ -33,7 +33,7 @@ function isErrorSection(section: ParsedHermesSection): boolean {
|
||||
|
||||
function getScheduleDisplay(value: string): string | null {
|
||||
const trimmed = value.trim()
|
||||
if (!isValidAutomationSchedule(trimmed)) {
|
||||
if (!isRunnableAutomationSchedule(trimmed)) {
|
||||
return null
|
||||
}
|
||||
return formatUiAutomationSchedule(trimmed)
|
||||
|
||||
@@ -9,8 +9,8 @@
|
||||
import type { Automation, ExternalAutomationJob } from '../../../../shared/automations-types'
|
||||
import { getAutomationRunRepoId } from '../../../../shared/automation-run-identity'
|
||||
import {
|
||||
isValidAutomationCronSchedule,
|
||||
isValidAutomationSchedule,
|
||||
isRunnableAutomationCronSchedule,
|
||||
isRunnableAutomationSchedule,
|
||||
tryParseAutomationRrule
|
||||
} from '../../../../shared/automation-schedule-parsing'
|
||||
import type { AutomationDraft } from './AutomationEditorDialog'
|
||||
@@ -19,7 +19,7 @@ import { getAutomationSetupDecisionDraftValue } from './automation-setup-decisio
|
||||
|
||||
export function buildAutomationEditDraft(automation: Automation): AutomationDraft {
|
||||
const schedule = tryParseAutomationRrule(automation.rrule)
|
||||
const hasCustomSchedule = !schedule && isValidAutomationSchedule(automation.rrule)
|
||||
const hasCustomSchedule = !schedule && isRunnableAutomationSchedule(automation.rrule)
|
||||
return {
|
||||
name: automation.name,
|
||||
prompt: automation.prompt,
|
||||
@@ -39,6 +39,7 @@ export function buildAutomationEditDraft(automation: Automation): AutomationDraf
|
||||
time: schedule ? formatTimeInput(schedule.hour, schedule.minute) : AUTOMATION_DEFAULT_TIME,
|
||||
dayOfWeek: String(schedule?.dayOfWeek ?? 1),
|
||||
customSchedule: hasCustomSchedule ? automation.rrule : '',
|
||||
savedSchedule: automation.rrule,
|
||||
missedRunGraceMinutes: String(automation.missedRunGraceMinutes),
|
||||
scheduleWarning:
|
||||
schedule || hasCustomSchedule
|
||||
@@ -52,7 +53,7 @@ export function buildExternalAutomationEditDraft(
|
||||
placement: { projectId: string; workspaceId: string }
|
||||
): AutomationDraft {
|
||||
const rawSchedule = job.rawSchedule?.trim() ?? ''
|
||||
const hasCustomSchedule = isValidAutomationCronSchedule(rawSchedule)
|
||||
const hasCustomSchedule = isRunnableAutomationCronSchedule(rawSchedule)
|
||||
return {
|
||||
name: job.name,
|
||||
prompt: job.prompt ?? job.promptPreview,
|
||||
@@ -69,6 +70,7 @@ export function buildExternalAutomationEditDraft(
|
||||
time: AUTOMATION_DEFAULT_TIME,
|
||||
dayOfWeek: '1',
|
||||
customSchedule: hasCustomSchedule ? rawSchedule : '',
|
||||
savedSchedule: rawSchedule || null,
|
||||
missedRunGraceMinutes: '720',
|
||||
scheduleWarning: hasCustomSchedule
|
||||
? null
|
||||
|
||||
@@ -0,0 +1,118 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import type { Automation } from '../../../../shared/automations-types'
|
||||
import {
|
||||
isRunnableAutomationSchedule,
|
||||
isValidAutomationSchedule
|
||||
} from '../../../../shared/automation-schedule-parsing'
|
||||
import { buildAutomationEditDraft } from './automation-edit-draft'
|
||||
import { getCronScheduleStatusLabel } from './AutomationCustomCronPanel'
|
||||
import { acceptsAutomationDraftSchedule } from './automation-schedule-input-gate'
|
||||
|
||||
// `*/90` on minutes was accepted before the oversized-step refusal (#15895) and still runs,
|
||||
// firing at :00. Only a row persisted by an older build can hold it.
|
||||
const LEGACY_OVERSIZED_STEP = '*/90 * * * *'
|
||||
|
||||
const makeAutomation = (overrides: Partial<Automation> = {}): Automation => ({
|
||||
id: 'a1',
|
||||
name: 'Nightly sweep',
|
||||
prompt: 'Check the repo',
|
||||
precheck: null,
|
||||
agentId: 'claude',
|
||||
projectId: 'r1',
|
||||
executionTargetType: 'local',
|
||||
executionTargetId: 'local',
|
||||
schedulerOwner: 'local_host_service',
|
||||
workspaceMode: 'existing',
|
||||
workspaceId: 'wt1',
|
||||
baseBranch: null,
|
||||
reuseSession: false,
|
||||
timezone: 'UTC',
|
||||
rrule: LEGACY_OVERSIZED_STEP,
|
||||
dtstart: 0,
|
||||
enabled: true,
|
||||
nextRunAt: 0,
|
||||
missedRunPolicy: 'run_once_within_grace',
|
||||
missedRunGraceMinutes: 720,
|
||||
createdAt: 0,
|
||||
updatedAt: 0,
|
||||
...overrides
|
||||
})
|
||||
|
||||
describe('editing an automation whose saved schedule predates the input gate', () => {
|
||||
it('still refuses the same expression as new input', () => {
|
||||
expect(isValidAutomationSchedule(LEGACY_OVERSIZED_STEP)).toBe(false)
|
||||
expect(isRunnableAutomationSchedule(LEGACY_OVERSIZED_STEP)).toBe(true)
|
||||
})
|
||||
|
||||
it('opens the editor with the saved schedule intact and no warning', () => {
|
||||
const draft = buildAutomationEditDraft(makeAutomation())
|
||||
expect(draft.preset).toBe('custom')
|
||||
expect(draft.customSchedule).toBe(LEGACY_OVERSIZED_STEP)
|
||||
expect(draft.scheduleWarning).toBeNull()
|
||||
})
|
||||
|
||||
// The regression this guards: a rename was blocked behind re-authoring a schedule the
|
||||
// user never touched and that is still firing.
|
||||
it('lets a rename through without re-authoring the schedule', () => {
|
||||
const automation = makeAutomation()
|
||||
const renamed = { ...buildAutomationEditDraft(automation), name: 'Renamed sweep' }
|
||||
expect(renamed.scheduleWarning).toBeNull()
|
||||
expect(
|
||||
acceptsAutomationDraftSchedule({
|
||||
customSchedule: renamed.customSchedule,
|
||||
savedRrule: automation.rrule,
|
||||
validate: isValidAutomationSchedule
|
||||
})
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('still refuses a schedule the user actually changes', () => {
|
||||
const automation = makeAutomation()
|
||||
for (const edited of ['*/91 * * * *', '0 */25 * * *', 'nonsense']) {
|
||||
expect(
|
||||
acceptsAutomationDraftSchedule({
|
||||
customSchedule: edited,
|
||||
savedRrule: automation.rrule,
|
||||
validate: isValidAutomationSchedule
|
||||
})
|
||||
).toBe(false)
|
||||
}
|
||||
})
|
||||
|
||||
it('refuses an oversized step on a new automation, which has nothing saved', () => {
|
||||
expect(
|
||||
acceptsAutomationDraftSchedule({
|
||||
customSchedule: LEGACY_OVERSIZED_STEP,
|
||||
savedRrule: null,
|
||||
validate: isValidAutomationSchedule
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
// The editor's live cron status runs the same gate, so an untouched legacy cadence is not
|
||||
// painted red with "fix this before saving" for a rule it only owes as new input.
|
||||
it('reports the saved schedule as valid in the editor cron status', () => {
|
||||
const draft = buildAutomationEditDraft(makeAutomation())
|
||||
const accepts = (schedule: string): boolean =>
|
||||
acceptsAutomationDraftSchedule({
|
||||
customSchedule: schedule,
|
||||
savedRrule: draft.savedSchedule,
|
||||
validate: isValidAutomationSchedule
|
||||
})
|
||||
expect(getCronScheduleStatusLabel(draft.customSchedule, accepts).kind).toBe('valid')
|
||||
// A different oversized step is new input, so it is still called out.
|
||||
expect(getCronScheduleStatusLabel('*/91 * * * *', accepts).kind).toBe('invalid')
|
||||
})
|
||||
|
||||
it('carries the saved schedule on the draft so the gate can see it', () => {
|
||||
expect(buildAutomationEditDraft(makeAutomation()).savedSchedule).toBe(LEGACY_OVERSIZED_STEP)
|
||||
})
|
||||
|
||||
// Leniency is scoped to the oversized-step gate; a schedule that cannot parse at all is
|
||||
// still unrepresentable and must keep warning rather than silently round-trip.
|
||||
it('keeps warning about a saved schedule that cannot be parsed', () => {
|
||||
const draft = buildAutomationEditDraft(makeAutomation({ rrule: '0 9 32 * *' }))
|
||||
expect(draft.customSchedule).toBe('')
|
||||
expect(draft.scheduleWarning).toBeTruthy()
|
||||
})
|
||||
})
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
isValidAutomationSchedule
|
||||
} from '../../../../shared/automation-schedule-parsing'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
import { acceptsAutomationDraftSchedule } from './automation-schedule-input-gate'
|
||||
import { parseDraftTime } from './automation-draft-model'
|
||||
import { saveHermesAutomation } from './automation-hermes-save'
|
||||
import { saveOrcaAutomation } from './automation-orca-save'
|
||||
@@ -51,7 +52,14 @@ export function createAutomationSaveAction(context: AutomationSaveContext) {
|
||||
const validateAdvancedSchedule = isHermesSave
|
||||
? isValidAutomationCronSchedule
|
||||
: isValidAutomationSchedule
|
||||
if (draft.preset === 'custom' && !validateAdvancedSchedule(draft.customSchedule)) {
|
||||
if (
|
||||
draft.preset === 'custom' &&
|
||||
!acceptsAutomationDraftSchedule({
|
||||
customSchedule: draft.customSchedule,
|
||||
savedRrule: draft.savedSchedule,
|
||||
validate: validateAdvancedSchedule
|
||||
})
|
||||
) {
|
||||
toast.error(
|
||||
translate(
|
||||
'auto.components.automations.AutomationsPage.6e91dab317',
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
/**
|
||||
* Decides when the editor's strict schedule gate applies.
|
||||
*
|
||||
* The gate judges a schedule the user is introducing or changing. A cadence already saved and
|
||||
* still running is left alone: rows written before the oversized-step refusal (#15895) stay
|
||||
* valid to run but not to re-enter, and re-judging one would block edits that never touched
|
||||
* the schedule — a rename, a prompt change — behind re-authoring it.
|
||||
*/
|
||||
export function acceptsAutomationDraftSchedule(input: {
|
||||
customSchedule: string
|
||||
savedRrule: string | null
|
||||
validate: (schedule: string) => boolean
|
||||
}): boolean {
|
||||
const schedule = input.customSchedule.trim()
|
||||
if (input.savedRrule !== null && input.savedRrule.trim() === schedule) {
|
||||
return true
|
||||
}
|
||||
return input.validate(schedule)
|
||||
}
|
||||
@@ -2,7 +2,7 @@ import type {
|
||||
ExternalAutomationJob,
|
||||
ExternalAutomationManager
|
||||
} from '../../../../shared/automations-types'
|
||||
import { isValidAutomationCronSchedule } from '../../../../shared/automation-schedule-parsing'
|
||||
import { isRunnableAutomationCronSchedule } from '../../../../shared/automation-schedule-parsing'
|
||||
import { formatUiAutomationSchedule } from './automation-schedule-label'
|
||||
import { translate } from '@/i18n/i18n'
|
||||
|
||||
@@ -27,7 +27,7 @@ export function getExternalAutomationScheduleDisplay(
|
||||
]
|
||||
|
||||
for (const candidate of candidateSchedules) {
|
||||
if (candidate && isValidAutomationCronSchedule(candidate)) {
|
||||
if (candidate && isRunnableAutomationCronSchedule(candidate)) {
|
||||
return { label: formatUiAutomationSchedule(candidate) }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -76,6 +76,7 @@ export function useAutomationEditorActions({
|
||||
dayOfWeek: '1',
|
||||
customSchedule: '',
|
||||
missedRunGraceMinutes: '720',
|
||||
savedSchedule: null,
|
||||
scheduleWarning: null
|
||||
}
|
||||
const nextDraft = template
|
||||
|
||||
@@ -147,6 +147,7 @@ export function useAutomationsPageLocalState(store: AutomationsPageStoreState) {
|
||||
dayOfWeek: '1',
|
||||
customSchedule: '',
|
||||
missedRunGraceMinutes: '720',
|
||||
savedSchedule: null,
|
||||
scheduleWarning: null
|
||||
})
|
||||
const draftRef = useRef(draft)
|
||||
|
||||
@@ -0,0 +1,133 @@
|
||||
// Cron field parsing for Orca's automation schedules.
|
||||
// A field step is bounded by the count of distinct values the field holds: a step of 90 on
|
||||
// minutes is one value at :00, never "every 90 minutes", so it is refused as input (#15895).
|
||||
export type CronParseOptions = {
|
||||
/** Input-time gate: reject a step wider than the field's domain instead of silently
|
||||
* degenerating to a single value. Off for persisted rows, which must keep running the
|
||||
* cadence they were saved with rather than start throwing mid-tick. */
|
||||
rejectOversizedStep?: boolean
|
||||
}
|
||||
|
||||
export const MONTH_NAMES: Record<string, number> = {
|
||||
JAN: 1,
|
||||
FEB: 2,
|
||||
MAR: 3,
|
||||
APR: 4,
|
||||
MAY: 5,
|
||||
JUN: 6,
|
||||
JUL: 7,
|
||||
AUG: 8,
|
||||
SEP: 9,
|
||||
OCT: 10,
|
||||
NOV: 11,
|
||||
DEC: 12
|
||||
}
|
||||
|
||||
export const DAY_NAMES: Record<string, number> = {
|
||||
SU: 0,
|
||||
MO: 1,
|
||||
TU: 2,
|
||||
WE: 3,
|
||||
TH: 4,
|
||||
FR: 5,
|
||||
SA: 6,
|
||||
SUN: 0,
|
||||
MON: 1,
|
||||
TUE: 2,
|
||||
WED: 3,
|
||||
THU: 4,
|
||||
FRI: 5,
|
||||
SAT: 6
|
||||
}
|
||||
|
||||
function parseCronNumber(
|
||||
value: string,
|
||||
names: Record<string, number> | null,
|
||||
field: string
|
||||
): number {
|
||||
const normalized = value.toUpperCase()
|
||||
const named = names?.[normalized]
|
||||
const parsed = named ?? Number(normalized)
|
||||
if (!Number.isInteger(parsed)) {
|
||||
throw new Error(`Invalid cron ${field}.`)
|
||||
}
|
||||
return parsed
|
||||
}
|
||||
|
||||
export function parseCronField(args: {
|
||||
value: string
|
||||
min: number
|
||||
max: number
|
||||
field: string
|
||||
names?: Record<string, number>
|
||||
normalize?: (value: number) => number
|
||||
// Distinct values the field holds, when `normalize` aliases some away — day of week
|
||||
// spans 0-7 but holds seven days, so `*/8` is oversized even though 8 <= 7-0+1.
|
||||
distinctValueCount?: number
|
||||
rejectOversizedStep?: boolean
|
||||
}): Set<number> {
|
||||
const result = new Set<number>()
|
||||
for (const rawPart of args.value.split(',')) {
|
||||
const part = rawPart.trim()
|
||||
if (!part) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
const stepParts = part.split('/')
|
||||
if (stepParts.length > 2) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
const [rangePart, stepPart] = stepParts
|
||||
if (!rangePart) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
const step = stepPart === undefined ? 1 : Number(stepPart)
|
||||
if (!Number.isInteger(step) || step < 1) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
const domainSize = args.distinctValueCount ?? args.max - args.min + 1
|
||||
if (args.rejectOversizedStep && step > domainSize) {
|
||||
throw new Error(`Cron ${args.field} step must be between 1 and ${domainSize}.`)
|
||||
}
|
||||
|
||||
let start: number
|
||||
let end: number
|
||||
if (rangePart === '*') {
|
||||
start = args.min
|
||||
end = args.max
|
||||
} else if (rangePart.includes('-')) {
|
||||
const rangeParts = rangePart.split('-')
|
||||
if (rangeParts.length !== 2 || !rangeParts[0] || !rangeParts[1]) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
const [startPart, endPart] = rangeParts
|
||||
start = parseCronNumber(startPart, args.names ?? null, args.field)
|
||||
end = parseCronNumber(endPart, args.names ?? null, args.field)
|
||||
} else {
|
||||
start = parseCronNumber(rangePart, args.names ?? null, args.field)
|
||||
end = start
|
||||
}
|
||||
|
||||
const normalizedStart = args.normalize?.(start) ?? start
|
||||
const normalizedEnd = args.normalize?.(end) ?? end
|
||||
if (
|
||||
start < args.min ||
|
||||
start > args.max ||
|
||||
end < args.min ||
|
||||
end > args.max ||
|
||||
normalizedStart < args.min ||
|
||||
normalizedStart > args.max ||
|
||||
normalizedEnd < args.min ||
|
||||
normalizedEnd > args.max ||
|
||||
start > end
|
||||
) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
for (let value = start; value <= end; value += step) {
|
||||
result.add(args.normalize?.(value) ?? value)
|
||||
}
|
||||
}
|
||||
if (result.size === 0) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
return result
|
||||
}
|
||||
@@ -0,0 +1,101 @@
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
import { nextAutomationOccurrenceAfter } from './automation-schedule-occurrences'
|
||||
import {
|
||||
isValidAutomationCronSchedule,
|
||||
isValidAutomationSchedule,
|
||||
isRunnableAutomationSchedule,
|
||||
parseCronExpression
|
||||
} from './automation-schedule-parsing'
|
||||
|
||||
const ascending = (values: Set<number>): number[] => [...values].sort((left, right) => left - right)
|
||||
|
||||
describe('cron oversized steps (#15895)', () => {
|
||||
it('refuses a step wider than the field domain at input time', () => {
|
||||
expect(isValidAutomationSchedule('*/90 * * * *')).toBe(false)
|
||||
expect(isValidAutomationCronSchedule('*/90 * * * *')).toBe(false)
|
||||
expect(isValidAutomationSchedule('0 */25 * * *')).toBe(false)
|
||||
expect(isValidAutomationSchedule('0 9 */32 * *')).toBe(false)
|
||||
expect(isValidAutomationSchedule('0 9 * */13 *')).toBe(false)
|
||||
// Day of week spans 0-7 but holds seven days, so 8 is oversized even though 8 <= 7-0+1.
|
||||
expect(isValidAutomationSchedule('0 9 * * */8')).toBe(false)
|
||||
expect(() => parseCronExpression('*/90 * * * *', { rejectOversizedStep: true })).toThrow(
|
||||
'Cron minute step must be between 1 and 60.'
|
||||
)
|
||||
})
|
||||
|
||||
it('keeps every step that fits its field domain', () => {
|
||||
expect(isValidAutomationSchedule('*/15 * * * *')).toBe(true)
|
||||
expect(isValidAutomationSchedule('*/60 * * * *')).toBe(true)
|
||||
expect(isValidAutomationSchedule('0 */24 * * *')).toBe(true)
|
||||
expect(isValidAutomationSchedule('0 9 */31 * *')).toBe(true)
|
||||
expect(isValidAutomationSchedule('0 9 * */12 *')).toBe(true)
|
||||
expect(isValidAutomationSchedule('0 9 * * */7')).toBe(true)
|
||||
})
|
||||
|
||||
// The gate is input-only. A row persisted before it keeps running the cadence it was saved
|
||||
// with rather than throwing mid-tick, which is what keeps it editable (see the editor tests).
|
||||
it('still runs a persisted oversized step, degenerating it to its single value', () => {
|
||||
expect(isRunnableAutomationSchedule('*/90 * * * *')).toBe(true)
|
||||
expect(ascending(parseCronExpression('*/90 * * * *').minutes)).toEqual([0])
|
||||
expect(
|
||||
nextAutomationOccurrenceAfter(
|
||||
'*/90 * * * *',
|
||||
new Date(2026, 4, 1, 0, 0).getTime(),
|
||||
new Date(2026, 4, 15, 9, 5).getTime()
|
||||
)
|
||||
).toBe(new Date(2026, 4, 15, 10, 0).getTime())
|
||||
})
|
||||
})
|
||||
|
||||
// Node reads the OS timezone on Windows and ignores a runtime process.env.TZ change, so the
|
||||
// stub — and the precondition asserting it took — cannot work there.
|
||||
describe.skipIf(process.platform === 'win32')('cron occurrence local-time controls', () => {
|
||||
afterEach(() => {
|
||||
vi.unstubAllEnvs()
|
||||
})
|
||||
|
||||
it('skips the wall-clock hour that local spring-forward removes', () => {
|
||||
vi.stubEnv('TZ', 'America/New_York')
|
||||
// Precondition: the stub took and 2026-03-08 really does lose an hour here.
|
||||
expect(new Date(2026, 2, 8, 12).getTimezoneOffset()).toBe(
|
||||
new Date(2026, 2, 8, 0).getTimezoneOffset() - 60
|
||||
)
|
||||
|
||||
expect(
|
||||
nextAutomationOccurrenceAfter(
|
||||
'30 2 * * *',
|
||||
new Date(2026, 0, 1).getTime(),
|
||||
new Date(2026, 2, 8, 0, 0).getTime()
|
||||
)
|
||||
).toBe(new Date(2026, 2, 9, 2, 30).getTime())
|
||||
})
|
||||
|
||||
it('fires both repeats of the wall-clock hour local fall-back replays', () => {
|
||||
vi.stubEnv('TZ', 'America/New_York')
|
||||
expect(new Date(2026, 10, 1, 12).getTimezoneOffset()).toBe(
|
||||
new Date(2026, 10, 1, 0).getTimezoneOffset() + 60
|
||||
)
|
||||
|
||||
const first = nextAutomationOccurrenceAfter(
|
||||
'30 1 * * *',
|
||||
new Date(2026, 0, 1).getTime(),
|
||||
new Date(2026, 10, 1, 0, 0).getTime()
|
||||
)
|
||||
const second = nextAutomationOccurrenceAfter(
|
||||
'30 1 * * *',
|
||||
new Date(2026, 0, 1).getTime(),
|
||||
first
|
||||
)
|
||||
|
||||
expect(new Date(first).getHours()).toBe(1)
|
||||
expect(new Date(second).getHours()).toBe(1)
|
||||
expect(second - first).toBe(60 * 60 * 1000)
|
||||
|
||||
// Pre-existing limit this change does not touch or fix (#20154): local 01:30 is ambiguous,
|
||||
// so flooring it rebuilds the earlier EDT instant and the scan lands back on the EST repeat
|
||||
// instead of tomorrow. Pinned so a later DST fix has to update it deliberately.
|
||||
expect(
|
||||
nextAutomationOccurrenceAfter('30 1 * * *', new Date(2026, 0, 1).getTime(), second)
|
||||
).toBe(second)
|
||||
})
|
||||
})
|
||||
@@ -2,6 +2,12 @@ import type { AutomationSchedulePreset } from './automations-types'
|
||||
import { cronHasPossibleOccurrence } from './automation-cron-occurrence'
|
||||
|
||||
import { isClipboardTextByteLengthOverLimit } from './clipboard-text'
|
||||
import {
|
||||
DAY_NAMES,
|
||||
MONTH_NAMES,
|
||||
parseCronField,
|
||||
type CronParseOptions
|
||||
} from './automation-cron-field-parsing'
|
||||
|
||||
export const AUTOMATION_CRON_EXPRESSION_MAX_BYTES = 2 * 1024
|
||||
export type ParsedRrule = {
|
||||
@@ -24,38 +30,10 @@ export type ParsedCron = {
|
||||
}
|
||||
|
||||
export type ParsedSchedule = ParsedRrule | ParsedCron
|
||||
export type { CronParseOptions }
|
||||
|
||||
const DAY_CODES = ['SU', 'MO', 'TU', 'WE', 'TH', 'FR', 'SA'] as const
|
||||
const WEEKDAY_CODES = ['MO', 'TU', 'WE', 'TH', 'FR'] as const
|
||||
const MONTH_NAMES: Record<string, number> = {
|
||||
JAN: 1,
|
||||
FEB: 2,
|
||||
MAR: 3,
|
||||
APR: 4,
|
||||
MAY: 5,
|
||||
JUN: 6,
|
||||
JUL: 7,
|
||||
AUG: 8,
|
||||
SEP: 9,
|
||||
OCT: 10,
|
||||
NOV: 11,
|
||||
DEC: 12
|
||||
}
|
||||
const DAY_NAMES: Record<string, number> = {
|
||||
SU: 0,
|
||||
MO: 1,
|
||||
TU: 2,
|
||||
WE: 3,
|
||||
TH: 4,
|
||||
FR: 5,
|
||||
SA: 6,
|
||||
SUN: 0,
|
||||
MON: 1,
|
||||
TUE: 2,
|
||||
WED: 3,
|
||||
THU: 4,
|
||||
FRI: 5,
|
||||
SAT: 6
|
||||
}
|
||||
|
||||
function parseRrule(rrule: string): ParsedRrule {
|
||||
const entries = new Map<string, string>()
|
||||
@@ -88,101 +66,22 @@ function parseRrule(rrule: string): ParsedRrule {
|
||||
return { kind: 'rrule', freq, byDay, byHour, byMinute }
|
||||
}
|
||||
|
||||
function parseCronNumber(
|
||||
value: string,
|
||||
names: Record<string, number> | null,
|
||||
field: string
|
||||
): number {
|
||||
const normalized = value.toUpperCase()
|
||||
const named = names?.[normalized]
|
||||
const parsed = named ?? Number(normalized)
|
||||
if (!Number.isInteger(parsed)) {
|
||||
throw new Error(`Invalid cron ${field}.`)
|
||||
}
|
||||
return parsed
|
||||
}
|
||||
|
||||
function parseCronField(args: {
|
||||
value: string
|
||||
min: number
|
||||
max: number
|
||||
field: string
|
||||
names?: Record<string, number>
|
||||
normalize?: (value: number) => number
|
||||
}): Set<number> {
|
||||
const result = new Set<number>()
|
||||
for (const rawPart of args.value.split(',')) {
|
||||
const part = rawPart.trim()
|
||||
if (!part) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
const stepParts = part.split('/')
|
||||
if (stepParts.length > 2) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
const [rangePart, stepPart] = stepParts
|
||||
if (!rangePart) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
const step = stepPart === undefined ? 1 : Number(stepPart)
|
||||
if (!Number.isInteger(step) || step < 1) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
|
||||
let start: number
|
||||
let end: number
|
||||
if (rangePart === '*') {
|
||||
start = args.min
|
||||
end = args.max
|
||||
} else if (rangePart.includes('-')) {
|
||||
const rangeParts = rangePart.split('-')
|
||||
if (rangeParts.length !== 2 || !rangeParts[0] || !rangeParts[1]) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
const [startPart, endPart] = rangeParts
|
||||
start = parseCronNumber(startPart, args.names ?? null, args.field)
|
||||
end = parseCronNumber(endPart, args.names ?? null, args.field)
|
||||
} else {
|
||||
start = parseCronNumber(rangePart, args.names ?? null, args.field)
|
||||
end = start
|
||||
}
|
||||
|
||||
const normalizedStart = args.normalize?.(start) ?? start
|
||||
const normalizedEnd = args.normalize?.(end) ?? end
|
||||
if (
|
||||
start < args.min ||
|
||||
start > args.max ||
|
||||
end < args.min ||
|
||||
end > args.max ||
|
||||
normalizedStart < args.min ||
|
||||
normalizedStart > args.max ||
|
||||
normalizedEnd < args.min ||
|
||||
normalizedEnd > args.max ||
|
||||
start > end
|
||||
) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
for (let value = start; value <= end; value += step) {
|
||||
result.add(args.normalize?.(value) ?? value)
|
||||
}
|
||||
}
|
||||
if (result.size === 0) {
|
||||
throw new Error(`Invalid cron ${args.field}.`)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
export function parseCronExpression(expression: string): ParsedCron {
|
||||
export function parseCronExpression(
|
||||
expression: string,
|
||||
options: CronParseOptions = {}
|
||||
): ParsedCron {
|
||||
const parts = getAutomationCronExpressionFields(expression, 6)
|
||||
if (parts.length !== 5) {
|
||||
throw new Error('Cron schedule must have five fields.')
|
||||
}
|
||||
const [minute, hour, dayOfMonth, month, dayOfWeek] = parts
|
||||
const rejectOversizedStep = options.rejectOversizedStep ?? false
|
||||
const daysOfMonth = parseCronField({
|
||||
value: dayOfMonth,
|
||||
min: 1,
|
||||
max: 31,
|
||||
field: 'day of month'
|
||||
field: 'day of month',
|
||||
rejectOversizedStep
|
||||
})
|
||||
const daysOfWeek = parseCronField({
|
||||
value: dayOfWeek,
|
||||
@@ -190,14 +89,29 @@ export function parseCronExpression(expression: string): ParsedCron {
|
||||
max: 7,
|
||||
field: 'day of week',
|
||||
names: DAY_NAMES,
|
||||
normalize: (value) => (value === 7 ? 0 : value)
|
||||
normalize: (value) => (value === 7 ? 0 : value),
|
||||
distinctValueCount: 7,
|
||||
rejectOversizedStep
|
||||
})
|
||||
return {
|
||||
kind: 'cron',
|
||||
minutes: parseCronField({ value: minute, min: 0, max: 59, field: 'minute' }),
|
||||
hours: parseCronField({ value: hour, min: 0, max: 23, field: 'hour' }),
|
||||
minutes: parseCronField({
|
||||
value: minute,
|
||||
min: 0,
|
||||
max: 59,
|
||||
field: 'minute',
|
||||
rejectOversizedStep
|
||||
}),
|
||||
hours: parseCronField({ value: hour, min: 0, max: 23, field: 'hour', rejectOversizedStep }),
|
||||
daysOfMonth,
|
||||
months: parseCronField({ value: month, min: 1, max: 12, field: 'month', names: MONTH_NAMES }),
|
||||
months: parseCronField({
|
||||
value: month,
|
||||
min: 1,
|
||||
max: 12,
|
||||
field: 'month',
|
||||
names: MONTH_NAMES,
|
||||
rejectOversizedStep
|
||||
}),
|
||||
daysOfWeek,
|
||||
dayOfMonthRestricted: daysOfMonth.size !== 31,
|
||||
dayOfWeekRestricted: daysOfWeek.size !== 7
|
||||
@@ -245,33 +159,50 @@ function isAutomationCronFieldWhitespace(code: number): boolean {
|
||||
)
|
||||
}
|
||||
|
||||
export function parseSchedule(schedule: string): ParsedSchedule {
|
||||
export function parseSchedule(schedule: string, options: CronParseOptions = {}): ParsedSchedule {
|
||||
const trimmed = schedule.trim()
|
||||
if (trimmed.includes('=')) {
|
||||
return parseRrule(trimmed)
|
||||
}
|
||||
return parseCronExpression(trimmed)
|
||||
return parseCronExpression(trimmed, options)
|
||||
}
|
||||
|
||||
export function isValidAutomationSchedule(schedule: string): boolean {
|
||||
function scheduleRuns(schedule: string, options: CronParseOptions): boolean {
|
||||
try {
|
||||
const parsed = parseSchedule(schedule)
|
||||
if (parsed.kind === 'cron' && !cronHasPossibleOccurrence(parsed, Date.now())) {
|
||||
throw new Error('Cron schedule has no possible run.')
|
||||
}
|
||||
return true
|
||||
const parsed = parseSchedule(schedule, options)
|
||||
return parsed.kind !== 'cron' || cronHasPossibleOccurrence(parsed, Date.now())
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
function cronScheduleRuns(schedule: string, options: CronParseOptions): boolean {
|
||||
try {
|
||||
return cronHasPossibleOccurrence(parseCronExpression(schedule.trim(), options), Date.now())
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
/** Accepts a schedule as new input, oversized-step refusal included (#15895). */
|
||||
export function isValidAutomationSchedule(schedule: string): boolean {
|
||||
return scheduleRuns(schedule, { rejectOversizedStep: true })
|
||||
}
|
||||
|
||||
export function isValidAutomationCronSchedule(schedule: string): boolean {
|
||||
try {
|
||||
const parsed = parseCronExpression(schedule.trim())
|
||||
return cronHasPossibleOccurrence(parsed, Date.now())
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
return cronScheduleRuns(schedule, { rejectOversizedStep: true })
|
||||
}
|
||||
|
||||
// Whether Orca can still run a schedule it did not just receive. A row saved before the
|
||||
// oversized-step gate, or one a provider owns, keeps running the cadence it has, so reading
|
||||
// it back must not re-judge it as input — otherwise renaming an automation would demand
|
||||
// re-authoring a schedule the user never touched.
|
||||
export function isRunnableAutomationSchedule(schedule: string): boolean {
|
||||
return scheduleRuns(schedule, {})
|
||||
}
|
||||
|
||||
export function isRunnableAutomationCronSchedule(schedule: string): boolean {
|
||||
return cronScheduleRuns(schedule, {})
|
||||
}
|
||||
|
||||
export function parseAutomationRrule(rrule: string): {
|
||||
|
||||
Reference in New Issue
Block a user