From e99c9be58de9dcceb9b9fa26b0213e159efb98e7 Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 14 Jul 2026 18:34:55 +0000 Subject: [PATCH 1/4] feat: add manage_tasks await for environment verification Block server-side on follow-up task settlement so environment-setup can own verification with one job call instead of model-driven poll loops. --- .../__tests__/await-task.test.ts | 317 +++++++++++++++++ .../__tests__/tool-descriptions.test.ts | 26 +- .../src/mcp/roomote-mcp-server/await-task.ts | 329 ++++++++++++++++++ .../src/mcp/roomote-mcp-server/index.ts | 38 +- .../__tests__/environmentSetupSkill.test.ts | 21 +- .../standard/environment-setup/SKILL.md | 54 ++- 6 files changed, 745 insertions(+), 40 deletions(-) create mode 100644 apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts create mode 100644 apps/worker/src/mcp/roomote-mcp-server/await-task.ts diff --git a/apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts b/apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts new file mode 100644 index 00000000..9dbfe686 --- /dev/null +++ b/apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts @@ -0,0 +1,317 @@ +import { + awaitTaskSettlement, + classifySettledSummary, + handleAwaitTask, + isTaskActiveForAwait, +} from '../await-task.js'; +import type { RoomoteConfig, TaskSummaryResponse } from '../types.js'; + +const config: RoomoteConfig = { + token: 'test-token', + platformApiUrl: 'https://test-api.example.com', +}; + +function summary( + overrides: Partial = {}, +): TaskSummaryResponse { + return { + id: 'task-1', + title: 'Verify environment', + mode: 'standard', + completed: false, + repositoryName: 'owner/repo', + harness: 'opencode-server', + createdAt: 1700000000, + taskRunStatus: 'running', + taskPhase: 'running', + taskRunError: null, + linkedEnvironmentId: 'env-1', + linkedEnvironmentName: 'Demo', + ...overrides, + }; +} + +describe('isTaskActiveForAwait', () => { + it('treats completed and exited runs as settled', () => { + expect(isTaskActiveForAwait(summary({ completed: true }))).toBe(false); + expect( + isTaskActiveForAwait( + summary({ taskRunStatus: 'failed', taskPhase: null }), + ), + ).toBe(false); + expect( + isTaskActiveForAwait( + summary({ taskRunStatus: 'canceled', taskPhase: null }), + ), + ).toBe(false); + }); + + it('treats Ready/Idle post-turn states as settled', () => { + expect( + isTaskActiveForAwait( + summary({ + taskRunStatus: 'running', + taskPhase: 'waiting_for_prompt', + }), + ), + ).toBe(false); + expect( + isTaskActiveForAwait( + summary({ taskRunStatus: 'idle', taskPhase: 'waiting_for_prompt' }), + ), + ).toBe(false); + expect( + isTaskActiveForAwait(summary({ taskRunStatus: 'idle', taskPhase: null })), + ).toBe(false); + }); + + it('keeps booting and working phases active', () => { + expect( + isTaskActiveForAwait( + summary({ taskRunStatus: 'preparing', taskPhase: null }), + ), + ).toBe(true); + expect( + isTaskActiveForAwait( + summary({ taskRunStatus: 'running', taskPhase: 'running' }), + ), + ).toBe(true); + expect( + isTaskActiveForAwait( + summary({ taskRunStatus: 'idle', taskPhase: 'running' }), + ), + ).toBe(true); + }); + + it('treats needs-input as settled', () => { + expect( + isTaskActiveForAwait( + summary({ + taskRunStatus: 'running', + taskPhase: 'waiting_for_user_input', + }), + ), + ).toBe(false); + }); +}); + +describe('classifySettledSummary', () => { + it('classifies completed ready success', () => { + expect( + classifySettledSummary(summary({ completed: true, taskRunError: null })), + ).toEqual({ + terminalLabel: 'Completed', + ready: true, + errorSummary: null, + }); + }); + + it('classifies failed runs', () => { + expect( + classifySettledSummary( + summary({ + completed: false, + taskRunStatus: 'failed', + taskRunError: 'boot failed', + }), + ), + ).toEqual({ + terminalLabel: 'Failed', + ready: false, + errorSummary: 'boot failed', + }); + }); + + it('classifies needs input', () => { + expect( + classifySettledSummary( + summary({ + taskRunStatus: 'running', + taskPhase: 'waiting_for_user_input', + }), + ), + ).toMatchObject({ + terminalLabel: 'NeedsInput', + ready: false, + }); + }); + + it('classifies status completed even when completed flag is false', () => { + expect( + classifySettledSummary( + summary({ + completed: false, + taskRunStatus: 'completed', + taskPhase: null, + taskRunError: null, + }), + ), + ).toEqual({ + terminalLabel: 'Completed', + ready: true, + errorSummary: null, + }); + }); + + it('classifies Ready and Idle settled labels', () => { + expect( + classifySettledSummary( + summary({ + taskRunStatus: 'running', + taskPhase: 'waiting_for_prompt', + }), + ), + ).toMatchObject({ terminalLabel: 'Ready', ready: true }); + + expect( + classifySettledSummary( + summary({ + taskRunStatus: 'idle', + taskPhase: null, + }), + ), + ).toMatchObject({ terminalLabel: 'Idle', ready: true }); + }); + + it('classifies canceled runs', () => { + expect( + classifySettledSummary( + summary({ + taskRunStatus: 'canceled', + taskPhase: null, + taskRunError: null, + }), + ), + ).toEqual({ + terminalLabel: 'Canceled', + ready: false, + errorSummary: 'Task was canceled', + }); + }); +}); + +describe('awaitTaskSettlement', () => { + it('polls until the task settles and returns ready', async () => { + const getTaskSummary = vi + .fn() + .mockResolvedValueOnce( + summary({ taskRunStatus: 'preparing', taskPhase: null }), + ) + .mockResolvedValueOnce( + summary({ taskRunStatus: 'running', taskPhase: 'running' }), + ) + .mockResolvedValueOnce( + summary({ + completed: true, + taskRunStatus: 'completed', + taskPhase: null, + }), + ); + + const sleep = vi.fn().mockResolvedValue(undefined); + let now = 1_000; + const result = await awaitTaskSettlement( + { taskId: 'task-1', timeoutMs: 60_000, pollIntervalMs: 1_000 }, + config, + { + getTaskSummary, + sleep, + now: () => { + now += 1_000; + return now; + }, + }, + ); + + expect(getTaskSummary).toHaveBeenCalledTimes(3); + expect(sleep).toHaveBeenCalledTimes(2); + expect(result.terminalLabel).toBe('Completed'); + expect(result.ready).toBe(true); + expect(result.timedOut).toBe(false); + expect(result.linkedEnvironmentId).toBe('env-1'); + }); + + it('returns TimedOut when still active after timeout', async () => { + const getTaskSummary = vi + .fn() + .mockResolvedValue( + summary({ taskRunStatus: 'running', taskPhase: 'running' }), + ); + const sleep = vi.fn().mockResolvedValue(undefined); + let now = 0; + + const result = await awaitTaskSettlement( + { taskId: 'task-1', timeoutMs: 5_000, pollIntervalMs: 1_000 }, + config, + { + getTaskSummary, + sleep, + now: () => { + const value = now; + now += 2_000; + return value; + }, + }, + ); + + expect(result.terminalLabel).toBe('TimedOut'); + expect(result.ready).toBe(false); + expect(result.timedOut).toBe(true); + expect(result.errorSummary).toMatch(/Timed out after 5000ms/); + }); + + it('returns Failed when summary settles failed', async () => { + const getTaskSummary = vi.fn().mockResolvedValue( + summary({ + taskRunStatus: 'failed', + taskPhase: null, + taskRunError: 'Sandbox failed to boot worker process', + }), + ); + + const result = await awaitTaskSettlement({ taskId: 'task-1' }, config, { + getTaskSummary, + sleep: vi.fn(), + now: () => 0, + }); + + expect(result.terminalLabel).toBe('Failed'); + expect(result.ready).toBe(false); + expect(result.errorSummary).toBe('Sandbox failed to boot worker process'); + expect(result.status).toBe('Failed'); + }); +}); + +describe('handleAwaitTask', () => { + it('formats a text summary for the settled await result', async () => { + const result = await handleAwaitTask({ taskId: 'task-1' }, config, { + getTaskSummary: vi.fn().mockResolvedValue( + summary({ + completed: true, + taskRunStatus: 'completed', + taskPhase: null, + }), + ), + sleep: vi.fn(), + now: () => 1000, + }); + + const text = result.content[0]?.text ?? ''; + expect(text).toContain('Task: Verify environment'); + expect(text).toContain('ID: task-1'); + expect(text).toContain('Terminal: Completed'); + expect(text).toContain('Ready: yes'); + expect(text).toContain('Timed out: no'); + expect(text).toContain('Linked Environment: Demo'); + }); + + it('surfaces API errors', async () => { + const failed = await handleAwaitTask({ taskId: 'missing' }, config, { + getTaskSummary: vi.fn().mockRejectedValue(new Error('Not found')), + sleep: vi.fn(), + now: () => 0, + }); + expect(failed.content[0]?.text).toContain('Not found'); + expect(failed.content[0]?.text).toContain('"success":false'); + }); +}); diff --git a/apps/worker/src/mcp/roomote-mcp-server/__tests__/tool-descriptions.test.ts b/apps/worker/src/mcp/roomote-mcp-server/__tests__/tool-descriptions.test.ts index 64a99f97..3fc2ab50 100644 --- a/apps/worker/src/mcp/roomote-mcp-server/__tests__/tool-descriptions.test.ts +++ b/apps/worker/src/mcp/roomote-mcp-server/__tests__/tool-descriptions.test.ts @@ -137,6 +137,29 @@ describe('roomote MCP tool descriptions', () => { ); }); + it('documents the await action on manage_tasks', async () => { + const { registeredTools } = await importRoomoteMcpServer(); + const manageTasksTool = getRegisteredTool(registeredTools, 'manage_tasks'); + const timeoutField = getInputSchemaField(manageTasksTool, 'timeoutMs'); + const pollIntervalField = getInputSchemaField( + manageTasksTool, + 'pollIntervalMs', + ); + + expect(manageTasksTool.config.description).toContain( + 'Use action "await" to block until a task settles', + ); + expect(manageTasksTool.config.description).toContain( + 'Prefer await after launch when this task must own the outcome of a follow-up task such as environment verification.', + ); + expect(timeoutField.description).toContain( + 'Optional max wait for action "await"', + ); + expect(pollIntervalField.description).toContain( + 'Optional poll interval for action "await"', + ); + }); + it('keeps debug actions out of manage_tasks', async () => { const { registeredTools } = await importRoomoteMcpServer(); const manageTasksTool = getRegisteredTool(registeredTools, 'manage_tasks'); @@ -153,6 +176,7 @@ describe('roomote MCP tool descriptions', () => { expect(actionField.options).toEqual([ 'search', 'get_summary', + 'await', 'get_compute_logs', 'get_messages', 'launch', @@ -161,7 +185,7 @@ describe('roomote MCP tool descriptions', () => { 'list_environments', ]); expect(taskIdField.description).toBe( - 'The task ID (required for get_summary, get_compute_logs, get_messages, cancel, and send_message)', + 'The task ID (required for get_summary, await, get_compute_logs, get_messages, cancel, and send_message)', ); expect(limitField.description).toBe( 'Max results for search (default 20, max 100) or max latest messages for get_messages (max 1000)', diff --git a/apps/worker/src/mcp/roomote-mcp-server/await-task.ts b/apps/worker/src/mcp/roomote-mcp-server/await-task.ts new file mode 100644 index 00000000..e4ebe783 --- /dev/null +++ b/apps/worker/src/mcp/roomote-mcp-server/await-task.ts @@ -0,0 +1,329 @@ +import { getTaskSummary } from './tasks-api-client.js'; +import { getHarnessLabel, getTaskStatusLabel } from './task-display.js'; +import { catchError, textResult } from './tool-result.js'; +import type { + RoomoteConfig, + TaskSummaryResponse, + ToolResult, +} from './types.js'; + +/** Default wait budget for environment prepare / verification boots. */ +export const DEFAULT_AWAIT_TASK_TIMEOUT_MS = 25 * 60_000; +export const MIN_AWAIT_TASK_TIMEOUT_MS = 5_000; +export const MAX_AWAIT_TASK_TIMEOUT_MS = 45 * 60_000; +export const DEFAULT_AWAIT_TASK_POLL_INTERVAL_MS = 5_000; +export const MIN_AWAIT_TASK_POLL_INTERVAL_MS = 1_000; +export const MAX_AWAIT_TASK_POLL_INTERVAL_MS = 60_000; + +export type AwaitTaskTerminalLabel = + | 'Completed' + | 'Failed' + | 'Canceled' + | 'TimedOut' + | 'NeedsInput' + | 'Ready' + | 'Idle'; + +export type AwaitTaskResult = { + taskId: string; + title: string | null; + status: string; + terminalLabel: AwaitTaskTerminalLabel; + ready: boolean; + timedOut: boolean; + waitedMs: number; + errorSummary: string | null; + taskRunStatus: string | null; + taskPhase: string | null; + completed: boolean; + harness: string | null; + linkedEnvironmentId: string | null; + linkedEnvironmentName: string | null; +}; + +export type AwaitTaskDeps = { + getTaskSummary: typeof getTaskSummary; + sleep: (ms: number) => Promise; + now: () => number; +}; + +const defaultDeps: AwaitTaskDeps = { + getTaskSummary, + sleep: (ms) => new Promise((resolve) => setTimeout(resolve, ms)), + now: () => Date.now(), +}; + +const TERMINAL_EXITED = new Set(['failed', 'canceled', 'completed']); + +/** + * Whether the target task is still usefully in-flight for `await`. + * Settled Ready/Idle (post-turn keepalive) is not active — environments that + * finish a one-shot verification turn look Idle/Ready, not Failed/Completed. + */ +export function isTaskActiveForAwait(summary: TaskSummaryResponse): boolean { + if (summary.completed) { + return false; + } + + const status = summary.taskRunStatus; + + if (!status) { + // No run row yet: keep waiting a bit during enqueue. + return true; + } + + if (TERMINAL_EXITED.has(status)) { + return false; + } + + const phase = summary.taskPhase; + + if (phase === 'waiting_for_user_input') { + return false; + } + + if (status === 'idle') { + // Follow-up turn mid-flight while row stays Idle. + return phase === 'running'; + } + + if (status === 'running') { + // waiting_for_prompt / idle phase => Ready/Idle labels (turn ended). + if ( + phase === 'waiting_for_prompt' || + phase === 'idle' || + phase === 'stopped' || + phase === 'shutting_down' + ) { + return false; + } + // null/unknown or running/working phases: still active. + return true; + } + + // Booting statuses (pending, preparing, spawning, connecting, ...) → active. + return true; +} + +export function classifySettledSummary( + summary: TaskSummaryResponse, +): Pick { + const statusLabel = getTaskStatusLabel(summary); + const errorSummary = summary.taskRunError?.trim() + ? summary.taskRunError.trim() + : null; + + if (summary.completed || summary.taskRunStatus === 'completed') { + return { + terminalLabel: 'Completed', + ready: !errorSummary, + errorSummary, + }; + } + + if (summary.taskRunStatus === 'failed') { + return { + terminalLabel: 'Failed', + ready: false, + errorSummary: errorSummary ?? 'Task run failed', + }; + } + + if (summary.taskRunStatus === 'canceled') { + return { + terminalLabel: 'Canceled', + ready: false, + errorSummary: errorSummary ?? 'Task was canceled', + }; + } + + if ( + summary.taskPhase === 'waiting_for_user_input' || + statusLabel === 'Needs input' + ) { + return { + terminalLabel: 'NeedsInput', + ready: false, + errorSummary: + errorSummary ?? + 'Task is waiting for user input instead of completing verification', + }; + } + + if (statusLabel === 'Idle' || summary.taskPhase === 'idle') { + return { + terminalLabel: 'Idle', + ready: !errorSummary, + errorSummary, + }; + } + + if (statusLabel === 'Ready' || summary.taskPhase === 'waiting_for_prompt') { + return { + terminalLabel: 'Ready', + ready: !errorSummary, + errorSummary, + }; + } + + // Fallback for unexpected settled shapes + return { + terminalLabel: statusLabel === 'Failed' ? 'Failed' : 'Idle', + ready: !errorSummary && statusLabel !== 'Failed', + errorSummary, + }; +} + +function clamp( + value: number | undefined, + fallback: number, + min: number, + max: number, +): number { + if (value === undefined || Number.isNaN(value)) { + return fallback; + } + return Math.min(max, Math.max(min, value)); +} + +function formatAwaitResult(result: AwaitTaskResult): string { + const harnessLabel = getHarnessLabel(result.harness); + const lines = [ + `Task: ${result.title || '(untitled)'}`, + `ID: ${result.taskId}`, + `Status: ${result.status}`, + `Terminal: ${result.terminalLabel}`, + `Ready: ${result.ready ? 'yes' : 'no'}`, + `Timed out: ${result.timedOut ? 'yes' : 'no'}`, + `Waited: ${Math.round(result.waitedMs / 1000)}s`, + result.errorSummary ? `Error: ${result.errorSummary}` : null, + harnessLabel ? `Harness: ${harnessLabel}` : null, + result.linkedEnvironmentName + ? `Linked Environment: ${result.linkedEnvironmentName}` + : null, + result.linkedEnvironmentId + ? `Linked Environment ID: ${result.linkedEnvironmentId}` + : null, + ].filter(Boolean); + + return lines.join('\n'); +} + +export async function awaitTaskSettlement( + params: { + taskId: string; + timeoutMs?: number; + pollIntervalMs?: number; + }, + config: RoomoteConfig, + deps: AwaitTaskDeps = defaultDeps, +): Promise { + const taskId = params.taskId.trim(); + const timeoutMs = clamp( + params.timeoutMs, + DEFAULT_AWAIT_TASK_TIMEOUT_MS, + MIN_AWAIT_TASK_TIMEOUT_MS, + MAX_AWAIT_TASK_TIMEOUT_MS, + ); + const pollIntervalMs = clamp( + params.pollIntervalMs, + DEFAULT_AWAIT_TASK_POLL_INTERVAL_MS, + MIN_AWAIT_TASK_POLL_INTERVAL_MS, + MAX_AWAIT_TASK_POLL_INTERVAL_MS, + ); + + const startedAt = deps.now(); + let lastSummary: TaskSummaryResponse | null = null; + + while (true) { + const summary = await deps.getTaskSummary(config, taskId); + lastSummary = summary; + + if (!isTaskActiveForAwait(summary)) { + const classified = classifySettledSummary(summary); + return { + taskId: summary.id, + title: summary.title, + status: getTaskStatusLabel(summary), + terminalLabel: classified.terminalLabel, + ready: classified.ready, + timedOut: false, + waitedMs: deps.now() - startedAt, + errorSummary: classified.errorSummary, + taskRunStatus: summary.taskRunStatus, + taskPhase: summary.taskPhase, + completed: summary.completed, + harness: summary.harness, + linkedEnvironmentId: summary.linkedEnvironmentId, + linkedEnvironmentName: summary.linkedEnvironmentName, + }; + } + + const elapsed = deps.now() - startedAt; + if (elapsed >= timeoutMs) { + break; + } + + const remaining = timeoutMs - elapsed; + await deps.sleep(Math.min(pollIntervalMs, remaining)); + } + + const summary = + lastSummary ?? + (await deps.getTaskSummary(config, taskId).catch(() => null)); + + if (!summary) { + return { + taskId, + title: null, + status: 'TimedOut', + terminalLabel: 'TimedOut', + ready: false, + timedOut: true, + waitedMs: deps.now() - startedAt, + errorSummary: `Timed out after ${timeoutMs}ms waiting for task ${taskId}`, + taskRunStatus: null, + taskPhase: null, + completed: false, + harness: null, + linkedEnvironmentId: null, + linkedEnvironmentName: null, + }; + } + + return { + taskId: summary.id, + title: summary.title, + status: getTaskStatusLabel(summary), + terminalLabel: 'TimedOut', + ready: false, + timedOut: true, + waitedMs: deps.now() - startedAt, + errorSummary: + summary.taskRunError?.trim() || + `Timed out after ${timeoutMs}ms while task was still active (status: ${getTaskStatusLabel(summary)})`, + taskRunStatus: summary.taskRunStatus, + taskPhase: summary.taskPhase, + completed: summary.completed, + harness: summary.harness, + linkedEnvironmentId: summary.linkedEnvironmentId, + linkedEnvironmentName: summary.linkedEnvironmentName, + }; +} + +export async function handleAwaitTask( + params: { + taskId: string; + timeoutMs?: number; + pollIntervalMs?: number; + }, + config: RoomoteConfig, + deps?: AwaitTaskDeps, +): Promise { + try { + const result = await awaitTaskSettlement(params, config, deps); + return textResult(formatAwaitResult(result)); + } catch (error) { + return catchError(error); + } +} diff --git a/apps/worker/src/mcp/roomote-mcp-server/index.ts b/apps/worker/src/mcp/roomote-mcp-server/index.ts index b76a19df..2f22e6d4 100644 --- a/apps/worker/src/mcp/roomote-mcp-server/index.ts +++ b/apps/worker/src/mcp/roomote-mcp-server/index.ts @@ -31,6 +31,7 @@ import { handleSearchTasks } from './search-tasks.js'; import { handleLaunchTask } from './launch-task.js'; import { handleGetTaskMessages } from './task-messages.js'; import { handleGetTaskSummary } from './task-summary.js'; +import { handleAwaitTask } from './await-task.js'; import { handleGetTaskComputeLogs } from './task-compute-logs.js'; import { handleCancelTask } from './cancel-task.js'; import { handleSendMessage } from './send-message.js'; @@ -397,6 +398,7 @@ const manageTasksToolDescription = 'Use action "list_environments" to list launch targets (named environments and the org-wide target). ' + 'Use action "search" to find tasks by query or status. ' + `Use action "get_summary" to inspect a specific task's latest status and failure details (requires taskId). ` + + 'Use action "await" to block until a task settles (Completed/Failed/Canceled/Ready/Idle/NeedsInput) or timeoutMs elapses, polling server-side so the current tool call stays mid-turn (requires taskId). Prefer await after launch when this task must own the outcome of a follow-up task such as environment verification. ' + 'Use action "get_compute_logs" to fetch all compute logs for a task, including per-job command output for compute providers that support output lookup when the job has both a machine id and sandbox command id (requires taskId). ' + 'Use action "get_messages" to retrieve the latest message history for a task (requires taskId, returns newest first). ' + `Use action "launch" to create and start a new task against an environment using ${PRODUCT_NAME}'s default standard workflow (requires prompt and environmentId). ` + @@ -408,6 +410,7 @@ const manageTasksInputSchema = { .enum([ 'search', 'get_summary', + 'await', 'get_compute_logs', 'get_messages', 'launch', @@ -416,13 +419,13 @@ const manageTasksInputSchema = { 'list_environments', ]) .describe( - 'The task action to perform. Call "list_environments" immediately before "launch".', + 'The task action to perform. Call "list_environments" immediately before "launch". After launch, preferred follow-up ownership uses "await" with the returned taskId.', ), taskId: z .string() .optional() .describe( - 'The task ID (required for get_summary, get_compute_logs, get_messages, cancel, and send_message)', + 'The task ID (required for get_summary, await, get_compute_logs, get_messages, cancel, and send_message)', ), message: z .string() @@ -471,6 +474,24 @@ const manageTasksInputSchema = { 'Call "list_environments" immediately before launching and copy one of the returned environmentId values.', ), branch: z.string().optional().describe('Branch to use (for launch)'), + timeoutMs: z + .number() + .int() + .min(5_000) + .max(45 * 60_000) + .optional() + .describe( + 'Optional max wait for action "await" in milliseconds (default 1500000 / 25 minutes, min 5000, max 2700000 / 45 minutes). Environment prepares can exceed 5 minutes.', + ), + pollIntervalMs: z + .number() + .int() + .min(1_000) + .max(60_000) + .optional() + .describe( + 'Optional poll interval for action "await" in milliseconds (default 5000, min 1000, max 60000).', + ), } satisfies Record; roomoteMcpServer.registerTool( @@ -511,6 +532,19 @@ roomoteMcpServer.registerTool( } return handleGetTaskSummary({ taskId: params.taskId }, config); } + case 'await': { + if (!params.taskId?.trim()) { + return errorResult('taskId is required for await'); + } + return handleAwaitTask( + { + taskId: params.taskId, + timeoutMs: params.timeoutMs, + pollIntervalMs: params.pollIntervalMs, + }, + config, + ); + } case 'get_compute_logs': { if (!params.taskId?.trim()) { return errorResult('taskId is required for get_compute_logs'); diff --git a/packages/cloud-agents/src/server/workflows/__tests__/environmentSetupSkill.test.ts b/packages/cloud-agents/src/server/workflows/__tests__/environmentSetupSkill.test.ts index 03c18e30..c2650b70 100644 --- a/packages/cloud-agents/src/server/workflows/__tests__/environmentSetupSkill.test.ts +++ b/packages/cloud-agents/src/server/workflows/__tests__/environmentSetupSkill.test.ts @@ -162,7 +162,7 @@ describe('environment-setup guidance', () => { const skillContent = readSkillContent(); expect(skillContent).toContain( - 'After successful environment persistence, use the Roomote MCP tool `mcp__roomote__manage_tasks` to launch a lightweight verification task against the created or updated environment and monitor it yourself instead of leaving verification as an implicit manual next step.', + 'After successful environment persistence, use the Roomote MCP tool `mcp__roomote__manage_tasks` to launch a lightweight verification task against the created or updated environment and await it with `action: "await"` yourself instead of leaving verification as an implicit manual next step or a multi-step `get_summary` poll loop.', ); expect(skillContent).toContain( 'For that follow-up task launch, call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "list_environments"` first so you can confirm the created or updated environment appears as a current launch target and copy the exact returned `environmentId`.', @@ -171,25 +171,25 @@ describe('environment-setup guidance', () => { 'Then call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "launch"`, `environmentId` set to that created or updated environment ID', ); expect(skillContent).toContain( - 'Immediately begin monitoring that verification task with the Roomote MCP tool `mcp__roomote__manage_tasks` using `action: "get_summary"` and the returned `taskId`.', + 'Immediately after launch succeeds, call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "await"` and that `taskId`.', ); expect(skillContent).toContain( - 'Narrate concise, plain-language progress updates while the follow-up check runs', + 'Prefer this single blocking await over model-driven `get_summary` poll loops or `bash` sleep', ); expect(skillContent).toContain( - 'Preparing the environment can take 5 minutes or more, so do not stop monitoring just because startup is taking a long time.', + 'Before or while that await runs, narrate one concise plain-language progress update', ); expect(skillContent).toContain( - 'If the monitored summary reaches `Ready`, `Idle`, or `Needs input`, do not keep polling that same state indefinitely.', + 'Preparing the environment can take 5 minutes or more — use the default await timeout', ); expect(skillContent).toContain( - 'If those latest task messages clearly report that the environment looks ready, treat that as a successful spawned-task run and report the observed success directly.', + 'If await returns `Terminal: Ready`, `Idle`, or `NeedsInput`', ); expect(skillContent).toContain( - 'If the monitored summary reaches `Completed` without a surfaced startup or runtime failure, treat that as a successful spawned-task run and report that observed outcome directly instead of asking the user to confirm it manually.', + 'Treat the await result as the source of truth for verification settlement.', ); expect(skillContent).toContain( - 'When the spawned verification task reveals a fixable setup or environment-definition error, try to fix it yourself, rerun any affected local validation, recreate or update the environment with the revised YAML, launch a fresh verification task, and repeat the monitoring process instead of stopping after the first failure.', + 'When the spawned verification task reveals a fixable setup or environment-definition error, try to fix it yourself, rerun any affected local validation, recreate or update the environment with the revised YAML, launch a fresh verification task, await the new taskId, and repeat instead of stopping after the first failure.', ); expect(skillContent).toContain( 'Retry at most 2 additional full environment-update-plus-verification attempts after the first spawned verification task', @@ -198,6 +198,9 @@ describe('environment-setup guidance', () => { 'If the observed verification error appears to require product or source-code changes outside environment-setup scope', ); expect(skillContent).not.toContain('monitoring limit'); + expect(skillContent).not.toContain( + 'Immediately begin monitoring that verification task with the Roomote MCP tool `mcp__roomote__manage_tasks` using `action: "get_summary"`', + ); expect(skillContent).toContain( 'When setup succeeds, begin with a plain-language outcome sentence such as `Your environment is ready.`', ); @@ -211,7 +214,7 @@ describe('environment-setup guidance', () => { 'This is the final visible paragraph; do not append an internal status summary after it.', ); expect(skillContent).toContain( - 'Do not mention a spawned task, task status, polling, or monitoring in those user-facing updates.', + 'Do not mention a spawned task, task status, polling, monitoring, or await in those user-facing updates.', ); expect(skillContent).not.toContain( 'When the verification task completed cleanly, report that the spawned verification task completed.', diff --git a/packages/cloud-agents/src/server/workflows/skills/standard/environment-setup/SKILL.md b/packages/cloud-agents/src/server/workflows/skills/standard/environment-setup/SKILL.md index ec5d7686..28da086b 100644 --- a/packages/cloud-agents/src/server/workflows/skills/standard/environment-setup/SKILL.md +++ b/packages/cloud-agents/src/server/workflows/skills/standard/environment-setup/SKILL.md @@ -8,7 +8,7 @@ You are an expert Roomote environment analyst. Analyze the already-checked-out r - Inspect repository evidence, infer the smallest correct Roomote environment config, validate install/test/start behavior locally on localhost when practical, create or update the environment when validation is sufficient, launch one lightweight verification task against the resulting environment, monitor that spawned task through the Roomote task summary surface while narrating concise progress updates, and when verification surfaces fixable setup errors, revise the environment and repeat the create-or-update plus verification cycle before returning a concise final outcome summary with explicit assumptions, validated observations, and blockers. + Inspect repository evidence, infer the smallest correct Roomote environment config, validate install/test/start behavior locally on localhost when practical, create or update the environment when validation is sufficient, launch one lightweight verification task against the resulting environment, block on that spawned task with Roomote MCP `manage_tasks` action `await` (server-side poll so this setup turn stays mid-turn) while narrating concise progress updates, and when verification surfaces fixable setup errors, revise the environment and repeat the create-or-update plus verification cycle before returning a concise final outcome summary with explicit assumptions, validated observations, and blockers. Produce one environment definition that is valid for the Roomote environment editor. Validate only local install, test, start, and localhost reachability. @@ -117,7 +117,7 @@ You are an expert Roomote environment analyst. Analyze the already-checked-out r 8. When the test failure instead appears to be a clearly pre-existing repository or unit-test failure outside environment-setup scope, record the exact command and failure, keep the suite referenced in `agentInstructions`, and continue only if install/start/localhost validation is otherwise sufficient. 9. If the app exposes an HTTP UI, set `initialUrl` to the best validated absolute localhost URL (or keep `about:blank` only when no better landing page exists), confirm that localhost URL through loopback HTTP reachability and startup evidence, and record the exact URL plus the evidence used. Do not use direct browser automation from `environment-setup`. 10. When the config includes a `ports` entry for a validated HTTP surface, confirm its `port` number matches the actual validated listening port and that any `initial_path` responds successfully over loopback. -10a. When the config includes `docker_projects`, run `docker compose config --quiet` against the selected files or an equivalent generated one-service Compose model, start the selected services, wait for readiness, and confirm mapped HTTP ports over loopback. Capture `docker compose ps --all` and recent service logs when startup fails. + 10a. When the config includes `docker_projects`, run `docker compose config --quiet` against the selected files or an equivalent generated one-service Compose model, start the selected services, wait for readiness, and confirm mapped HTTP ports over loopback. Capture `docker compose ps --all` and recent service logs when startup fails. 11. If the app exposes only an HTTP API or a non-browser surface, verify localhost reachability using loopback addresses only. 12. If any command in the draft config fails or cannot be confirmed, either revise or remove that command from the YAML, or report the exact blocker; do not leave unrun or unconfirmed commands in the final config. 13. As soon as repository evidence or early validation makes it clear that specific environment variables or secrets will be required and values are unavailable, request them immediately instead of waiting for a later failure. @@ -135,21 +135,19 @@ You are an expert Roomote environment analyst. Analyze the already-checked-out r 25. For that follow-up task launch, call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "list_environments"` first so you can confirm the created or updated environment appears as a current launch target and copy the exact returned `environmentId`. 26. Then call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "launch"`, `environmentId` set to that created or updated environment ID, and a concrete prompt such as `Confirm that this environment is running correctly. Use localhost or the environment's initial URL to verify the expected service responds successfully, and confirm there are no obvious startup failures blocking basic use. Preparing the environment can take 5 minutes or more, so be patient before deciding startup is stuck. Report the exact step that fails plus any visible error messages or logs. If everything works, say that the environment looks ready.`. 27. Keep the returned `taskId` for internal monitoring only. Do not expose the spawned verification task link in the user-facing response. -28. Immediately begin monitoring that verification task with the Roomote MCP tool `mcp__roomote__manage_tasks` using `action: "get_summary"` and the returned `taskId`. Use that per-task summary as the source of truth for task state, including any surfaced startup or runtime failure details. -29. Narrate concise, plain-language progress updates while the follow-up check runs, so the user can see that setup is still being checked without being asked to wait. Say what is being checked (for example, `I'm confirming the environment starts cleanly`) rather than mentioning a spawned task, task status, polling, or monitoring. -30. Continue checking the verification task while the summary shows an active startup or running state, or until it clearly reports a startup or runtime blocker through the summary. Preparing the environment can take 5 minutes or more, so do not stop monitoring just because startup is taking a long time. -31. If the monitored summary reaches `Completed` without a surfaced startup or runtime failure, treat that as a successful spawned-task run and report that observed outcome directly instead of asking the user to confirm it manually. -32. If the monitored summary reaches `Ready`, `Idle`, or `Needs input`, do not keep polling that same state indefinitely. Inspect the latest task messages to determine whether the verification task already reported success, surfaced a blocker, or is unexpectedly waiting for follow-up input. -33. If those latest task messages clearly report that the environment looks ready, treat that as a successful spawned-task run and report the observed success directly. -34. If those latest task messages surface a startup or runtime blocker, request unexpected user input, or otherwise fail to give a clear success outcome, treat that as a blocker or verification failure instead of pretending the environment is verified. -35. If the monitored summary reaches `Failed`, `Canceled`, or exposes a startup or runtime error, inspect the exact status and error and decide whether the problem appears fixable within environment-setup scope, such as revising commands, services, environment variables, startup order, readiness checks, or other environment-definition details. -36. When the spawned verification task reveals a fixable setup or environment-definition error, try to fix it yourself, rerun any affected local validation, recreate or update the environment with the revised YAML, launch a fresh verification task, and repeat the monitoring process instead of stopping after the first failure. -37. Treat each fix attempt as a real retry loop: revise based on the observed error, revalidate the affected setup steps, persist the updated environment again, relaunch the verification task, and monitor the new task summary rather than assuming the old failure is resolved. -38. Keep this verification-repair loop bounded. Retry at most 2 additional full environment-update-plus-verification attempts after the first spawned verification task unless the task context explicitly justifies a smaller limit. -39. If the observed verification error appears to require product or source-code changes outside environment-setup scope, missing external credentials, unsupported infrastructure, or another user decision you cannot safely make, report that blocker instead of pretending the environment can be repaired automatically. -40. If the verification task remains in an active startup or running state without surfacing a blocker you can act on, keep monitoring instead of handing the waiting back to the user. -41. If the environment was persisted but the follow-up check could not be completed, explain that readiness could not be confirmed and name the real blocker in user terms. Do not pretend the environment is ready. -42. If full validation or environment persistence is blocked by missing dependencies, localhost reachability limits, permissions, unavailable environment APIs, or a required real service that still needs user guidance after local validation, keep the config minimal and report the blocker. +28. Immediately after launch succeeds, call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "await"` and that `taskId`. Prefer this single blocking await over model-driven `get_summary` poll loops or `bash` sleep; await polls server-side and keeps the current setup tool call mid-turn so this parent task stays Working until the verification task settles or times out. Preparing the environment can take 5 minutes or more — use the default await timeout (or raise `timeoutMs` up to the tool maximum when needed) and do not abandon early solely because startup is slow. +29. Before or while that await runs, narrate one concise plain-language progress update so the user can see that setup is still being checked without being asked to wait. Say what is being checked (for example, `I'm confirming the environment starts cleanly`) rather than mentioning a spawned task, task status, polling, monitoring, or the await action. +30. Treat the await result as the source of truth for verification settlement. Map `Terminal: Completed` with `Ready: yes`, or terminal `Ready`/`Idle` without an error, as a successful boot when corresponding task messages do not contradict readiness. +31. If await returns `Terminal: Ready`, `Idle`, or `NeedsInput`, or if readiness remains ambiguous, optionally call `action: "get_messages"` once to confirm the verification task reported success versus a blocker or unexpected input request. Only allow true success when messages clearly report the environment looks ready (or await reported Completed with Ready: yes and no error). +32. If latest messages surface a startup or runtime blocker, request unexpected user input, or otherwise fail to give a clear success outcome, treat that as a blocker or verification failure instead of pretending the environment is verified. +33. If await returns `Terminal: Failed`, `Canceled`, or `TimedOut`, or exposes a startup or runtime error, inspect the exact status and error and decide whether the problem appears fixable within environment-setup scope, such as revising commands, services, environment variables, startup order, readiness checks, or other environment-definition details. +34. When the spawned verification task reveals a fixable setup or environment-definition error, try to fix it yourself, rerun any affected local validation, recreate or update the environment with the revised YAML, launch a fresh verification task, await the new taskId, and repeat instead of stopping after the first failure. +35. Treat each fix attempt as a real retry loop: revise based on the observed error, revalidate the affected setup steps, persist the updated environment again, relaunch the verification task, and await the new task rather than assuming the old failure is resolved. +36. Keep this verification-repair loop bounded. Retry at most 2 additional full environment-update-plus-verification attempts after the first spawned verification task unless the task context explicitly justifies a smaller limit. +37. If the observed verification error appears to require product or source-code changes outside environment-setup scope, missing external credentials, unsupported infrastructure, or another user decision you cannot safely make, report that blocker instead of pretending the environment can be repaired automatically. +38. Do not substitute multi-step agent-driven get_summary polling for await. If await is unavailable for an explicit tool or platform reason, report that readiness could not be confirmed rather than inventing a long sleep/poll loop. +39. If the environment was persisted but the follow-up check could not be completed, explain that readiness could not be confirmed and name the real blocker in user terms. Do not pretend the environment is ready. +40. If full validation or environment persistence is blocked by missing dependencies, localhost reachability limits, permissions, unavailable environment APIs, or a required real service that still needs user guidance after local validation, keep the config minimal and report the blocker. The final config reflects observed install/test/start behavior where practical and clearly reports any validation limits. @@ -192,11 +190,11 @@ You are an expert Roomote environment analyst. Analyze the already-checked-out r When a repository test suite is practical to run, it is executed and the result is reported explicitly. Test failures block environment creation when they indicate an environment-definition or setup problem; clearly pre-existing repository failures may be carried forward only when install/start validation is otherwise sufficient. The final environment definition is best-effort and should be runnable once required environment variables are provided. Environment creation or update is attempted only after local install/test/start validation is successful enough to proceed. -When environment persistence succeeds, a lightweight Roomote verification task is launched against that environment and monitored by calling the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "get_summary"` before this setup task finishes, unless an explicit blocker prevents launch or monitoring. +When environment persistence succeeds, a lightweight Roomote verification task is launched against that environment and awaited by calling the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "await"` and the returned `taskId` before this setup task finishes, unless an explicit blocker prevents launch or await. The final response summarizes the environment name, whether it was created or updated, key validation outcomes, and any blockers - without including the raw YAML config or internal orchestration details. A successful onboarding final response ends by telling the user that the environment is ready and linking `[Create a new task](/)` as the clear next action. -If the follow-up verification task reaches `Completed` without a surfaced startup or runtime failure, the final response reports that the environment is ready to use instead of asking the user to confirm it manually or describing the verification task. -If the spawned verification task reveals a fixable setup or environment-definition error, the skill attempts to repair it, update the environment, relaunch verification, and report the final bounded retry outcome instead of stopping after the first failed verification task. +If the follow-up verification await reports ready success (`Terminal: Completed` with `Ready: yes`, or settled Ready/Idle without error after optional message confirmation), the final response reports that the environment is ready to use instead of asking the user to confirm it manually or describing the verification task. +If the spawned verification task reveals a fixable setup or environment-definition error, the skill attempts to repair it, update the environment, relaunch verification, await the new task, and report the final bounded retry outcome instead of stopping after the first failed verification task. If the app exposes a browser UI and local startup succeeds, the localhost URL is verified through loopback reachability or other non-browser startup evidence before persistence continues. If required environment variables or secrets are known but unavailable, the skill requests them immediately through `request_environment_variables` in web tasks and Slack-started setup tasks, or asks the user to set them locally in the current task before proceeding with affected validation. Every command present in the final `repositories[].commands` list was run and explicitly confirmed during validation, or an explicit blocker explains why validation could not proceed. @@ -329,17 +327,17 @@ You are an expert Roomote environment analyst. Analyze the already-checked-out r In non-web surfaces, ask only for local environment variable additions in the current task, and provide exact variable names and exact actions. For this skill, create a new environment or update the specified existing environment only after localhost startup and loopback reachability are successful enough to proceed, including validation of `initialUrl` through non-browser evidence when the app exposes a browser UI. When the task explicitly identifies an existing environment to revise, update that environment instead of creating a duplicate. -After successful environment persistence, use the Roomote MCP tool `mcp__roomote__manage_tasks` to launch a lightweight verification task against the created or updated environment and monitor it yourself instead of leaving verification as an implicit manual next step. +After successful environment persistence, use the Roomote MCP tool `mcp__roomote__manage_tasks` to launch a lightweight verification task against the created or updated environment and await it with `action: "await"` yourself instead of leaving verification as an implicit manual next step or a multi-step `get_summary` poll loop. Before launching that verification task, call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "list_environments"` so the environment target is grounded in current Roomote data and you can copy the exact returned `environmentId`. -When the verification task launch succeeds, monitor it with the Roomote MCP tool `mcp__roomote__manage_tasks` using `action: "get_summary"` and use that per-task summary surface as the source of truth for task status and surfaced startup failures. -While the follow-up check runs, narrate concise progress updates in plain language instead of silently waiting or pushing the waiting back onto the user. Do not mention a spawned task, task status, polling, or monitoring in those user-facing updates. -Preparing the environment can take 5 minutes or more. Do not stop monitoring solely because the verification task is taking a long time to start; keep checking until it reaches a terminal state or surfaces a blocker you can report or act on. -If the monitored summary settles into `Ready`, `Idle`, or `Needs input`, inspect the latest task messages instead of polling that state forever, and only treat it as success when those messages clearly report that the environment looks ready. -If the spawned verification task surfaces a fixable setup or environment-definition problem, attempt to fix it yourself, recreate or update the environment, launch a fresh verification task, and repeat the monitoring loop instead of stopping after the first discovered error. +When the verification task launch succeeds, immediately call `mcp__roomote__manage_tasks` with `action: "await"` and the returned `taskId`. Treat that blocking await result as the source of truth for verification settlement instead of agent-driven summary polling or bash sleep loops. +Before or while the follow-up check runs, narrate a concise progress update in plain language instead of silently waiting or pushing the waiting back onto the user. Do not mention a spawned task, task status, polling, monitoring, or await in those user-facing updates. +Preparing the environment can take 5 minutes or more. Do not abandon await solely because the verification task is taking a long time to start; use the tool's default timeout or an appropriate higher `timeoutMs` within tool limits, then report timeout as failure to confirm readiness. +If await settles into `Ready`, `Idle`, or `NeedsInput`, inspect latest task messages when success is not already clear from the await payload, and only treat it as success when evidence clearly reports that the environment looks ready. +If the spawned verification task surfaces a fixable setup or environment-definition problem, attempt to fix it yourself, recreate or update the environment, launch a fresh verification task, await the new taskId, and repeat the loop instead of stopping after the first discovered error. Bound that environment-repair loop to at most 2 additional full retries after the first spawned verification task unless the task context clearly justifies fewer attempts. Do not claim automatic repair for failures that actually require product/source changes outside environment-setup scope, unsupported infrastructure, missing external credentials, or a user decision you cannot safely infer; report those as blockers. -Do not tell the user to verify the environment in that spawned task before clicking Continue, and do not include the spawned verification task link in the user-facing response; report the monitored outcome yourself. -If the follow-up check cannot be launched or completed after the environment is persisted, report that readiness could not be confirmed and explain the real blocker in user terms; do not imply the environment is ready. +Do not tell the user to verify the environment in that spawned task before clicking Continue, and do not include the spawned verification task link in the user-facing response; report the awaited outcome yourself. +If the follow-up check cannot be launched or awaited after the environment is persisted, report that readiness could not be confirmed and explain the real blocker in user terms; do not imply the environment is ready. When a browser UI is validated locally, report the exact localhost URL and the loopback or startup evidence that confirmed it rather than capturing screenshots from this skill. Any long-running service command (for example `dev`, `start`, `serve`, `preview`, or watchers) must use `detached: true`. Any command with `detached: true` must set `logfile` to capture runtime output. From d04db05f1081112303cc69f8b303e3522883b1ca Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 14 Jul 2026 18:36:11 +0000 Subject: [PATCH 2/4] fix: keep await-task helpers package-private for knip --- .../src/mcp/roomote-mcp-server/await-task.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/apps/worker/src/mcp/roomote-mcp-server/await-task.ts b/apps/worker/src/mcp/roomote-mcp-server/await-task.ts index e4ebe783..19a5c03d 100644 --- a/apps/worker/src/mcp/roomote-mcp-server/await-task.ts +++ b/apps/worker/src/mcp/roomote-mcp-server/await-task.ts @@ -8,14 +8,14 @@ import type { } from './types.js'; /** Default wait budget for environment prepare / verification boots. */ -export const DEFAULT_AWAIT_TASK_TIMEOUT_MS = 25 * 60_000; -export const MIN_AWAIT_TASK_TIMEOUT_MS = 5_000; -export const MAX_AWAIT_TASK_TIMEOUT_MS = 45 * 60_000; -export const DEFAULT_AWAIT_TASK_POLL_INTERVAL_MS = 5_000; -export const MIN_AWAIT_TASK_POLL_INTERVAL_MS = 1_000; -export const MAX_AWAIT_TASK_POLL_INTERVAL_MS = 60_000; - -export type AwaitTaskTerminalLabel = +const DEFAULT_AWAIT_TASK_TIMEOUT_MS = 25 * 60_000; +const MIN_AWAIT_TASK_TIMEOUT_MS = 5_000; +const MAX_AWAIT_TASK_TIMEOUT_MS = 45 * 60_000; +const DEFAULT_AWAIT_TASK_POLL_INTERVAL_MS = 5_000; +const MIN_AWAIT_TASK_POLL_INTERVAL_MS = 1_000; +const MAX_AWAIT_TASK_POLL_INTERVAL_MS = 60_000; + +type AwaitTaskTerminalLabel = | 'Completed' | 'Failed' | 'Canceled' @@ -24,7 +24,7 @@ export type AwaitTaskTerminalLabel = | 'Ready' | 'Idle'; -export type AwaitTaskResult = { +type AwaitTaskResult = { taskId: string; title: string | null; status: string; @@ -41,7 +41,7 @@ export type AwaitTaskResult = { linkedEnvironmentName: string | null; }; -export type AwaitTaskDeps = { +type AwaitTaskDeps = { getTaskSummary: typeof getTaskSummary; sleep: (ms: number) => Promise; now: () => number; From 3e5cfd36d6c5777c4e94425308a7f3be5efa645d Mon Sep 17 00:00:00 2001 From: Roomote Date: Tue, 14 Jul 2026 18:45:18 +0000 Subject: [PATCH 3/4] fix: harden await settlement and require verify message success Do not settle stopped/shutting_down runs as ready, prefer latest run failed/canceled over completed, and require get_messages before env setup reports readiness. --- .../__tests__/await-task.test.ts | 72 +++++++++++++++++++ .../src/mcp/roomote-mcp-server/await-task.ts | 53 +++++++------- .../__tests__/environmentSetupSkill.test.ts | 11 +-- .../standard/environment-setup/SKILL.md | 12 ++-- 4 files changed, 114 insertions(+), 34 deletions(-) diff --git a/apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts b/apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts index 9dbfe686..0ab041df 100644 --- a/apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts +++ b/apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts @@ -32,6 +32,46 @@ function summary( } describe('isTaskActiveForAwait', () => { + it('keeps stopped/shutting_down running phases active', () => { + expect( + isTaskActiveForAwait( + summary({ + taskRunStatus: 'running', + taskPhase: 'stopped', + }), + ), + ).toBe(true); + expect( + isTaskActiveForAwait( + summary({ + taskRunStatus: 'running', + taskPhase: 'shutting_down', + }), + ), + ).toBe(true); + }); + + it('prioritizes latest failed/canceled status over aggregate completed', () => { + expect( + isTaskActiveForAwait( + summary({ + completed: true, + taskRunStatus: 'failed', + taskPhase: null, + }), + ), + ).toBe(false); + expect( + isTaskActiveForAwait( + summary({ + completed: true, + taskRunStatus: 'canceled', + taskPhase: null, + }), + ), + ).toBe(false); + }); + it('treats completed and exited runs as settled', () => { expect(isTaskActiveForAwait(summary({ completed: true }))).toBe(false); expect( @@ -106,6 +146,38 @@ describe('classifySettledSummary', () => { }); }); + it('classifies failed/canceled above aggregate completed', () => { + expect( + classifySettledSummary( + summary({ + completed: true, + taskRunStatus: 'failed', + taskPhase: null, + taskRunError: 'boot failed after resume', + }), + ), + ).toEqual({ + terminalLabel: 'Failed', + ready: false, + errorSummary: 'boot failed after resume', + }); + + expect( + classifySettledSummary( + summary({ + completed: true, + taskRunStatus: 'canceled', + taskPhase: null, + taskRunError: null, + }), + ), + ).toEqual({ + terminalLabel: 'Canceled', + ready: false, + errorSummary: 'Task was canceled', + }); + }); + it('classifies failed runs', () => { expect( classifySettledSummary( diff --git a/apps/worker/src/mcp/roomote-mcp-server/await-task.ts b/apps/worker/src/mcp/roomote-mcp-server/await-task.ts index 19a5c03d..04064aed 100644 --- a/apps/worker/src/mcp/roomote-mcp-server/await-task.ts +++ b/apps/worker/src/mcp/roomote-mcp-server/await-task.ts @@ -59,23 +59,27 @@ const TERMINAL_EXITED = new Set(['failed', 'canceled', 'completed']); * Whether the target task is still usefully in-flight for `await`. * Settled Ready/Idle (post-turn keepalive) is not active — environments that * finish a one-shot verification turn look Idle/Ready, not Failed/Completed. + * Prefer the latest run's terminal status when it conflicts with the aggregate + * completed flag (failed/canceled resumes can leave completed=true). */ export function isTaskActiveForAwait(summary: TaskSummaryResponse): boolean { - if (summary.completed) { + const status = summary.taskRunStatus; + + // Latest run status always wins: a failed/canceled (or status-completed) run + // is settled even when the task aggregate completed flag is inconsistent. + if (status && TERMINAL_EXITED.has(status)) { return false; } - const status = summary.taskRunStatus; + if (summary.completed) { + return false; + } if (!status) { // No run row yet: keep waiting a bit during enqueue. return true; } - if (TERMINAL_EXITED.has(status)) { - return false; - } - const phase = summary.taskPhase; if (phase === 'waiting_for_user_input') { @@ -89,15 +93,12 @@ export function isTaskActiveForAwait(summary: TaskSummaryResponse): boolean { if (status === 'running') { // waiting_for_prompt / idle phase => Ready/Idle labels (turn ended). - if ( - phase === 'waiting_for_prompt' || - phase === 'idle' || - phase === 'stopped' || - phase === 'shutting_down' - ) { + // stopped / shutting_down usually precede a final failed/canceled/idle + // status transition — keep polling instead of treating them as ready. + if (phase === 'waiting_for_prompt' || phase === 'idle') { return false; } - // null/unknown or running/working phases: still active. + // null/unknown, running, stopped, shutting_down: still active. return true; } @@ -113,14 +114,8 @@ export function classifySettledSummary( ? summary.taskRunError.trim() : null; - if (summary.completed || summary.taskRunStatus === 'completed') { - return { - terminalLabel: 'Completed', - ready: !errorSummary, - errorSummary, - }; - } - + // Prefer the latest run terminal status over the aggregate completed flag so + // a failed/canceled resume is never reported as a successful completion. if (summary.taskRunStatus === 'failed') { return { terminalLabel: 'Failed', @@ -137,6 +132,14 @@ export function classifySettledSummary( }; } + if (summary.taskRunStatus === 'completed' || summary.completed) { + return { + terminalLabel: 'Completed', + ready: !errorSummary, + errorSummary, + }; + } + if ( summary.taskPhase === 'waiting_for_user_input' || statusLabel === 'Needs input' @@ -166,11 +169,13 @@ export function classifySettledSummary( }; } - // Fallback for unexpected settled shapes + // Fallback for unexpected settled shapes — never claim ready here. return { terminalLabel: statusLabel === 'Failed' ? 'Failed' : 'Idle', - ready: !errorSummary && statusLabel !== 'Failed', - errorSummary, + ready: false, + errorSummary: + errorSummary ?? + `Task settled in an unexpected state (${statusLabel}) and readiness was not confirmed`, }; } diff --git a/packages/cloud-agents/src/server/workflows/__tests__/environmentSetupSkill.test.ts b/packages/cloud-agents/src/server/workflows/__tests__/environmentSetupSkill.test.ts index c2650b70..d7a420e7 100644 --- a/packages/cloud-agents/src/server/workflows/__tests__/environmentSetupSkill.test.ts +++ b/packages/cloud-agents/src/server/workflows/__tests__/environmentSetupSkill.test.ts @@ -177,16 +177,19 @@ describe('environment-setup guidance', () => { 'Prefer this single blocking await over model-driven `get_summary` poll loops or `bash` sleep', ); expect(skillContent).toContain( - 'Before or while that await runs, narrate one concise plain-language progress update', + 'After await settles without an explicit failure', ); expect(skillContent).toContain( - 'Preparing the environment can take 5 minutes or more — use the default await timeout', + 'always call `action: "get_messages"` for that `taskId` before claiming success', ); expect(skillContent).toContain( - 'If await returns `Terminal: Ready`, `Idle`, or `NeedsInput`', + 'do not infer readiness from Terminal/Ready alone', + ); + expect(skillContent).not.toContain( + 'optionally call `action: "get_messages"`', ); expect(skillContent).toContain( - 'Treat the await result as the source of truth for verification settlement.', + 'Before or while that await runs, narrate one concise plain-language progress update', ); expect(skillContent).toContain( 'When the spawned verification task reveals a fixable setup or environment-definition error, try to fix it yourself, rerun any affected local validation, recreate or update the environment with the revised YAML, launch a fresh verification task, await the new taskId, and repeat instead of stopping after the first failure.', diff --git a/packages/cloud-agents/src/server/workflows/skills/standard/environment-setup/SKILL.md b/packages/cloud-agents/src/server/workflows/skills/standard/environment-setup/SKILL.md index 28da086b..697b7883 100644 --- a/packages/cloud-agents/src/server/workflows/skills/standard/environment-setup/SKILL.md +++ b/packages/cloud-agents/src/server/workflows/skills/standard/environment-setup/SKILL.md @@ -137,10 +137,10 @@ You are an expert Roomote environment analyst. Analyze the already-checked-out r 27. Keep the returned `taskId` for internal monitoring only. Do not expose the spawned verification task link in the user-facing response. 28. Immediately after launch succeeds, call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "await"` and that `taskId`. Prefer this single blocking await over model-driven `get_summary` poll loops or `bash` sleep; await polls server-side and keeps the current setup tool call mid-turn so this parent task stays Working until the verification task settles or times out. Preparing the environment can take 5 minutes or more — use the default await timeout (or raise `timeoutMs` up to the tool maximum when needed) and do not abandon early solely because startup is slow. 29. Before or while that await runs, narrate one concise plain-language progress update so the user can see that setup is still being checked without being asked to wait. Say what is being checked (for example, `I'm confirming the environment starts cleanly`) rather than mentioning a spawned task, task status, polling, monitoring, or the await action. -30. Treat the await result as the source of truth for verification settlement. Map `Terminal: Completed` with `Ready: yes`, or terminal `Ready`/`Idle` without an error, as a successful boot when corresponding task messages do not contradict readiness. -31. If await returns `Terminal: Ready`, `Idle`, or `NeedsInput`, or if readiness remains ambiguous, optionally call `action: "get_messages"` once to confirm the verification task reported success versus a blocker or unexpected input request. Only allow true success when messages clearly report the environment looks ready (or await reported Completed with Ready: yes and no error). +30. Treat the await result as the source of truth for _settlement timing_, not for readiness content. A verification task turn can end (`Completed` / `Ready` / `Idle`) even when the agent reported a startup blocker. +31. After await settles without an explicit failure (`Failed`, `Canceled`, `TimedOut`, `NeedsInput`, or an await error), always call `action: "get_messages"` for that `taskId` before claiming success. Require a clear success assertion in the latest messages (for example that the environment looks ready / works). If messages report a blocker, request input, or are ambiguous, treat that as verification failure — do not infer readiness from Terminal/Ready alone. 32. If latest messages surface a startup or runtime blocker, request unexpected user input, or otherwise fail to give a clear success outcome, treat that as a blocker or verification failure instead of pretending the environment is verified. -33. If await returns `Terminal: Failed`, `Canceled`, or `TimedOut`, or exposes a startup or runtime error, inspect the exact status and error and decide whether the problem appears fixable within environment-setup scope, such as revising commands, services, environment variables, startup order, readiness checks, or other environment-definition details. +33. If await returns `Terminal: Failed`, `Canceled`, or `TimedOut`, or exposes a startup or runtime error, inspect the exact status and error (and get_messages when helpful) and decide whether the problem appears fixable within environment-setup scope, such as revising commands, services, environment variables, startup order, readiness checks, or other environment-definition details. 34. When the spawned verification task reveals a fixable setup or environment-definition error, try to fix it yourself, rerun any affected local validation, recreate or update the environment with the revised YAML, launch a fresh verification task, await the new taskId, and repeat instead of stopping after the first failure. 35. Treat each fix attempt as a real retry loop: revise based on the observed error, revalidate the affected setup steps, persist the updated environment again, relaunch the verification task, and await the new task rather than assuming the old failure is resolved. 36. Keep this verification-repair loop bounded. Retry at most 2 additional full environment-update-plus-verification attempts after the first spawned verification task unless the task context explicitly justifies a smaller limit. @@ -193,7 +193,7 @@ You are an expert Roomote environment analyst. Analyze the already-checked-out r When environment persistence succeeds, a lightweight Roomote verification task is launched against that environment and awaited by calling the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "await"` and the returned `taskId` before this setup task finishes, unless an explicit blocker prevents launch or await. The final response summarizes the environment name, whether it was created or updated, key validation outcomes, and any blockers - without including the raw YAML config or internal orchestration details. A successful onboarding final response ends by telling the user that the environment is ready and linking `[Create a new task](/)` as the clear next action. -If the follow-up verification await reports ready success (`Terminal: Completed` with `Ready: yes`, or settled Ready/Idle without error after optional message confirmation), the final response reports that the environment is ready to use instead of asking the user to confirm it manually or describing the verification task. +If the follow-up verification await settles without an explicit failure and `get_messages` clearly asserts the environment looks ready, the final response reports that the environment is ready to use instead of asking the user to confirm it manually or describing the verification task. Do not report readiness from Terminal alone without that message assertion. If the spawned verification task reveals a fixable setup or environment-definition error, the skill attempts to repair it, update the environment, relaunch verification, await the new task, and report the final bounded retry outcome instead of stopping after the first failed verification task. If the app exposes a browser UI and local startup succeeds, the localhost URL is verified through loopback reachability or other non-browser startup evidence before persistence continues. If required environment variables or secrets are known but unavailable, the skill requests them immediately through `request_environment_variables` in web tasks and Slack-started setup tasks, or asks the user to set them locally in the current task before proceeding with affected validation. @@ -329,10 +329,10 @@ You are an expert Roomote environment analyst. Analyze the already-checked-out r When the task explicitly identifies an existing environment to revise, update that environment instead of creating a duplicate. After successful environment persistence, use the Roomote MCP tool `mcp__roomote__manage_tasks` to launch a lightweight verification task against the created or updated environment and await it with `action: "await"` yourself instead of leaving verification as an implicit manual next step or a multi-step `get_summary` poll loop. Before launching that verification task, call the Roomote MCP tool `mcp__roomote__manage_tasks` with `action: "list_environments"` so the environment target is grounded in current Roomote data and you can copy the exact returned `environmentId`. -When the verification task launch succeeds, immediately call `mcp__roomote__manage_tasks` with `action: "await"` and the returned `taskId`. Treat that blocking await result as the source of truth for verification settlement instead of agent-driven summary polling or bash sleep loops. +When the verification task launch succeeds, immediately call `mcp__roomote__manage_tasks` with `action: "await"` and the returned `taskId`. Treat that blocking await result as the source of truth for _when_ the verification task settled instead of agent-driven summary polling or bash sleep loops. After a non-failure settle, always call `action: "get_messages"` and require a clear success assertion before reporting the environment ready. Before or while the follow-up check runs, narrate a concise progress update in plain language instead of silently waiting or pushing the waiting back onto the user. Do not mention a spawned task, task status, polling, monitoring, or await in those user-facing updates. Preparing the environment can take 5 minutes or more. Do not abandon await solely because the verification task is taking a long time to start; use the tool's default timeout or an appropriate higher `timeoutMs` within tool limits, then report timeout as failure to confirm readiness. -If await settles into `Ready`, `Idle`, or `NeedsInput`, inspect latest task messages when success is not already clear from the await payload, and only treat it as success when evidence clearly reports that the environment looks ready. +If await settles into `Ready`, `Idle`, `Completed`, or `NeedsInput`, always inspect latest task messages before success; only treat it as success when those messages clearly report that the environment looks ready. Never infer readiness from Terminal labels alone. If the spawned verification task surfaces a fixable setup or environment-definition problem, attempt to fix it yourself, recreate or update the environment, launch a fresh verification task, await the new taskId, and repeat the loop instead of stopping after the first discovered error. Bound that environment-repair loop to at most 2 additional full retries after the first spawned verification task unless the task context clearly justifies fewer attempts. Do not claim automatic repair for failures that actually require product/source changes outside environment-setup scope, unsupported infrastructure, missing external credentials, or a user decision you cannot safely infer; report those as blockers. From 51bbbc48e63df260cd71f12ed2b9dae68e75eca3 Mon Sep 17 00:00:00 2001 From: daniel-lxs Date: Tue, 14 Jul 2026 13:52:07 -0500 Subject: [PATCH 4/4] fix: keep manage_tasks await polling through transient status reads A single failed getTaskSummary poll (platform-API stall, rolling restart, or a launch/enqueue race before the run row exists) previously aborted the whole await. Absorb transient read failures and keep polling until the task settles; surface a retryable error only once reads keep failing past a bounded grace window (2m) or the overall timeout elapses. Also document why holding one tool call open for minutes is safe in the OpenCode runtime. Co-Authored-By: Claude Opus 4.8 --- .../__tests__/await-task.test.ts | 53 +++++++++++++++-- .../src/mcp/roomote-mcp-server/await-task.ts | 58 ++++++++++++++++++- 2 files changed, 105 insertions(+), 6 deletions(-) diff --git a/apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts b/apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts index 0ab041df..fee50cc9 100644 --- a/apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts +++ b/apps/worker/src/mcp/roomote-mcp-server/__tests__/await-task.test.ts @@ -352,6 +352,42 @@ describe('awaitTaskSettlement', () => { expect(result.errorSummary).toBe('Sandbox failed to boot worker process'); expect(result.status).toBe('Failed'); }); + + it('absorbs a transient poll failure and keeps waiting for settlement', async () => { + const getTaskSummary = vi + .fn() + .mockRejectedValueOnce(new Error('502 Bad Gateway')) + .mockResolvedValueOnce( + summary({ taskRunStatus: 'running', taskPhase: 'running' }), + ) + .mockResolvedValueOnce( + summary({ + completed: true, + taskRunStatus: 'completed', + taskPhase: null, + }), + ); + + const sleep = vi.fn().mockResolvedValue(undefined); + let now = 0; + const result = await awaitTaskSettlement( + { taskId: 'task-1', timeoutMs: 600_000, pollIntervalMs: 1_000 }, + config, + { + getTaskSummary, + sleep, + now: () => { + now += 1_000; + return now; + }, + }, + ); + + expect(getTaskSummary).toHaveBeenCalledTimes(3); + expect(result.terminalLabel).toBe('Completed'); + expect(result.ready).toBe(true); + expect(result.timedOut).toBe(false); + }); }); describe('handleAwaitTask', () => { @@ -377,12 +413,21 @@ describe('handleAwaitTask', () => { expect(text).toContain('Linked Environment: Demo'); }); - it('surfaces API errors', async () => { + it('surfaces API errors after the transient-failure grace window', async () => { + const getTaskSummary = vi.fn().mockRejectedValue(new Error('Not found')); + let clock = 0; const failed = await handleAwaitTask({ taskId: 'missing' }, config, { - getTaskSummary: vi.fn().mockRejectedValue(new Error('Not found')), - sleep: vi.fn(), - now: () => 0, + getTaskSummary, + sleep: vi.fn().mockResolvedValue(undefined), + now: () => { + const value = clock; + clock += 60_000; + return value; + }, }); + // The read failed on every poll, but await retried across the grace window + // instead of aborting on the first failure. + expect(getTaskSummary.mock.calls.length).toBeGreaterThan(1); expect(failed.content[0]?.text).toContain('Not found'); expect(failed.content[0]?.text).toContain('"success":false'); }); diff --git a/apps/worker/src/mcp/roomote-mcp-server/await-task.ts b/apps/worker/src/mcp/roomote-mcp-server/await-task.ts index 04064aed..5ca1c52a 100644 --- a/apps/worker/src/mcp/roomote-mcp-server/await-task.ts +++ b/apps/worker/src/mcp/roomote-mcp-server/await-task.ts @@ -14,6 +14,15 @@ const MAX_AWAIT_TASK_TIMEOUT_MS = 45 * 60_000; const DEFAULT_AWAIT_TASK_POLL_INTERVAL_MS = 5_000; const MIN_AWAIT_TASK_POLL_INTERVAL_MS = 1_000; const MAX_AWAIT_TASK_POLL_INTERVAL_MS = 60_000; +/** + * Grace window for consecutive failed status polls. A single failed read + * (platform-API stall, a rolling restart, or a launch/enqueue race before the + * run row exists) must not abort a multi-minute await, so transient failures + * are absorbed and the poll is retried. Only once reads keep failing for this + * long — well under the overall wait budget — is the failure surfaced as a + * retryable tool error instead of silently burning the full timeout. + */ +const MAX_CONSECUTIVE_POLL_ERROR_MS = 2 * 60_000; type AwaitTaskTerminalLabel = | 'Completed' @@ -214,6 +223,14 @@ function formatAwaitResult(result: AwaitTaskResult): string { return lines.join('\n'); } +/** + * Blocks the current tool call, polling the task summary until it settles or + * the timeout elapses. Holding one tool call open for minutes is deliberate and + * only safe because of two OpenCode-runtime properties: the client waits on an + * MCP tool indefinitely (no per-call request timeout), and the turn-stall + * watchdog re-arms instead of aborting while a tool part is still `running`. If + * either assumption changes, this long block needs its own keepalive. + */ export async function awaitTaskSettlement( params: { taskId: string; @@ -239,9 +256,44 @@ export async function awaitTaskSettlement( const startedAt = deps.now(); let lastSummary: TaskSummaryResponse | null = null; + let firstPollErrorAt: number | null = null; + let lastPollError: Error | null = null; while (true) { - const summary = await deps.getTaskSummary(config, taskId); + let summary: TaskSummaryResponse; + try { + summary = await deps.getTaskSummary(config, taskId); + } catch (error) { + // A single failed poll must not abort a multi-minute await. Absorb + // transient read failures and keep polling; surface only once failures + // persist past the grace window or the overall budget runs out. + lastPollError = error instanceof Error ? error : new Error(String(error)); + const erroredAt = deps.now(); + if (firstPollErrorAt === null) { + firstPollErrorAt = erroredAt; + } + + const elapsed = erroredAt - startedAt; + if (elapsed >= timeoutMs) { + break; + } + + const erroringForMs = erroredAt - firstPollErrorAt; + if (erroringForMs >= Math.min(MAX_CONSECUTIVE_POLL_ERROR_MS, timeoutMs)) { + throw new Error( + `Unable to read task ${taskId} status for ${Math.round( + erroringForMs / 1000, + )}s: ${lastPollError.message}`, + ); + } + + await deps.sleep(Math.min(pollIntervalMs, timeoutMs - elapsed)); + continue; + } + + // A successful read clears any transient-failure streak. + firstPollErrorAt = null; + lastPollError = null; lastSummary = summary; if (!isTaskActiveForAwait(summary)) { @@ -286,7 +338,9 @@ export async function awaitTaskSettlement( ready: false, timedOut: true, waitedMs: deps.now() - startedAt, - errorSummary: `Timed out after ${timeoutMs}ms waiting for task ${taskId}`, + errorSummary: lastPollError + ? `Timed out after ${timeoutMs}ms; last status read for task ${taskId} failed: ${lastPollError.message}` + : `Timed out after ${timeoutMs}ms waiting for task ${taskId}`, taskRunStatus: null, taskPhase: null, completed: false,