diff --git a/packages/opencode/migration/20260619090000_automation_run_history_survives_definition_delete/migration.sql b/packages/opencode/migration/20260619090000_automation_run_history_survives_definition_delete/migration.sql new file mode 100644 index 000000000..5905f3c46 --- /dev/null +++ b/packages/opencode/migration/20260619090000_automation_run_history_survives_definition_delete/migration.sql @@ -0,0 +1,40 @@ +CREATE TABLE `automation_run_next` ( + `id` text PRIMARY KEY, + `automation_id` text NOT NULL, + `project_id` text NOT NULL, + `owner_directory` text NOT NULL, + `triggered_at` integer NOT NULL, + `data` text NOT NULL, + `time_created` integer NOT NULL, + `time_updated` integer NOT NULL, + CONSTRAINT `fk_automation_run_project_id_project_id_fk` FOREIGN KEY (`project_id`) REFERENCES `project`(`id`) ON DELETE CASCADE +); +--> statement-breakpoint +INSERT INTO `automation_run_next` ( + `id`, + `automation_id`, + `project_id`, + `owner_directory`, + `triggered_at`, + `data`, + `time_created`, + `time_updated` +) +SELECT + `id`, + `automation_id`, + `project_id`, + `owner_directory`, + `triggered_at`, + `data`, + `time_created`, + `time_updated` +FROM `automation_run`; +--> statement-breakpoint +DROP TABLE `automation_run`; +--> statement-breakpoint +ALTER TABLE `automation_run_next` RENAME TO `automation_run`; +--> statement-breakpoint +CREATE INDEX `automation_run_automation_triggered_idx` ON `automation_run` (`automation_id`,`triggered_at`,`id`); +--> statement-breakpoint +CREATE INDEX `automation_run_project_owner_idx` ON `automation_run` (`project_id`,`owner_directory`); diff --git a/packages/opencode/src/automation/__test_hooks.ts b/packages/opencode/src/automation/__test_hooks.ts index 804ce9c46..06fbbabdd 100644 --- a/packages/opencode/src/automation/__test_hooks.ts +++ b/packages/opencode/src/automation/__test_hooks.ts @@ -3,10 +3,8 @@ 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 + * Production code MUST NOT import or write to this module. By living outside + * the `Automation` namespace, these seams are invisible to anyone consuming the * public `Automation.*` API surface. * * Tests assign hooks here and MUST clear them in a `finally` block so a @@ -14,4 +12,5 @@ import type { Automation } from "./index" */ export const internalTestHooks: { beforeReplaceDefinition?: (previous: Automation.Definition) => void + beforeExecuteRun?: (run: Automation.Run) => void | Promise } = {} diff --git a/packages/opencode/src/automation/automation.sql.ts b/packages/opencode/src/automation/automation.sql.ts index 2c478cea0..ed76aa478 100644 --- a/packages/opencode/src/automation/automation.sql.ts +++ b/packages/opencode/src/automation/automation.sql.ts @@ -26,9 +26,7 @@ export const AutomationRunTable = sqliteTable( "automation_run", { id: text().primaryKey().$type(), - automation_id: text() - .notNull() - .references(() => AutomationDefinitionTable.id, { onDelete: "cascade" }), + automation_id: text().notNull(), project_id: text() .$type() .notNull() diff --git a/packages/opencode/src/automation/index.ts b/packages/opencode/src/automation/index.ts index 0e02947dc..bde3e4300 100644 --- a/packages/opencode/src/automation/index.ts +++ b/packages/opencode/src/automation/index.ts @@ -66,10 +66,6 @@ export namespace Automation { .object({ error: z.literal("automation_conflict"), message: z.string() }) .strict() .meta({ ref: "AutomationConflictError" }) - export const ActiveRunStillRunningErrorResponse = z - .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 @@ -800,13 +796,17 @@ export namespace Automation { return true } - export async function remove(id: string): Promise<{ tombstone: Tombstone; stoppedRun?: Run }> { + export async function remove(id: string): Promise<{ tombstone: Tombstone }> { const previous = get(id) - const stoppedRun = stopActiveRun(id) - const liveRun = await getLiveActiveRun(id) - if (liveRun) throw new ActiveRunStillRunningError(liveRun.id) Database.use((db) => db.delete(AutomationDefinitionTable).where(eq(AutomationDefinitionTable.id, id)).run()) - return { tombstone: { id: previous.id, deleted: true, revision: previous.revision + 1 }, stoppedRun } + return { tombstone: { id: previous.id, deleted: true, revision: previous.revision + 1 } } + } + + async function stopLiveRunForSourceDelete(id: string) { + const active = state().activeRuns.get(id) + if (!active) return + const stopped = stopRunByID(active.runID, "cancelled") + if (stopped) await publishRunUpdated(stopped) } // A continue automation lives inside the conversation it was created in @@ -819,11 +819,11 @@ export namespace Automation { for (const definition of list()) { if (definition.context !== "continue" || definition.sourceSessionID !== sessionID) continue try { + await stopLiveRunForSourceDelete(definition.id) const removed = await remove(definition.id) await Bus.publish(Event.DefinitionDeleted, removed.tombstone) - if (removed.stoppedRun) await publishRunUpdated(removed.stoppedRun) } catch (error) { - if (NotFoundError.isInstance(error) || error instanceof ActiveRunStillRunningError) continue + if (NotFoundError.isInstance(error)) continue throw error } } @@ -892,39 +892,6 @@ export namespace Automation { ) } - function stopActiveRun(automationID: string) { - const active = state().activeRuns.get(automationID) - if (!active) return undefined - active.controller.abort() - const current = getRun(active.runID) - return current ? stopRun(current, "cancelled") : undefined - } - - async function getLiveActiveRun(automationID: string) { - get(automationID) - const projectID = Instance.project.id - const ownerDirectory = Instance.directory - const rows = Database.use((db) => - db - .select() - .from(AutomationRunTable) - .where( - and( - eq(AutomationRunTable.automation_id, automationID), - eq(AutomationRunTable.project_id, projectID), - eq(AutomationRunTable.owner_directory, ownerDirectory), - sql`json_extract(${AutomationRunTable.data}, '$.state') in ('scheduled', 'running', 'awaiting_input')`, - ), - ) - .all(), - ) - for (const row of rows) { - const run = Run.parse(row.data) - if (!isActiveRun(run)) continue - if (await hasLiveRunLease(run.id)) return run - } - } - export function stopRunByID( runID: string, stopReason: Extract["stopReason"], @@ -1021,10 +988,9 @@ export namespace Automation { return false } - function hasDurableActiveWriter(run: Run, writerKey: string) { - const definition = get(run.automationID) - const projectID = definition.where.projectID - const ownerDirectory = Instance.directory + function hasDurableActiveWriter(run: Run, writerKey: string, scope: Scope) { + const projectID = scope.projectID + const ownerDirectory = scope.ownerDirectory return Database.transaction( (db) => { const rows = db @@ -1062,7 +1028,10 @@ export namespace Automation { if (row.id === run.id) return false const item = Run.parse(row.data) if (!isActiveRun(item)) return false - return writerKeys.get(item.automationID) === writerKey + const rowWriterKey = writerKeys.get(item.automationID) + // A deleted definition leaves no writer key; while its run is active, + // keep the writer guard conservative within this project scope. + return rowWriterKey === undefined || rowWriterKey === writerKey }) }, { behavior: "immediate" }, @@ -1180,8 +1149,10 @@ export namespace Automation { const runID = AutomationID.Run.ascending() const lease = await Flock.acquire(runLeaseKey(Instance.directory, runID)) try { - const initial = runNow(id, { now: options.now, runID }) - queueMicrotask(() => void executeRun(initial, options.executor, options.attendance ?? "attended", lease)) + const scope = currentScope() + const definition = get(id, scope) + const initial = runNow(id, { now: options.now, runID, scope }) + queueMicrotask(() => void executeRun(initial, definition, scope, options.executor, options.attendance ?? "attended", lease)) return initial } catch (error) { await lease.release().catch(() => undefined) @@ -1189,16 +1160,23 @@ export namespace Automation { } } - async function executeRun(initial: Run, executor: RunExecutor, attendance: AutomationRunAttendance, lease: Flock.Lease) { + async function executeRun( + initial: Run, + definition: Definition, + scope: Scope, + executor: RunExecutor, + attendance: AutomationRunAttendance, + lease: Flock.Lease, + ) { const data = state() const controller = new AbortController() let writerKey: string | undefined let current = initial try { - const definition = get(initial.automationID) + await internalTestHooks.beforeExecuteRun?.(initial) writerKey = getWriterKey(definition) - for (const run of await reconcileInterruptedRuns()) await publishRunUpdated(run) - if (data.activeWriters.has(writerKey) || hasDurableActiveWriter(initial, writerKey)) { + for (const run of await reconcileInterruptedRuns({ scope })) await publishRunUpdated(run) + if (data.activeWriters.has(writerKey) || hasDurableActiveWriter(initial, writerKey, scope)) { const stopped = reviseRun(initial, { state: "stopped", completedAt: Date.now(), @@ -1327,9 +1305,7 @@ export namespace Automation { patch: UpdateInput, options?: { now?: number }, ) => Effect.Effect - readonly remove: ( - id: string, - ) => Effect.Effect<{ tombstone: Tombstone; stoppedRun?: Run }, ActiveRunStillRunningError> + readonly remove: (id: string) => Effect.Effect<{ tombstone: Tombstone }> readonly runNowExecuting: ( id: string, options: { executor: RunExecutor; attendance?: AutomationRunAttendance; now?: number }, @@ -1376,9 +1352,7 @@ export namespace Automation { }), remove: (id) => Effect.tryPromise({ try: () => remove(id), catch: (error) => error }).pipe( - Effect.catch((error) => - error instanceof ActiveRunStillRunningError ? Effect.fail(error) : Effect.die(error), - ), + Effect.catch((error) => Effect.die(error)), ), runNowExecuting: (id, options) => Effect.promise(() => runNowExecuting(id, options)), runs: (input) => Effect.sync(() => runs(input)), @@ -1409,10 +1383,3 @@ export class ConflictError extends Error { this.name = "AutomationConflictError" } } - -export class ActiveRunStillRunningError extends Error { - constructor(readonly runID: string) { - super(`Automation run is still running: ${runID}`) - this.name = "AutomationActiveRunStillRunningError" - } -} diff --git a/packages/opencode/src/server/instance/automation.ts b/packages/opencode/src/server/instance/automation.ts index 4649179ff..7760da2b6 100644 --- a/packages/opencode/src/server/instance/automation.ts +++ b/packages/opencode/src/server/instance/automation.ts @@ -3,7 +3,7 @@ import type { Context } from "hono" import { describeRoute, resolver, validator } from "hono-openapi" import { Cause, Effect, Exit } from "effect" import z from "zod" -import { ActiveRunStillRunningError, Automation, AutomationID, ConflictError, ValidationError } from "@/automation" +import { Automation, AutomationID, ConflictError, ValidationError } from "@/automation" import { sessionPromptExecutor } from "@/automation/runner" import { AutomationScheduler } from "@/automation/scheduler" import { validateModelAndVariant } from "@/automation/validation" @@ -19,13 +19,6 @@ function conflictError(error: ConflictError) { return Automation.ConflictErrorResponse.parse({ error: "automation_conflict", message: error.message }) } -function activeRunStillRunningError(error: ActiveRunStillRunningError) { - return Automation.ActiveRunStillRunningErrorResponse.parse({ - error: "active_run_still_running", - runID: error.runID, - }) -} - const AutomationIDParam = z.object({ automationID: AutomationID.Definition.zod }) const AutomationRunsQuery = z.object({ limit: z.coerce.number().int().positive().max(100).optional(), @@ -55,7 +48,6 @@ function runRoute(c: Context, effect: Effect.Effect scheduler.cancel(removed.tombstone.id)) - if (removed.stoppedRun) yield* automation.publishRunUpdated(removed.stoppedRun) yield* automation.publishDefinitionDeleted(removed.tombstone) return c.json(removed.tombstone) }) @@ -353,17 +344,13 @@ export const AutomationRoutes = (): Hono => describeRoute({ summary: "Delete automation", description: - "Delete an automation definition and return a tombstone. If a run is active in this process, stop it and publish the stopped run before publishing the tombstone. If a live run is owned by another process, return 409 without deleting.", + "Delete an automation definition, cancel future scheduling, and return a tombstone. Already-started runs continue to completion.", operationId: "automation.delete", responses: { 200: { description: "Automation deletion tombstone", content: { "application/json": { schema: resolver(Automation.Tombstone) } }, }, - 409: { - description: "Automation has a live run owned by another process", - content: { "application/json": { schema: resolver(Automation.ActiveRunStillRunningErrorResponse) } }, - }, ...errors(404), }, }), diff --git a/packages/opencode/src/session/prompt/pawwork.txt b/packages/opencode/src/session/prompt/pawwork.txt index 866f15a97..23a5f3be9 100644 --- a/packages/opencode/src/session/prompt/pawwork.txt +++ b/packages/opencode/src/session/prompt/pawwork.txt @@ -51,11 +51,13 @@ If the user has already specified a path, execute it directly without re-asking. # Scheduling, reminders, and recurring work -When the user asks to do something later, be reminded, send something at a specific time, or repeat work on a schedule, create a PawWork Automation with the `automate` tool — for one-time and recurring tasks alike. Automations appear in the Automations panel and run with the session's project context, model, and credentials. +When the user asks to do something later, be reminded, send or check something at a specific time, repeat work on a schedule, or monitor, poll, or watch something over time (check periodically, every N minutes/hours, until a status changes, or tell them when something happens), create a PawWork Automation with the `automate` tool for one-time and recurring tasks alike. Automations appear in the Automations panel and run with the session's project context, model, and credentials. When the user asks to list, pause, resume, delete, remove, or cancel an existing PawWork Automation, activate `automate_manage` via `tool_info` and manage it there. Do not send the user away to the Automations panel unless they explicitly want to use the UI. -Never install OS-level schedulers for these requests with any tool: no `at`, `cron`, `crontab`, `launchd` or LaunchAgents plists, systemd timers, `schtasks`, background scripts, or sleep loops — neither by running commands nor by writing files. Use other tools only to gather information the scheduled prompt will need. OS schedulers are acceptable only when the user explicitly asks for a system-level scheduler outside PawWork. +Use short bounded waits only inside the current turn: fixed sleeps and retry loops must be capped at 60 seconds total and only wait for immediate readiness, such as a local server booting, a page loading, or a command producing first output. For minute-scale polling, repeated checks, or background monitoring, use `automate`. + +Never install OS-level schedulers for these requests with any tool: no `at`, `cron`, `crontab`, `launchd` or LaunchAgents plists, systemd timers, `schtasks`, background scripts, or sleep loops; neither by running commands nor by writing files. Use other tools only to gather information the scheduled prompt will need. OS schedulers are acceptable only when the user explicitly asks for a system-level scheduler outside PawWork. # Browsing and operating websites diff --git a/packages/opencode/src/tool/automate-manage.ts b/packages/opencode/src/tool/automate-manage.ts index 289c51eca..d1a20a369 100644 --- a/packages/opencode/src/tool/automate-manage.ts +++ b/packages/opencode/src/tool/automate-manage.ts @@ -1,5 +1,5 @@ import { Cause, Effect, Schema } from "effect" -import { ActiveRunStillRunningError, Automation } from "@/automation" +import { Automation } from "@/automation" import { NotFoundError } from "@/storage/db" import * as Tool from "./tool" @@ -21,7 +21,6 @@ type Metadata = { automationDefinitions?: Automation.Definition[] automationDefinition?: Automation.Definition automationTombstone?: Automation.Tombstone - stoppedRun?: Automation.Run } function schedule(definition: Automation.Definition) { @@ -52,12 +51,6 @@ function readableAutomationError(error: unknown, id: string) { if (NotFoundError.isInstance(error)) { return new Error(`Automation not found: ${id}. Run automate_manage list to get a current id.`, { cause: error }) } - if (error instanceof ActiveRunStillRunningError) { - return new Error( - `Cannot delete automation ${id}: active_run_still_running (${error.runID}). Try again after the active run finishes.`, - { cause: error }, - ) - } return error } @@ -116,11 +109,10 @@ export function createAutomateManageDefinition( metadata: { action: "delete", id, title: previous.title }, }) const removed = yield* readableAutomationEffect(automation.remove(id), id) - if (removed.stoppedRun) yield* automation.publishRunUpdated(removed.stoppedRun) yield* automation.publishDefinitionDeleted(removed.tombstone) return { title: "Automation deleted", - metadata: { automationTombstone: removed.tombstone, stoppedRun: removed.stoppedRun }, + metadata: { automationTombstone: removed.tombstone }, output: JSON.stringify(removed.tombstone, null, 2), } }), diff --git a/packages/opencode/src/tool/automate.ts b/packages/opencode/src/tool/automate.ts index f75ccb9a1..fe26b3d6e 100644 --- a/packages/opencode/src/tool/automate.ts +++ b/packages/opencode/src/tool/automate.ts @@ -113,7 +113,7 @@ export function createAutomateDefinition( // me", "later", "every weekday") against the first sentences, so triggers // lead and behavioral detail lives in the field descriptions above. description: [ - "Create a PawWork Automation: a scheduled task, reminder, or recurring job that PawWork runs for the user. Use this whenever the user asks to do something later, at a specific time or date, one time, daily, weekly, on weekdays, or on any other schedule — including scheduled messages and reminders. Never set up OS schedulers (at, cron, launchd, schtasks) for these requests unless the user explicitly asks for an OS-level scheduler outside PawWork.", + "Create a PawWork Automation: a scheduled task, reminder, recurring job, or background monitor that PawWork runs for the user. Use this whenever the user asks to do something later, at a specific time or date, one time, daily, weekly, on weekdays, or on any other schedule. Also use it to monitor, poll, watch, or check periodically over time, such as every 5 minutes, until a status changes, or when the user says to tell them when something happens. Never set up OS schedulers (at, cron, launchd, schtasks) for these requests unless the user explicitly asks for an OS-level scheduler outside PawWork.", "Automations appear in PawWork's Automations panel, where the user can pause or delete them, and each run uses this session's project context, model, and credentials. Creating the definition schedules the future run; it does not run the prompt now. After creating one, tell the user when it will fire (the result includes the schedule).", ].join("\n\n"), parameters: AutomateParameters, diff --git a/packages/opencode/src/tool/shell.txt b/packages/opencode/src/tool/shell.txt index 9cbcec5a4..e16c9d90c 100644 --- a/packages/opencode/src/tool/shell.txt +++ b/packages/opencode/src/tool/shell.txt @@ -37,7 +37,7 @@ Usage notes: - Edit files: Use Edit (NOT sed/awk) - Write files: Use Write (NOT echo >/cat < { }) }) - test("delete rejects a live run owned by another process", async () => { + test("delete removes the definition while a live run is owned by another process", async () => { await withAutomationApp(async ({ app, projectID }) => { const created = Automation.create(recurringInput(projectID), { now: 100 }) const active = Automation.runNow(created.id, { now: 200 }) await using _ = await Flock.acquire(`automation-run:${Instance.directory}:${active.id}`) - const response = await app.request(`/automation/${created.id}`, { method: "DELETE" }) + const deleted = await json(app, `/automation/${created.id}`, { method: "DELETE" }) - expect(response.status).toBe(409) - expect(await response.json()).toEqual({ error: "active_run_still_running", runID: active.id }) - expect(Automation.get(created.id).id).toBe(created.id) - expect(Automation.runs({ automationID: created.id }).items[0]).toMatchObject({ + expect(deleted).toEqual({ id: created.id, deleted: true, revision: 2 }) + expect(() => Automation.get(created.id)).toThrow() + const row = Database.use((db) => db.select().from(AutomationRunTable).where(eq(AutomationRunTable.id, active.id)).get()) + expect(row ? Automation.Run.parse(row.data) : undefined).toMatchObject({ id: active.id, state: "scheduled", }) @@ -1238,28 +1240,29 @@ describe("automation routes", () => { const update409 = paths["/automation/{automationID}"].put.responses["409"].content["application/json"].schema const pause409 = paths["/automation/{automationID}/pause"].post.responses["409"].content["application/json"].schema const resume409 = paths["/automation/{automationID}/resume"].post.responses["409"].content["application/json"].schema - const delete409 = paths["/automation/{automationID}"].delete.responses["409"].content["application/json"].schema + const deleteResponses = paths["/automation/{automationID}"].delete.responses expect(create422).toEqual({ $ref: "#/components/schemas/AutomationValidationError" }) expect(update422).toEqual({ $ref: "#/components/schemas/AutomationValidationError" }) expect(update409).toEqual({ $ref: "#/components/schemas/AutomationConflictError" }) expect(pause409).toEqual({ $ref: "#/components/schemas/AutomationConflictError" }) expect(resume409).toEqual({ $ref: "#/components/schemas/AutomationConflictError" }) - expect(delete409).toEqual({ $ref: "#/components/schemas/AutomationActiveRunStillRunningError" }) + expect(deleteResponses).not.toHaveProperty("409") expect(spec.components?.schemas).toHaveProperty("AutomationValidationError") expect(spec.components?.schemas).toHaveProperty("AutomationConflictError") - expect(spec.components?.schemas).toHaveProperty("AutomationActiveRunStillRunningError") + expect(spec.components?.schemas).not.toHaveProperty("AutomationActiveRunStillRunningError") }) - test("openapi describes delete active-run stop side effect", async () => { + test("openapi describes delete as preserving already-started runs", async () => { const { Server } = await import("../../src/server/server") const spec = await Server.openapi() const paths = spec.paths as Record const description = paths["/automation/{automationID}"].delete.description - expect(description).toContain("If a run is active") - expect(description).toContain("publish the stopped run") - expect(description).toContain("live run is owned by another process") + expect(description).toContain("Already-started runs continue") + expect(description).not.toContain("run stop endpoint") + expect(description).not.toContain("publish the stopped run") + expect(description).not.toContain("live run is owned by another process") }) test("runNow returns the queued run before background execution updates it", async () => { diff --git a/packages/opencode/test/server/automation-runner.test.ts b/packages/opencode/test/server/automation-runner.test.ts index c2299f077..d93f9de4e 100644 --- a/packages/opencode/test/server/automation-runner.test.ts +++ b/packages/opencode/test/server/automation-runner.test.ts @@ -17,6 +17,7 @@ import { SessionID } from "../../src/session/schema" import { AutomationRunContext, AutomationStepCapError } from "../../src/automation/run-context" import { Flock } from "../../src/util/flock" import { Worktree } from "../../src/worktree" +import { internalTestHooks } from "../../src/automation/__test_hooks" import { tmpdir } from "../fixture/fixture" const RUN_WAIT_TIMEOUT_MS = 10_000 @@ -93,6 +94,21 @@ async function waitForTerminalRun(automationID: string) { throw new Error("Timed out waiting for terminal automation run") } +function readRun(runID: string) { + const row = Database.use((db) => db.select().from(AutomationRunTable).where(eq(AutomationRunTable.id, runID)).get()) + return row ? Automation.Run.parse(row.data) : undefined +} + +async function waitForRunByID(runID: string, state: Automation.Run["state"]) { + const deadline = Date.now() + RUN_WAIT_TIMEOUT_MS + while (Date.now() < deadline) { + const run = readRun(runID) + if (run?.state === state) return run + await Bun.sleep(10) + } + throw new Error(`Timed out waiting for run ${runID} to reach ${state}`) +} + function defer() { let resolve!: (value: T | PromiseLike) => void const promise = new Promise((done) => { @@ -111,7 +127,7 @@ function automationSessionsForTitle(title: string) { ) } -function hangingChat(ready: () => void) { +function hangingChat(ready: () => void, delayMs = 10_000) { const encoder = new TextEncoder() let timer: ReturnType | undefined const first = `data: ${JSON.stringify({ @@ -141,7 +157,7 @@ function hangingChat(ready: () => void) { timer = setTimeout(() => { ctrl.enqueue(encoder.encode(rest)) ctrl.close() - }, 10_000) + }, delayMs) }, cancel() { if (timer) clearTimeout(timer) @@ -149,17 +165,6 @@ function hangingChat(ready: () => void) { }) } -async function waitForAbortedAssistant(sessionID: SessionID) { - const deadline = Date.now() + 1_000 - while (Date.now() < deadline) { - const messages = await Session.messages({ sessionID }) - const assistant = messages.findLast((message) => message.info.role === "assistant") - if (assistant?.info.role === "assistant" && assistant.info.error?.name === "MessageAbortedError") return assistant - await Bun.sleep(10) - } - throw new Error("Timed out waiting for aborted assistant message") -} - describe("automation runNow execution", () => { test("executes a run and records the terminal result", async () => { await withAutomation(async (projectID) => { @@ -262,6 +267,31 @@ describe("automation runNow execution", () => { }) }) + test("blocks a run when a deleted automation still has a durable active writer", async () => { + await withAutomation(async (projectID) => { + const first = Automation.create(input(projectID, { title: "Deleted active writer" })) + const second = Automation.create(input(projectID, { title: "Second automation" })) + const active = Automation.runNow(first.id, { now: 100 }) + await using _ = await Flock.acquire(`automation-run:${Instance.directory}:${active.id}`) + Automation.remove(first.id) + let entered = false + + await Automation.runNowExecuting(second.id, { + now: 200, + executor: async () => { + entered = true + return { sessionID: SessionID.descending(), result: "second", cost: 0 } + }, + }) + + const stopped = await waitForRun(second.id, "stopped") + if (stopped.state !== "stopped") throw new Error("expected stopped run") + expect(stopped.stopReason).toBe("previous_run_awaiting_input") + expect(readRun(active.id)?.state).toBe("scheduled") + expect(entered).toBe(false) + }) + }) + test("reconciles stale durable writers before executing a manual run", async () => { await withAutomation(async (projectID) => { const first = Automation.create(input(projectID, { title: "Stale writer" })) @@ -413,6 +443,38 @@ describe("automation runNow execution", () => { }) }) + test("deleteBySourceSession cancels an active continue run bound to the deleted conversation", async () => { + await withAutomation(async (projectID) => { + const sourceSessionID = SessionID.descending() + const definition = Automation.create(input(projectID, { context: "continue" }), { sourceSessionID }) + const started = Promise.withResolvers() + const release = Promise.withResolvers() + let sawAbort = false + + const initial = await Automation.runNowExecuting(definition.id, { + executor: async ({ signal }) => { + signal.addEventListener("abort", () => { + sawAbort = true + release.resolve() + }) + started.resolve() + await release.promise + return { sessionID: sourceSessionID, result: "done", cost: 0 } + }, + }) + + await started.promise + await Automation.deleteBySourceSession(sourceSessionID) + release.resolve() + + expect(() => Automation.get(definition.id)).toThrow() + const stopped = await waitForRunByID(initial.id, "stopped") + if (stopped.state !== "stopped") throw new Error("expected stopped run") + expect(stopped.stopReason).toBe("cancelled") + expect(sawAbort).toBe(true) + }) + }) + test("does not revive a continue automation deleted during execution", async () => { await withAutomation(async (projectID) => { const definition = Automation.create(input(projectID, { context: "continue" }), { @@ -424,16 +486,16 @@ describe("automation runNow execution", () => { }) let removed!: Awaited> - await Automation.runNowExecuting(definition.id, { + const initial = await Automation.runNowExecuting(definition.id, { executor: async () => { removed = await Automation.remove(definition.id) return { sessionID: SessionID.descending(), result: "done", cost: 0 } }, }) - await Bun.sleep(20) + await waitForRunByID(initial.id, "succeeded") unsubscribeDefinition() - expect(removed.stoppedRun).toMatchObject({ state: "stopped", stopReason: "cancelled" }) + expect(removed.tombstone).toEqual({ id: definition.id, deleted: true, revision: 2 }) expect(() => Automation.get(definition.id)).toThrow() expect(definitionEvents).toHaveLength(0) }) @@ -460,7 +522,7 @@ describe("automation runNow execution", () => { }) }) - test("aborts an active run when its automation is deleted", async () => { + test("keeps an active run alive when its automation is deleted", async () => { await withAutomation(async (projectID) => { const definition = Automation.create(input(projectID)) const sessionID = SessionID.descending() @@ -472,7 +534,7 @@ describe("automation runNow execution", () => { if (event.properties.automationID === definition.id) runEvents.push(event.properties) }) - await Automation.runNowExecuting(definition.id, { + const initial = await Automation.runNowExecuting(definition.id, { executor: async ({ run, signal }) => { Automation.markRunStarted(run, sessionID, { now: run.triggeredAt }) signal.addEventListener("abort", () => { @@ -481,33 +543,76 @@ describe("automation runNow execution", () => { }) started.resolve() await release.promise - return { sessionID, result: "should not succeed", cost: 0 } + return { sessionID, result: "done", cost: 0 } }, }) await started.promise const removed = await Automation.remove(definition.id) + release.resolve() - expect(sawAbort).toBe(true) - expect(removed.stoppedRun).toMatchObject({ - state: "stopped", + expect(sawAbort).toBe(false) + expect(removed).not.toHaveProperty("stoppedRun") + expect(() => Automation.get(definition.id)).toThrow() + const succeeded = await waitForRunByID(initial.id, "succeeded") + expect(succeeded).toMatchObject({ + state: "succeeded", sessionID, - stopReason: "cancelled", + result: "done", }) - await Bun.sleep(20) unsubscribeRun() - expect(runEvents.some((event) => event.state === "succeeded")).toBe(false) + expect(runEvents.some((event) => event.state === "stopped")).toBe(false) + }) + }) + + test("keeps a queued run alive when its automation is deleted before the runner reads the definition", async () => { + await withAutomation(async (projectID) => { + const definition = Automation.create(input(projectID)) + const sessionID = SessionID.descending() + const runnerEntered = Promise.withResolvers() + const releaseRunner = Promise.withResolvers() + let entered = false + + internalTestHooks.beforeExecuteRun = async (run) => { + runnerEntered.resolve(run) + await releaseRunner.promise + } + try { + const initial = await Automation.runNowExecuting(definition.id, { + executor: async () => { + entered = true + return { sessionID, result: "done", cost: 0 } + }, + }) + const queued = await runnerEntered.promise + expect(queued.id).toBe(initial.id) + const removed = await Automation.remove(definition.id) + releaseRunner.resolve() + + expect(removed.tombstone).toEqual({ id: definition.id, deleted: true, revision: 2 }) + expect(() => Automation.get(definition.id)).toThrow() + const succeeded = await waitForRunByID(initial.id, "succeeded") + expect(succeeded).toMatchObject({ + state: "succeeded", + sessionID, + result: "done", + }) + expect(entered).toBe(true) + } finally { + releaseRunner.resolve() + delete internalTestHooks.beforeExecuteRun + } }) }) - test("deleting an active automation cancels the real session prompt", async () => { + test("deleting an active automation lets the real session prompt finish", async () => { const ready = defer() const server = Bun.serve({ port: 0, fetch(req) { const url = new URL(req.url) if (!url.pathname.endsWith("/chat/completions")) return new Response("not found", { status: 404 }) - return new Response(hangingChat(() => ready.resolve()), { + return new Response(hangingChat(() => ready.resolve(), 10), { status: 200, headers: { "Content-Type": "text/event-stream" }, }) @@ -546,15 +651,18 @@ describe("automation runNow execution", () => { fn: async () => { const definition = Automation.create(input(Instance.project.id, { title: "Cancel real prompt" })) - await Automation.runNowExecuting(definition.id, { executor: sessionPromptExecutor }) + const initial = await Automation.runNowExecuting(definition.id, { executor: sessionPromptExecutor }) await ready.promise const removed = await Automation.remove(definition.id) - const stoppedRun = removed.stoppedRun - expect(stoppedRun).toMatchObject({ state: "stopped", stopReason: "cancelled" }) - if (!stoppedRun?.sessionID) throw new Error("expected stopped run to keep its sessionID") - - await waitForAbortedAssistant(stoppedRun.sessionID) + expect(removed).not.toHaveProperty("stoppedRun") + + const succeeded = await waitForRunByID(initial.id, "succeeded") + if (!succeeded.sessionID) throw new Error("expected succeeded run to keep its sessionID") + const messages = await Session.messages({ sessionID: succeeded.sessionID }) + expect( + messages.some((message) => message.info.role === "assistant" && message.info.error?.name === "MessageAbortedError"), + ).toBe(false) }, }) } finally { @@ -974,7 +1082,7 @@ describe("automation runNow execution", () => { } }) - test("deleting after run start but before prompt runner is busy does not call the provider", async () => { + test("deleting after run start but before prompt runner is busy still lets the prompt run", async () => { let providerCalls = 0 const server = Bun.serve({ port: 0, @@ -982,7 +1090,7 @@ describe("automation runNow execution", () => { const url = new URL(req.url) if (!url.pathname.endsWith("/chat/completions")) return new Response("not found", { status: 404 }) providerCalls++ - return new Response(hangingChat(() => undefined), { + return new Response(hangingChat(() => undefined, 10), { status: 200, headers: { "Content-Type": "text/event-stream" }, }) @@ -1026,7 +1134,7 @@ describe("automation runNow execution", () => { void Automation.remove(definition.id).then(removed.resolve, removed.reject) }) - await Automation.runNowExecuting(definition.id, { executor: sessionPromptExecutor }) + const initial = await Automation.runNowExecuting(definition.id, { executor: sessionPromptExecutor }) let result: Awaited> try { result = await Promise.race([ @@ -1039,13 +1147,9 @@ describe("automation runNow execution", () => { unsubscribe() } - expect(result.stoppedRun).toMatchObject({ state: "stopped", stopReason: "cancelled" }) - await Bun.sleep(50) - expect(providerCalls).toBe(0) - if (result.stoppedRun?.sessionID) { - const messages = await Session.messages({ sessionID: result.stoppedRun.sessionID }) - expect(messages.some((message) => message.info.role === "assistant")).toBe(false) - } + expect(result).not.toHaveProperty("stoppedRun") + await waitForRunByID(initial.id, "succeeded") + expect(providerCalls).toBe(1) }, }) } finally { diff --git a/packages/opencode/test/tool/automate-manage.test.ts b/packages/opencode/test/tool/automate-manage.test.ts index 402592f24..580dc9d30 100644 --- a/packages/opencode/test/tool/automate-manage.test.ts +++ b/packages/opencode/test/tool/automate-manage.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, test } from "bun:test" import { Effect, ManagedRuntime, Schema } from "effect" import { Automation } from "../../src/automation" +import { AutomationRunTable } from "../../src/automation/automation.sql" import { AutomationScheduler } from "../../src/automation/scheduler" import { Bus } from "../../src/bus" import { Instance } from "../../src/project/instance" @@ -9,6 +10,7 @@ import { MessageID, SessionID } from "../../src/session/schema" import { AutomateManageParameters, createAutomateManageDefinition } from "../../src/tool/automate-manage" import { toJsonSchema } from "../../src/util/effect-zod" import { Flock } from "../../src/util/flock" +import { Database, eq } from "../../src/storage/db" import { tmpdir } from "../fixture/fixture" import { fakeAutomationProvider } from "../fake/provider" @@ -299,7 +301,7 @@ describe("automate_manage tool", () => { }) }) - test("delete preserves the automation when a live active run is still running", async () => { + test("delete removes the automation while a live active run is still running", async () => { await using tmp = await tmpdir({ git: true }) await Instance.provide({ directory: tmp.path, @@ -309,11 +311,17 @@ describe("automate_manage tool", () => { const active = Automation.runNow(created.id, { now: 200 }) await using _lease = await Flock.acquire(`automation-run:${Instance.directory}:${active.id}`) - await expect( - Effect.runPromise(tool().execute({ action: "delete", id: created.id }, toolContext())), - ).rejects.toThrow(`Cannot delete automation ${created.id}: active_run_still_running (${active.id})`) + const result = await Effect.runPromise(tool().execute({ action: "delete", id: created.id }, toolContext())) - expect(Automation.get(created.id).id).toBe(created.id) + expect(result.title).toBe("Automation deleted") + expect(result.metadata.automationTombstone).toEqual({ id: created.id, deleted: true, revision: 2 }) + expect(result.metadata).not.toHaveProperty("stoppedRun") + expect(() => Automation.get(created.id)).toThrow() + const row = Database.use((db) => db.select().from(AutomationRunTable).where(eq(AutomationRunTable.id, active.id)).get()) + expect(row ? Automation.Run.parse(row.data) : undefined).toMatchObject({ + id: active.id, + state: "scheduled", + }) }, }) }) diff --git a/packages/opencode/test/tool/automate.test.ts b/packages/opencode/test/tool/automate.test.ts index 3a6ffa3f9..a624b8515 100644 --- a/packages/opencode/test/tool/automate.test.ts +++ b/packages/opencode/test/tool/automate.test.ts @@ -41,6 +41,11 @@ describe("automate tool", () => { const tool = createAutomateDefinition(fakeProviderInterface, automation) expect(tool.description).toContain("reminder") expect(tool.description).toContain("one time") + expect(tool.description).toContain("monitor") + expect(tool.description).toContain("poll") + expect(tool.description).toContain("check periodically") + expect(tool.description).toContain("every 5 minutes") + expect(tool.description).toContain("until a status changes") expect(tool.description).toContain("Never set up OS schedulers") // The exception must stay aligned with pawwork.txt's scheduling section: // an explicit user request for a system-level scheduler is legitimate work. diff --git a/packages/opencode/test/tool/registry.test.ts b/packages/opencode/test/tool/registry.test.ts index 75842b97f..96676e250 100644 --- a/packages/opencode/test/tool/registry.test.ts +++ b/packages/opencode/test/tool/registry.test.ts @@ -113,6 +113,37 @@ describe("tool.registry", () => { }) }) + test("tool_info describes automate_manage delete as preserving already-started runs", async () => { + await using tmp = await tmpdir() + + await withMockedConfigInstall(async () => { + await Instance.provide({ + directory: tmp.path, + fn: async () => { + const tools = await ToolRegistry.tools({ + providerID: ProviderID.make("openai"), + modelID: ModelID.make("gpt-5"), + agent: { name: "build", mode: "primary" as const, permission: [], options: {} }, + }) + const toolInfo = tools.find((tool) => tool.id === "tool_info")! + const ctx = { + sessionID: SessionID.descending(), + messageID: MessageID.ascending(), + agent: "build", + abort: new AbortController().signal, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + } + + const result = await Effect.runPromise(toolInfo.execute({ name: "automate_manage" }, ctx)) + expect(result.output).toContain("already-started runs continue") + expect(result.output).not.toContain("active run prevents removal") + }, + }) + }) + }) + test("keeps trash removal contract across prompt and package surfaces", async () => { const shellDescription = await Bun.file(new URL("../../src/tool/shell.txt", import.meta.url)).text() expect(shellDescription).not.toContain("trash tool") @@ -148,11 +179,19 @@ describe("tool.registry", () => { test("keeps scheduling routing contract across prompt surfaces", async () => { const shellDescription = await Bun.file(new URL("../../src/tool/shell.txt", import.meta.url)).text() expect(shellDescription).toContain("Scheduled or delayed tasks: Use the automate tool") + expect(shellDescription).toContain("polling, monitoring, or repeated-check tasks") + expect(shellDescription).toContain("over 60 seconds") expect(shellDescription).toContain("Existing PawWork Automations: Use automate_manage via tool_info") const systemPrompt = await Bun.file(new URL("../../src/session/prompt/pawwork.txt", import.meta.url)).text() expect(systemPrompt).toContain("# Scheduling, reminders, and recurring work") expect(systemPrompt).toContain("`automate` tool") + expect(systemPrompt).toContain("monitor, poll, or watch something over time") + expect(systemPrompt).toContain("every N minutes") + expect(systemPrompt).toContain("until a status changes") + expect(systemPrompt).toContain("short bounded waits") + expect(systemPrompt).toContain("60 seconds total") + expect(systemPrompt).toContain("minute-scale polling") expect(systemPrompt).toContain("`automate_manage`") expect(systemPrompt).toContain("launchd") expect(systemPrompt).toContain("by writing files") diff --git a/packages/sdk/js/src/v2/gen/sdk.gen.ts b/packages/sdk/js/src/v2/gen/sdk.gen.ts index ab5cb75ae..9b0f08536 100644 --- a/packages/sdk/js/src/v2/gen/sdk.gen.ts +++ b/packages/sdk/js/src/v2/gen/sdk.gen.ts @@ -3576,7 +3576,7 @@ export class Automation extends HeyApiClient { /** * Delete automation * - * Delete an automation definition and return a tombstone. If a run is active in this process, stop it and publish the stopped run before publishing the tombstone. If a live run is owned by another process, return 409 without deleting. + * Delete an automation definition, cancel future scheduling, and return a tombstone. Already-started runs continue to completion. */ public delete( parameters: { diff --git a/packages/sdk/js/src/v2/gen/types.gen.ts b/packages/sdk/js/src/v2/gen/types.gen.ts index 668b5a125..e76f02f95 100644 --- a/packages/sdk/js/src/v2/gen/types.gen.ts +++ b/packages/sdk/js/src/v2/gen/types.gen.ts @@ -2497,11 +2497,6 @@ export type AutomationUpdateInput = { variant?: string | null } -export type AutomationActiveRunStillRunningError = { - error: "active_run_still_running" - runID: string -} - export type AutomationRunsResponse = { items: Array nextCursor: string | null @@ -5918,10 +5913,6 @@ export type AutomationDeleteErrors = { * Not found */ 404: NotFoundError - /** - * Automation has a live run owned by another process - */ - 409: AutomationActiveRunStillRunningError } export type AutomationDeleteError = AutomationDeleteErrors[keyof AutomationDeleteErrors]