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
12 changes: 12 additions & 0 deletions packages/computer/src/agent-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,11 @@ type ExtractedComputerInitArgs = Partial<
AgentBehaviorInitArgs
>;

export interface ComputerMidsceneToolsOptions {
/** Keep CLI-owned Xvfb alive until process exit so Xlib clients stay valid. */
keepXvfbAliveUntilProcessExit?: boolean;
}

function adaptComputerInitArgs(
extracted: ExtractedComputerInitArgs | undefined,
): ComputerInitArgs | undefined {
Expand Down Expand Up @@ -186,6 +191,10 @@ export class ComputerMidsceneTools extends BaseMidsceneTools<
> {
private lastInitArgsSignature?: string;

constructor(private readonly options: ComputerMidsceneToolsOptions = {}) {
super();
}

protected getCliReportSessionName() {
return 'midscene-computer';
}
Expand Down Expand Up @@ -246,6 +255,9 @@ export class ComputerMidsceneTools extends BaseMidsceneTools<
...(displayId ? { displayId } : {}),
...(headless !== undefined ? { headless } : {}),
...(keyboardTypeDelay !== undefined ? { keyboardTypeDelay } : {}),
...(this.options.keepXvfbAliveUntilProcessExit
? { keepXvfbAliveUntilProcessExit: true }
: {}),
...(extractAgentBehaviorInitArgs(opts) ?? {}),
...(reportOptions ?? {}),
};
Expand Down
1 change: 1 addition & 0 deletions packages/computer/src/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function createLocalComputerDevice(
keyboardDriver: opts?.keyboardDriver,
headless: opts?.headless,
xvfbResolution: opts?.xvfbResolution,
keepXvfbAliveUntilProcessExit: opts?.keepXvfbAliveUntilProcessExit,
});
}

Expand Down
4 changes: 3 additions & 1 deletion packages/computer/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { reportCLIError, runToolsCLI } from '@midscene/shared/cli';
import { ComputerMidsceneTools } from './agent-tools';

