From e23d53401b8c23061dd25f3d6081f16050491a34 Mon Sep 17 00:00:00 2001 From: Matt Rubens <2600+mrubens@users.noreply.github.com> Date: Fri, 6 Nov 2026 13:56:37 -0500 Subject: [PATCH] fix: let long tasks run and make safety stops resumable Remove the per-pass 200-tool and 100-step caps, which stopped legitimate long tasks well under the lifetime request ceiling. Creating a resume run now releases a pass-level safety stop in the run-creation transaction; the request counter is never reset and a request-ceiling stop stays latched. Parent-session stop notices are keyed per run so a later stop is not dropped. --- .changeset/agent-safety-long-tasks.md | 6 +++ apps/docs/tasks.mdx | 19 +++---- .../opencode-server-subagent-watchdog.test.ts | 8 ++- .../src/server/__tests__/enqueue-task.test.ts | 50 +++++++++++++++++++ .../cloud-agents/src/server/task-run-queue.ts | 4 ++ packages/db/agent-safety.md | 24 +++++---- .../db/src/lib/__tests__/agent-safety.test.ts | 43 ++++++++++++++++ packages/db/src/lib/agent-safety.ts | 34 +++++++++++-- .../types/src/__tests__/agent-safety.test.ts | 18 +++---- packages/types/src/agent-safety.ts | 7 +-- 10 files changed, 171 insertions(+), 42 deletions(-) create mode 100644 .changeset/agent-safety-long-tasks.md diff --git a/.changeset/agent-safety-long-tasks.md b/.changeset/agent-safety-long-tasks.md new file mode 100644 index 0000000000..d7c7cd7252 --- /dev/null +++ b/.changeset/agent-safety-long-tasks.md @@ -0,0 +1,6 @@ +--- +'@roomote/worker': patch +'@roomote/cloud-agents': patch +--- + +Long tasks are no longer stopped after 200 tool calls or 100 model steps, and replying to a task stopped for repeated activity resumes it from its retained work. The 1,000-request lifetime ceiling and repetition detection still apply. diff --git a/apps/docs/tasks.mdx b/apps/docs/tasks.mdx index 4f12af3249..307550d9f6 100644 --- a/apps/docs/tasks.mdx +++ b/apps/docs/tasks.mdx @@ -110,11 +110,11 @@ The read-only task-update polling tool has a larger, finite allowance of 20 identical completions. Shell commands and search tools have no polling exception. Subagent passes, including review, judge, explore, and custom subagents, also -stop at 100 model steps, 200 completed tools, or 20 minutes. Activity does not -extend the deadline. The main task also has repetition and completed-tool -protection. A task has a lifetime ceiling of 1,000 admitted provider requests, -shared across its models, children, retries, and resumed runs. Unknown prices -do not disable that request ceiling. +stop after 20 minutes. Activity does not extend the deadline. The main task +also has repetition protection. A task has a lifetime ceiling of 1,000 admitted +provider requests, shared across its models, children, retries, and resumed +runs. Unknown prices do not disable that request ceiling. Long tasks are not +stopped for the number of tool calls or model steps alone. A safety stop cancels the task's agent tree instead of asking the parent agent to retry the stopped pass. A notice is saved in the task and its owning session, @@ -123,10 +123,11 @@ if a notice has not appeared in an already-open view. Partial work is retained; an interrupted review must not be treated as a clean review. An already-admitted provider request can still finish and incur cost while cancellation settles. -Automatic retry, model fallback, restart, and resume do not reset a stopped -task's admission state. Inspect the partial work and the stop reason before -explicitly starting a new task with a narrower request. There is no automatic -budget reset or in-product allowance-extension control. +Automatic retry, model fallback, and restart do not continue a stopped task. +Resuming the task, for example by replying to it, continues from the retained +work after a repetition or time stop. The request count is never reset: a task +that reached the 1,000-request ceiling stays stopped, and continuing that work +requires a new task. There is no in-product allowance-extension control. These are request and execution bounds, not a dollar spending limit. Use your inference provider's own billing controls for a financial cap; see diff --git a/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-subagent-watchdog.test.ts b/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-subagent-watchdog.test.ts index 4b01cb8f22..3100e580f3 100644 --- a/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-subagent-watchdog.test.ts +++ b/apps/worker/src/sandbox-server/lib/harnesses/__tests__/opencode-server-subagent-watchdog.test.ts @@ -474,12 +474,12 @@ describe('OpenCode subagent run tracking', () => { } }); - it('caps unique model steps even without tool calls', async () => { + it('does not stop a pass for its number of model steps', async () => { const { client, harness } = createHarness(); try { await connectHarness(harness, client); await armSpawn(client, harness); - for (let index = 0; index < 100; index++) { + for (let index = 0; index < 250; index++) { await client.emit({ type: 'message.updated', properties: { @@ -492,9 +492,7 @@ describe('OpenCode subagent run tracking', () => { }, }); } - expect(client.abort).toHaveBeenCalledWith( - expect.objectContaining({ sessionId: 'ses_1' }), - ); + expect(client.abort).not.toHaveBeenCalled(); } finally { harness.dispose(); } diff --git a/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts b/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts index bd755e6453..5dceb17ed2 100644 --- a/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts +++ b/packages/cloud-agents/src/server/__tests__/enqueue-task.test.ts @@ -310,6 +310,56 @@ describe('pinned launch providers without an eligible default', () => { ); }); +describe('snapshot resume agent safety', () => { + it('releases a pass-level safety stop but keeps the lifetime request count', async () => { + const userId = await createUser(); + const run = await launchFresh({ + task: standardTaskInput({ computeProvider: 'modal' }), + initiator: { kind: 'user', userId }, + workflow: 'standard', + surface: 'web', + trigger: 'manual', + }); + await db + .update(tasks) + .set({ + inferenceRequests: 98, + safetyStoppedAt: new Date(), + safetyStopReason: 'pass_tool_limit', + }) + .where(eq(tasks.id, run.taskId!)); + await enqueueTask( + { + task: { + type: TaskPayloadKind.SnapshotResume, + computeProvider: 'modal', + payload: { + repo: 'acme/widgets', + sourceSnapshotId: 'safety-resume-snapshot', + sourceRunId: run.id, + }, + } as SnapshotResumeTask, + actingUserId: userId, + }, + { enqueue: false }, + ); + expect( + await db.query.tasks.findFirst({ + where: eq(tasks.id, run.taskId!), + columns: { + inferenceRequests: true, + safetyStoppedAt: true, + safetyStopReason: true, + }, + }), + ).toEqual({ + inferenceRequests: 98, + safetyStoppedAt: null, + safetyStopReason: null, + }); + }); +}); + describe('shouldCaptureTaskCreatedEvent', () => { it('excludes environment snapshot maintenance from task analytics', () => { expect( diff --git a/packages/cloud-agents/src/server/task-run-queue.ts b/packages/cloud-agents/src/server/task-run-queue.ts index 10c51e495b..5cfdfeb064 100644 --- a/packages/cloud-agents/src/server/task-run-queue.ts +++ b/packages/cloud-agents/src/server/task-run-queue.ts @@ -47,6 +47,7 @@ import { type TaskRun, type DatabaseTransaction, db, + releaseAgentSafetyStopForResume, deploymentSettings, ensureAutomationRowsOnce, ensureSessionForTask, @@ -3092,6 +3093,9 @@ async function enqueueSnapshotResume( await options.afterCreateInTransaction(tx, insertedRun); } + // Resuming continues a pass-level safety stop; the request ceiling holds. + await releaseAgentSafetyStopForResume(tx, sourceRun.taskId); + // The new resume run is non-terminal (pending/dequeued), so re-derive the // task state: a task that had gone terminal on a prior attempt flips back // to 'active' now that another attempt is queued. diff --git a/packages/db/agent-safety.md b/packages/db/agent-safety.md index a59375a4bb..046e2119a2 100644 --- a/packages/db/agent-safety.md +++ b/packages/db/agent-safety.md @@ -3,11 +3,13 @@ The task row is the durable admission authority. `inference_request_limit` defaults to 1,000; `inference_requests` counts admitted attempts, not successful responses or accounting events. `safety_stopped_at` and `safety_stop_reason` -are an irreversible application-level latch for the task. No resume, new run, -provider fallback, or ordinary message clears these fields. There is no UI or -public API to grant more allowance in this release; explicit continuation is a -new task after inspecting the retained work. Operators should not clear a latch -while an old runtime is alive. +are the task's stop latch. Provider fallback, in-run retries, and ordinary +admission never clear it. Creating a snapshot-resume run releases a pass-level +latch (`releaseAgentSafetyStopForResume`, in the run-creation transaction) +unless the reason is `task_request_limit` or the counter is already at the +ceiling; `inference_requests` is never reset. There is no UI or public API to +grant more allowance; continuing past the ceiling is a new task. Operators +should not clear a latch while an old runtime is alive. ## Admission guarantees @@ -49,9 +51,12 @@ updates for the same invocation are ignored. The exact read-only `manage_tasks`/`roomote_manage_tasks` action `get_updates` gets 20 identical completions; it is not an unlimited polling exception. -Each live child gets its own 100 unique assistant-message starts, 200 unique -terminal tool completions, and 20-minute wall deadline, independent of progress -and siblings. A pending task-tool spawn is time-bounded even before its child ID +Each live child gets its own 20-minute wall deadline, independent of progress +and siblings. There is no per-pass tool-completion or step cap: legitimate long +passes routinely exceed any fixed count, and the lifetime request ceiling plus +repetition detection bound runaway spend. `pass_step_limit` and +`pass_tool_limit` remain accepted reasons only so older workers can report +them (N-1). A pending task-tool spawn is time-bounded even before its child ID arrives. The worker checks wall deadlines every second. Root tool completions are guarded too. Control-plane prompts retain their normal shorter deadlines (120 seconds by default); `timeoutMs: null` and larger timeouts cannot disable @@ -62,7 +67,8 @@ retry/fallback queues, aborts the root and discovered descendants with bounded abort requests, emits the terminal task event to release worker/parent waits, and refuses late local commands/events. This intentionally does not let the parent spawn a replacement pass. The durable task latch refuses all subsequent -managed provider dispatch even if the worker restarts. The existing parent +managed provider dispatch even if the worker restarts, until a resume run +releases a pass-level stop. The existing parent settle/error relays do not wake parent inference for a safety-stopped task. ## Boundaries to keep in mind diff --git a/packages/db/src/lib/__tests__/agent-safety.test.ts b/packages/db/src/lib/__tests__/agent-safety.test.ts index 5b5057a397..f513da176a 100644 --- a/packages/db/src/lib/__tests__/agent-safety.test.ts +++ b/packages/db/src/lib/__tests__/agent-safety.test.ts @@ -1,5 +1,6 @@ import { admitTaskInferenceRequest, + releaseAgentSafetyStopForResume, stopTaskForAgentSafety, db, eq, @@ -104,3 +105,45 @@ it('persists exactly one parent notice and stop latch together, independent of i const restarted = await runFactory.create({ taskId: task.id }); expect(await admitTaskInferenceRequest(task.id, restarted.id)).toBe(false); }); + +it('releases a pass-level stop for a resume without resetting the request counter', async () => { + const { task, run } = await seed(); + expect(await admitTaskInferenceRequest(task.id, run.id)).toBe(true); + await stopTaskForAgentSafety({ + taskId: task.id, + runId: run.id, + reason: 'identical_tool_loop', + }); + const resumed = await runFactory.create({ taskId: task.id, kind: 'resume' }); + expect( + await db.transaction((tx) => releaseAgentSafetyStopForResume(tx, task.id)), + ).toBe(true); + expect(await admitTaskInferenceRequest(task.id, resumed.id)).toBe(true); + const row = await db.query.tasks.findFirst({ where: eq(tasks.id, task.id) }); + expect(row).toMatchObject({ + inferenceRequests: 2, + safetyStoppedAt: null, + safetyStopReason: null, + }); + await stopTaskForAgentSafety({ + taskId: task.id, + runId: resumed.id, + reason: 'identical_tool_loop', + }); + expect( + await db.query.taskMessages.findMany({ + where: eq(taskMessages.taskId, task.id), + }), + ).toHaveLength(2); +}); + +it('keeps a request-ceiling stop latched across resumes', async () => { + const { task, run } = await seed(1); + expect(await admitTaskInferenceRequest(task.id, run.id)).toBe(true); + expect(await admitTaskInferenceRequest(task.id, run.id)).toBe(false); + expect( + await db.transaction((tx) => releaseAgentSafetyStopForResume(tx, task.id)), + ).toBe(false); + const resumed = await runFactory.create({ taskId: task.id, kind: 'resume' }); + expect(await admitTaskInferenceRequest(task.id, resumed.id)).toBe(false); +}); diff --git a/packages/db/src/lib/agent-safety.ts b/packages/db/src/lib/agent-safety.ts index e064aee89f..4164444c70 100644 --- a/packages/db/src/lib/agent-safety.ts +++ b/packages/db/src/lib/agent-safety.ts @@ -1,4 +1,4 @@ -import { and, desc, eq } from 'drizzle-orm'; +import { and, desc, eq, isNotNull, isNull, lt, ne, or } from 'drizzle-orm'; import { AGENT_SAFETY_STOP_TEXT, ACP_ENVELOPE_EVENT_TYPES, @@ -81,8 +81,9 @@ async function stopInTransaction( .insert(fastAgentMessages) .values({ conversationId: parentConversation.id, - eventId: `agent-safety:${taskId}`, - turnId: `agent-safety:${taskId}`, + // Per run: a resume can release the latch and a later run stop again. + eventId: `agent-safety:${taskId}:${run.id}`, + turnId: `agent-safety:${taskId}:${run.id}`, turnSeq: 0, ts, eventType: ACP_ENVELOPE_EVENT_TYPES.AgentSafetyStop, @@ -142,3 +143,30 @@ export async function admitTaskInferenceRequest( return true; }); } + +/** + * A new resume run is an explicit continuation, so it releases a pass-level + * safety stop (for example an identical-tool loop). The lifetime request + * counter is never reset, and a `task_request_limit` stop stays latched. + */ +export async function releaseAgentSafetyStopForResume( + tx: Transaction, + taskId: string, +): Promise { + const released = await tx + .update(tasks) + .set({ safetyStoppedAt: null, safetyStopReason: null }) + .where( + and( + eq(tasks.id, taskId), + isNotNull(tasks.safetyStoppedAt), + or( + isNull(tasks.safetyStopReason), + ne(tasks.safetyStopReason, 'task_request_limit'), + ), + lt(tasks.inferenceRequests, tasks.inferenceRequestLimit), + ), + ) + .returning({ id: tasks.id }); + return released.length > 0; +} diff --git a/packages/types/src/__tests__/agent-safety.test.ts b/packages/types/src/__tests__/agent-safety.test.ts index f948df99c1..380f130dde 100644 --- a/packages/types/src/__tests__/agent-safety.test.ts +++ b/packages/types/src/__tests__/agent-safety.test.ts @@ -46,12 +46,10 @@ describe('agent safety pass', () => { canonicalSafetyJson({ b: 2, a: 1 }), ); }); - it('resets repetition on changed results or arguments, but still caps the pass', () => { + it('resets repetition on changed results or arguments without a per-pass tool cap', () => { const guard = pass(); - for (let id = 0; id < AGENT_SAFETY.passTools; id++) - expect(guard.completed(call(id, { value: id }))).toBe( - id === AGENT_SAFETY.passTools - 1 ? 'pass_tool_limit' : undefined, - ); + for (let id = 0; id < 1_000; id++) + expect(guard.completed(call(id, { value: id }))).toBeUndefined(); }); it('permits bounded status polling, never arbitrary search or shell exemptions', () => { const guard = pass(); @@ -65,13 +63,13 @@ describe('agent safety pass', () => { }), ).toBe(id === 19 ? 'identical_tool_loop' : undefined); }); - it('counts unique model steps and has an activity-independent deadline', () => { + it('counts unique model steps without a cap and has an activity-independent deadline', () => { const guard = pass(); - for (let id = 0; id < 100; id++) { - const expected = id === 99 ? 'pass_step_limit' : undefined; - expect(guard.step(String(id))).toBe(expected); - expect(guard.step(String(id))).toBe(expected); + for (let id = 0; id < 1_000; id++) { + expect(guard.step(String(id))).toBeUndefined(); + expect(guard.step(String(id))).toBeUndefined(); } + expect(guard.snapshot(0).steps).toBe(1_000); expect(pass().checkTime(AGENT_SAFETY.passTimeoutMs)).toBe( 'pass_time_limit', ); diff --git a/packages/types/src/agent-safety.ts b/packages/types/src/agent-safety.ts index 3dc41c8156..46f7a8a725 100644 --- a/packages/types/src/agent-safety.ts +++ b/packages/types/src/agent-safety.ts @@ -2,14 +2,13 @@ export const AGENT_SAFETY = { identicalCompletions: 5, pollingCompletions: 20, - passSteps: 100, - passTools: 200, passTimeoutMs: 20 * 60_000, taskRequests: 1_000, } as const; export type AgentSafetyReason = | 'identical_tool_loop' + // N-1: no longer raised; kept so older workers can still report them. | 'pass_step_limit' | 'pass_tool_limit' | 'pass_time_limit' @@ -89,8 +88,6 @@ export class AgentSafetyPass { step(id: string): AgentSafetyReason | undefined { if (this.stopped) return this.stopped; this.steps.add(id); - if (this.steps.size >= AGENT_SAFETY.passSteps) - this.stopped ??= 'pass_step_limit'; return this.stopped; } @@ -122,8 +119,6 @@ export class AgentSafetyPass { : AGENT_SAFETY.identicalCompletions) ) this.stopped = 'identical_tool_loop'; - if (this.completedCalls.size >= AGENT_SAFETY.passTools) - this.stopped ??= 'pass_tool_limit'; return this.stopped; } }