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
33 changes: 33 additions & 0 deletions docs/computer-use-runtime-hardening.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
# Computer Use Runtime Hardening

This follow-up addresses lifecycle gaps found during review of PR #892.

## Problems

- `clearSession()` did not create a stop tombstone when no session-state record
existed yet, so a first queued invocation could activate after cleanup.
- Read-only host actions did not acquire a session lease and could continue
after `user_stopped`.
- Later lifecycle events could overwrite `blocked_url` or `user_stopped`.

## Root Cause

The Runtime treated observation and mutation leases as the only operations that
needed lifecycle fencing. Cleanup also mutated only an already-created state
record, while terminal transitions shared the same unrestricted transition
helper as recoverable states.

## Fix

- Create the same-turn stop tombstone unconditionally during `clearSession()`.
- Require an observation lease for every host-reading or waiting action.
- Make `blocked_url` and `user_stopped` absorb later lifecycle events.

A new turn still creates a fresh Computer Use session state, preserving the
existing explicit recovery boundary.

## Verification

- `npm --workspace @maka/runtime run typecheck`
- focused Computer Use and session-state tests: 52 passed
- `git diff --check`
93 changes: 93 additions & 0 deletions packages/runtime/src/__tests__/computer-use-tools.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1292,6 +1292,99 @@ describe('buildComputerUseTools — the `maka_computer` MakaTool', () => {
assert.doesNotMatch(nextTurn.text, /user_stopped/);
});

test('clearSession fences a first invocation that is already queued', async () => {
let observeAppCalls = 0;
const backend = fakeBackend() as CuDispatchBackend & {
observeApp: NonNullable<CuDispatchBackend['observeApp']>;
};
backend.observeApp = async () => {
observeAppCalls += 1;
return observation();
};
const tools = buildComputerUseTools({ backend });
const tool = tools[0];

const pending = tool.impl({
action: 'observe',
app: 'Fixture',
} as never, ctx());
tools.clearSession('s1');

const result = await pending as { text: string };
assert.match(result.text, /user_stopped/);
assert.equal(observeAppCalls, 0);
});

test('clearSession after a non-CU turn does not block the next turn observe', async () => {
let observeAppCalls = 0;
const backend = fakeBackend() as CuDispatchBackend & {
observeApp: NonNullable<CuDispatchBackend['observeApp']>;
};
backend.observeApp = async () => {
observeAppCalls += 1;
return observation();
};
const tools = buildComputerUseTools({ backend });
const tool = tools[0];

tools.clearSession('s1');
const result = await tool.impl(
{ action: 'observe', app: 'Fixture' } as never,
ctx(undefined, { turnId: 'next-turn', toolCallId: 'observe-next' }),
) as { text: string };

assert.doesNotMatch(result.text, /user_stopped/);
assert.equal(observeAppCalls, 1);
});

test('clearSession fences host-reading results that complete after stop', async () => {
for (const input of [
{ action: 'list_apps' },
{ action: 'screenshot', app: 'Fixture' },
{ action: 'cursor_position' },
{ action: 'wait', duration: 0.001 },
] as const) {
let release!: () => void;
let started!: () => void;
const gate = new Promise<void>((resolve) => { release = resolve; });
const entered = new Promise<void>((resolve) => { started = resolve; });
const backend = fakeBackend() as CuDispatchBackend & {
listApps: NonNullable<CuDispatchBackend['listApps']>;
observeApp: NonNullable<CuDispatchBackend['observeApp']>;
};
backend.listApps = async () => {
started();
await gate;
return [];
};
backend.observeApp = async () => {
started();
await gate;
return observation();
};
backend.run = async (action) => {
started();
await gate;
return action.type === 'cursor_position'
? {
outcome: { ok: true, tier: 'coordinate-background' },
resolvedScreenPoint: { x: 10, y: 20 },
}
: { outcome: { ok: true, tier: 'coordinate-background' } };
};
const tools = buildComputerUseTools({ backend });
const tool = tools[0];
const pending = tool.impl(input as never, ctx());
await entered;
tools.clearSession('s1');
release();

const result = await pending as { text: string; screenshot?: unknown };
assert.match(result.text, /user_stopped/, input.action);
assert.equal(result.screenshot, undefined, input.action);
}
});

