Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions src/daytona/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ import {
AUTOBRIN_BUNDLED_COMPUTER_USE_SKILL,
AUTOBRIN_FLUE_DIR,
BENCHPRESS_ROOT,
COMPUTER_USE_START_POLL_INTERVAL_MS,
COMPUTER_USE_START_TIMEOUT_MS,
DEFAULT_COMPUTER_USE_BASE_URL,
} from './constants.js';
import { executeChecked, executeOptional } from './sandbox-exec.js';
Expand Down Expand Up @@ -63,6 +65,58 @@ export async function checkComputerUseScreenshot(
};
}

export type ComputerUseStartOptions = {
baseUrl?: string;
timeoutMs?: number;
pollIntervalMs?: number;
};

/**
* Starts the sandbox's computer-use process stack (Xvfb, xfce4, x11vnc, novnc) via the Daytona
* SDK and polls a real screenshot capture until it succeeds or `timeoutMs` elapses.
*
* `sandbox.computerUse.start()` resolving does not mean the desktop is screenshot-ready yet --
* these are supervised processes that take a moment to come up. Without this wait, callers race
* ahead and see the exact `computerUseScreenshotOk: false` symptom this fixes (see
* https://github.com/superagent-ai/benchpress/issues/38).
*
* Never throws: a failed start() call or a readiness timeout is logged clearly and this returns
* `false` so callers proceed without computer-use rather than aborting the whole engagement --
* some sandboxes genuinely don't support it, and most engagements only need it for optional
* visual confirmation (see `ensureComputerUseAssets` below, which treats the same signals as
* non-fatal for the same reason).
*/
export async function ensureComputerUseStarted(
sandbox: Sandbox,
options: ComputerUseStartOptions = {},
): Promise<boolean> {
const baseUrl = options.baseUrl ?? DEFAULT_COMPUTER_USE_BASE_URL;
const timeoutMs = options.timeoutMs ?? COMPUTER_USE_START_TIMEOUT_MS;
const pollIntervalMs = options.pollIntervalMs ?? COMPUTER_USE_START_POLL_INTERVAL_MS;

try {
await sandbox.computerUse.start();
} catch (error) {
console.warn(
`sandbox.computerUse.start() failed on sandbox ${sandbox.id}: ${error instanceof Error ? error.message : String(error)}. Continuing without computer-use.`,
);
return false;
}

const deadline = Date.now() + timeoutMs;
while (Date.now() < deadline) {
if ((await checkComputerUseScreenshot(sandbox, baseUrl)).ok) {
return true;
}
await new Promise((resolve) => setTimeout(resolve, pollIntervalMs));
}

console.warn(
`Computer-use did not become screenshot-ready within ${timeoutMs}ms of sandbox.computerUse.start() on sandbox ${sandbox.id} (Xvfb/xfce4/x11vnc/novnc may not be supported on this image). Continuing without confirmed computer-use readiness.`,
);
return false;
}

