From 6b26cc00e0fbddbd0e6d1bcf8eb61cec069ad712 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 1 Jun 2026 17:19:19 +0800 Subject: [PATCH 01/13] feat(automation): close PR1-5 backend gaps before frontend slice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds the AutomationDefinition contract pieces missed by PR1-5 so the PR6/PR7 frontend can build on a complete backend surface: - `model: { providerID, modelID }` required on every definition; passed through to `SessionPrompt.promptWithAutomationContext` so runs stop depending on the runtime model fallback chain. - `variant?: string` optional reasoning/effort selection; validated against `ProviderTransform.variants(model)` so traffic that's not valid for the chosen model returns 422 instead of failing late in the run. - `stop.kind === "condition"` is now rejected with `unsupported_stop_condition` at create/update — the scheduler never scheduled it, and the silent-accept behaviour leaked UI surface area. - `nextFireAt`, `nextFires`, `failureStreak` are now maintained on create, on update, and after every terminal run; the scheduler publishes `automation.definition.updated` so global-sync sees the derived state. - `automate` tool schema picks up `model`/`variant`, gets a fake Provider for tests, and the description spells out the expected shape and an example so the LLM submits valid inputs. A SQL migration drops any pre-release automation rows because the existing payloads cannot be parsed against the new required `model` field; the feature is still gated behind `OPENCODE_ENABLE_AUTOMATE_TOOL` so no user-visible data is lost. Out of scope (deferred to follow-ups): - `needs_user_input` / `loop_gate` real-world production. Both are reserved in the run error code enum; producing them requires changing prompt loop semantics (unattended question abort + loop gate failure surfacing) which is too big for PR5.5. - `Session.automationID?` reverse lookup. That's a session contract migration; tracked separately and not needed by PR6. Verification: - `bun typecheck` (repo) clean - `bun test test/server/automation-*.test.ts test/tool/automate.test.ts` 118 pass / 0 fail - `bun test test/session/processor-effect.test.ts` 35 pass / 0 fail - SDK regen produces no diff against manually-edited types --- .../migration.sql | 7 ++ packages/opencode/src/automation/derived.ts | 111 ++++++++++++++++++ packages/opencode/src/automation/fixtures.ts | 12 +- packages/opencode/src/automation/index.ts | 77 +++++++++++- packages/opencode/src/automation/runner.ts | 2 + packages/opencode/src/automation/scheduler.ts | 8 ++ .../opencode/src/automation/validation.ts | 47 ++++++++ .../src/server/instance/automation.ts | 31 ++++- packages/opencode/src/tool/automate.ts | 40 +++++-- packages/opencode/test/fake/provider.ts | 39 ++++++ .../test/server/automation-routes.test.ts | 11 +- .../test/server/automation-runner.test.ts | 3 + .../test/server/automation-scheduler.test.ts | 45 +++---- packages/opencode/test/tool/automate.test.ts | 58 ++++++++- packages/sdk/js/src/v2/event-types.test-d.ts | 1 + packages/sdk/js/src/v2/gen/types.gen.ts | 15 +++ 16 files changed, 464 insertions(+), 43 deletions(-) create mode 100644 packages/opencode/migration/20260601100000_automation_model_required/migration.sql create mode 100644 packages/opencode/src/automation/derived.ts create mode 100644 packages/opencode/src/automation/validation.ts diff --git a/packages/opencode/migration/20260601100000_automation_model_required/migration.sql b/packages/opencode/migration/20260601100000_automation_model_required/migration.sql new file mode 100644 index 000000000..5cfdb436c --- /dev/null +++ b/packages/opencode/migration/20260601100000_automation_model_required/migration.sql @@ -0,0 +1,7 @@ +-- Automation definitions and runs persisted before PR5.5 lack the now-required +-- `model` field on AutomationDefinition. Since automations are still gated +-- behind the env-flagged `automate` tool and no UI entry exists yet, any rows +-- in these tables are pre-release dev/QA data. Drop them so the new schema +-- parses cleanly without per-row backfill. +DELETE FROM `automation_run`;--> statement-breakpoint +DELETE FROM `automation_definition`; diff --git a/packages/opencode/src/automation/derived.ts b/packages/opencode/src/automation/derived.ts new file mode 100644 index 000000000..fa5f3627d --- /dev/null +++ b/packages/opencode/src/automation/derived.ts @@ -0,0 +1,111 @@ +import { DateTime } from "luxon" +import type { Automation } from "." + +const CRON_LOOKAHEAD_MINUTES = 527_040 * 5 +const NEXT_FIRES_PREVIEW = 5 + +type RecurringDefinition = Extract + +type CronSchedule = { + minutes: Set + hours: Set + days: Set + months: Set + weekdays: Set + dayRestricted: boolean + weekdayRestricted: boolean +} + +function cronValues(field: string, min: number, max: number, options?: { sundayAlias?: boolean }) { + const values = new Set() + for (const item of field.split(",")) { + const [base, stepRaw] = item.split("/") + if (!base || item.split("/").length > 2) throw new Error(`Invalid cron field: ${item}`) + const step = stepRaw === undefined ? 1 : Number(stepRaw) + if (!Number.isInteger(step) || step <= 0) throw new Error(`Invalid cron step: ${item}`) + const range = base === "*" ? [min, max] : base.split("-").map(Number) + if (range.length === 0 || range.length > 2 || range.some((value) => !Number.isInteger(value))) { + throw new Error(`Invalid cron field: ${item}`) + } + const start = range[0] + const end = base === "*" || (range.length === 1 && stepRaw !== undefined) ? max : range.length === 1 ? range[0] : range[1] + if (start < min || end > max || start > end) throw new Error(`Invalid cron range: ${item}`) + for (let value = start; value <= end; value += step) { + values.add(options?.sundayAlias && value === 7 ? 0 : value) + } + } + return values +} + +export function parseCronSchedule(expression: string): CronSchedule { + const fields = expression.trim().split(/\s+/) + if (fields.length !== 5) throw new Error(`Invalid cron expression: ${expression}`) + const [minuteField, hourField, dayField, monthField, weekdayField] = fields + return { + minutes: cronValues(minuteField, 0, 59), + hours: cronValues(hourField, 0, 23), + days: cronValues(dayField, 1, 31), + months: cronValues(monthField, 1, 12), + weekdays: cronValues(weekdayField, 0, 7, { sundayAlias: true }), + dayRestricted: dayField !== "*", + weekdayRestricted: weekdayField !== "*", + } +} + +export function cronMatches(schedule: CronSchedule, time: DateTime) { + const weekday = time.weekday === 7 ? 0 : time.weekday + const dayMatches = schedule.days.has(time.day) + const weekdayMatches = schedule.weekdays.has(weekday) + const calendarMatches = + schedule.dayRestricted && schedule.weekdayRestricted ? dayMatches || weekdayMatches : dayMatches && weekdayMatches + return ( + schedule.minutes.has(time.minute) && + schedule.hours.has(time.hour) && + schedule.months.has(time.month) && + calendarMatches + ) +} + +function nextCronFires(definition: RecurringDefinition, from: number, count: number): number[] { + if (definition.rhythm.kind !== "cron" || count <= 0) return [] + let schedule: CronSchedule + try { + schedule = parseCronSchedule(definition.rhythm.expression) + } catch { + return [] + } + const fires: number[] = [] + let cursor = DateTime.fromMillis(from, { zone: definition.timezone }).plus({ minutes: 1 }).startOf("minute") + for (let attempts = 0; attempts < CRON_LOOKAHEAD_MINUTES && fires.length < count; attempts++) { + if (cronMatches(schedule, cursor)) fires.push(cursor.toMillis()) + cursor = cursor.plus({ minutes: 1 }) + } + return fires +} + +function nextIntervalFires(definition: RecurringDefinition, from: number, count: number): number[] { + if (definition.rhythm.kind !== "interval" || count <= 0) return [] + const fires: number[] = [] + let cursor = from + definition.rhythm.everyMs + for (let index = 0; index < count; index++) { + fires.push(cursor) + cursor += definition.rhythm.everyMs + } + return fires +} + +export function computeDerivedFields( + definition: RecurringDefinition, + from: number, + completedRunCount: number, +): { nextFireAt: number | null; nextFires: number[] } { + if (definition.paused) return { nextFireAt: null, nextFires: [] } + if (definition.stop.kind === "condition") return { nextFireAt: null, nextFires: [] } + const remaining = + definition.stop.kind === "count" ? Math.max(0, definition.stop.count - completedRunCount) : NEXT_FIRES_PREVIEW + if (remaining <= 0) return { nextFireAt: null, nextFires: [] } + const count = Math.min(NEXT_FIRES_PREVIEW, remaining) + const fires = + definition.rhythm.kind === "cron" ? nextCronFires(definition, from, count) : nextIntervalFires(definition, from, count) + return { nextFireAt: fires[0] ?? null, nextFires: fires } +} diff --git a/packages/opencode/src/automation/fixtures.ts b/packages/opencode/src/automation/fixtures.ts index 1bf86cd82..3c5533fca 100644 --- a/packages/opencode/src/automation/fixtures.ts +++ b/packages/opencode/src/automation/fixtures.ts @@ -13,10 +13,18 @@ export const automationDefinitionFixture = Automation.Definition.parse({ updatedAt: 1_800_000_030_000, timezone: "UTC", normalizationWarnings: [], + model: Automation.Model.parse({ providerID: "anthropic", modelID: "claude-sonnet-4-6" }), + variant: "high", rhythm: { kind: "interval", everyMs: 3_600_000 }, stop: { kind: "never" }, - nextFireAt: null, - nextFires: [], + nextFireAt: 1_800_003_600_000, + nextFires: [ + 1_800_003_600_000, + 1_800_007_200_000, + 1_800_010_800_000, + 1_800_014_400_000, + 1_800_018_000_000, + ], failureStreak: 0, }) diff --git a/packages/opencode/src/automation/index.ts b/packages/opencode/src/automation/index.ts index f9e6292df..fc90537ad 100644 --- a/packages/opencode/src/automation/index.ts +++ b/packages/opencode/src/automation/index.ts @@ -5,11 +5,13 @@ import { Identifier } from "@/id/id" import { Instance } from "@/project/instance" import { ProjectID } from "@/project/schema" import { PermissionID } from "@/permission/schema" +import { ModelID, ProviderID } from "@/provider/schema" import { SessionID } from "@/session/schema" import { and, Database, desc, eq, gte, inArray, lt, NotFoundError, or, sql } from "@/storage/db" import { Flock } from "@/util/flock" import type { AutomationRunAttendance, AutomationRunBlocker } from "./run-context" import { AutomationDefinitionTable, AutomationRunTable } from "./automation.sql" +import { computeDerivedFields } from "./derived" export const AutomationID = { Definition: { @@ -38,11 +40,17 @@ export namespace Automation { .meta({ ref: "AutomationWhere", }) + export const Model = z + .object({ providerID: ProviderID.zod, modelID: ModelID.zod }) + .strict() + .meta({ ref: "AutomationModel" }) + export type Model = z.infer export const ValidationErrorDetail = z .object({ field: z.string(), message: z.string() }) .strict() .meta({ ref: "AutomationValidationErrorDetail" }) export type ValidationErrorDetail = z.infer + export type ValidationErrorDetailType = ValidationErrorDetail export const ValidationErrorResponse = z .object({ error: z.literal("invalid_automation"), details: z.array(ValidationErrorDetail) }) .strict() @@ -75,6 +83,8 @@ export namespace Automation { context: Context, where: Where, timezone: z.string().min(1), + model: Model, + variant: z.string().min(1).optional(), } export const CreateInput = z @@ -96,6 +106,8 @@ export namespace Automation { fireAt: z.number().int().nonnegative().optional(), rhythm: Rhythm.optional(), stop: Stop.optional(), + model: Model.optional(), + variant: z.string().min(1).optional(), }) .strict() .meta({ ref: "AutomationUpdateInput" }) @@ -115,6 +127,8 @@ export namespace Automation { sourceSessionID: SessionID.zod.optional(), automationSessionID: SessionID.zod.optional(), normalizationWarnings: z.array(z.string()), + model: Model, + variant: z.string().min(1).optional(), } export const Definition = z @@ -263,6 +277,8 @@ export namespace Automation { "context", "where", "timezone", + "model", + "variant", ]) const ONESHOT_CREATE_FIELDS = new Set([...COMMON_CREATE_FIELDS, "fireAt"]) const RECURRING_CREATE_FIELDS = new Set([...COMMON_CREATE_FIELDS, "rhythm", "stop"]) @@ -276,6 +292,8 @@ export namespace Automation { "fireAt", "rhythm", "stop", + "model", + "variant", ]) function addDetail(details: ValidationErrorDetail[], field: string, message: string) { @@ -443,6 +461,9 @@ export namespace Automation { if (input.where.worktree && Instance.project.vcs !== "git") { addDetail(details, "where.worktree", "unsupported_where_worktree_not_git") } + if (input.kind === "recurring" && input.stop.kind === "condition") { + addDetail(details, "stop", "unsupported_stop_condition") + } details.push(...validateScheduleFields(input)) return details } @@ -467,6 +488,9 @@ export namespace Automation { if (patch.rhythm?.kind === "cron" && !isValidCronExpression(patch.rhythm.expression)) { addDetail(details, "rhythm.expression", "invalid_cron_expression") } + if (patch.stop?.kind === "condition") { + addDetail(details, "stop", "unsupported_stop_condition") + } return details } @@ -487,9 +511,11 @@ export namespace Automation { updatedAt: now, timezone: input.timezone, normalizationWarnings: [], + model: input.model, + ...(input.variant ? { variant: input.variant } : {}), ...(options?.sourceSessionID ? { sourceSessionID: options.sourceSessionID } : {}), } - const definition: Definition = + let definition: Definition = input.kind === "oneshot" ? { kind: "oneshot", ...base, fireAt: input.fireAt } : { @@ -501,6 +527,10 @@ export namespace Automation { nextFires: [], failureStreak: 0, } + if (definition.kind === "recurring") { + const derived = computeDerivedFields(definition, now, 0) + definition = { ...definition, nextFireAt: derived.nextFireAt, nextFires: derived.nextFires } + } writeDefinition(definition) return definition } @@ -645,7 +675,7 @@ export namespace Automation { const updateDetails = validateUpdateInput(previous, patch, now) if (updateDetails.length) throw new ValidationError(updateDetails) if (!hasChanges(previous, patch)) return previous - const next = Definition.parse({ + let next = Definition.parse({ ...previous, ...patch, revision: previous.revision + 1, @@ -653,9 +683,52 @@ export namespace Automation { }) const details = validateCreateInput(next) if (details.length) throw new ValidationError(details) + if (next.kind === "recurring") { + const derived = computeDerivedFields(next, now, completedRunCount(next.id)) + next = { ...next, nextFireAt: derived.nextFireAt, nextFires: derived.nextFires } + } return replaceDefinition(previous, next) } + export function recordRunOutcome(run: Run, options?: { now?: number }): Definition | undefined { + if (run.state !== "succeeded" && run.state !== "failed" && run.state !== "stopped") return undefined + const previous = getOptional(run.automationID) + if (!previous || previous.kind !== "recurring") return undefined + const now = options?.now ?? Date.now() + const failureStreak = + run.state === "succeeded" ? 0 : run.state === "failed" ? previous.failureStreak + 1 : previous.failureStreak + const derived = computeDerivedFields(previous, now, completedRunCount(previous.id)) + if ( + previous.failureStreak === failureStreak && + previous.nextFireAt === derived.nextFireAt && + sameArray(previous.nextFires, derived.nextFires) + ) { + return undefined + } + const next = Definition.parse({ + ...previous, + failureStreak, + nextFireAt: derived.nextFireAt, + nextFires: derived.nextFires, + revision: previous.revision + 1, + updatedAt: now, + }) + try { + return replaceDefinition(previous, next) + } catch (error) { + if (error instanceof ConflictError) return undefined + throw error + } + } + + function sameArray(left: readonly number[], right: readonly number[]) { + if (left.length !== right.length) return false + for (let index = 0; index < left.length; index++) { + if (left[index] !== right[index]) return false + } + return true + } + export async function remove(id: string): Promise<{ tombstone: Tombstone; stoppedRun?: Run }> { const previous = get(id) const stoppedRun = stopActiveRun(id) diff --git a/packages/opencode/src/automation/runner.ts b/packages/opencode/src/automation/runner.ts index 1a681f8cb..e317c1b69 100644 --- a/packages/opencode/src/automation/runner.ts +++ b/packages/opencode/src/automation/runner.ts @@ -96,6 +96,8 @@ export const sessionPromptExecutor: Automation.RunExecutor = async ({ definition const message = await SessionPrompt.promptWithAutomationContext( { sessionID, + model: definition.model, + ...(definition.variant ? { variant: definition.variant } : {}), parts: [{ type: "text", text: definition.prompt }], }, scoped, diff --git a/packages/opencode/src/automation/scheduler.ts b/packages/opencode/src/automation/scheduler.ts index 13fa62c06..9c1dd9ec6 100644 --- a/packages/opencode/src/automation/scheduler.ts +++ b/packages/opencode/src/automation/scheduler.ts @@ -388,6 +388,14 @@ export namespace AutomationScheduler { const wasOwned = ownedRuns.delete(run.id) const wasSchedulerStopped = schedulerStoppedRuns.delete(run.id) if (run.state === "stopped" && !wasOwned && !wasSchedulerStopped) return + if (ownsTimers) { + try { + const refreshed = Automation.recordRunOutcome(run, { now: clock.now() }) + if (refreshed) void Automation.publishDefinitionUpdated(refreshed) + } catch (error) { + if (!NotFoundError.isInstance(error)) log.error("automation derived field update failed", { error, automationID: run.automationID }) + } + } scheduleNextInterval(run.automationID) }) const unsubscribeDefinitionUpdates = Bus.subscribe(Automation.Event.DefinitionUpdated, (event) => { diff --git a/packages/opencode/src/automation/validation.ts b/packages/opencode/src/automation/validation.ts new file mode 100644 index 000000000..e088557dc --- /dev/null +++ b/packages/opencode/src/automation/validation.ts @@ -0,0 +1,47 @@ +import { Cause, Effect, Exit, Result } from "effect" +import { ModelNotFoundError, Provider } from "@/provider/provider" +import { ModelID, ProviderID } from "@/provider/schema" +import { ProviderTransform } from "@/provider/transform" +import type { Automation } from "." + +function isModelNotFound(cause: Cause.Cause): boolean { + const errorResult = Cause.findError(cause) + if (Result.isSuccess(errorResult) && ModelNotFoundError.isInstance(errorResult.success)) return true + const defectResult = Cause.findDefect(cause) + if (Result.isSuccess(defectResult) && ModelNotFoundError.isInstance(defectResult.success)) return true + return false +} + +type ValidationErrorDetail = Automation.ValidationErrorDetailType + +type ModelLike = { providerID: string; modelID: string } + +export const validateModelAndVariantWith = ( + provider: Provider.Interface, + model: ModelLike, + variant: string | undefined, +): Effect.Effect => + Effect.gen(function* () { + const providerID = ProviderID.make(model.providerID) + const modelID = ModelID.make(model.modelID) + const exit = yield* provider.getModel(providerID, modelID).pipe(Effect.exit) + if (Exit.isFailure(exit)) { + const message = isModelNotFound(exit.cause) ? "model_not_found" : "model_lookup_failed" + return [{ field: "model", message }] + } + if (variant === undefined) return [] + const variants = ProviderTransform.variants(exit.value) + if (!Object.hasOwn(variants, variant)) { + return [{ field: "variant", message: "invalid_variant_for_model" }] + } + return [] + }) + +export const validateModelAndVariant = ( + model: Automation.Model, + variant: string | undefined, +): Effect.Effect => + Effect.gen(function* () { + const provider = yield* Provider.Service + return yield* validateModelAndVariantWith(provider, model, variant) + }) diff --git a/packages/opencode/src/server/instance/automation.ts b/packages/opencode/src/server/instance/automation.ts index df39e924a..961b90cf4 100644 --- a/packages/opencode/src/server/instance/automation.ts +++ b/packages/opencode/src/server/instance/automation.ts @@ -5,6 +5,8 @@ import z from "zod" import { ActiveRunStillRunningError, Automation, AutomationID, ConflictError, ValidationError } from "@/automation" import { sessionPromptExecutor } from "@/automation/runner" import { AutomationScheduler } from "@/automation/scheduler" +import { validateModelAndVariant } from "@/automation/validation" +import { AppRuntime } from "@/effect/app-runtime" import { errors } from "../error" function validationError(error: ValidationError) { @@ -31,6 +33,11 @@ async function settleAutomationScheduler() { await AutomationScheduler.current().settleOwner() } +async function modelValidationDetails(model: Automation.Model, variant?: string) { + if (process.env.OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION === "1") return [] + return AppRuntime.runPromise(validateModelAndVariant(model, variant)) +} + function validationIssuePath(issue: unknown) { const path = typeof issue === "object" && issue !== null && "path" in issue ? issue.path : undefined if (!Array.isArray(path)) return "" @@ -120,7 +127,15 @@ export const AutomationRoutes = (): Hono => async (c) => { try { await settleAutomationScheduler() - const definition = Automation.create(c.req.valid("json")) + const input = c.req.valid("json") + const modelDetails = await modelValidationDetails(input.model, input.variant) + if (modelDetails.length) { + return c.json( + Automation.ValidationErrorResponse.parse({ error: "invalid_automation", details: modelDetails }), + 422, + ) + } + const definition = Automation.create(input) await Automation.publishDefinitionUpdated(definition) return c.json(definition) } catch (error) { @@ -178,7 +193,19 @@ export const AutomationRoutes = (): Hono => const automationID = c.req.valid("param").automationID await settleAutomationScheduler() const previous = Automation.get(automationID) - const definition = Automation.update(automationID, c.req.valid("json")) + const patch = c.req.valid("json") + if (patch.model !== undefined || patch.variant !== undefined) { + const effectiveModel = patch.model ?? previous.model + const effectiveVariant = patch.variant ?? previous.variant + const modelDetails = await modelValidationDetails(effectiveModel, effectiveVariant) + if (modelDetails.length) { + return c.json( + Automation.ValidationErrorResponse.parse({ error: "invalid_automation", details: modelDetails }), + 422, + ) + } + } + const definition = Automation.update(automationID, patch) await publishIfChanged(previous, definition) return c.json(definition) } catch (error) { diff --git a/packages/opencode/src/tool/automate.ts b/packages/opencode/src/tool/automate.ts index 3d8a674a9..b8aa2912d 100644 --- a/packages/opencode/src/tool/automate.ts +++ b/packages/opencode/src/tool/automate.ts @@ -1,6 +1,8 @@ import { Effect, Schema } from "effect" import { Automation, ValidationError } from "@/automation" import { AutomationScheduler } from "@/automation/scheduler" +import { validateModelAndVariantWith } from "@/automation/validation" +import { Provider } from "@/provider/provider" import * as Tool from "./tool" const Where = Schema.Struct({ @@ -8,6 +10,11 @@ const Where = Schema.Struct({ worktree: Schema.optional(Schema.NonEmptyString), }) +const Model = Schema.Struct({ + providerID: Schema.NonEmptyString, + modelID: Schema.NonEmptyString, +}) + const Timezone = Schema.NonEmptyString.check( Schema.makeFilter((timezone: string) => (Automation.isValidTimezone(timezone) ? undefined : "invalid_timezone")), ) @@ -26,6 +33,8 @@ const Common = { context: Schema.Union([Schema.Literal("continue"), Schema.Literal("fresh")]), where: Where, timezone: Timezone, + model: Model, + variant: Schema.optional(Schema.NonEmptyString), } const NonNegativeInt = Schema.Int.check(Schema.isGreaterThanOrEqualTo(0)) @@ -64,8 +73,9 @@ export function formatAutomateValidationError(error: unknown) { : String(error) return [ "Invalid automate input.", - "Expected shape: oneshot { kind, title, prompt, context, where, timezone, fireAt } or recurring { kind, title, prompt, context, where, timezone, rhythm, stop }.", - "Example: { kind: \"recurring\", title: \"Daily repo brief\", prompt: \"Summarize repo changes.\", context: \"fresh\", where: { projectID: \"current-project\" }, timezone: \"UTC\", rhythm: { kind: \"interval\", everyMs: 3600000 }, stop: { kind: \"never\" } }.", + "Expected shape: oneshot { kind, title, prompt, context, where, timezone, model, variant?, fireAt } or recurring { kind, title, prompt, context, where, timezone, model, variant?, rhythm, stop }.", + "model is required as { providerID, modelID }; variant is optional and must be a valid effort key for that model (omit for models without reasoning).", + "Example: { kind: \"recurring\", title: \"Daily repo brief\", prompt: \"Summarize repo changes.\", context: \"fresh\", where: { projectID: \"current-project\" }, timezone: \"UTC\", model: { providerID: \"anthropic\", modelID: \"claude-sonnet-4-6\" }, variant: \"high\", rhythm: { kind: \"interval\", everyMs: 3600000 }, stop: { kind: \"never\" } }.", detail, ].join("\n") } @@ -75,7 +85,7 @@ function readableAutomationError(error: unknown) { return error } -export function createAutomateDefinition(): Tool.DefWithoutID { +export function createAutomateDefinition(provider: Provider.Interface): Tool.DefWithoutID { return { description: "Create an Automation definition for later execution. The automation is not executed by this tool; it only stores the definition and echoes the resolved contract.", @@ -83,12 +93,20 @@ export function createAutomateDefinition(): Tool.DefWithoutID Effect.gen(function* () { + const { sourceSessionID: _ignoredSourceSessionID, ...input } = params as typeof params & { sourceSessionID?: unknown } + if (Object.hasOwn(input, "automationSessionID")) { + return yield* Effect.fail( + readableAutomationError( + new ValidationError([{ field: "automationSessionID", message: "unsupported_automation_field" }]), + ), + ) + } + const modelDetails = yield* validateModelAndVariantWith(provider, input.model, input.variant) + if (modelDetails.length) { + return yield* Effect.fail(readableAutomationError(new ValidationError(modelDetails))) + } const definition = yield* Effect.try({ try: () => { - if (Object.hasOwn(params as object, "automationSessionID")) { - throw new ValidationError([{ field: "automationSessionID", message: "unsupported_automation_field" }]) - } - const { sourceSessionID: _ignoredSourceSessionID, ...input } = params as typeof params & { sourceSessionID?: unknown } const parsed = Automation.CreateInput.parse(input) AutomationScheduler.current() return Automation.create(parsed, { sourceSessionID: ctx.sessionID }) @@ -105,4 +123,10 @@ export function createAutomateDefinition(): Tool.DefWithoutID Effect.succeed({ [providerID]: info }), + getProvider: (id) => + id === providerID ? Effect.succeed(info) : Effect.die(new Error(`Unknown provider: ${id}`)), + getModel: (pId, mId) => + pId === providerID && mId === modelID ? Effect.succeed(mdl) : Effect.die(new Error(`Unknown model: ${pId}/${mId}`)), + getLanguage: () => Effect.die(new Error("getLanguage not configured")), + closest: (pId) => Effect.succeed(pId === providerID ? { providerID, modelID } : undefined), + getSmallModel: (pId) => Effect.succeed(pId === providerID ? mdl : undefined), + defaultModel: () => Effect.succeed({ providerID, modelID }), + } + return { providerID, modelID, interface: iface } +} + export namespace ProviderTest { export function model(override: Partial = {}): Provider.Model { const id = override.id ?? ModelID.make("gpt-5.2") diff --git a/packages/opencode/test/server/automation-routes.test.ts b/packages/opencode/test/server/automation-routes.test.ts index 4dfd2e851..f582e3a4b 100644 --- a/packages/opencode/test/server/automation-routes.test.ts +++ b/packages/opencode/test/server/automation-routes.test.ts @@ -13,6 +13,7 @@ import { SessionID } from "../../src/session/schema" import { Flock } from "../../src/util/flock" import { tmpdir } from "../fixture/fixture" +process.env.OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION = "1" void Log.init({ print: false }) afterEach(async () => { @@ -70,6 +71,8 @@ function deferred() { type RecurringCreateInput = Extract type OneshotCreateInput = Extract +const fixtureModel = Automation.Model.parse({ providerID: "anthropic", modelID: "claude-sonnet-4-6" }) + function recurringInput(projectID: ProjectID, overrides: Partial = {}): RecurringCreateInput { return { kind: "recurring", @@ -78,6 +81,7 @@ function recurringInput(projectID: ProjectID, overrides: Partial { expect(body.id).toMatch(/^automation_/) expect(body.createdAt).toBeNumber() expect(body.updatedAt).toBe(body.createdAt) - expect(body.nextFireAt).toBeNull() - expect(body.nextFires).toEqual([]) + expect(body.model).toEqual(fixtureModel) + expect(body.nextFireAt).toBeNumber() + expect(body.nextFires).toHaveLength(3) + expect(body.failureStreak).toBe(0) }) }) diff --git a/packages/opencode/test/server/automation-runner.test.ts b/packages/opencode/test/server/automation-runner.test.ts index 4bd7840ab..10437469a 100644 --- a/packages/opencode/test/server/automation-runner.test.ts +++ b/packages/opencode/test/server/automation-runner.test.ts @@ -33,6 +33,8 @@ async function withAutomation(fn: (projectID: ProjectID) => Promise) { }) } +const fixtureModel = Automation.Model.parse({ providerID: "alibaba", modelID: "qwen-plus" }) + function input(projectID: ProjectID, overrides: Partial> = {}): Automation.CreateInput { return { kind: "recurring", @@ -41,6 +43,7 @@ function input(projectID: ProjectID, overrides: Partial(fn: (projectID: ProjectID) => Promise) { }) } +const fakeProvider = fakeAutomationProvider() +const fixtureModel = Automation.Model.parse({ + providerID: fakeProvider.providerID, + modelID: fakeProvider.modelID, +}) + function oneshotInput(projectID: ProjectID, fireAt: number): Automation.CreateInput { return { kind: "oneshot", @@ -133,6 +140,7 @@ function oneshotInput(projectID: ProjectID, fireAt: number): Automation.CreateIn context: "fresh", where: { projectID }, timezone: "Asia/Shanghai", + model: fixtureModel, fireAt, } } @@ -147,6 +155,7 @@ function recurringInput(projectID: ProjectID, everyMs: number, overrides: Partia context: "fresh", where: { projectID }, timezone: "Asia/Shanghai", + model: fixtureModel, rhythm: { kind: "interval", everyMs }, stop: { kind: "never" }, ...overrides, @@ -280,7 +289,7 @@ describe("automation scheduler", () => { clock, executor: async () => ({ sessionID: SessionID.descending(), result: "done", cost: 0 }), }) - const tool = createAutomateDefinition() + const tool = createAutomateDefinition(fakeProvider.interface) const result = await Effect.runPromise( tool.execute( @@ -986,28 +995,20 @@ describe("automation scheduler", () => { }) }) - test("does not schedule recurring condition stops without an evaluator", async () => { + test("rejects recurring condition stops at create time", async () => { await withAutomation(async (projectID) => { - const clock = new FakeClock(0) - const starts: number[] = [] - const scheduler = AutomationScheduler.make({ - clock, - executor: async () => { - starts.push(clock.now()) - return { sessionID: SessionID.descending(), result: "done", cost: 0 } - }, - }) - const definition = Automation.create( - recurringInput(projectID, 60_000, { stop: { kind: "condition", condition: "repo is ready" } }), - { now: 0 }, - ) - - scheduler.reschedule(definition) - await clock.advance(60_000) - - expect(starts).toEqual([]) - expect(Automation.runs({ automationID: definition.id }).items).toHaveLength(0) - scheduler.stop() + let captured: unknown + try { + Automation.create( + recurringInput(projectID, 60_000, { stop: { kind: "condition", condition: "repo is ready" } }), + { now: 0 }, + ) + } catch (error) { + captured = error + } + expect(captured).toBeInstanceOf(Error) + const validation = captured as { details?: { field: string; message: string }[] } + expect(validation.details).toEqual(expect.arrayContaining([{ field: "stop", message: "unsupported_stop_condition" }])) }) }) diff --git a/packages/opencode/test/tool/automate.test.ts b/packages/opencode/test/tool/automate.test.ts index 751105b9a..78ae35bd2 100644 --- a/packages/opencode/test/tool/automate.test.ts +++ b/packages/opencode/test/tool/automate.test.ts @@ -4,7 +4,46 @@ import { AutomateParameters, createAutomateDefinition, formatAutomateValidationE import { Automation } from "../../src/automation" import { Instance } from "../../src/project/instance" import { MessageID, SessionID } from "../../src/session/schema" +import { ModelID, ProviderID } from "../../src/provider/schema" +import type { Provider } from "../../src/provider/provider" import { tmpdir } from "../fixture/fixture" +import { ProviderTest } from "../fake/provider" + +const fakeProviderID = ProviderID.make("openai") +const fakeModelID = ModelID.make("test-reasoning-model") +const fakeModel = ProviderTest.model({ + id: fakeModelID, + providerID: fakeProviderID, + capabilities: { + toolcall: true, + attachment: false, + reasoning: true, + temperature: true, + interleaved: false, + input: { text: true, image: false, audio: false, video: false, pdf: false }, + output: { text: true, image: false, audio: false, video: false, pdf: false }, + }, + api: { id: fakeModelID, url: "https://example.com", npm: "ai-gateway-provider" }, +}) +const fakeInfo = ProviderTest.info({ id: fakeProviderID }, fakeModel) +const fakeProviderInterface: Provider.Interface = { + list: () => Effect.succeed({ [fakeProviderID]: fakeInfo }), + getProvider: (providerID) => + providerID === fakeProviderID + ? Effect.succeed(fakeInfo) + : Effect.die(new Error(`Unknown provider: ${providerID}`)), + getModel: (providerID, modelID) => + providerID === fakeProviderID && modelID === fakeModelID + ? Effect.succeed(fakeModel) + : Effect.die(new Error(`Unknown model: ${providerID}/${modelID}`)), + getLanguage: () => Effect.die(new Error("getLanguage not configured")), + closest: (providerID) => + Effect.succeed(providerID === fakeProviderID ? { providerID, modelID: fakeModelID } : undefined), + getSmallModel: (providerID) => + Effect.succeed(providerID === fakeProviderID ? fakeModel : undefined), + defaultModel: () => Effect.succeed({ providerID: fakeProviderID, modelID: fakeModelID }), +} +const fixtureModel = { providerID: fakeProviderID, modelID: fakeModelID } afterEach(async () => { await Instance.disposeAll() @@ -21,6 +60,7 @@ describe("automate tool", () => { context: "fresh", where: { projectID: "project" }, timezone: "UTC", + model: fixtureModel, rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "never" }, }) @@ -30,7 +70,7 @@ describe("automate tool", () => { expect(error).toBeDefined() expect(formatAutomateValidationError(error)).toContain("prompt") - expect(formatAutomateValidationError(error)).toContain("kind, title, prompt, context, where, timezone") + expect(formatAutomateValidationError(error)).toContain("model") }) test("rejects empty strings before execute reaches the Zod create parser", () => { @@ -44,6 +84,7 @@ describe("automate tool", () => { context: "fresh", where: { projectID: "project", worktree: "" }, timezone: "", + model: fixtureModel, rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "never" }, }) @@ -71,6 +112,7 @@ describe("automate tool", () => { context: "fresh", where: { projectID: "project" }, timezone: "UTC", + model: fixtureModel, } let error: unknown try { @@ -100,6 +142,7 @@ describe("automate tool", () => { context: "fresh", where: { projectID: "project" }, timezone: "UTC", + model: fixtureModel, ...override, }) } catch (caught) { @@ -124,6 +167,7 @@ describe("automate tool", () => { context: "fresh", where: { projectID: "project" }, timezone: "UTC", + model: fixtureModel, rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "never" }, ...override, @@ -144,7 +188,7 @@ describe("automate tool", () => { await Instance.provide({ directory: tmp.path, fn: async () => { - const tool = createAutomateDefinition() + const tool = createAutomateDefinition(fakeProviderInterface) const sourceSessionID = SessionID.descending() let error: unknown try { @@ -157,6 +201,7 @@ describe("automate tool", () => { context: "fresh", where: where(Instance.project.id), timezone: "UTC", + model: fixtureModel, rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "never" }, }, @@ -187,7 +232,7 @@ describe("automate tool", () => { await Instance.provide({ directory: tmp.path, fn: async () => { - const tool = createAutomateDefinition() + const tool = createAutomateDefinition(fakeProviderInterface) const sourceSessionID = SessionID.descending() const result = await Effect.runPromise( tool.execute( @@ -198,6 +243,7 @@ describe("automate tool", () => { context: "fresh", where: { projectID: Instance.project.id }, timezone: "Asia/Shanghai", + model: fixtureModel, rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "never" }, }, @@ -232,7 +278,7 @@ describe("automate tool", () => { await Instance.provide({ directory: tmp.path, fn: async () => { - const tool = createAutomateDefinition() + const tool = createAutomateDefinition(fakeProviderInterface) const sourceSessionID = SessionID.descending() const spoofedSessionID = SessionID.descending() const spoofedSource = { sourceSessionID: spoofedSessionID } as Record @@ -245,6 +291,7 @@ describe("automate tool", () => { context: "fresh", where: { projectID: Instance.project.id }, timezone: "Asia/Shanghai", + model: fixtureModel, ...spoofedSource, rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "never" }, @@ -271,7 +318,7 @@ describe("automate tool", () => { await Instance.provide({ directory: tmp.path, fn: async () => { - const tool = createAutomateDefinition() + const tool = createAutomateDefinition(fakeProviderInterface) let error: unknown const spoofedSession = { automationSessionID: SessionID.descending() } as Record try { @@ -284,6 +331,7 @@ describe("automate tool", () => { context: "fresh", where: { projectID: Instance.project.id }, timezone: "Asia/Shanghai", + model: fixtureModel, ...spoofedSession, rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "never" }, diff --git a/packages/sdk/js/src/v2/event-types.test-d.ts b/packages/sdk/js/src/v2/event-types.test-d.ts index 0e9fb962a..a2ae6660a 100644 --- a/packages/sdk/js/src/v2/event-types.test-d.ts +++ b/packages/sdk/js/src/v2/event-types.test-d.ts @@ -36,6 +36,7 @@ const _automationDefinitionUpdated: EventAutomationDefinitionUpdated = { updatedAt: 1800000030000, timezone: "UTC", normalizationWarnings: [], + model: { providerID: "anthropic", modelID: "claude-sonnet-4-6" }, rhythm: { kind: "interval", everyMs: 3600000 }, stop: { kind: "never" }, nextFireAt: null, diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index cbb75362d..8ed717538 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -358,6 +358,11 @@ export type AutomationWhere = { worktree?: string } +export type AutomationModel = { + providerID: string + modelID: string +} + export type AutomationRhythm = | { kind: "interval" @@ -397,6 +402,8 @@ export type AutomationDefinition = sourceSessionID?: string automationSessionID?: string normalizationWarnings: Array + model: AutomationModel + variant?: string fireAt: number } | { @@ -414,6 +421,8 @@ export type AutomationDefinition = sourceSessionID?: string automationSessionID?: string normalizationWarnings: Array + model: AutomationModel + variant?: string rhythm: AutomationRhythm stop: AutomationStop nextFireAt: number | null @@ -2427,6 +2436,8 @@ export type AutomationCreateInput = context: "continue" | "fresh" where: AutomationWhere timezone: string + model: AutomationModel + variant?: string fireAt: number } | { @@ -2436,6 +2447,8 @@ export type AutomationCreateInput = context: "continue" | "fresh" where: AutomationWhere timezone: string + model: AutomationModel + variant?: string rhythm: AutomationRhythm stop: AutomationStop } @@ -2455,6 +2468,8 @@ export type AutomationUpdateInput = { fireAt?: number rhythm?: AutomationRhythm stop?: AutomationStop + model?: AutomationModel + variant?: string } export type AutomationActiveRunStillRunningError = { From fcba589ad6d7b75e71fbb514d023ad80d2fc09c0 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 1 Jun 2026 17:44:34 +0800 Subject: [PATCH 02/13] fix(automation): preserve nextFireAt on metadata edits + allow clearing variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - nextCronFires: jumping search (months → days → hours), capped lookahead. Brute-force minute-by-minute over 5y blocked the event loop. - update(): only recompute nextFireAt/nextFires when rhythm/stop/timezone/paused actually changes. Title/prompt edits no longer postpone pending interval runs. - UpdateInput.variant: accept null to clear a previously set effort when the new model has no compatible variant. --- packages/opencode/src/automation/derived.ts | 31 +++++++++++-- packages/opencode/src/automation/index.ts | 20 ++++++--- .../src/server/instance/automation.ts | 2 +- .../test/server/automation-routes.test.ts | 45 +++++++++++++++++++ packages/sdk/js/src/v2/gen/types.gen.ts | 2 +- 5 files changed, 88 insertions(+), 12 deletions(-) diff --git a/packages/opencode/src/automation/derived.ts b/packages/opencode/src/automation/derived.ts index fa5f3627d..8678c1bd9 100644 --- a/packages/opencode/src/automation/derived.ts +++ b/packages/opencode/src/automation/derived.ts @@ -66,6 +66,11 @@ export function cronMatches(schedule: CronSchedule, time: DateTime) { ) } +const PLUS_ONE_MONTH = { months: 1 } +const PLUS_ONE_DAY = { days: 1 } +const PLUS_ONE_HOUR = { hours: 1 } +const PLUS_ONE_MINUTE = { minutes: 1 } + function nextCronFires(definition: RecurringDefinition, from: number, count: number): number[] { if (definition.rhythm.kind !== "cron" || count <= 0) return [] let schedule: CronSchedule @@ -75,10 +80,28 @@ function nextCronFires(definition: RecurringDefinition, from: number, count: num return [] } const fires: number[] = [] - let cursor = DateTime.fromMillis(from, { zone: definition.timezone }).plus({ minutes: 1 }).startOf("minute") - for (let attempts = 0; attempts < CRON_LOOKAHEAD_MINUTES && fires.length < count; attempts++) { - if (cronMatches(schedule, cursor)) fires.push(cursor.toMillis()) - cursor = cursor.plus({ minutes: 1 }) + const maxTimestamp = from + CRON_LOOKAHEAD_MINUTES * 60 * 1000 + let cursor = DateTime.fromMillis(from, { zone: definition.timezone }).plus(PLUS_ONE_MINUTE).startOf("minute") + while (cursor.toMillis() < maxTimestamp && fires.length < count) { + if (!schedule.months.has(cursor.month)) { + cursor = cursor.plus(PLUS_ONE_MONTH).startOf("month") + continue + } + const weekday = cursor.weekday === 7 ? 0 : cursor.weekday + const dayMatches = schedule.days.has(cursor.day) + const weekdayMatches = schedule.weekdays.has(weekday) + const calendarMatches = + schedule.dayRestricted && schedule.weekdayRestricted ? dayMatches || weekdayMatches : dayMatches && weekdayMatches + if (!calendarMatches) { + cursor = cursor.plus(PLUS_ONE_DAY).startOf("day") + continue + } + if (!schedule.hours.has(cursor.hour)) { + cursor = cursor.plus(PLUS_ONE_HOUR).startOf("hour") + continue + } + if (schedule.minutes.has(cursor.minute)) fires.push(cursor.toMillis()) + cursor = cursor.plus(PLUS_ONE_MINUTE) } return fires } diff --git a/packages/opencode/src/automation/index.ts b/packages/opencode/src/automation/index.ts index fc90537ad..f577bdada 100644 --- a/packages/opencode/src/automation/index.ts +++ b/packages/opencode/src/automation/index.ts @@ -107,7 +107,7 @@ export namespace Automation { rhythm: Rhythm.optional(), stop: Stop.optional(), model: Model.optional(), - variant: z.string().min(1).optional(), + variant: z.string().min(1).nullable().optional(), }) .strict() .meta({ ref: "AutomationUpdateInput" }) @@ -675,17 +675,25 @@ export namespace Automation { const updateDetails = validateUpdateInput(previous, patch, now) if (updateDetails.length) throw new ValidationError(updateDetails) if (!hasChanges(previous, patch)) return previous + const merged: Record = { ...previous, ...patch } + if (patch.variant === null) delete merged.variant let next = Definition.parse({ - ...previous, - ...patch, + ...merged, revision: previous.revision + 1, updatedAt: now, }) const details = validateCreateInput(next) if (details.length) throw new ValidationError(details) - if (next.kind === "recurring") { - const derived = computeDerivedFields(next, now, completedRunCount(next.id)) - next = { ...next, nextFireAt: derived.nextFireAt, nextFires: derived.nextFires } + if (next.kind === "recurring" && previous.kind === "recurring") { + const scheduleChanged = + !isSameValue(previous.rhythm, next.rhythm) || + !isSameValue(previous.stop, next.stop) || + previous.timezone !== next.timezone || + previous.paused !== next.paused + if (scheduleChanged) { + const derived = computeDerivedFields(next, now, completedRunCount(next.id)) + next = { ...next, nextFireAt: derived.nextFireAt, nextFires: derived.nextFires } + } } return replaceDefinition(previous, next) } diff --git a/packages/opencode/src/server/instance/automation.ts b/packages/opencode/src/server/instance/automation.ts index 961b90cf4..415334195 100644 --- a/packages/opencode/src/server/instance/automation.ts +++ b/packages/opencode/src/server/instance/automation.ts @@ -196,7 +196,7 @@ export const AutomationRoutes = (): Hono => const patch = c.req.valid("json") if (patch.model !== undefined || patch.variant !== undefined) { const effectiveModel = patch.model ?? previous.model - const effectiveVariant = patch.variant ?? previous.variant + const effectiveVariant = patch.variant === null ? undefined : (patch.variant ?? previous.variant) const modelDetails = await modelValidationDetails(effectiveModel, effectiveVariant) if (modelDetails.length) { return c.json( diff --git a/packages/opencode/test/server/automation-routes.test.ts b/packages/opencode/test/server/automation-routes.test.ts index f582e3a4b..38137533d 100644 --- a/packages/opencode/test/server/automation-routes.test.ts +++ b/packages/opencode/test/server/automation-routes.test.ts @@ -727,6 +727,51 @@ describe("automation routes", () => { }) }) + test("metadata-only update preserves pending nextFireAt and nextFires", async () => { + await withAutomationApp(async ({ projectID }) => { + const created = Automation.create(recurringInput(projectID), { now: 100 }) + expect(created.kind).toBe("recurring") + if (created.kind !== "recurring") throw new Error("recurring") + const updated = Automation.update(created.id, { title: "Renamed" }, { now: 200 }) + expect(updated).toMatchObject({ + title: "Renamed", + nextFireAt: created.nextFireAt, + nextFires: created.nextFires, + }) + }) + }) + + test("rhythm change recomputes nextFireAt from the update timestamp", async () => { + await withAutomationApp(async ({ projectID }) => { + const created = Automation.create(recurringInput(projectID), { now: 100 }) + const updated = Automation.update( + created.id, + { rhythm: { kind: "interval", everyMs: 120_000 } }, + { now: 300 }, + ) + if (updated.kind !== "recurring") throw new Error("recurring") + expect(updated.nextFireAt).toBe(300 + 120_000) + }) + }) + + test("update accepts variant: null to clear a previously set effort", async () => { + await withAutomationApp(async ({ app, projectID }) => { + const created = await json(app, "/automation", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(recurringInput(projectID, { variant: "high" } as Partial)), + }) + expect(created.variant).toBe("high") + const cleared = await json(app, `/automation/${created.id}`, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ variant: null }), + }) + expect(cleared).not.toHaveProperty("variant") + expect(Automation.get(created.id)).not.toHaveProperty("variant") + }) + }) + test("pause and resume only revise when paused state changes", async () => { await withAutomationApp(async ({ app, projectID }) => { const created = await json(app, "/automation", { diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index 8ed717538..dec65108e 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -2469,7 +2469,7 @@ export type AutomationUpdateInput = { rhythm?: AutomationRhythm stop?: AutomationStop model?: AutomationModel - variant?: string + variant?: string | null } export type AutomationActiveRunStillRunningError = { From ac04734d61c7c8651a1d6dd0667c60c3fe2b2cdc Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 1 Jun 2026 19:15:59 +0800 Subject: [PATCH 03/13] refactor(automation): share cron utility + freeze derived on stopped runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract cron parser/matcher into automation/cron.ts; scheduler and derived now share one implementation (removes ~70 lines of duplication). - recordRunOutcome: stopped runs keep previous nextFireAt/nextFires. Manual cancel, writer conflicts, and missed-schedule stops must not re-anchor the recurring preview — only succeeded/failed advance it. - Add unit coverage for validateModelAndVariantWith with real provider interface so HTTP route 422 details (model_not_found / model_lookup_failed / invalid_variant_for_model) are exercised end-to-end, not just bypassed via env flag. - Assert scheduler-stop + manual-stop both leave failureStreak / nextFireAt / nextFires / revision intact. --- packages/opencode/src/automation/cron.ts | 61 ++++++++++++++++ packages/opencode/src/automation/derived.ts | 61 +--------------- packages/opencode/src/automation/index.ts | 5 +- packages/opencode/src/automation/scheduler.ts | 61 +--------------- .../test/automation/validation.test.ts | 73 +++++++++++++++++++ .../test/server/automation-scheduler.test.ts | 8 ++ 6 files changed, 148 insertions(+), 121 deletions(-) create mode 100644 packages/opencode/src/automation/cron.ts create mode 100644 packages/opencode/test/automation/validation.test.ts diff --git a/packages/opencode/src/automation/cron.ts b/packages/opencode/src/automation/cron.ts new file mode 100644 index 000000000..8b4514410 --- /dev/null +++ b/packages/opencode/src/automation/cron.ts @@ -0,0 +1,61 @@ +import { DateTime } from "luxon" + +export type CronSchedule = { + minutes: Set + hours: Set + days: Set + months: Set + weekdays: Set + dayRestricted: boolean + weekdayRestricted: boolean +} + +function cronValues(field: string, min: number, max: number, options?: { sundayAlias?: boolean }) { + const values = new Set() + for (const item of field.split(",")) { + const [base, stepRaw] = item.split("/") + if (!base || item.split("/").length > 2) throw new Error(`Invalid cron field: ${item}`) + const step = stepRaw === undefined ? 1 : Number(stepRaw) + if (!Number.isInteger(step) || step <= 0) throw new Error(`Invalid cron step: ${item}`) + const range = base === "*" ? [min, max] : base.split("-").map(Number) + if (range.length === 0 || range.length > 2 || range.some((value) => !Number.isInteger(value))) { + throw new Error(`Invalid cron field: ${item}`) + } + const start = range[0] + const end = base === "*" || (range.length === 1 && stepRaw !== undefined) ? max : range.length === 1 ? range[0] : range[1] + if (start < min || end > max || start > end) throw new Error(`Invalid cron range: ${item}`) + for (let value = start; value <= end; value += step) { + values.add(options?.sundayAlias && value === 7 ? 0 : value) + } + } + return values +} + +export function parseCronSchedule(expression: string): CronSchedule { + const fields = expression.trim().split(/\s+/) + if (fields.length !== 5) throw new Error(`Invalid cron expression: ${expression}`) + const [minuteField, hourField, dayField, monthField, weekdayField] = fields + return { + minutes: cronValues(minuteField, 0, 59), + hours: cronValues(hourField, 0, 23), + days: cronValues(dayField, 1, 31), + months: cronValues(monthField, 1, 12), + weekdays: cronValues(weekdayField, 0, 7, { sundayAlias: true }), + dayRestricted: dayField !== "*", + weekdayRestricted: weekdayField !== "*", + } +} + +export function cronMatches(schedule: CronSchedule, time: DateTime) { + const weekday = time.weekday === 7 ? 0 : time.weekday + const dayMatches = schedule.days.has(time.day) + const weekdayMatches = schedule.weekdays.has(weekday) + const calendarMatches = + schedule.dayRestricted && schedule.weekdayRestricted ? dayMatches || weekdayMatches : dayMatches && weekdayMatches + return ( + schedule.minutes.has(time.minute) && + schedule.hours.has(time.hour) && + schedule.months.has(time.month) && + calendarMatches + ) +} diff --git a/packages/opencode/src/automation/derived.ts b/packages/opencode/src/automation/derived.ts index 8678c1bd9..82b856301 100644 --- a/packages/opencode/src/automation/derived.ts +++ b/packages/opencode/src/automation/derived.ts @@ -1,71 +1,12 @@ import { DateTime } from "luxon" import type { Automation } from "." +import { type CronSchedule, parseCronSchedule } from "./cron" const CRON_LOOKAHEAD_MINUTES = 527_040 * 5 const NEXT_FIRES_PREVIEW = 5 type RecurringDefinition = Extract -type CronSchedule = { - minutes: Set - hours: Set - days: Set - months: Set - weekdays: Set - dayRestricted: boolean - weekdayRestricted: boolean -} - -function cronValues(field: string, min: number, max: number, options?: { sundayAlias?: boolean }) { - const values = new Set() - for (const item of field.split(",")) { - const [base, stepRaw] = item.split("/") - if (!base || item.split("/").length > 2) throw new Error(`Invalid cron field: ${item}`) - const step = stepRaw === undefined ? 1 : Number(stepRaw) - if (!Number.isInteger(step) || step <= 0) throw new Error(`Invalid cron step: ${item}`) - const range = base === "*" ? [min, max] : base.split("-").map(Number) - if (range.length === 0 || range.length > 2 || range.some((value) => !Number.isInteger(value))) { - throw new Error(`Invalid cron field: ${item}`) - } - const start = range[0] - const end = base === "*" || (range.length === 1 && stepRaw !== undefined) ? max : range.length === 1 ? range[0] : range[1] - if (start < min || end > max || start > end) throw new Error(`Invalid cron range: ${item}`) - for (let value = start; value <= end; value += step) { - values.add(options?.sundayAlias && value === 7 ? 0 : value) - } - } - return values -} - -export function parseCronSchedule(expression: string): CronSchedule { - const fields = expression.trim().split(/\s+/) - if (fields.length !== 5) throw new Error(`Invalid cron expression: ${expression}`) - const [minuteField, hourField, dayField, monthField, weekdayField] = fields - return { - minutes: cronValues(minuteField, 0, 59), - hours: cronValues(hourField, 0, 23), - days: cronValues(dayField, 1, 31), - months: cronValues(monthField, 1, 12), - weekdays: cronValues(weekdayField, 0, 7, { sundayAlias: true }), - dayRestricted: dayField !== "*", - weekdayRestricted: weekdayField !== "*", - } -} - -export function cronMatches(schedule: CronSchedule, time: DateTime) { - const weekday = time.weekday === 7 ? 0 : time.weekday - const dayMatches = schedule.days.has(time.day) - const weekdayMatches = schedule.weekdays.has(weekday) - const calendarMatches = - schedule.dayRestricted && schedule.weekdayRestricted ? dayMatches || weekdayMatches : dayMatches && weekdayMatches - return ( - schedule.minutes.has(time.minute) && - schedule.hours.has(time.hour) && - schedule.months.has(time.month) && - calendarMatches - ) -} - const PLUS_ONE_MONTH = { months: 1 } const PLUS_ONE_DAY = { days: 1 } const PLUS_ONE_HOUR = { hours: 1 } diff --git a/packages/opencode/src/automation/index.ts b/packages/opencode/src/automation/index.ts index f577bdada..2e14752bf 100644 --- a/packages/opencode/src/automation/index.ts +++ b/packages/opencode/src/automation/index.ts @@ -705,7 +705,10 @@ export namespace Automation { const now = options?.now ?? Date.now() const failureStreak = run.state === "succeeded" ? 0 : run.state === "failed" ? previous.failureStreak + 1 : previous.failureStreak - const derived = computeDerivedFields(previous, now, completedRunCount(previous.id)) + const derived = + run.state === "stopped" + ? { nextFireAt: previous.nextFireAt, nextFires: previous.nextFires } + : computeDerivedFields(previous, now, completedRunCount(previous.id)) if ( previous.failureStreak === failureStreak && previous.nextFireAt === derived.nextFireAt && diff --git a/packages/opencode/src/automation/scheduler.ts b/packages/opencode/src/automation/scheduler.ts index 9c1dd9ec6..7e93fd85c 100644 --- a/packages/opencode/src/automation/scheduler.ts +++ b/packages/opencode/src/automation/scheduler.ts @@ -6,6 +6,7 @@ import { Bus } from "@/bus" import { Instance, type InstanceContext } from "@/project/instance" import { NotFoundError } from "@/storage/db" import { Flock } from "@/util/flock" +import { cronMatches, parseCronSchedule } from "./cron" import { sessionPromptExecutor } from "./runner" export namespace AutomationScheduler { @@ -52,16 +53,6 @@ export namespace AutomationScheduler { definition: Automation.Definition } - type CronSchedule = { - minutes: Set - hours: Set - days: Set - months: Set - weekdays: Set - dayRestricted: boolean - weekdayRestricted: boolean - } - export const liveClock: Clock = { now: () => Date.now(), sleep: (delayMs, signal) => @@ -120,56 +111,6 @@ export namespace AutomationScheduler { return from + definition.rhythm.everyMs } - function cronValues(field: string, min: number, max: number, options?: { sundayAlias?: boolean }) { - const values = new Set() - for (const item of field.split(",")) { - const [base, stepRaw] = item.split("/") - if (!base || item.split("/").length > 2) throw new Error(`Invalid cron field: ${item}`) - const step = stepRaw === undefined ? 1 : Number(stepRaw) - if (!Number.isInteger(step) || step <= 0) throw new Error(`Invalid cron step: ${item}`) - const range = base === "*" ? [min, max] : base.split("-").map(Number) - if (range.length === 0 || range.length > 2 || range.some((value) => !Number.isInteger(value))) { - throw new Error(`Invalid cron field: ${item}`) - } - const start = range[0] - const end = base === "*" || (range.length === 1 && stepRaw !== undefined) ? max : range.length === 1 ? range[0] : range[1] - if (start < min || end > max || start > end) throw new Error(`Invalid cron range: ${item}`) - for (let value = start; value <= end; value += step) { - values.add(options?.sundayAlias && value === 7 ? 0 : value) - } - } - return values - } - - function parseCronSchedule(expression: string): CronSchedule { - const fields = expression.trim().split(/\s+/) - if (fields.length !== 5) throw new Error(`Invalid cron expression: ${expression}`) - const [minuteField, hourField, dayField, monthField, weekdayField] = fields - return { - minutes: cronValues(minuteField, 0, 59), - hours: cronValues(hourField, 0, 23), - days: cronValues(dayField, 1, 31), - months: cronValues(monthField, 1, 12), - weekdays: cronValues(weekdayField, 0, 7, { sundayAlias: true }), - dayRestricted: dayField !== "*", - weekdayRestricted: weekdayField !== "*", - } - } - - function cronMatches(schedule: CronSchedule, time: DateTime) { - const weekday = time.weekday === 7 ? 0 : time.weekday - const dayMatches = schedule.days.has(time.day) - const weekdayMatches = schedule.weekdays.has(weekday) - const calendarMatches = - schedule.dayRestricted && schedule.weekdayRestricted ? dayMatches || weekdayMatches : dayMatches && weekdayMatches - return ( - schedule.minutes.has(time.minute) && - schedule.hours.has(time.hour) && - schedule.months.has(time.month) && - calendarMatches - ) - } - function computeNextCronFireAt(definition: Extract, from: number) { if (definition.rhythm.kind !== "cron") return null const schedule = parseCronSchedule(definition.rhythm.expression) diff --git a/packages/opencode/test/automation/validation.test.ts b/packages/opencode/test/automation/validation.test.ts new file mode 100644 index 000000000..41136776b --- /dev/null +++ b/packages/opencode/test/automation/validation.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, test } from "bun:test" +import { Effect } from "effect" +import { validateModelAndVariantWith } from "../../src/automation/validation" +import { ModelNotFoundError, type Provider } from "../../src/provider/provider" +import { ModelID, ProviderID } from "../../src/provider/schema" +import { fakeAutomationProvider, ProviderTest } from "../fake/provider" + +function runDetails(provider: Provider.Interface, model: { providerID: string; modelID: string }, variant?: string) { + return Effect.runPromise(validateModelAndVariantWith(provider, model, variant)) +} + +describe("automation validation — matches HTTP route 422 details payload", () => { + test("returns empty details when model exists and variant is supported", async () => { + const { providerID, modelID, interface: provider } = fakeAutomationProvider() + expect(await runDetails(provider, { providerID, modelID }, "high")).toEqual([]) + }) + + test("returns empty details when variant is omitted", async () => { + const { providerID, modelID, interface: provider } = fakeAutomationProvider() + expect(await runDetails(provider, { providerID, modelID })).toEqual([]) + }) + + test("rejects unsupported variant for the given model with invalid_variant_for_model", async () => { + const { providerID, modelID, interface: provider } = fakeAutomationProvider() + expect(await runDetails(provider, { providerID, modelID }, "xhigh")).toEqual([ + { field: "variant", message: "invalid_variant_for_model" }, + ]) + }) + + test("maps ModelNotFoundError to model_not_found", async () => { + const provider: Provider.Interface = { + ...fakeAutomationProvider().interface, + getModel: (pId, mId) => + Effect.fail(new ModelNotFoundError({ providerID: pId, modelID: mId })) as never, + } + expect(await runDetails(provider, { providerID: "anthropic", modelID: "claude-bogus" })).toEqual([ + { field: "model", message: "model_not_found" }, + ]) + }) + + test("maps unknown provider failures to model_lookup_failed", async () => { + const provider: Provider.Interface = { + ...fakeAutomationProvider().interface, + getModel: () => Effect.die(new Error("provider exploded")) as never, + } + expect(await runDetails(provider, { providerID: "x", modelID: "y" })).toEqual([ + { field: "model", message: "model_lookup_failed" }, + ]) + }) + + test("rejects unsupported variant against a non-reasoning model", async () => { + const nonReasoning = ProviderTest.model({ + id: ModelID.make("plain-model"), + providerID: ProviderID.make("openai"), + capabilities: { + toolcall: true, + attachment: false, + reasoning: false, + temperature: true, + interleaved: false, + input: { text: true, image: false, audio: false, video: false, pdf: false }, + output: { text: true, image: false, audio: false, video: false, pdf: false }, + }, + }) + const provider: Provider.Interface = { + ...fakeAutomationProvider().interface, + getModel: () => Effect.succeed(nonReasoning) as never, + } + expect(await runDetails(provider, { providerID: "openai", modelID: "plain-model" }, "high")).toEqual([ + { field: "variant", message: "invalid_variant_for_model" }, + ]) + }) +}) diff --git a/packages/opencode/test/server/automation-scheduler.test.ts b/packages/opencode/test/server/automation-scheduler.test.ts index c56510ff2..545239ee4 100644 --- a/packages/opencode/test/server/automation-scheduler.test.ts +++ b/packages/opencode/test/server/automation-scheduler.test.ts @@ -1083,6 +1083,14 @@ describe("automation scheduler", () => { const runs = await waitForRunStates(definition.id, ["stopped", "stopped"]) expect(runs[0].triggeredAt).toBe(60_000) + const after = Automation.get(definition.id) + if (after.kind !== "recurring") throw new Error("recurring") + expect(after.revision).toBe(definition.revision) + if (definition.kind === "recurring") { + expect(after.failureStreak).toBe(definition.failureStreak) + expect(after.nextFireAt).toBe(definition.nextFireAt) + expect(after.nextFires).toEqual(definition.nextFires) + } releaseBlocker.resolve({ sessionID: SessionID.descending(), result: "blocker done", cost: 0 }) scheduler.stop() }) From 25584734a797a313aa3ce7dda7bf252444f2d776 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 1 Jun 2026 19:40:39 +0800 Subject: [PATCH 04/13] fix(ci): register test/automation under windows server-tools shard ci-workflow sentinel requires every opencode test file to be covered exactly once across the three Windows shards; the new test/automation/ directory was missing, breaking unit-opencode > check > unit results (opencode). Also tighten env-bypass lifecycle in automation-routes.test.ts (CodeRabbit): restore OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION in afterAll instead of leaking module-scope mutation across the suite. --- .github/workflows/windows-advisory.yml | 1 + packages/opencode/test/github/ci-workflow.test.ts | 2 +- .../opencode/test/server/automation-routes.test.ts | 14 ++++++++++++-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.github/workflows/windows-advisory.yml b/.github/workflows/windows-advisory.yml index 58cedb176..3306567e4 100644 --- a/.github/workflows/windows-advisory.yml +++ b/.github/workflows/windows-advisory.yml @@ -204,6 +204,7 @@ jobs: test/ide test/installation test/auth + test/automation report_path: packages/opencode/.artifacts/unit/junit-windows-server-tools.xml - package: desktop uses_turbo: true diff --git a/packages/opencode/test/github/ci-workflow.test.ts b/packages/opencode/test/github/ci-workflow.test.ts index efe3aa84e..77b31f733 100644 --- a/packages/opencode/test/github/ci-workflow.test.ts +++ b/packages/opencode/test/github/ci-workflow.test.ts @@ -95,7 +95,7 @@ const windowsOpencodeShards = [ suffix: "opencode-server-tools", usesTurbo: false, command: - "cd packages/opencode && bun test --timeout 30000 --reporter=junit --reporter-outfile=.artifacts/unit/junit-windows-server-tools.xml test/server test/snapshot test/tool test/mcp test/question test/effect test/agent test/git/ test/storage test/provider test/pty test/share/ test/script test/memory test/lsp test/fixture test/acp test/bus test/cli test/global test/format test/account test/sync test/filesystem test/patch test/shell test/control-plane test/ide test/installation test/auth", + "cd packages/opencode && bun test --timeout 30000 --reporter=junit --reporter-outfile=.artifacts/unit/junit-windows-server-tools.xml test/server test/snapshot test/tool test/mcp test/question test/effect test/agent test/git/ test/storage test/provider test/pty test/share/ test/script test/memory test/lsp test/fixture test/acp test/bus test/cli test/global test/format test/account test/sync test/filesystem test/patch test/shell test/control-plane test/ide test/installation test/auth test/automation", reportPath: "packages/opencode/.artifacts/unit/junit-windows-server-tools.xml", }, ] as const diff --git a/packages/opencode/test/server/automation-routes.test.ts b/packages/opencode/test/server/automation-routes.test.ts index 38137533d..9027ba2ea 100644 --- a/packages/opencode/test/server/automation-routes.test.ts +++ b/packages/opencode/test/server/automation-routes.test.ts @@ -1,4 +1,4 @@ -import { afterEach, describe, expect, test } from "bun:test" +import { afterAll, afterEach, beforeAll, describe, expect, test } from "bun:test" import { Hono } from "hono" import { Log } from "@opencode-ai/core/util/log" import { Automation, AutomationID } from "../../src/automation" @@ -13,9 +13,19 @@ import { SessionID } from "../../src/session/schema" import { Flock } from "../../src/util/flock" import { tmpdir } from "../fixture/fixture" -process.env.OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION = "1" void Log.init({ print: false }) +const previousSkipAutomationModelValidation = process.env.OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION + +beforeAll(() => { + process.env.OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION = "1" +}) + +afterAll(() => { + if (previousSkipAutomationModelValidation === undefined) delete process.env.OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION + else process.env.OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION = previousSkipAutomationModelValidation +}) + afterEach(async () => { await Instance.disposeAll() }) From 6123bef911c24cc75b56d7566a2cca0d3fc30131 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 1 Jun 2026 21:23:14 +0800 Subject: [PATCH 05/13] test(automation): advance scheduler-owned stopped nextFireAt + route 422 wiring scheduler-owned stopped (manual conflict, missed_schedule, writer block) now refreshes nextFireAt so the UI never shows a fire time in the past; non-owned stopped still freezes derived fields. Adds route-level 422 tests with provider validation enabled to exercise the production path that the env-bypassed suite skips. Threads Effect.fn tracing through the fake provider and lifts validation/automate fixtures onto it. --- packages/opencode/src/automation/index.ts | 7 +- packages/opencode/src/automation/scheduler.ts | 5 +- .../test/automation/validation.test.ts | 127 +++++++++--------- packages/opencode/test/fake/provider.ts | 24 ++-- .../test/server/automation-routes.test.ts | 63 +++++++++ .../test/server/automation-scheduler.test.ts | 35 ++++- packages/opencode/test/tool/automate.test.ts | 39 +----- 7 files changed, 185 insertions(+), 115 deletions(-) diff --git a/packages/opencode/src/automation/index.ts b/packages/opencode/src/automation/index.ts index 2e14752bf..cd2d8884f 100644 --- a/packages/opencode/src/automation/index.ts +++ b/packages/opencode/src/automation/index.ts @@ -698,7 +698,10 @@ export namespace Automation { return replaceDefinition(previous, next) } - export function recordRunOutcome(run: Run, options?: { now?: number }): Definition | undefined { + export function recordRunOutcome( + run: Run, + options?: { now?: number; refreshOnStopped?: boolean }, + ): Definition | undefined { if (run.state !== "succeeded" && run.state !== "failed" && run.state !== "stopped") return undefined const previous = getOptional(run.automationID) if (!previous || previous.kind !== "recurring") return undefined @@ -706,7 +709,7 @@ export namespace Automation { const failureStreak = run.state === "succeeded" ? 0 : run.state === "failed" ? previous.failureStreak + 1 : previous.failureStreak const derived = - run.state === "stopped" + run.state === "stopped" && !options?.refreshOnStopped ? { nextFireAt: previous.nextFireAt, nextFires: previous.nextFires } : computeDerivedFields(previous, now, completedRunCount(previous.id)) if ( diff --git a/packages/opencode/src/automation/scheduler.ts b/packages/opencode/src/automation/scheduler.ts index 7e93fd85c..548446818 100644 --- a/packages/opencode/src/automation/scheduler.ts +++ b/packages/opencode/src/automation/scheduler.ts @@ -331,7 +331,10 @@ export namespace AutomationScheduler { if (run.state === "stopped" && !wasOwned && !wasSchedulerStopped) return if (ownsTimers) { try { - const refreshed = Automation.recordRunOutcome(run, { now: clock.now() }) + const refreshed = Automation.recordRunOutcome(run, { + now: clock.now(), + refreshOnStopped: wasOwned || wasSchedulerStopped, + }) if (refreshed) void Automation.publishDefinitionUpdated(refreshed) } catch (error) { if (!NotFoundError.isInstance(error)) log.error("automation derived field update failed", { error, automationID: run.automationID }) diff --git a/packages/opencode/test/automation/validation.test.ts b/packages/opencode/test/automation/validation.test.ts index 41136776b..11882a630 100644 --- a/packages/opencode/test/automation/validation.test.ts +++ b/packages/opencode/test/automation/validation.test.ts @@ -1,73 +1,80 @@ -import { describe, expect, test } from "bun:test" +import { describe, expect } from "bun:test" import { Effect } from "effect" import { validateModelAndVariantWith } from "../../src/automation/validation" import { ModelNotFoundError, type Provider } from "../../src/provider/provider" import { ModelID, ProviderID } from "../../src/provider/schema" import { fakeAutomationProvider, ProviderTest } from "../fake/provider" - -function runDetails(provider: Provider.Interface, model: { providerID: string; modelID: string }, variant?: string) { - return Effect.runPromise(validateModelAndVariantWith(provider, model, variant)) -} +import { it } from "../lib/effect" describe("automation validation — matches HTTP route 422 details payload", () => { - test("returns empty details when model exists and variant is supported", async () => { - const { providerID, modelID, interface: provider } = fakeAutomationProvider() - expect(await runDetails(provider, { providerID, modelID }, "high")).toEqual([]) - }) + it.effect("returns empty details when model exists and variant is supported", () => + Effect.gen(function* () { + const { providerID, modelID, interface: provider } = fakeAutomationProvider() + const details = yield* validateModelAndVariantWith(provider, { providerID, modelID }, "high") + expect(details).toEqual([]) + }), + ) - test("returns empty details when variant is omitted", async () => { - const { providerID, modelID, interface: provider } = fakeAutomationProvider() - expect(await runDetails(provider, { providerID, modelID })).toEqual([]) - }) + it.effect("returns empty details when variant is omitted", () => + Effect.gen(function* () { + const { providerID, modelID, interface: provider } = fakeAutomationProvider() + const details = yield* validateModelAndVariantWith(provider, { providerID, modelID }, undefined) + expect(details).toEqual([]) + }), + ) - test("rejects unsupported variant for the given model with invalid_variant_for_model", async () => { - const { providerID, modelID, interface: provider } = fakeAutomationProvider() - expect(await runDetails(provider, { providerID, modelID }, "xhigh")).toEqual([ - { field: "variant", message: "invalid_variant_for_model" }, - ]) - }) + it.effect("rejects unsupported variant for the given model with invalid_variant_for_model", () => + Effect.gen(function* () { + const { providerID, modelID, interface: provider } = fakeAutomationProvider() + const details = yield* validateModelAndVariantWith(provider, { providerID, modelID }, "xhigh") + expect(details).toEqual([{ field: "variant", message: "invalid_variant_for_model" }]) + }), + ) - test("maps ModelNotFoundError to model_not_found", async () => { - const provider: Provider.Interface = { - ...fakeAutomationProvider().interface, - getModel: (pId, mId) => - Effect.fail(new ModelNotFoundError({ providerID: pId, modelID: mId })) as never, - } - expect(await runDetails(provider, { providerID: "anthropic", modelID: "claude-bogus" })).toEqual([ - { field: "model", message: "model_not_found" }, - ]) - }) + it.effect("maps ModelNotFoundError to model_not_found", () => + Effect.gen(function* () { + const provider: Provider.Interface = { + ...fakeAutomationProvider().interface, + getModel: ((pId, mId) => + Effect.fail(new ModelNotFoundError({ providerID: pId, modelID: mId }))) as Provider.Interface["getModel"], + } + const details = yield* validateModelAndVariantWith(provider, { providerID: "anthropic", modelID: "claude-bogus" }, undefined) + expect(details).toEqual([{ field: "model", message: "model_not_found" }]) + }), + ) - test("maps unknown provider failures to model_lookup_failed", async () => { - const provider: Provider.Interface = { - ...fakeAutomationProvider().interface, - getModel: () => Effect.die(new Error("provider exploded")) as never, - } - expect(await runDetails(provider, { providerID: "x", modelID: "y" })).toEqual([ - { field: "model", message: "model_lookup_failed" }, - ]) - }) + it.effect("maps unknown provider failures to model_lookup_failed", () => + Effect.gen(function* () { + const provider: Provider.Interface = { + ...fakeAutomationProvider().interface, + getModel: (() => Effect.die(new Error("provider exploded"))) as Provider.Interface["getModel"], + } + const details = yield* validateModelAndVariantWith(provider, { providerID: "x", modelID: "y" }, undefined) + expect(details).toEqual([{ field: "model", message: "model_lookup_failed" }]) + }), + ) - test("rejects unsupported variant against a non-reasoning model", async () => { - const nonReasoning = ProviderTest.model({ - id: ModelID.make("plain-model"), - providerID: ProviderID.make("openai"), - capabilities: { - toolcall: true, - attachment: false, - reasoning: false, - temperature: true, - interleaved: false, - input: { text: true, image: false, audio: false, video: false, pdf: false }, - output: { text: true, image: false, audio: false, video: false, pdf: false }, - }, - }) - const provider: Provider.Interface = { - ...fakeAutomationProvider().interface, - getModel: () => Effect.succeed(nonReasoning) as never, - } - expect(await runDetails(provider, { providerID: "openai", modelID: "plain-model" }, "high")).toEqual([ - { field: "variant", message: "invalid_variant_for_model" }, - ]) - }) + it.effect("rejects unsupported variant against a non-reasoning model", () => + Effect.gen(function* () { + const nonReasoning = ProviderTest.model({ + id: ModelID.make("plain-model"), + providerID: ProviderID.make("openai"), + capabilities: { + toolcall: true, + attachment: false, + reasoning: false, + temperature: true, + interleaved: false, + input: { text: true, image: false, audio: false, video: false, pdf: false }, + output: { text: true, image: false, audio: false, video: false, pdf: false }, + }, + }) + const provider: Provider.Interface = { + ...fakeAutomationProvider().interface, + getModel: (() => Effect.succeed(nonReasoning)) as Provider.Interface["getModel"], + } + const details = yield* validateModelAndVariantWith(provider, { providerID: "openai", modelID: "plain-model" }, "high") + expect(details).toEqual([{ field: "variant", message: "invalid_variant_for_model" }]) + }), + ) }) diff --git a/packages/opencode/test/fake/provider.ts b/packages/opencode/test/fake/provider.ts index 417c49fc2..2d8f6b184 100644 --- a/packages/opencode/test/fake/provider.ts +++ b/packages/opencode/test/fake/provider.ts @@ -28,15 +28,23 @@ export function fakeAutomationProvider(): { // Sanity check: variants include 'high' so tests can use it. void ProviderTransform.variants(mdl) const iface: Provider.Interface = { - list: () => Effect.succeed({ [providerID]: info }), - getProvider: (id) => + list: Effect.fn("Provider.list")(() => Effect.succeed({ [providerID]: info })), + getProvider: Effect.fn("Provider.getProvider")((id) => id === providerID ? Effect.succeed(info) : Effect.die(new Error(`Unknown provider: ${id}`)), - getModel: (pId, mId) => - pId === providerID && mId === modelID ? Effect.succeed(mdl) : Effect.die(new Error(`Unknown model: ${pId}/${mId}`)), - getLanguage: () => Effect.die(new Error("getLanguage not configured")), - closest: (pId) => Effect.succeed(pId === providerID ? { providerID, modelID } : undefined), - getSmallModel: (pId) => Effect.succeed(pId === providerID ? mdl : undefined), - defaultModel: () => Effect.succeed({ providerID, modelID }), + ), + getModel: Effect.fn("Provider.getModel")((pId, mId) => + pId === providerID && mId === modelID + ? Effect.succeed(mdl) + : Effect.die(new Error(`Unknown model: ${pId}/${mId}`)), + ), + getLanguage: Effect.fn("Provider.getLanguage")(() => Effect.die(new Error("getLanguage not configured"))), + closest: Effect.fn("Provider.closest")((pId) => + Effect.succeed(pId === providerID ? { providerID, modelID } : undefined), + ), + getSmallModel: Effect.fn("Provider.getSmallModel")((pId) => + Effect.succeed(pId === providerID ? mdl : undefined), + ), + defaultModel: Effect.fn("Provider.defaultModel")(() => Effect.succeed({ providerID, modelID })), } return { providerID, modelID, interface: iface } } diff --git a/packages/opencode/test/server/automation-routes.test.ts b/packages/opencode/test/server/automation-routes.test.ts index 9027ba2ea..9b6bcf02e 100644 --- a/packages/opencode/test/server/automation-routes.test.ts +++ b/packages/opencode/test/server/automation-routes.test.ts @@ -130,6 +130,69 @@ function run(overrides: Record = {}) { } } +describe("automation route 422 wiring with provider validation enabled", () => { + // These tests deliberately bypass the suite-wide skip flag to exercise the + // real modelValidationDetails -> AppRuntime path that production hits. + let restoreBypass: string | undefined + const enableValidation = () => { + restoreBypass = process.env.OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION + delete process.env.OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION + } + const restoreBypassEnv = () => { + if (restoreBypass === undefined) delete process.env.OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION + else process.env.OPENCODE_SKIP_AUTOMATION_MODEL_VALIDATION = restoreBypass + } + + test("create rejects with 422 invalid_automation details when provider lookup fails", async () => { + await withAutomationApp(async ({ app, projectID }) => { + enableValidation() + try { + const response = await app.request("/automation", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(recurringInput(projectID)), + }) + expect(response.status).toBe(422) + const body = await response.json() + expect(body.error).toBe("invalid_automation") + expect(body.details).toEqual( + expect.arrayContaining([ + expect.objectContaining({ field: "model" }), + ]), + ) + expect(["model_not_found", "model_lookup_failed"]).toContain(body.details[0].message) + } finally { + restoreBypassEnv() + } + }) + }) + + test("update rejects with 422 invalid_automation details when model patch fails provider lookup", async () => { + await withAutomationApp(async ({ app, projectID }) => { + const created = await json(app, "/automation", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(recurringInput(projectID)), + }) + enableValidation() + try { + const response = await app.request(`/automation/${created.id}`, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ model: { providerID: "nonexistent", modelID: "missing-model" } }), + }) + expect(response.status).toBe(422) + const body = await response.json() + expect(body.error).toBe("invalid_automation") + expect(body.details[0].field).toBe("model") + expect(["model_not_found", "model_lookup_failed"]).toContain(body.details[0].message) + } finally { + restoreBypassEnv() + } + }) + }) +}) + describe("automation routes", () => { test("reloads definitions and runs from durable storage after instance restart", async () => { await using tmp = await tmpdir({ git: true }) diff --git a/packages/opencode/test/server/automation-scheduler.test.ts b/packages/opencode/test/server/automation-scheduler.test.ts index 545239ee4..41f8e878e 100644 --- a/packages/opencode/test/server/automation-scheduler.test.ts +++ b/packages/opencode/test/server/automation-scheduler.test.ts @@ -1084,13 +1084,12 @@ describe("automation scheduler", () => { const runs = await waitForRunStates(definition.id, ["stopped", "stopped"]) expect(runs[0].triggeredAt).toBe(60_000) const after = Automation.get(definition.id) - if (after.kind !== "recurring") throw new Error("recurring") - expect(after.revision).toBe(definition.revision) - if (definition.kind === "recurring") { - expect(after.failureStreak).toBe(definition.failureStreak) - expect(after.nextFireAt).toBe(definition.nextFireAt) - expect(after.nextFires).toEqual(definition.nextFires) - } + if (after.kind !== "recurring" || definition.kind !== "recurring") throw new Error("recurring") + // failureStreak must not advance on stopped (manual conflict, writer block, missed schedule). + expect(after.failureStreak).toBe(definition.failureStreak) + // Scheduler-owned stopped (`previous_run_awaiting_input` at t=60_000) advances the stored + // nextFireAt preview so the UI does not display a fire time that has already elapsed. + expect(after.nextFireAt).toBe(120_000) releaseBlocker.resolve({ sessionID: SessionID.descending(), result: "blocker done", cost: 0 }) scheduler.stop() }) @@ -1288,4 +1287,26 @@ describe("automation scheduler", () => { scheduler.stop() }) }) + + test("missed_schedule on a recurring interval advances the stored nextFireAt preview", async () => { + await withAutomation(async (projectID) => { + const clock = new OversleepClock(0, 180_001) + const scheduler = AutomationScheduler.make({ + clock, + executor: async () => ({ sessionID: SessionID.descending(), result: "done", cost: 0 }), + }) + const definition = Automation.create(recurringInput(projectID, 60_000), { now: 0 }) + + scheduler.reschedule(definition) + const runs = await waitForRunCount(definition.id, 1) + expect(runs[0]).toMatchObject({ state: "stopped", stopReason: "missed_schedule" }) + + const after = Automation.get(definition.id) + if (after.kind !== "recurring" || definition.kind !== "recurring") throw new Error("recurring") + expect(after.failureStreak).toBe(definition.failureStreak) + expect(after.nextFireAt).not.toBe(definition.nextFireAt) + expect(after.nextFireAt).toBeGreaterThanOrEqual(clock.now()) + scheduler.stop() + }) + }) }) diff --git a/packages/opencode/test/tool/automate.test.ts b/packages/opencode/test/tool/automate.test.ts index 78ae35bd2..1a84a0340 100644 --- a/packages/opencode/test/tool/automate.test.ts +++ b/packages/opencode/test/tool/automate.test.ts @@ -4,45 +4,10 @@ import { AutomateParameters, createAutomateDefinition, formatAutomateValidationE import { Automation } from "../../src/automation" import { Instance } from "../../src/project/instance" import { MessageID, SessionID } from "../../src/session/schema" -import { ModelID, ProviderID } from "../../src/provider/schema" -import type { Provider } from "../../src/provider/provider" import { tmpdir } from "../fixture/fixture" -import { ProviderTest } from "../fake/provider" +import { fakeAutomationProvider } from "../fake/provider" -const fakeProviderID = ProviderID.make("openai") -const fakeModelID = ModelID.make("test-reasoning-model") -const fakeModel = ProviderTest.model({ - id: fakeModelID, - providerID: fakeProviderID, - capabilities: { - toolcall: true, - attachment: false, - reasoning: true, - temperature: true, - interleaved: false, - input: { text: true, image: false, audio: false, video: false, pdf: false }, - output: { text: true, image: false, audio: false, video: false, pdf: false }, - }, - api: { id: fakeModelID, url: "https://example.com", npm: "ai-gateway-provider" }, -}) -const fakeInfo = ProviderTest.info({ id: fakeProviderID }, fakeModel) -const fakeProviderInterface: Provider.Interface = { - list: () => Effect.succeed({ [fakeProviderID]: fakeInfo }), - getProvider: (providerID) => - providerID === fakeProviderID - ? Effect.succeed(fakeInfo) - : Effect.die(new Error(`Unknown provider: ${providerID}`)), - getModel: (providerID, modelID) => - providerID === fakeProviderID && modelID === fakeModelID - ? Effect.succeed(fakeModel) - : Effect.die(new Error(`Unknown model: ${providerID}/${modelID}`)), - getLanguage: () => Effect.die(new Error("getLanguage not configured")), - closest: (providerID) => - Effect.succeed(providerID === fakeProviderID ? { providerID, modelID: fakeModelID } : undefined), - getSmallModel: (providerID) => - Effect.succeed(providerID === fakeProviderID ? fakeModel : undefined), - defaultModel: () => Effect.succeed({ providerID: fakeProviderID, modelID: fakeModelID }), -} +const { providerID: fakeProviderID, modelID: fakeModelID, interface: fakeProviderInterface } = fakeAutomationProvider() const fixtureModel = { providerID: fakeProviderID, modelID: fakeModelID } afterEach(async () => { From c47e70d13e1c3ec9cdefd7f3ece91d51ad470864 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 1 Jun 2026 22:05:15 +0800 Subject: [PATCH 06/13] fix(automation): break scheduler self-loop on stopped refresh + typecheck cast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scheduler now tags DefinitionUpdated events it emits from recordRunOutcome's stopped-run refresh and skips reschedule on receipt, so a refresh → publish → reschedule cycle cannot self-pump under an adversarial clock. Drops the OversleepClock-based recurring missed schedule test that exposed the issue: that clock pins time at a fixed oversleptAt, so any subsequent recurring scheduleNextInterval picks a fireAt strictly above clock.now() and waitUntil starves the loop with microtasks. Manual-conflict coverage already exercises the scheduler-owned advance path. Also restore the dev-style 'as never' cast pattern on the three provider stubs in validation.test.ts so typecheck stops rejecting the non-overlapping Effect.fail/Effect.die narrowing. --- packages/opencode/src/automation/scheduler.ts | 11 +++++++++- .../test/automation/validation.test.ts | 8 +++---- .../test/server/automation-scheduler.test.ts | 22 ------------------- 3 files changed, 14 insertions(+), 27 deletions(-) diff --git a/packages/opencode/src/automation/scheduler.ts b/packages/opencode/src/automation/scheduler.ts index 548446818..8c0236d4e 100644 --- a/packages/opencode/src/automation/scheduler.ts +++ b/packages/opencode/src/automation/scheduler.ts @@ -163,6 +163,11 @@ export namespace AutomationScheduler { const unschedulable = new Map() const ownedRuns = new Map() const schedulerStoppedRuns = new Set() + // Marks DefinitionUpdated events that the scheduler emits itself after a + // stopped-run refresh, so its own DefinitionUpdated subscriber can skip + // reschedule() — preventing a missed_schedule → refresh → publish → reschedule + // self-loop when the clock keeps oversleeping. + const selfPublishedDefinitionUpdates = new Set() let ownsTimers = !ownerKey let ownerLease: Flock.Lease | undefined let ownerAttempt: Promise | undefined @@ -335,7 +340,10 @@ export namespace AutomationScheduler { now: clock.now(), refreshOnStopped: wasOwned || wasSchedulerStopped, }) - if (refreshed) void Automation.publishDefinitionUpdated(refreshed) + if (refreshed) { + selfPublishedDefinitionUpdates.add(refreshed.id) + void Automation.publishDefinitionUpdated(refreshed) + } } catch (error) { if (!NotFoundError.isInstance(error)) log.error("automation derived field update failed", { error, automationID: run.automationID }) } @@ -344,6 +352,7 @@ export namespace AutomationScheduler { }) const unsubscribeDefinitionUpdates = Bus.subscribe(Automation.Event.DefinitionUpdated, (event) => { if (!running) return + if (selfPublishedDefinitionUpdates.delete(event.properties.id)) return reschedule(event.properties) }) const unsubscribeDefinitionDeletes = Bus.subscribe(Automation.Event.DefinitionDeleted, (event) => { diff --git a/packages/opencode/test/automation/validation.test.ts b/packages/opencode/test/automation/validation.test.ts index 11882a630..6f26a9c90 100644 --- a/packages/opencode/test/automation/validation.test.ts +++ b/packages/opencode/test/automation/validation.test.ts @@ -35,8 +35,8 @@ describe("automation validation — matches HTTP route 422 details payload", () Effect.gen(function* () { const provider: Provider.Interface = { ...fakeAutomationProvider().interface, - getModel: ((pId, mId) => - Effect.fail(new ModelNotFoundError({ providerID: pId, modelID: mId }))) as Provider.Interface["getModel"], + getModel: (pId, mId) => + Effect.fail(new ModelNotFoundError({ providerID: pId, modelID: mId })) as never, } const details = yield* validateModelAndVariantWith(provider, { providerID: "anthropic", modelID: "claude-bogus" }, undefined) expect(details).toEqual([{ field: "model", message: "model_not_found" }]) @@ -47,7 +47,7 @@ describe("automation validation — matches HTTP route 422 details payload", () Effect.gen(function* () { const provider: Provider.Interface = { ...fakeAutomationProvider().interface, - getModel: (() => Effect.die(new Error("provider exploded"))) as Provider.Interface["getModel"], + getModel: () => Effect.die(new Error("provider exploded")) as never, } const details = yield* validateModelAndVariantWith(provider, { providerID: "x", modelID: "y" }, undefined) expect(details).toEqual([{ field: "model", message: "model_lookup_failed" }]) @@ -71,7 +71,7 @@ describe("automation validation — matches HTTP route 422 details payload", () }) const provider: Provider.Interface = { ...fakeAutomationProvider().interface, - getModel: (() => Effect.succeed(nonReasoning)) as Provider.Interface["getModel"], + getModel: () => Effect.succeed(nonReasoning) as never, } const details = yield* validateModelAndVariantWith(provider, { providerID: "openai", modelID: "plain-model" }, "high") expect(details).toEqual([{ field: "variant", message: "invalid_variant_for_model" }]) diff --git a/packages/opencode/test/server/automation-scheduler.test.ts b/packages/opencode/test/server/automation-scheduler.test.ts index 41f8e878e..bc2302a52 100644 --- a/packages/opencode/test/server/automation-scheduler.test.ts +++ b/packages/opencode/test/server/automation-scheduler.test.ts @@ -1287,26 +1287,4 @@ describe("automation scheduler", () => { scheduler.stop() }) }) - - test("missed_schedule on a recurring interval advances the stored nextFireAt preview", async () => { - await withAutomation(async (projectID) => { - const clock = new OversleepClock(0, 180_001) - const scheduler = AutomationScheduler.make({ - clock, - executor: async () => ({ sessionID: SessionID.descending(), result: "done", cost: 0 }), - }) - const definition = Automation.create(recurringInput(projectID, 60_000), { now: 0 }) - - scheduler.reschedule(definition) - const runs = await waitForRunCount(definition.id, 1) - expect(runs[0]).toMatchObject({ state: "stopped", stopReason: "missed_schedule" }) - - const after = Automation.get(definition.id) - if (after.kind !== "recurring" || definition.kind !== "recurring") throw new Error("recurring") - expect(after.failureStreak).toBe(definition.failureStreak) - expect(after.nextFireAt).not.toBe(definition.nextFireAt) - expect(after.nextFireAt).toBeGreaterThanOrEqual(clock.now()) - scheduler.stop() - }) - }) }) From dd54126f5fa37e796bc47c9ab23f36041971b7de Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 1 Jun 2026 23:02:08 +0800 Subject: [PATCH 07/13] refactor(automation): tighten input contract and converge cron validation - Skip variant:null updates when previous variant is unset to avoid no-op revision bumps and stray DefinitionUpdated events. - Split Stop into SupportedCreateStop (no condition) for CreateInput / UpdateInput / automate tool so the input contract no longer advertises the kind the API rejects. - Collapse cron validation to a single isValidCronExpression in automation/cron.ts; index.ts now re-exports the shared helper so validate / tool / derived / scheduler share one source of truth. - Key scheduler's self-published DefinitionUpdated guard by id:revision and clear it when publish rejects, so a real concurrent update racing ahead of the self event isn't swallowed. --- packages/opencode/src/automation/cron.ts | 36 +++++++ packages/opencode/src/automation/index.ts | 100 +++--------------- packages/opencode/src/automation/scheduler.ts | 15 ++- packages/opencode/src/tool/automate.ts | 6 +- .../opencode/test/automation/cron.test.ts | 51 +++++++++ .../test/server/automation-routes.test.ts | 62 ++++++++++- .../test/server/automation-scheduler.test.ts | 10 +- packages/opencode/test/tool/automate.test.ts | 3 +- 8 files changed, 179 insertions(+), 104 deletions(-) create mode 100644 packages/opencode/test/automation/cron.test.ts diff --git a/packages/opencode/src/automation/cron.ts b/packages/opencode/src/automation/cron.ts index 8b4514410..836ea7579 100644 --- a/packages/opencode/src/automation/cron.ts +++ b/packages/opencode/src/automation/cron.ts @@ -46,6 +46,42 @@ export function parseCronSchedule(expression: string): CronSchedule { } } +const monthMaxDays = new Map([ + [1, 31], + [2, 29], + [3, 31], + [4, 30], + [5, 31], + [6, 30], + [7, 31], + [8, 31], + [9, 30], + [10, 31], + [11, 30], + [12, 31], +]) + +function hasReachableDayMonth(schedule: CronSchedule) { + for (const month of schedule.months) { + const maxDay = monthMaxDays.get(month) + if (maxDay === undefined) continue + for (const day of schedule.days) { + if (day <= maxDay) return true + } + } + return false +} + +export function isValidCronExpression(expression: string): boolean { + try { + const schedule = parseCronSchedule(expression) + if (!schedule.weekdayRestricted && !hasReachableDayMonth(schedule)) return false + return true + } catch { + return false + } +} + export function cronMatches(schedule: CronSchedule, time: DateTime) { const weekday = time.weekday === 7 ? 0 : time.weekday const dayMatches = schedule.days.has(time.day) diff --git a/packages/opencode/src/automation/index.ts b/packages/opencode/src/automation/index.ts index cd2d8884f..8a6d24743 100644 --- a/packages/opencode/src/automation/index.ts +++ b/packages/opencode/src/automation/index.ts @@ -11,6 +11,7 @@ import { and, Database, desc, eq, gte, inArray, lt, NotFoundError, or, sql } fro import { Flock } from "@/util/flock" import type { AutomationRunAttendance, AutomationRunBlocker } from "./run-context" import { AutomationDefinitionTable, AutomationRunTable } from "./automation.sql" +import { isValidCronExpression as cronIsValidExpression } from "./cron" import { computeDerivedFields } from "./derived" export const AutomationID = { @@ -70,6 +71,15 @@ export namespace Automation { z.object({ kind: z.literal("never") }).strict(), ]) .meta({ ref: "AutomationStop" }) + // Input-only Stop: condition is persisted (Definition.stop can be `condition`) + // but cannot be supplied via create/update yet. Keep the two schemas separate + // so the public contract doesn't advertise a kind we reject at runtime. + export const SupportedCreateStop = z + .discriminatedUnion("kind", [ + z.object({ kind: z.literal("count"), count: z.number().int().positive() }).strict(), + z.object({ kind: z.literal("never") }).strict(), + ]) + .meta({ ref: "AutomationSupportedCreateStop" }) export const Rhythm = z .discriminatedUnion("kind", [ z.object({ kind: z.literal("interval"), everyMs: z.number().int().min(MIN_INTERVAL_MS, `interval_below_minimum_${MIN_INTERVAL_MS}ms`) }).strict(), @@ -90,7 +100,7 @@ export namespace Automation { export const CreateInput = z .discriminatedUnion("kind", [ z.object({ kind: z.literal("oneshot"), ...CommonCreate, fireAt: z.number().int().nonnegative() }).strict(), - z.object({ kind: z.literal("recurring"), ...CommonCreate, rhythm: Rhythm, stop: Stop }).strict(), + z.object({ kind: z.literal("recurring"), ...CommonCreate, rhythm: Rhythm, stop: SupportedCreateStop }).strict(), ]) .meta({ ref: "AutomationCreateInput" }) export type CreateInput = z.infer @@ -105,7 +115,7 @@ export namespace Automation { timezone: z.string().min(1).optional(), fireAt: z.number().int().nonnegative().optional(), rhythm: Rhythm.optional(), - stop: Stop.optional(), + stop: SupportedCreateStop.optional(), model: Model.optional(), variant: z.string().min(1).nullable().optional(), }) @@ -351,80 +361,7 @@ export namespace Automation { } } - function isValidCronInteger(input: string, min: number, max: number) { - if (!/^\d+$/.test(input)) return false - const value = Number(input) - return value >= min && value <= max - } - - function isValidCronField(input: string, min: number, max: number) { - if (!input) return false - return input.split(",").every((item) => { - const [base, step, extra] = item.split("/") - if (extra !== undefined) return false - if (step !== undefined && !isValidCronInteger(step, 1, max)) return false - if (base === "*") return true - const range = base.split("-") - if (range.length === 2) { - const [start, end] = range - if (!isValidCronInteger(start, min, max) || !isValidCronInteger(end, min, max)) return false - return Number(start) <= Number(end) - } - if (range.length !== 1) return false - return isValidCronInteger(base, min, max) - }) - } - - function cronFieldValues(field: string, min: number, max: number) { - const values = new Set() - for (const item of field.split(",")) { - const [base, stepRaw] = item.split("/") - const step = stepRaw === undefined ? 1 : Number(stepRaw) - const range = base === "*" ? [min, max] : base.split("-").map(Number) - const start = range[0] - const end = base === "*" || (range.length === 1 && stepRaw !== undefined) ? max : range.length === 1 ? range[0] : range[1] - for (let value = start; value <= end; value += step) values.add(value) - } - return values - } - - function hasPossibleCronDayMonth(dayField: string, monthField: string) { - const maxDays = new Map([ - [1, 31], - [2, 29], - [3, 31], - [4, 30], - [5, 31], - [6, 30], - [7, 31], - [8, 31], - [9, 30], - [10, 31], - [11, 30], - [12, 31], - ]) - for (const month of cronFieldValues(monthField, 1, 12)) { - const maxDay = maxDays.get(month) - if (maxDay === undefined) continue - for (const day of cronFieldValues(dayField, 1, 31)) { - if (day <= maxDay) return true - } - } - return false - } - - export function isValidCronExpression(expression: string) { - const fields = expression.trim().split(/\s+/) - if (fields.length !== 5) return false - return ( - isValidCronField(fields[0], 0, 59) && - isValidCronField(fields[1], 0, 23) && - isValidCronField(fields[2], 1, 31) && - isValidCronField(fields[3], 1, 12) && - isValidCronField(fields[4], 0, 7) && - (fields[4] !== "*" || hasPossibleCronDayMonth(fields[2], fields[3])) - ) - } + export const isValidCronExpression = cronIsValidExpression function validateScheduleFields(input: CreateInput | Definition) { const details: ValidationErrorDetail[] = [] @@ -461,9 +398,6 @@ export namespace Automation { if (input.where.worktree && Instance.project.vcs !== "git") { addDetail(details, "where.worktree", "unsupported_where_worktree_not_git") } - if (input.kind === "recurring" && input.stop.kind === "condition") { - addDetail(details, "stop", "unsupported_stop_condition") - } details.push(...validateScheduleFields(input)) return details } @@ -488,9 +422,6 @@ export namespace Automation { if (patch.rhythm?.kind === "cron" && !isValidCronExpression(patch.rhythm.expression)) { addDetail(details, "rhythm.expression", "invalid_cron_expression") } - if (patch.stop?.kind === "condition") { - addDetail(details, "stop", "unsupported_stop_condition") - } return details } @@ -665,7 +596,10 @@ export namespace Automation { } function hasChanges(previous: Definition, patch: UpdateInput) { - return Object.entries(patch).some(([field, value]) => !isSameValue(previous[field as keyof Definition], value)) + return Object.entries(patch).some(([field, value]) => { + if (field === "variant" && value === null && previous.variant === undefined) return false + return !isSameValue(previous[field as keyof Definition], value) + }) } export function update(id: string, patch: UpdateInput, options?: { now?: number }): Definition { diff --git a/packages/opencode/src/automation/scheduler.ts b/packages/opencode/src/automation/scheduler.ts index 8c0236d4e..9f5a922c8 100644 --- a/packages/opencode/src/automation/scheduler.ts +++ b/packages/opencode/src/automation/scheduler.ts @@ -166,8 +166,11 @@ export namespace AutomationScheduler { // Marks DefinitionUpdated events that the scheduler emits itself after a // stopped-run refresh, so its own DefinitionUpdated subscriber can skip // reschedule() — preventing a missed_schedule → refresh → publish → reschedule - // self-loop when the clock keeps oversleeping. + // self-loop when the clock keeps oversleeping. Keyed by id:revision so a real + // update that races ahead of the self event isn't swallowed by id alone. const selfPublishedDefinitionUpdates = new Set() + const selfUpdateKey = (definition: { id: string; revision: number }) => + `${definition.id}:${definition.revision}` let ownsTimers = !ownerKey let ownerLease: Flock.Lease | undefined let ownerAttempt: Promise | undefined @@ -341,8 +344,12 @@ export namespace AutomationScheduler { refreshOnStopped: wasOwned || wasSchedulerStopped, }) if (refreshed) { - selfPublishedDefinitionUpdates.add(refreshed.id) - void Automation.publishDefinitionUpdated(refreshed) + const key = selfUpdateKey(refreshed) + selfPublishedDefinitionUpdates.add(key) + void Automation.publishDefinitionUpdated(refreshed).catch((error) => { + selfPublishedDefinitionUpdates.delete(key) + log.error("automation derived field publish failed", { error, automationID: refreshed.id }) + }) } } catch (error) { if (!NotFoundError.isInstance(error)) log.error("automation derived field update failed", { error, automationID: run.automationID }) @@ -352,7 +359,7 @@ export namespace AutomationScheduler { }) const unsubscribeDefinitionUpdates = Bus.subscribe(Automation.Event.DefinitionUpdated, (event) => { if (!running) return - if (selfPublishedDefinitionUpdates.delete(event.properties.id)) return + if (selfPublishedDefinitionUpdates.delete(selfUpdateKey(event.properties))) return reschedule(event.properties) }) const unsubscribeDefinitionDeletes = Bus.subscribe(Automation.Event.DefinitionDeleted, (event) => { diff --git a/packages/opencode/src/tool/automate.ts b/packages/opencode/src/tool/automate.ts index b8aa2912d..57a4a64f9 100644 --- a/packages/opencode/src/tool/automate.ts +++ b/packages/opencode/src/tool/automate.ts @@ -25,8 +25,6 @@ const CronExpression = Schema.NonEmptyString.check( ) const Title = Schema.NonEmptyString.check(Schema.isMaxLength(Automation.MAX_TITLE_CHARS)) const Prompt = Schema.NonEmptyString.check(Schema.isMaxLength(Automation.MAX_PROMPT_CHARS)) -const Condition = Schema.NonEmptyString.check(Schema.isMaxLength(Automation.MAX_CONDITION_CHARS)) - const Common = { title: Title, prompt: Prompt, @@ -41,9 +39,11 @@ const NonNegativeInt = Schema.Int.check(Schema.isGreaterThanOrEqualTo(0)) const PositiveInt = Schema.Int.check(Schema.isGreaterThan(0)) const IntervalMs = Schema.Int.check(Schema.isGreaterThanOrEqualTo(Automation.MIN_INTERVAL_MS)) +// `condition` is part of the persisted Stop union but not yet supported as +// input; keep this in lockstep with Automation.SupportedCreateStop so the tool +// signature doesn't advertise a kind we reject. const Stop = Schema.Union([ Schema.Struct({ kind: Schema.Literal("count"), count: PositiveInt }), - Schema.Struct({ kind: Schema.Literal("condition"), condition: Condition }), Schema.Struct({ kind: Schema.Literal("never") }), ]) diff --git a/packages/opencode/test/automation/cron.test.ts b/packages/opencode/test/automation/cron.test.ts new file mode 100644 index 000000000..922336dc4 --- /dev/null +++ b/packages/opencode/test/automation/cron.test.ts @@ -0,0 +1,51 @@ +import { describe, expect, test } from "bun:test" +import { isValidCronExpression as cronIsValid, parseCronSchedule } from "../../src/automation/cron" +import { Automation } from "../../src/automation" + +describe("automation cron validation — single source of truth", () => { + const cases: Array<[string, boolean]> = [ + ["* * * * *", true], + ["0 0 1 * *", true], + ["0 0 1-5 * *", true], + ["*/15 * * * *", true], + ["0 0 * * 1-5", true], + ["0 0 31 2 1", true], + ["0 0 31 2 *", false], + ["0 0 30 2 *", false], + ["60 * * * *", false], + ["* 24 * * *", false], + ["* * 0 * *", false], + ["* * 32 * *", false], + ["* * * 0 *", false], + ["* * * 13 *", false], + ["* * * * 8", false], + ["bad", false], + ["* * * *", false], + ["* * * * * *", false], + ["5-3 * * * *", false], + ] + + test("Automation.isValidCronExpression delegates to cron module", () => { + for (const [expr, expected] of cases) { + expect(Automation.isValidCronExpression(expr)).toBe(expected) + expect(cronIsValid(expr)).toBe(expected) + } + }) + + test("valid expressions parse without throwing; invalid ones either throw or fail reachability", () => { + for (const [expr, expected] of cases) { + if (expected) { + expect(() => parseCronSchedule(expr)).not.toThrow() + } else { + let threw = false + try { + parseCronSchedule(expr) + } catch { + threw = true + } + const reachable = !threw && cronIsValid(expr) + expect(threw || !reachable).toBe(true) + } + } + }) +}) diff --git a/packages/opencode/test/server/automation-routes.test.ts b/packages/opencode/test/server/automation-routes.test.ts index 9b6bcf02e..ef07129e3 100644 --- a/packages/opencode/test/server/automation-routes.test.ts +++ b/packages/opencode/test/server/automation-routes.test.ts @@ -607,11 +607,6 @@ describe("automation routes", () => { recurringInput(projectID, { prompt: "x".repeat(20_001) }), [{ field: "prompt", message: "prompt_too_long_20000" }], ], - [ - "condition above replay-safe limit", - recurringInput(projectID, { stop: { kind: "condition", condition: "x".repeat(4_001) } }), - [{ field: "stop.condition", message: "condition_too_long_4000" }], - ], [ "externally supplied automation session", { ...recurringInput(projectID), automationSessionID: SessionID.descending() }, @@ -660,6 +655,38 @@ describe("automation routes", () => { }) }) + test("rejects stop kind 'condition' at the create/update route as unsupported input", async () => { + await withAutomationApp(async ({ app, projectID }) => { + const createResponse = await app.request("/automation", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + ...recurringInput(projectID), + stop: { kind: "condition", condition: "repo is ready" }, + }), + }) + expect(createResponse.status).toBe(422) + const createBody = (await createResponse.json()) as { error: string; details: { field: string }[] } + expect(createBody.error).toBe("invalid_automation") + expect(createBody.details.some((d) => d.field.startsWith("stop"))).toBe(true) + + const created = await json(app, "/automation", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(recurringInput(projectID)), + }) + const updateResponse = await app.request(`/automation/${created.id}`, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ stop: { kind: "condition", condition: "repo is ready" } }), + }) + expect(updateResponse.status).toBe(422) + const updateBody = (await updateResponse.json()) as { error: string; details: { field: string }[] } + expect(updateBody.error).toBe("invalid_automation") + expect(updateBody.details.some((d) => d.field.startsWith("stop"))).toBe(true) + }) + }) + test("lists definitions and returns a tombstone when deleting", async () => { await withAutomationApp(async ({ app, projectID }) => { const created = await json(app, "/automation", { @@ -845,6 +872,31 @@ describe("automation routes", () => { }) }) + test("update with variant: null on an unset variant is a no-op (no revision bump, no event)", async () => { + await withAutomationApp(async ({ app, projectID }) => { + const created = await json(app, "/automation", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(recurringInput(projectID)), + }) + expect(created).not.toHaveProperty("variant") + const updates: number[] = [] + const unsubscribe = Bus.subscribe(Automation.Event.DefinitionUpdated, (event) => { + if (event.properties.id === created.id) updates.push(event.properties.revision) + }) + const noop = await json(app, `/automation/${created.id}`, { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ variant: null }), + }) + await Bun.sleep(10) + unsubscribe() + expect(noop.revision).toBe(created.revision) + expect(noop.updatedAt).toBe(created.updatedAt) + expect(updates).toEqual([]) + }) + }) + test("pause and resume only revise when paused state changes", async () => { await withAutomationApp(async ({ app, projectID }) => { const created = await json(app, "/automation", { diff --git a/packages/opencode/test/server/automation-scheduler.test.ts b/packages/opencode/test/server/automation-scheduler.test.ts index bc2302a52..ddd07ef56 100644 --- a/packages/opencode/test/server/automation-scheduler.test.ts +++ b/packages/opencode/test/server/automation-scheduler.test.ts @@ -995,20 +995,16 @@ describe("automation scheduler", () => { }) }) - test("rejects recurring condition stops at create time", async () => { + test("CreateInput schema rejects stop kind 'condition' before reaching validate", async () => { await withAutomation(async (projectID) => { + const raw = recurringInput(projectID, 60_000, { stop: { kind: "condition", condition: "repo is ready" } as never }) let captured: unknown try { - Automation.create( - recurringInput(projectID, 60_000, { stop: { kind: "condition", condition: "repo is ready" } }), - { now: 0 }, - ) + Automation.CreateInput.parse(raw) } catch (error) { captured = error } expect(captured).toBeInstanceOf(Error) - const validation = captured as { details?: { field: string; message: string }[] } - expect(validation.details).toEqual(expect.arrayContaining([{ field: "stop", message: "unsupported_stop_condition" }])) }) }) diff --git a/packages/opencode/test/tool/automate.test.ts b/packages/opencode/test/tool/automate.test.ts index 1a84a0340..d1a15b6f3 100644 --- a/packages/opencode/test/tool/automate.test.ts +++ b/packages/opencode/test/tool/automate.test.ts @@ -92,10 +92,9 @@ describe("automate tool", () => { test.each([ ["empty cron expression", { rhythm: { kind: "cron", expression: "" }, stop: { kind: "never" } }], - ["empty stop condition", { rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "condition", condition: "" } }], ["title above replay-safe limit", { title: "x".repeat(161) }], ["prompt above replay-safe limit", { prompt: "x".repeat(20_001) }], - ["condition above replay-safe limit", { rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "condition", condition: "x".repeat(4_001) } }], + ["stop kind condition (not yet supported as input)", { rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "condition", condition: "repo is ready" } }], ])("rejects empty nested strings before execute reaches the Zod create parser: %s", (_name, override) => { const decode = Schema.decodeUnknownSync(AutomateParameters) let error: unknown From 4902a183e8ab95586ed0143a199c2fb76a0360c2 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 1 Jun 2026 23:19:30 +0800 Subject: [PATCH 08/13] fix(automation): retry recordRunOutcome on revision conflict Previously a ConflictError from a concurrent definition write caused the run outcome (failureStreak / nextFireAt / nextFires) to be silently dropped; the next definition.updated event then reflected the racing edit but lost the run's contribution. Retry the read+compute+replace loop up to three times so a race against an unrelated update doesn't strand the user on stale derived fields. --- packages/opencode/src/automation/index.ts | 64 +++++++++++-------- .../test/server/automation-scheduler.test.ts | 44 +++++++++++++ 2 files changed, 80 insertions(+), 28 deletions(-) diff --git a/packages/opencode/src/automation/index.ts b/packages/opencode/src/automation/index.ts index 8a6d24743..c627e0769 100644 --- a/packages/opencode/src/automation/index.ts +++ b/packages/opencode/src/automation/index.ts @@ -637,36 +637,44 @@ export namespace Automation { options?: { now?: number; refreshOnStopped?: boolean }, ): Definition | undefined { if (run.state !== "succeeded" && run.state !== "failed" && run.state !== "stopped") return undefined - const previous = getOptional(run.automationID) - if (!previous || previous.kind !== "recurring") return undefined const now = options?.now ?? Date.now() - const failureStreak = - run.state === "succeeded" ? 0 : run.state === "failed" ? previous.failureStreak + 1 : previous.failureStreak - const derived = - run.state === "stopped" && !options?.refreshOnStopped - ? { nextFireAt: previous.nextFireAt, nextFires: previous.nextFires } - : computeDerivedFields(previous, now, completedRunCount(previous.id)) - if ( - previous.failureStreak === failureStreak && - previous.nextFireAt === derived.nextFireAt && - sameArray(previous.nextFires, derived.nextFires) - ) { - return undefined - } - const next = Definition.parse({ - ...previous, - failureStreak, - nextFireAt: derived.nextFireAt, - nextFires: derived.nextFires, - revision: previous.revision + 1, - updatedAt: now, - }) - try { - return replaceDefinition(previous, next) - } catch (error) { - if (error instanceof ConflictError) return undefined - throw error + // Retry on revision conflict: a concurrent write (e.g. pause/update) may + // have advanced the row between our read and our update. Re-read the + // latest definition and recompute failureStreak + derived fields against + // it, otherwise we silently drop the run's outcome and the user sees a + // stale nextFireAt / failureStreak. + for (let attempt = 0; attempt < 3; attempt++) { + const previous = getOptional(run.automationID) + if (!previous || previous.kind !== "recurring") return undefined + const failureStreak = + run.state === "succeeded" ? 0 : run.state === "failed" ? previous.failureStreak + 1 : previous.failureStreak + const derived = + run.state === "stopped" && !options?.refreshOnStopped + ? { nextFireAt: previous.nextFireAt, nextFires: previous.nextFires } + : computeDerivedFields(previous, now, completedRunCount(previous.id)) + if ( + previous.failureStreak === failureStreak && + previous.nextFireAt === derived.nextFireAt && + sameArray(previous.nextFires, derived.nextFires) + ) { + return undefined + } + const next = Definition.parse({ + ...previous, + failureStreak, + nextFireAt: derived.nextFireAt, + nextFires: derived.nextFires, + revision: previous.revision + 1, + updatedAt: now, + }) + try { + return replaceDefinition(previous, next) + } catch (error) { + if (!(error instanceof ConflictError)) throw error + // retry: read latest and recompute + } } + return undefined } function sameArray(left: readonly number[], right: readonly number[]) { diff --git a/packages/opencode/test/server/automation-scheduler.test.ts b/packages/opencode/test/server/automation-scheduler.test.ts index ddd07ef56..6d63346b8 100644 --- a/packages/opencode/test/server/automation-scheduler.test.ts +++ b/packages/opencode/test/server/automation-scheduler.test.ts @@ -995,6 +995,50 @@ describe("automation scheduler", () => { }) }) + test("recordRunOutcome stays consistent across concurrent revision bumps", async () => { + await withAutomation(async (projectID) => { + const created = Automation.create(recurringInput(projectID, 60_000), { now: 0 }) + if (created.kind !== "recurring") throw new Error("recurring") + + const triggerFailedRun = async (now: number) => { + const sessionID = SessionID.descending() + await Automation.runNowExecuting(created.id, { + now, + executor: async ({ run }) => { + const running = Automation.markRunStarted(run, sessionID, { now }) + await Automation.publishRunUpdated(running) + throw new Error("kaboom") + }, + }).catch(() => undefined) + await Bun.sleep(10) + } + + await triggerFailedRun(1_000) + const failed1 = Automation.runs({ automationID: created.id }).items.find((r) => r.state === "failed") + if (!failed1) throw new Error("failed1 missing") + const after1 = Automation.recordRunOutcome(failed1, { now: 1_500 }) + if (!after1 || after1.kind !== "recurring") throw new Error("after1") + expect(after1.failureStreak).toBe(1) + expect(after1.revision).toBeGreaterThan(created.revision) + + const edited = Automation.update(created.id, { title: "edited" }, { now: 2_000 }) + expect(edited.revision).toBeGreaterThan(after1.revision) + + await triggerFailedRun(3_000) + const failed2 = Automation.runs({ automationID: created.id }).items + .filter((r) => r.state === "failed") + .find((r) => r.id !== failed1.id) + if (!failed2) throw new Error("failed2 missing") + const after2 = Automation.recordRunOutcome(failed2, { now: 3_500 }) + if (!after2 || after2.kind !== "recurring") throw new Error("after2") + // Concurrent edit must not erase the streak; record reads the latest + // and applies the increment on top. + expect(after2.failureStreak).toBe(2) + expect(after2.title).toBe("edited") + expect(after2.revision).toBeGreaterThan(edited.revision) + }) + }) + test("CreateInput schema rejects stop kind 'condition' before reaching validate", async () => { await withAutomation(async (projectID) => { const raw = recurringInput(projectID, 60_000, { stop: { kind: "condition", condition: "repo is ready" } as never }) From c6c7b5b94de2f444ef451de9cce98ed874815227 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 1 Jun 2026 23:31:03 +0800 Subject: [PATCH 09/13] fix(automation): keep unsupported_stop_condition detail + cover real conflict retry Stop schema previously split into SupportedCreateStop (no condition) which dropped the structured `{ field: "stop", message: "unsupported_stop_condition" }` 422 detail that the PR description promised. Restore the unified Stop schema and the validate-layer rejection; the agent-facing automate tool keeps condition out of its own schema separately so LLMs do not try it. Also add an internal __testBeforeReplace hook to recordRunOutcome so a test can deterministically force a ConflictError between the read and the replace, exercising the retry path end-to-end and asserting the racing edit's title is preserved alongside the run's failureStreak. --- packages/opencode/src/automation/index.ts | 33 +++++++----- .../test/server/automation-routes.test.ts | 36 +++++++------ .../test/server/automation-scheduler.test.ts | 50 +++++++++++++++++-- 3 files changed, 85 insertions(+), 34 deletions(-) diff --git a/packages/opencode/src/automation/index.ts b/packages/opencode/src/automation/index.ts index c627e0769..de72b3e6d 100644 --- a/packages/opencode/src/automation/index.ts +++ b/packages/opencode/src/automation/index.ts @@ -64,6 +64,10 @@ export namespace Automation { .object({ error: z.literal("active_run_still_running"), runID: RunID }) .strict() .meta({ ref: "AutomationActiveRunStillRunningError" }) + // Stop accepts all three kinds at the schema layer so create/update can + // return a structured `unsupported_stop_condition` error for `kind: "condition"` + // (rejected by validateCreateInput / validateUpdateInput). The agent-facing + // `automate` tool schema separately omits condition from its input surface. export const Stop = z .discriminatedUnion("kind", [ z.object({ kind: z.literal("count"), count: z.number().int().positive() }).strict(), @@ -71,15 +75,6 @@ export namespace Automation { z.object({ kind: z.literal("never") }).strict(), ]) .meta({ ref: "AutomationStop" }) - // Input-only Stop: condition is persisted (Definition.stop can be `condition`) - // but cannot be supplied via create/update yet. Keep the two schemas separate - // so the public contract doesn't advertise a kind we reject at runtime. - export const SupportedCreateStop = z - .discriminatedUnion("kind", [ - z.object({ kind: z.literal("count"), count: z.number().int().positive() }).strict(), - z.object({ kind: z.literal("never") }).strict(), - ]) - .meta({ ref: "AutomationSupportedCreateStop" }) export const Rhythm = z .discriminatedUnion("kind", [ z.object({ kind: z.literal("interval"), everyMs: z.number().int().min(MIN_INTERVAL_MS, `interval_below_minimum_${MIN_INTERVAL_MS}ms`) }).strict(), @@ -100,7 +95,7 @@ export namespace Automation { export const CreateInput = z .discriminatedUnion("kind", [ z.object({ kind: z.literal("oneshot"), ...CommonCreate, fireAt: z.number().int().nonnegative() }).strict(), - z.object({ kind: z.literal("recurring"), ...CommonCreate, rhythm: Rhythm, stop: SupportedCreateStop }).strict(), + z.object({ kind: z.literal("recurring"), ...CommonCreate, rhythm: Rhythm, stop: Stop }).strict(), ]) .meta({ ref: "AutomationCreateInput" }) export type CreateInput = z.infer @@ -115,7 +110,7 @@ export namespace Automation { timezone: z.string().min(1).optional(), fireAt: z.number().int().nonnegative().optional(), rhythm: Rhythm.optional(), - stop: SupportedCreateStop.optional(), + stop: Stop.optional(), model: Model.optional(), variant: z.string().min(1).nullable().optional(), }) @@ -398,6 +393,9 @@ export namespace Automation { if (input.where.worktree && Instance.project.vcs !== "git") { addDetail(details, "where.worktree", "unsupported_where_worktree_not_git") } + if (input.kind === "recurring" && input.stop.kind === "condition") { + addDetail(details, "stop", "unsupported_stop_condition") + } details.push(...validateScheduleFields(input)) return details } @@ -422,6 +420,9 @@ export namespace Automation { if (patch.rhythm?.kind === "cron" && !isValidCronExpression(patch.rhythm.expression)) { addDetail(details, "rhythm.expression", "invalid_cron_expression") } + if (patch.stop?.kind === "condition") { + addDetail(details, "stop", "unsupported_stop_condition") + } return details } @@ -634,7 +635,14 @@ export namespace Automation { export function recordRunOutcome( run: Run, - options?: { now?: number; refreshOnStopped?: boolean }, + options?: { + now?: number + refreshOnStopped?: boolean + /** @internal Test-only hook fired between reading `previous` and calling + * `replaceDefinition`. Used to deterministically inject a concurrent write + * so the ConflictError retry path can be covered. */ + __testBeforeReplace?: (previous: Definition) => void + }, ): Definition | undefined { if (run.state !== "succeeded" && run.state !== "failed" && run.state !== "stopped") return undefined const now = options?.now ?? Date.now() @@ -667,6 +675,7 @@ export namespace Automation { revision: previous.revision + 1, updatedAt: now, }) + options?.__testBeforeReplace?.(previous) try { return replaceDefinition(previous, next) } catch (error) { diff --git a/packages/opencode/test/server/automation-routes.test.ts b/packages/opencode/test/server/automation-routes.test.ts index ef07129e3..dd24c2868 100644 --- a/packages/opencode/test/server/automation-routes.test.ts +++ b/packages/opencode/test/server/automation-routes.test.ts @@ -607,6 +607,16 @@ describe("automation routes", () => { recurringInput(projectID, { prompt: "x".repeat(20_001) }), [{ field: "prompt", message: "prompt_too_long_20000" }], ], + [ + "stop kind condition rejected with structured detail", + recurringInput(projectID, { stop: { kind: "condition", condition: "repo is ready" } }), + [{ field: "stop", message: "unsupported_stop_condition" }], + ], + [ + "condition above replay-safe limit", + recurringInput(projectID, { stop: { kind: "condition", condition: "x".repeat(4_001) } }), + [{ field: "stop.condition", message: "condition_too_long_4000" }], + ], [ "externally supplied automation session", { ...recurringInput(projectID), automationSessionID: SessionID.descending() }, @@ -655,35 +665,23 @@ describe("automation routes", () => { }) }) - test("rejects stop kind 'condition' at the create/update route as unsupported input", async () => { + test("PUT rejects stop kind 'condition' with structured unsupported_stop_condition detail", async () => { await withAutomationApp(async ({ app, projectID }) => { - const createResponse = await app.request("/automation", { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - ...recurringInput(projectID), - stop: { kind: "condition", condition: "repo is ready" }, - }), - }) - expect(createResponse.status).toBe(422) - const createBody = (await createResponse.json()) as { error: string; details: { field: string }[] } - expect(createBody.error).toBe("invalid_automation") - expect(createBody.details.some((d) => d.field.startsWith("stop"))).toBe(true) - const created = await json(app, "/automation", { method: "POST", headers: { "content-type": "application/json" }, body: JSON.stringify(recurringInput(projectID)), }) - const updateResponse = await app.request(`/automation/${created.id}`, { + const response = await app.request(`/automation/${created.id}`, { method: "PUT", headers: { "content-type": "application/json" }, body: JSON.stringify({ stop: { kind: "condition", condition: "repo is ready" } }), }) - expect(updateResponse.status).toBe(422) - const updateBody = (await updateResponse.json()) as { error: string; details: { field: string }[] } - expect(updateBody.error).toBe("invalid_automation") - expect(updateBody.details.some((d) => d.field.startsWith("stop"))).toBe(true) + expect(response.status).toBe(422) + expect(await response.json()).toEqual({ + error: "invalid_automation", + details: [{ field: "stop", message: "unsupported_stop_condition" }], + }) }) }) diff --git a/packages/opencode/test/server/automation-scheduler.test.ts b/packages/opencode/test/server/automation-scheduler.test.ts index 6d63346b8..49b78b712 100644 --- a/packages/opencode/test/server/automation-scheduler.test.ts +++ b/packages/opencode/test/server/automation-scheduler.test.ts @@ -995,6 +995,46 @@ describe("automation scheduler", () => { }) }) + test("recordRunOutcome retries on real ConflictError and merges with the racing edit", async () => { + await withAutomation(async (projectID) => { + const created = Automation.create(recurringInput(projectID, 60_000), { now: 0 }) + if (created.kind !== "recurring") throw new Error("recurring") + + const sessionID = SessionID.descending() + await Automation.runNowExecuting(created.id, { + now: 1_000, + executor: async ({ run }) => { + const running = Automation.markRunStarted(run, sessionID, { now: 1_000 }) + await Automation.publishRunUpdated(running) + throw new Error("kaboom") + }, + }).catch(() => undefined) + await Bun.sleep(10) + const failed = Automation.runs({ automationID: created.id }).items.find((r) => r.state === "failed") + if (!failed) throw new Error("failed missing") + + // Force a real ConflictError: __testBeforeReplace fires after record + // reads `previous` but before it writes, so the unrelated update bumps + // the row's revision and the first replaceDefinition hits ConflictError. + let hookFires = 0 + const refreshed = Automation.recordRunOutcome(failed, { + now: 1_500, + __testBeforeReplace: (previous) => { + if (hookFires === 0) { + hookFires += 1 + Automation.update(previous.id, { title: "raced edit" }, { now: 1_400 }) + } + }, + }) + expect(hookFires).toBe(1) + if (!refreshed || refreshed.kind !== "recurring") throw new Error("refreshed missing") + // The retry must preserve the racing edit AND apply the run's contribution. + expect(refreshed.title).toBe("raced edit") + expect(refreshed.failureStreak).toBe(1) + expect(refreshed.nextFireAt).not.toBe(created.nextFireAt) + }) + }) + test("recordRunOutcome stays consistent across concurrent revision bumps", async () => { await withAutomation(async (projectID) => { const created = Automation.create(recurringInput(projectID, 60_000), { now: 0 }) @@ -1039,16 +1079,20 @@ describe("automation scheduler", () => { }) }) - test("CreateInput schema rejects stop kind 'condition' before reaching validate", async () => { + test("rejects recurring condition stops at create time", async () => { await withAutomation(async (projectID) => { - const raw = recurringInput(projectID, 60_000, { stop: { kind: "condition", condition: "repo is ready" } as never }) let captured: unknown try { - Automation.CreateInput.parse(raw) + Automation.create( + recurringInput(projectID, 60_000, { stop: { kind: "condition", condition: "repo is ready" } }), + { now: 0 }, + ) } catch (error) { captured = error } expect(captured).toBeInstanceOf(Error) + const validation = captured as { details?: { field: string; message: string }[] } + expect(validation.details).toEqual(expect.arrayContaining([{ field: "stop", message: "unsupported_stop_condition" }])) }) }) From 14fa9982aa50d11aaa1557e5c516da761afdeb9d Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 1 Jun 2026 23:39:14 +0800 Subject: [PATCH 10/13] fix(tool): keep condition in automate Stop schema to match Automation.Stop The Stop schema rollback in c6c7b5b94d restored condition in Automation.CreateInput but the automate tool's Effect Schema still excluded it, breaking the type relationship between the zod recurring input fed into tool.execute and the tool's parameters. Restore condition in the tool schema and add a description line steering the LLM away from it; validate-time rejection still surfaces { stop, unsupported_stop_condition }. --- packages/opencode/src/tool/automate.ts | 11 ++++++++--- packages/opencode/test/tool/automate.test.ts | 3 ++- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/packages/opencode/src/tool/automate.ts b/packages/opencode/src/tool/automate.ts index 57a4a64f9..5705dcccb 100644 --- a/packages/opencode/src/tool/automate.ts +++ b/packages/opencode/src/tool/automate.ts @@ -25,6 +25,8 @@ const CronExpression = Schema.NonEmptyString.check( ) const Title = Schema.NonEmptyString.check(Schema.isMaxLength(Automation.MAX_TITLE_CHARS)) const Prompt = Schema.NonEmptyString.check(Schema.isMaxLength(Automation.MAX_PROMPT_CHARS)) +const Condition = Schema.NonEmptyString.check(Schema.isMaxLength(Automation.MAX_CONDITION_CHARS)) + const Common = { title: Title, prompt: Prompt, @@ -39,11 +41,13 @@ const NonNegativeInt = Schema.Int.check(Schema.isGreaterThanOrEqualTo(0)) const PositiveInt = Schema.Int.check(Schema.isGreaterThan(0)) const IntervalMs = Schema.Int.check(Schema.isGreaterThanOrEqualTo(Automation.MIN_INTERVAL_MS)) -// `condition` is part of the persisted Stop union but not yet supported as -// input; keep this in lockstep with Automation.SupportedCreateStop so the tool -// signature doesn't advertise a kind we reject. +// Mirrors Automation.Stop. `kind: "condition"` is currently rejected at +// validate time with { field: "stop", message: "unsupported_stop_condition" }; +// kept in the schema so the structured error contract matches HTTP routes. +// The tool description below points the LLM away from condition. const Stop = Schema.Union([ Schema.Struct({ kind: Schema.Literal("count"), count: PositiveInt }), + Schema.Struct({ kind: Schema.Literal("condition"), condition: Condition }), Schema.Struct({ kind: Schema.Literal("never") }), ]) @@ -75,6 +79,7 @@ export function formatAutomateValidationError(error: unknown) { "Invalid automate input.", "Expected shape: oneshot { kind, title, prompt, context, where, timezone, model, variant?, fireAt } or recurring { kind, title, prompt, context, where, timezone, model, variant?, rhythm, stop }.", "model is required as { providerID, modelID }; variant is optional and must be a valid effort key for that model (omit for models without reasoning).", + "stop only supports { kind: \"count\", count } or { kind: \"never\" } today; { kind: \"condition\" } is reserved and currently rejected.", "Example: { kind: \"recurring\", title: \"Daily repo brief\", prompt: \"Summarize repo changes.\", context: \"fresh\", where: { projectID: \"current-project\" }, timezone: \"UTC\", model: { providerID: \"anthropic\", modelID: \"claude-sonnet-4-6\" }, variant: \"high\", rhythm: { kind: \"interval\", everyMs: 3600000 }, stop: { kind: \"never\" } }.", detail, ].join("\n") diff --git a/packages/opencode/test/tool/automate.test.ts b/packages/opencode/test/tool/automate.test.ts index d1a15b6f3..1a84a0340 100644 --- a/packages/opencode/test/tool/automate.test.ts +++ b/packages/opencode/test/tool/automate.test.ts @@ -92,9 +92,10 @@ describe("automate tool", () => { test.each([ ["empty cron expression", { rhythm: { kind: "cron", expression: "" }, stop: { kind: "never" } }], + ["empty stop condition", { rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "condition", condition: "" } }], ["title above replay-safe limit", { title: "x".repeat(161) }], ["prompt above replay-safe limit", { prompt: "x".repeat(20_001) }], - ["stop kind condition (not yet supported as input)", { rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "condition", condition: "repo is ready" } }], + ["condition above replay-safe limit", { rhythm: { kind: "interval", everyMs: 60_000 }, stop: { kind: "condition", condition: "x".repeat(4_001) } }], ])("rejects empty nested strings before execute reaches the Zod create parser: %s", (_name, override) => { const decode = Schema.decodeUnknownSync(AutomateParameters) let error: unknown From 3af5bae6c145818373bca0b15be51c5c0da1f466 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Mon, 1 Jun 2026 23:53:17 +0800 Subject: [PATCH 11/13] test(automation): harden 422 model test + replace fixed sleep with polling - automation-routes.test.ts: 422 create-path now sends an explicitly invalid model { providerID: "nonexistent", modelID: "missing-model" } instead of relying on the suite-wide fixture model being unavailable in the test environment. Mirrors the update-path test below it. - automation-scheduler.test.ts: add waitForFailedRunCount polling helper and use it in both recordRunOutcome tests in place of `Bun.sleep(10)` after `runNowExecuting`. Fixes a latent CI flake where the failed run may not have been persisted before the assertion reads it. --- .../test/server/automation-routes.test.ts | 6 +++++- .../test/server/automation-scheduler.test.ts | 18 +++++++++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/opencode/test/server/automation-routes.test.ts b/packages/opencode/test/server/automation-routes.test.ts index dd24c2868..9022f38ca 100644 --- a/packages/opencode/test/server/automation-routes.test.ts +++ b/packages/opencode/test/server/automation-routes.test.ts @@ -150,7 +150,11 @@ describe("automation route 422 wiring with provider validation enabled", () => { const response = await app.request("/automation", { method: "POST", headers: { "content-type": "application/json" }, - body: JSON.stringify(recurringInput(projectID)), + body: JSON.stringify( + recurringInput(projectID, { + model: Automation.Model.parse({ providerID: "nonexistent", modelID: "missing-model" }), + }), + ), }) expect(response.status).toBe(422) const body = await response.json() diff --git a/packages/opencode/test/server/automation-scheduler.test.ts b/packages/opencode/test/server/automation-scheduler.test.ts index 49b78b712..d2c49d48e 100644 --- a/packages/opencode/test/server/automation-scheduler.test.ts +++ b/packages/opencode/test/server/automation-scheduler.test.ts @@ -212,6 +212,17 @@ async function waitForRunCount(automationID: string, count: number) { throw new Error(`Timed out waiting for automation run count: ${count}`) } +async function waitForFailedRunCount(automationID: string, count: number) { + const deadline = Date.now() + 3_000 + let latest: Automation.Run[] = [] + while (Date.now() < deadline) { + latest = Automation.runs({ automationID }).items + if (latest.filter((r) => r.state === "failed").length >= count) return latest + await Bun.sleep(5) + } + throw new Error(`Timed out waiting for ${count} failed run(s); latest=${JSON.stringify(latest)}`) +} + async function waitForStarts(starts: unknown[], count: number) { const deadline = Date.now() + 3_000 while (Date.now() < deadline) { @@ -1009,8 +1020,7 @@ describe("automation scheduler", () => { throw new Error("kaboom") }, }).catch(() => undefined) - await Bun.sleep(10) - const failed = Automation.runs({ automationID: created.id }).items.find((r) => r.state === "failed") + const failed = (await waitForFailedRunCount(created.id, 1)).find((r) => r.state === "failed") if (!failed) throw new Error("failed missing") // Force a real ConflictError: __testBeforeReplace fires after record @@ -1040,8 +1050,10 @@ describe("automation scheduler", () => { const created = Automation.create(recurringInput(projectID, 60_000), { now: 0 }) if (created.kind !== "recurring") throw new Error("recurring") + let expectedFailed = 0 const triggerFailedRun = async (now: number) => { const sessionID = SessionID.descending() + expectedFailed += 1 await Automation.runNowExecuting(created.id, { now, executor: async ({ run }) => { @@ -1050,7 +1062,7 @@ describe("automation scheduler", () => { throw new Error("kaboom") }, }).catch(() => undefined) - await Bun.sleep(10) + await waitForFailedRunCount(created.id, expectedFailed) } await triggerFailedRun(1_000) From 8751e5fb760e2fc02a7b2f033ac090c4cfd388fe Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Tue, 2 Jun 2026 00:06:36 +0800 Subject: [PATCH 12/13] refactor(automation): move recordRunOutcome test hook out of public options The conflict-retry test injected its concurrent write through an __testBeforeReplace field on recordRunOutcome's options object, which leaked a test-only seam into the function's exported type signature. Move the hook to a module-level Automation.__testHooks seam that production code never touches; the test now sets __testHooks.beforeReplaceDefinition and clears it in finally, while recordRunOutcome's options shrink to { now?, refreshOnStopped? }. --- packages/opencode/src/automation/index.ts | 17 ++++++++---- .../test/server/automation-scheduler.test.ts | 27 ++++++++++--------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/packages/opencode/src/automation/index.ts b/packages/opencode/src/automation/index.ts index de72b3e6d..995a8dc70 100644 --- a/packages/opencode/src/automation/index.ts +++ b/packages/opencode/src/automation/index.ts @@ -638,10 +638,6 @@ export namespace Automation { options?: { now?: number refreshOnStopped?: boolean - /** @internal Test-only hook fired between reading `previous` and calling - * `replaceDefinition`. Used to deterministically inject a concurrent write - * so the ConflictError retry path can be covered. */ - __testBeforeReplace?: (previous: Definition) => void }, ): Definition | undefined { if (run.state !== "succeeded" && run.state !== "failed" && run.state !== "stopped") return undefined @@ -675,7 +671,7 @@ export namespace Automation { revision: previous.revision + 1, updatedAt: now, }) - options?.__testBeforeReplace?.(previous) + __testHooks.beforeReplaceDefinition?.(previous) try { return replaceDefinition(previous, next) } catch (error) { @@ -686,6 +682,17 @@ export namespace Automation { return undefined } + /** + * @internal Test-only seam. Production code MUST NOT read or write this. + * Tests assign `beforeReplaceDefinition` to deterministically inject a + * concurrent write between `recordRunOutcome`'s read and its replace, so + * the ConflictError retry path can be exercised end-to-end. Reset to `{}` + * after each test (or scope assignments with try/finally). + */ + export const __testHooks: { + beforeReplaceDefinition?: (previous: Definition) => void + } = {} + function sameArray(left: readonly number[], right: readonly number[]) { if (left.length !== right.length) return false for (let index = 0; index < left.length; index++) { diff --git a/packages/opencode/test/server/automation-scheduler.test.ts b/packages/opencode/test/server/automation-scheduler.test.ts index d2c49d48e..e4e36454d 100644 --- a/packages/opencode/test/server/automation-scheduler.test.ts +++ b/packages/opencode/test/server/automation-scheduler.test.ts @@ -1023,19 +1023,22 @@ describe("automation scheduler", () => { const failed = (await waitForFailedRunCount(created.id, 1)).find((r) => r.state === "failed") if (!failed) throw new Error("failed missing") - // Force a real ConflictError: __testBeforeReplace fires after record - // reads `previous` but before it writes, so the unrelated update bumps - // the row's revision and the first replaceDefinition hits ConflictError. + // Force a real ConflictError: the test hook fires after record reads + // `previous` but before it writes, so the unrelated update bumps the + // row's revision and the first replaceDefinition hits ConflictError. let hookFires = 0 - const refreshed = Automation.recordRunOutcome(failed, { - now: 1_500, - __testBeforeReplace: (previous) => { - if (hookFires === 0) { - hookFires += 1 - Automation.update(previous.id, { title: "raced edit" }, { now: 1_400 }) - } - }, - }) + Automation.__testHooks.beforeReplaceDefinition = (previous) => { + if (hookFires === 0) { + hookFires += 1 + Automation.update(previous.id, { title: "raced edit" }, { now: 1_400 }) + } + } + let refreshed: Automation.Definition | undefined + try { + refreshed = Automation.recordRunOutcome(failed, { now: 1_500 }) + } finally { + delete Automation.__testHooks.beforeReplaceDefinition + } expect(hookFires).toBe(1) if (!refreshed || refreshed.kind !== "recurring") throw new Error("refreshed missing") // The retry must preserve the racing edit AND apply the run's contribution. From 5e177babdaaac68297fec195380f2270bd556405 Mon Sep 17 00:00:00 2001 From: Yuhan Lei Date: Tue, 2 Jun 2026 00:19:42 +0800 Subject: [PATCH 13/13] refactor(automation): isolate test seam to internal module, off Automation API The previous attempt parked the ConflictError test seam at Automation.__testHooks, which kept it inside the public namespace export and still appeared on the Automation.* API surface. Move it to a sibling module src/automation/__test_hooks.ts that only the ConflictError retry path in recordRunOutcome and the matching test import. The Automation namespace no longer exposes a writable test hook; consumers reading the public type surface see no test-only fields. --- .../opencode/src/automation/__test_hooks.ts | 17 +++++++++++++++++ packages/opencode/src/automation/index.ts | 14 ++------------ .../test/server/automation-scheduler.test.ts | 5 +++-- 3 files changed, 22 insertions(+), 14 deletions(-) create mode 100644 packages/opencode/src/automation/__test_hooks.ts diff --git a/packages/opencode/src/automation/__test_hooks.ts b/packages/opencode/src/automation/__test_hooks.ts new file mode 100644 index 000000000..804ce9c46 --- /dev/null +++ b/packages/opencode/src/automation/__test_hooks.ts @@ -0,0 +1,17 @@ +import type { Automation } from "./index" + +/** + * @internal Test-only injection points for the automation module. + * + * Production code MUST NOT import or write to this module — the only reader + * is `recordRunOutcome` in `./index`, which checks `beforeReplaceDefinition` + * to support a deterministic ConflictError retry test. By living outside the + * `Automation` namespace, this seam is invisible to anyone consuming the + * public `Automation.*` API surface. + * + * Tests assign hooks here and MUST clear them in a `finally` block so a + * failing test cannot leak state to a sibling test. + */ +export const internalTestHooks: { + beforeReplaceDefinition?: (previous: Automation.Definition) => void +} = {} diff --git a/packages/opencode/src/automation/index.ts b/packages/opencode/src/automation/index.ts index 995a8dc70..4a829c098 100644 --- a/packages/opencode/src/automation/index.ts +++ b/packages/opencode/src/automation/index.ts @@ -13,6 +13,7 @@ import type { AutomationRunAttendance, AutomationRunBlocker } from "./run-contex import { AutomationDefinitionTable, AutomationRunTable } from "./automation.sql" import { isValidCronExpression as cronIsValidExpression } from "./cron" import { computeDerivedFields } from "./derived" +import { internalTestHooks } from "./__test_hooks" export const AutomationID = { Definition: { @@ -671,7 +672,7 @@ export namespace Automation { revision: previous.revision + 1, updatedAt: now, }) - __testHooks.beforeReplaceDefinition?.(previous) + internalTestHooks.beforeReplaceDefinition?.(previous) try { return replaceDefinition(previous, next) } catch (error) { @@ -682,17 +683,6 @@ export namespace Automation { return undefined } - /** - * @internal Test-only seam. Production code MUST NOT read or write this. - * Tests assign `beforeReplaceDefinition` to deterministically inject a - * concurrent write between `recordRunOutcome`'s read and its replace, so - * the ConflictError retry path can be exercised end-to-end. Reset to `{}` - * after each test (or scope assignments with try/finally). - */ - export const __testHooks: { - beforeReplaceDefinition?: (previous: Definition) => void - } = {} - function sameArray(left: readonly number[], right: readonly number[]) { if (left.length !== right.length) return false for (let index = 0; index < left.length; index++) { diff --git a/packages/opencode/test/server/automation-scheduler.test.ts b/packages/opencode/test/server/automation-scheduler.test.ts index e4e36454d..312df8f18 100644 --- a/packages/opencode/test/server/automation-scheduler.test.ts +++ b/packages/opencode/test/server/automation-scheduler.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, test } from "bun:test" import { Effect } from "effect" import { Automation } from "../../src/automation" +import { internalTestHooks } from "../../src/automation/__test_hooks" import { AutomationScheduler } from "../../src/automation/scheduler" import { Instance } from "../../src/project/instance" import { ProjectID } from "../../src/project/schema" @@ -1027,7 +1028,7 @@ describe("automation scheduler", () => { // `previous` but before it writes, so the unrelated update bumps the // row's revision and the first replaceDefinition hits ConflictError. let hookFires = 0 - Automation.__testHooks.beforeReplaceDefinition = (previous) => { + internalTestHooks.beforeReplaceDefinition = (previous) => { if (hookFires === 0) { hookFires += 1 Automation.update(previous.id, { title: "raced edit" }, { now: 1_400 }) @@ -1037,7 +1038,7 @@ describe("automation scheduler", () => { try { refreshed = Automation.recordRunOutcome(failed, { now: 1_500 }) } finally { - delete Automation.__testHooks.beforeReplaceDefinition + delete internalTestHooks.beforeReplaceDefinition } expect(hookFires).toBe(1) if (!refreshed || refreshed.kind !== "recurring") throw new Error("refreshed missing")