declare const __VERSION__: string;
const tools = new ComputerMidsceneTools();
const tools = new ComputerMidsceneTools({
keepXvfbAliveUntilProcessExit: true,
});
runToolsCLI(tools, 'midscene-computer', {
stripPrefix: 'computer_',
version: __VERSION__,
Expand Down
72 changes: 46 additions & 26 deletions packages/computer/src/device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ import {
import type { XvfbInstance } from './xvfb';
import {
checkXvfbInstalled,
createXvfbSigintCleanup,
createXvfbSignalCleanup,
needsXvfb,
scheduleXvfbStopAfterProcessExit,
startXvfb,
} from './xvfb';

Expand Down Expand Up @@ -732,6 +733,14 @@ export interface ComputerDeviceOpt extends ComputerDeviceInputOpt {
* Resolution for Xvfb virtual display (default '1920x1080x24')
*/
xvfbResolution?: string;
/**
* Keep a managed Xvfb server alive until process exit.
*
* @internal The foreground CLI uses this because libnut keeps a process-wide
* X11 connection open. Stopping Xvfb during normal CLI teardown would make
* Xlib terminate an otherwise successful command with exit code 1.
*/
keepXvfbAliveUntilProcessExit?: boolean;
}

export class ComputerDevice implements AbstractInterface {
Expand All @@ -743,7 +752,7 @@ export class ComputerDevice implements AbstractInterface {
private destroyed = false;
private xvfbInstance?: XvfbInstance;
private xvfbCleanup?: () => void;
private xvfbSigintCleanup?: () => void;
private xvfbSignalCleanup?: () => void;
private readonly inputDriver = new ComputerInputDriver({
getLibnut: () => libnut,
useAppleScript: () => this.useAppleScript,
Expand Down Expand Up @@ -971,22 +980,27 @@ export class ComputerDevice implements AbstractInterface {
this.xvfbInstance = await startXvfb({
resolution: this.options?.xvfbResolution,
});
if (this.options?.keepXvfbAliveUntilProcessExit) {
scheduleXvfbStopAfterProcessExit(this.xvfbInstance);
}
process.env.DISPLAY = this.xvfbInstance.display;
debugDevice(`Xvfb started on display ${this.xvfbInstance.display}`);

// Clean up Xvfb on process exit (stored for removal in destroy())
this.xvfbCleanup = () => {
if (this.xvfbInstance) {
this.xvfbInstance.stop();
this.xvfbInstance = undefined;
}
};
this.xvfbSigintCleanup = createXvfbSigintCleanup(() =>
this.xvfbCleanup?.(),
);
process.on('exit', this.xvfbCleanup);
process.on('SIGINT', this.xvfbSigintCleanup);
process.on('SIGTERM', this.xvfbCleanup);
if (!this.options?.keepXvfbAliveUntilProcessExit) {
// Clean up SDK-owned Xvfb during device teardown or process exit.
this.xvfbCleanup = () => {
if (this.xvfbInstance) {
this.xvfbInstance.stop();
this.xvfbInstance = undefined;
}
};
this.xvfbSignalCleanup = createXvfbSignalCleanup(() =>
this.xvfbCleanup?.(),
);
process.on('exit', this.xvfbCleanup);
process.on('SIGINT', this.xvfbSignalCleanup);
process.on('SIGTERM', this.xvfbSignalCleanup);
}
}

// Load libnut on first connect
Expand All @@ -1013,17 +1027,19 @@ Available Displays: ${displays.length > 0 ? displays.map((d) => d.name).join(',
} catch (error) {
// Clean up Xvfb on connection failure
if (this.xvfbInstance) {
this.xvfbInstance.stop();
if (!this.options?.keepXvfbAliveUntilProcessExit) {
this.xvfbInstance.stop();
}
this.xvfbInstance = undefined;
}
if (this.xvfbCleanup) {
process.removeListener('exit', this.xvfbCleanup);
process.removeListener('SIGTERM', this.xvfbCleanup);
this.xvfbCleanup = undefined;
}
if (this.xvfbSigintCleanup) {
process.removeListener('SIGINT', this.xvfbSigintCleanup);
this.xvfbSigintCleanup = undefined;
if (this.xvfbSignalCleanup) {
process.removeListener('SIGINT', this.xvfbSignalCleanup);
process.removeListener('SIGTERM', this.xvfbSignalCleanup);
this.xvfbSignalCleanup = undefined;
}
debugDevice(`Failed to connect: ${error}`);
throw new Error(`Unable to connect to computer device: ${error}`);
Expand Down Expand Up @@ -1591,18 +1607,22 @@ $g.Dispose(); $bmp.Dispose(); $ms.Dispose()
this.destroyed = true;
this.inputDriver.destroy();

const keepXvfbAliveUntilProcessExit =
this.options?.keepXvfbAliveUntilProcessExit === true;
if (this.xvfbInstance) {
this.xvfbInstance.stop();
if (!keepXvfbAliveUntilProcessExit) {
this.xvfbInstance.stop();
}
this.xvfbInstance = undefined;
}
if (this.xvfbCleanup) {
if (this.xvfbCleanup && !keepXvfbAliveUntilProcessExit) {
process.removeListener('exit', this.xvfbCleanup);
process.removeListener('SIGTERM', this.xvfbCleanup);
this.xvfbCleanup = undefined;
}
if (this.xvfbSigintCleanup) {
process.removeListener('SIGINT', this.xvfbSigintCleanup);
this.xvfbSigintCleanup = undefined;
if (this.xvfbSignalCleanup) {
process.removeListener('SIGINT', this.xvfbSignalCleanup);
process.removeListener('SIGTERM', this.xvfbSignalCleanup);
this.xvfbSignalCleanup = undefined;
}

debugDevice('Computer device destroyed');
Expand Down
61 changes: 58 additions & 3 deletions packages/computer/src/xvfb.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,66 @@ export interface XvfbInstance {
stop(): void;
}

const xvfbCleanupMonitorScript = String.raw`
const parentPid = Number(process.argv[1]);
const xvfbPid = Number(process.argv[2]);
const timer = setInterval(() => {
try {
process.kill(xvfbPid, 0);
} catch {
clearInterval(timer);
process.exit(0);
}
try {
process.kill(parentPid, 0);
return;
} catch {
// The owner is gone, so its X11 clients can no longer receive XIO errors.
}
try {
process.kill(xvfbPid, 'SIGTERM');
} catch {
// Xvfb may have already exited.
}
clearInterval(timer);
}, 100);
`;

/**
* Let a detached monitor stop Xvfb only after the owning process has exited.
*
* libnut keeps a process-wide X11 connection open and exposes no close API.
* Killing Xvfb from that same process makes Xlib call exit(1), even after a
* successful CLI command. The monitor runs outside the owner, waits until its
* X11 sockets have closed with process exit, and then stops the server.
*/
export function scheduleXvfbStopAfterProcessExit(
instance: XvfbInstance,
parentPid = process.pid,
): ChildProcess {
const xvfbPid = instance.process.pid;
if (!xvfbPid) {
throw new Error('Cannot schedule Xvfb cleanup before its process starts');
}

const monitor = spawn(
process.execPath,
['-e', xvfbCleanupMonitorScript, String(parentPid), String(xvfbPid)],
{ detached: true, stdio: 'ignore' },
);
monitor.on('error', (error) => {
debugXvfb(`Xvfb cleanup monitor failed: ${error.message}`);
});
instance.process.unref();
monitor.unref();
return monitor;
}

/**
* Keep Xvfb alive while the foreground recorder handles SIGINT and saves its
* artifact. Other SIGINT listeners do not defer cleanup.
* Keep Xvfb alive while the foreground recorder handles a termination signal
* and saves its artifact. Other signal listeners do not defer cleanup.
*/
export function createXvfbSigintCleanup(
export function createXvfbSignalCleanup(
cleanup: () => void,
source: CliInterruptSource = process,
): () => void {
Expand Down
20 changes: 20 additions & 0 deletions packages/computer/tests/unit-test/agent-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,26 @@ describe('ComputerMidsceneTools', () => {
});
});

it('keeps CLI-owned Xvfb alive until process exit when configured', async () => {
const tools = new ComputerMidsceneTools({
keepXvfbAliveUntilProcessExit: true,
});
await tools.initTools();

const takeScreenshotTool = tools
.getToolDefinitions()
.find((tool) => tool.name === 'take_screenshot');

await takeScreenshotTool?.handler({
computer: { headless: true },
});

expect(agentFromComputer).toHaveBeenCalledWith({
headless: true,
keepXvfbAliveUntilProcessExit: true,
});
});

it('passes common agent behavior args to local agent creation', async () => {
const tools = new ComputerMidsceneTools();
await tools.initTools();
Expand Down
30 changes: 29 additions & 1 deletion packages/computer/tests/unit-test/device.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { describe, expect, it } from '@rstest/core';
import { describe, expect, it, rs } from '@rstest/core';
import { ComputerDevice, checkComputerEnvironment } from '../../src';

const needsDisplay = process.platform === 'linux' && !process.env.DISPLAY;
Expand All @@ -15,6 +15,34 @@ describe('ComputerDevice', () => {
expect(device).toBeDefined();
});

it('leaves CLI-owned Xvfb for the process exit cleanup', async () => {
const device = new ComputerDevice({
keepXvfbAliveUntilProcessExit: true,
});
const stop = rs.fn();
const deviceInternals = device as unknown as {
xvfbInstance?: { stop(): void };
};
deviceInternals.xvfbInstance = { stop };

await device.destroy();

expect(stop).not.toHaveBeenCalled();
});

it('stops API-owned Xvfb during normal device teardown', async () => {
const device = new ComputerDevice({});
const stop = rs.fn();
const deviceInternals = device as unknown as {
xvfbInstance?: { stop(): void };
};
deviceInternals.xvfbInstance = { stop };

await device.destroy();

expect(stop).toHaveBeenCalledOnce();
});

it.skipIf(needsDisplay)('should list displays', async () => {
const displays = await ComputerDevice.listDisplays();
expect(Array.isArray(displays)).toBe(true);
Expand Down
Loading
Loading