export async function ensureComputerUseAssets(sandbox: Sandbox): Promise<ComputerUseAssetStatus> {
const bundledSkillPath = `${AUTOBRIN_FLUE_DIR}/${AUTOBRIN_BUNDLED_COMPUTER_USE_SKILL}`;
const bundledCheck = await executeOptional(
Expand Down
8 changes: 8 additions & 0 deletions src/daytona/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ export const DEFAULT_AUTOBRIN_FLUE_MODEL = 'kimi-azure/kimi-k2.6';
export const DEFAULT_AUTOBRIN_FLUE_REPOSITORY = 'https://github.com/superagent-ai/autobrin-flue.git';
export const DEFAULT_COMPUTER_USE_BASE_URL = 'http://127.0.0.1:2280';

/**
* Sane bound on how long to wait for `sandbox.computerUse.start()` (Xvfb/xfce4/x11vnc/novnc) to
* become screenshot-ready before giving up and proceeding without confirmed readiness.
* See https://github.com/superagent-ai/benchpress/issues/38.
*/
export const COMPUTER_USE_START_TIMEOUT_MS = 60_000;
export const COMPUTER_USE_START_POLL_INTERVAL_MS = 2_000;

export const ALLOWED_FLUE_REFS = ['staging', 'main'] as const;
export type AllowedFlueRef = (typeof ALLOWED_FLUE_REFS)[number];

Expand Down
17 changes: 16 additions & 1 deletion src/daytona/launcher.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Image, Sandbox } from '@daytona/sdk';
import { ensureComputerUseAssets } from './assets.js';
import { ensureComputerUseAssets, ensureComputerUseStarted } from './assets.js';
import { bootstrapAutobrinFlue, prepareRepoTarget, prepareWebappTarget } from './bootstrap.js';
import {
applyAutoStopSafetyNet,
Expand Down Expand Up @@ -79,6 +79,16 @@ export async function runDaytonaEngagement(options: DaytonaRunOptions): Promise<
console.error(`Daytona sandbox created: ${sandbox.id}`);
await applyAutoStopSafetyNet(sandbox);

// Xvfb/xfce4/x11vnc/novnc are supervised processes that Daytona never starts on its own --
// they must be explicitly started via the SDK. Started (and awaited-ready) before bootstrap
// so the desktop stack has the most time to come up before anything tries to use it. Skipped
// entirely for engagements that already opt out via AUTOBRIN_COMPUTER_USE=none, since start()
// has real side effects (spins up processes) unlike the read-only checks in
// ensureComputerUseAssets below. See https://github.com/superagent-ai/benchpress/issues/38.
if (sandboxEnv.AUTOBRIN_COMPUTER_USE !== 'none') {
await ensureComputerUseStarted(sandbox, { baseUrl: sandboxEnv.AUTOBRIN_COMPUTER_USE_BASE_URL });
}

await bootstrapAutobrinFlue(sandbox, {
ref: sandboxEnv.AUTOBRIN_FLUE_REF,
repository: sandboxEnv.AUTOBRIN_FLUE_REPOSITORY,
Expand Down Expand Up @@ -111,6 +121,11 @@ export async function runDaytonaEngagement(options: DaytonaRunOptions): Promise<
keptSandbox,
};
} finally {
// No explicit sandbox.computerUse.stop() here: deleting the sandbox already terminates every
// supervised process inside it (Xvfb/xfce4/x11vnc/novnc included), so there is nothing left
// to release. Calling stop() first would only add another fallible round-trip to the teardown
// path for no additional safety, and would be actively wrong for --keep-sandbox, where the
// caller wants the desktop to keep running.
if (sandbox && !options.keepSandbox) {
try {
await deleteDaytonaSandbox(sandbox.id, env);
Expand Down
115 changes: 114 additions & 1 deletion tests/daytona-doctor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { AddressInfo } from 'node:net';
import type { Sandbox } from '@daytona/sdk';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { checkComputerUseScreenshot, ensureComputerUseAssets } from '../src/daytona/assets.js';
import { checkComputerUseScreenshot, ensureComputerUseAssets, ensureComputerUseStarted } from '../src/daytona/assets.js';
import { describeAutobrinFlueCloneFailure } from '../src/daytona/bootstrap.js';

const clientMocks = vi.hoisted(() => ({
Expand Down Expand Up @@ -46,6 +46,34 @@ function scriptedSandbox(
return fakeSandbox(executeCommand);
}

/**
* Sandbox stub for `ensureComputerUseStarted`: a mockable `computerUse.start()` plus a
* screenshot-check response sequence (the last entry repeats once exhausted), so tests can model
* "ready immediately", "ready after N polls", and "never ready" without real sleeps/servers.
*/
function computerUseStartSandbox(options: {
start?: () => Promise<{ message?: string }>;
screenshotResults: ExecuteCommandResult[];
id?: string;
}): Sandbox & { screenshotCallCount: () => number } {
let callCount = 0;
const start = options.start ?? (async () => ({ message: 'started' }));
const executeCommand: ExecuteCommandFn = async (command) => {
if (command.includes('computeruse/screenshot')) {
const index = Math.min(callCount, options.screenshotResults.length - 1);
callCount += 1;
return options.screenshotResults[index];
}
return { exitCode: 0, result: '' };
};
return {
id: options.id ?? 'fake-sandbox',
process: { executeCommand },
computerUse: { start },
screenshotCallCount: () => callCount,
} as unknown as Sandbox & { screenshotCallCount: () => number };
}

/** Sandbox stub that really runs the command via bash, so the actual curl/mktemp/wc script is exercised. */
function realShellSandbox(): Sandbox {
const executeCommand: ExecuteCommandFn = (command) =>
Expand Down Expand Up @@ -122,6 +150,91 @@ describe('checkComputerUseScreenshot (real curl/mktemp/wc script over loopback H
});
});

describe('ensureComputerUseStarted (superagent-ai/benchpress#38 — start() is never fire-and-forget)', () => {
let warnSpy: ReturnType<typeof vi.spyOn>;

beforeEach(() => {
warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
});

afterEach(() => {
warnSpy.mockRestore();
});

it('calls sandbox.computerUse.start() and returns true immediately when the first screenshot check already succeeds', async () => {
const start = vi.fn(async () => ({ message: 'started' }));
const sandbox = computerUseStartSandbox({
start,
screenshotResults: [{ exitCode: 0, result: '54321' }],
});

const ready = await ensureComputerUseStarted(sandbox, { timeoutMs: 200, pollIntervalMs: 10 });

expect(ready).toBe(true);
expect(start).toHaveBeenCalledTimes(1);
expect(sandbox.screenshotCallCount()).toBe(1);
expect(warnSpy).not.toHaveBeenCalled();
});

it('polls until the screenshot check succeeds, not just on the first attempt', async () => {
const sandbox = computerUseStartSandbox({
screenshotResults: [
{ exitCode: 1, result: '' },
{ exitCode: 1, result: '' },
{ exitCode: 0, result: '98765' },
],
});

const ready = await ensureComputerUseStarted(sandbox, { timeoutMs: 500, pollIntervalMs: 5 });

expect(ready).toBe(true);
expect(sandbox.screenshotCallCount()).toBeGreaterThanOrEqual(3);
expect(warnSpy).not.toHaveBeenCalled();
});

it('gives up and warns clearly after the timeout when the desktop never becomes screenshot-ready (fails clearly, does not hang)', async () => {
const sandbox = computerUseStartSandbox({
screenshotResults: [{ exitCode: 1, result: '' }],
});

const ready = await ensureComputerUseStarted(sandbox, { timeoutMs: 30, pollIntervalMs: 10 });

expect(ready).toBe(false);
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('did not become screenshot-ready'));
}, 10_000);

it('warns and returns false without ever polling when sandbox.computerUse.start() itself throws', async () => {
const start = vi.fn(async () => {
throw new Error('computer use not supported on this image');
});
const sandbox = computerUseStartSandbox({
start,
screenshotResults: [{ exitCode: 0, result: '54321' }],
});

const ready = await ensureComputerUseStarted(sandbox, { timeoutMs: 200, pollIntervalMs: 10 });

expect(ready).toBe(false);
expect(sandbox.screenshotCallCount()).toBe(0);
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('computer use not supported on this image'));
});

it('uses the provided baseUrl when checking readiness (real curl against a fixture server)', async () => {
const fixture = await startFixtureServer(FAKE_PNG_BYTES);
try {
const sandbox = { ...realShellSandbox(), computerUse: { start: async () => ({ message: 'started' }) } } as Sandbox;
const ready = await ensureComputerUseStarted(sandbox, {
baseUrl: fixture.baseUrl,
timeoutMs: 200,
pollIntervalMs: 10,
});
expect(ready).toBe(true);
} finally {
await fixture.close();
}
});
});

describe('ensureComputerUseAssets (superagent-ai/benchpress#4 — cua-driver is informational only)', () => {
let warnSpy: ReturnType<typeof vi.spyOn>;
let errorSpy: ReturnType<typeof vi.spyOn>;
Expand Down
71 changes: 71 additions & 0 deletions tests/daytona-launcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const assetsMocks = vi.hoisted(() => ({
visionHelperPresent: true,
usedFallback: false,
})),
ensureComputerUseStarted: vi.fn(async () => true),
}));
vi.mock('../src/daytona/assets.js', () => assetsMocks);

Expand Down Expand Up @@ -60,6 +61,7 @@ describe('runDaytonaEngagement afterEngagement hook', () => {
vi.spyOn(console, 'error').mockImplementation(() => undefined);
clientMocks.createSandbox.mockReset().mockResolvedValue({ id: 'sandbox-1' });
clientMocks.deleteDaytonaSandbox.mockReset().mockResolvedValue(undefined);
assetsMocks.ensureComputerUseStarted.mockReset().mockResolvedValue(true);
engagementMocks.runEngagementViaHttp.mockReset().mockResolvedValue({
exitCode: 0,
streamLogPath: '/logs/stream.jsonl',
Expand Down Expand Up @@ -150,3 +152,72 @@ describe('runDaytonaEngagement afterEngagement hook', () => {
expect(clientMocks.deleteDaytonaSandbox).not.toHaveBeenCalled();
});
});

// Regression (superagent-ai/benchpress#38): runDaytonaEngagement never called
// sandbox.computerUse.start(), so Xvfb/xfce4/x11vnc/novnc never launched and computer-use
// confirmation always failed. ensureComputerUseStarted (assets.js) owns the actual start+poll
// logic and is unit-tested in daytona-doctor.test.ts; these tests only cover runDaytonaEngagement's
// gating and sequencing contract around it.
describe('runDaytonaEngagement computer-use start gating (superagent-ai/benchpress#38)', () => {
beforeEach(() => {
vi.spyOn(console, 'error').mockImplementation(() => undefined);
clientMocks.createSandbox.mockReset().mockResolvedValue({ id: 'sandbox-1' });
clientMocks.deleteDaytonaSandbox.mockReset().mockResolvedValue(undefined);
bootstrapMocks.bootstrapAutobrinFlue.mockReset().mockResolvedValue(undefined);
assetsMocks.ensureComputerUseStarted.mockReset().mockResolvedValue(true);
engagementMocks.runEngagementViaHttp.mockReset().mockResolvedValue({
exitCode: 0,
streamLogPath: '/logs/stream.jsonl',
resultPath: '/result.json',
resultJson: { status: 'ok' },
});
});

afterEach(() => {
vi.restoreAllMocks();
});

const baseOptions = {
image: 'test-image',
payload: { modality: 'repo' as const, repo: 'owner/repo' },
env: { DAYTONA_API_KEY: 'test' },
};

it('starts computer-use (with the resolved base URL) after sandbox creation and before bootstrap, by default', async () => {
const order: string[] = [];
assetsMocks.ensureComputerUseStarted.mockImplementation(async () => {
order.push('computerUseStarted');
return true;
});
bootstrapMocks.bootstrapAutobrinFlue.mockImplementation(async () => {
order.push('bootstrap');
});

await runDaytonaEngagement(baseOptions);

expect(order).toEqual(['computerUseStarted', 'bootstrap']);
expect(assetsMocks.ensureComputerUseStarted).toHaveBeenCalledWith(
{ id: 'sandbox-1' },
{ baseUrl: 'http://127.0.0.1:2280' },
);
});

it('skips computer-use start entirely when AUTOBRIN_COMPUTER_USE=none', async () => {
await runDaytonaEngagement({
...baseOptions,
env: { ...baseOptions.env, AUTOBRIN_COMPUTER_USE: 'none' },
});

expect(assetsMocks.ensureComputerUseStarted).not.toHaveBeenCalled();
expect(bootstrapMocks.bootstrapAutobrinFlue).toHaveBeenCalled();
});

it('still completes the engagement when computer-use never becomes ready (non-fatal)', async () => {
assetsMocks.ensureComputerUseStarted.mockResolvedValue(false);

const result = await runDaytonaEngagement(baseOptions);

expect(result.engagement.exitCode).toBe(0);
expect(clientMocks.deleteDaytonaSandbox).toHaveBeenCalledWith('sandbox-1', baseOptions.env);
});
});
Loading