test('S17: surfaces the typed backend failure code without leaking raw driver text', async () => {
const backend = fakeBackend({ result: { outcome: { ok: false, error: 'capture_failed', message: 'AXPress err -25202', completedSubSteps: 0 } } });
const r = await callComputer(backend, { action: 'wait' });
Expand Down
24 changes: 24 additions & 0 deletions packages/runtime/src/__tests__/cua-session-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,30 @@ describe('CuaSessionState', () => {
});
});

test('terminal states absorb later lifecycle events', () => {
const blocked = new CuaSessionState('blocked');
blocked.blockedUrlDetected();
blocked.screenLocked();
blocked.reobserveRequired();
blocked.physicalUserIntervened();
blocked.userStopped();
assert.deepEqual(blocked.snapshot(), {
status: 'blocked_url',
generation: 1,
});

const stopped = new CuaSessionState('stopped');
stopped.userStopped();
stopped.blockedUrlDetected();
stopped.screenLocked();
stopped.reobserveRequired();
stopped.physicalUserIntervened();
assert.deepEqual(stopped.snapshot(), {
status: 'user_stopped',
generation: 1,
});
});

test('dynamic content changes neither synthesize intervention nor fence a lease', () => {
const state = new CuaSessionState('session-1');
state.freshObservationSucceeded();
Expand Down
64 changes: 60 additions & 4 deletions packages/runtime/src/computer-use-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ export function buildComputerUseTools(deps: {
const presentationWaiters = new Map<string, Set<() => void>>();
const presentationQueueWaiters = new Map<string, Set<() => void>>();
const presentationGenerations = new Map<string, number>();
const pendingInvocationTurns = new Map<string, Set<string>>();
let presentationQueue = Promise.resolve();
interface SessionObservationRecord {
turnId: string;
Expand Down Expand Up @@ -611,6 +612,16 @@ export function buildComputerUseTools(deps: {
return next;
}

function trackPendingInvocation(sessionId: string, turnId: string): () => void {
const turns = pendingInvocationTurns.get(sessionId) ?? new Set<string>();
turns.add(turnId);
pendingInvocationTurns.set(sessionId, turns);
return () => {
turns.delete(turnId);
if (turns.size === 0) pendingInvocationTurns.delete(sessionId);
};
}

function invalidateObservation(sessionId: string): void {
const record = observations.get(sessionId);
if (!record) return;
Expand Down Expand Up @@ -1079,9 +1090,18 @@ export function buildComputerUseTools(deps: {
if (abortSignal.aborted) return { text: 'computer aborted before start' };
const input = snapshotComputerParams(computerParams.parse(args));
const invocationGeneration = presentationGenerations.get(sessionId) ?? 0;
return withInvocationQueue(sessionId, abortSignal, async () => {
const releasePendingInvocation = trackPendingInvocation(sessionId, turnId);
try {
return await withInvocationQueue(sessionId, abortSignal, async () => {
const state = sessionState(sessionId, turnId);
const observationLease = input.action === 'observe'
const requiresObservationLease = (
input.action === 'observe'
|| input.action === 'screenshot'
|| input.action === 'list_apps'
|| input.action === 'cursor_position'
|| input.action === 'wait'
);
const observationLease = requiresObservationLease
? state.beforeObservation()
: undefined;
if (observationLease && !observationLease.ok) {
Expand Down Expand Up @@ -1125,6 +1145,15 @@ export function buildComputerUseTools(deps: {
return { text: 'maka_computer.list_apps failed: unsupported_action' };
}
const apps = await deps.backend.listApps(abortSignal);
if (
!observationLease?.ok
|| !state.validateObservationLease(observationLease.lease).ok
) {
const blocked = state.beforeAction();
return sessionFailure(
blocked.ok ? 'reobserve_required' : blocked.reason,
);
}
return {
text: JSON.stringify({
app_count: apps.length,
Expand Down Expand Up @@ -1208,6 +1237,15 @@ export function buildComputerUseTools(deps: {
windowId: input.window_id,
includeScreenshot: true,
}, abortSignal, runCtx);
if (
!observationLease?.ok
|| !state.validateObservationLease(observationLease.lease).ok
) {
const blocked = state.beforeAction();
return sessionFailure(
blocked.ok ? 'reobserve_required' : blocked.reason,
);
}
if (!screenshotObservation.screenshot) {
return { text: 'maka_computer.screenshot failed: capture_failed' };
}
Expand Down Expand Up @@ -1443,6 +1481,15 @@ export function buildComputerUseTools(deps: {
if (presentation.blocked) return presentation.blocked;
result = presentation.result;
if (result) applyTypedOutcomeState(state, result.outcome);
if (observationLease?.ok) {
const validated = state.validateObservationLease(
observationLease.lease,
);
if (!validated.ok) {
presentation.finish();
return sessionFailure(validated.reason);
}
}
if (actionLease) {
const leaseFailure = validateActionLease(state, actionLease);
if (leaseFailure) {
Expand Down Expand Up @@ -1511,7 +1558,10 @@ export function buildComputerUseTools(deps: {
}
: { text, modelText };
}
});
});
} finally {
releasePendingInvocation();
}
},
// Map the raw result into model-visible content: the summary as text, plus the
// screenshot as a native image block when present. `image-data` becomes the
Expand Down Expand Up @@ -1546,7 +1596,13 @@ export function buildComputerUseTools(deps: {
);
for (const wake of presentationQueueWaiters.get(sessionId) ?? []) wake();
for (const wake of presentationWaiters.get(sessionId) ?? []) wake();
sessionStates.get(sessionId)?.state.userStopped();
const current = sessionStates.get(sessionId);
if (current) {
current.state.userStopped();
} else {
const pendingTurn = pendingInvocationTurns.get(sessionId)?.values().next().value;
if (pendingTurn) sessionState(sessionId, pendingTurn).userStopped();
}
invalidateObservation(sessionId);
observations.delete(sessionId);
deps.backend.clearSession?.(sessionId);
Expand Down
9 changes: 9 additions & 0 deletions packages/runtime/src/cua-session-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export class CuaSessionState {
}

physicalUserIntervened(): CuaSessionSnapshot {
if (this.isTerminal()) return this.snapshot();
return this.transition('intervention_debounce');
}

Expand All @@ -92,10 +93,12 @@ export class CuaSessionState {
}

reobserveRequired(): CuaSessionSnapshot {
if (this.isTerminal()) return this.snapshot();
return this.transition('reobserve_required');
}

screenLocked(): CuaSessionSnapshot {
if (this.isTerminal()) return this.snapshot();
return this.transition('screen_locked');
}

Expand All @@ -106,10 +109,12 @@ export class CuaSessionState {
}

blockedUrlDetected(): CuaSessionSnapshot {
if (this.isTerminal()) return this.snapshot();
return this.transition('blocked_url');
}

userStopped(): CuaSessionSnapshot {
if (this.isTerminal()) return this.snapshot();
return this.transition('user_stopped');
}

Expand All @@ -128,6 +133,10 @@ export class CuaSessionState {
|| this.status === 'reobserve_required';
}

private isTerminal(): boolean {
return this.status === 'blocked_url' || this.status === 'user_stopped';
}

private transition(status: CuaSessionStatus): CuaSessionSnapshot {
this.generation += 1;
this.status = status;
Expand Down
Loading