Skip to content
Closed
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
165 changes: 165 additions & 0 deletions apps/server/src/mcp/PreviewAutomationBroker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,141 @@ it.effect("atomically registers a connected host and correlates its response", (
),
);

it.effect("starts a fresh response deadline when the host begins navigation", () =>
Effect.scoped(
Effect.gen(function* () {
const broker = yield* makeBroker;
const started = yield* Deferred.make<void>();
const releaseResponse = yield* Deferred.make<void>();
const requests = requestsFrom(yield* broker.connect(makeHost()));
yield* Stream.runForEach(requests, (request) =>
Effect.gen(function* () {
expect(request.supportsStartedResponse).toBe(true);
yield* broker.respond({
clientId: "client-1",
connectionId: request.connectionId,
requestId: request.requestId,
phase: "started",
ok: true,
});
yield* Deferred.succeed(started, undefined);
yield* Deferred.await(releaseResponse);
yield* broker.respond({
clientId: "client-1",
connectionId: request.connectionId,
requestId: request.requestId,
ok: true,
result: { loading: false },
});
}),
).pipe(Effect.forkScoped);
yield* Effect.yieldNow;

const invocation = yield* broker
.invoke<{ loading: boolean }>({
scope,
operation: "navigate",
input: { url: "https://example.com", timeoutMs: 100 },
timeoutMs: 100,
})
.pipe(Effect.forkScoped);
yield* Deferred.await(started);
yield* TestClock.adjust("100 millis");

expect(invocation.pollUnsafe()).toBeUndefined();
yield* Deferred.succeed(releaseResponse, undefined);
expect(yield* Fiber.join(invocation)).toEqual({ loading: false });
}),
).pipe(Effect.provide(TestClock.layer())),
);

it.effect("allows the host a setup budget before navigation starts", () =>
Effect.scoped(
Effect.gen(function* () {
const broker = yield* makeBroker;
const requestReceived = yield* Deferred.make<void>();
const releaseStarted = yield* Deferred.make<void>();
const requests = requestsFrom(yield* broker.connect(makeHost()));
yield* Stream.runForEach(requests, (request) =>
Effect.gen(function* () {
yield* Deferred.succeed(requestReceived, undefined);
yield* Deferred.await(releaseStarted);
yield* broker.respond({
clientId: "client-1",
connectionId: request.connectionId,
requestId: request.requestId,
phase: "started",
ok: true,
});
yield* broker.respond({
clientId: "client-1",
connectionId: request.connectionId,
requestId: request.requestId,
ok: true,
result: { loading: false },
});
}),
).pipe(Effect.forkScoped);
yield* Effect.yieldNow;

const invocation = yield* broker
.invoke<{ loading: boolean }>({
scope,
operation: "navigate",
input: { url: "https://example.com", timeoutMs: 100 },
timeoutMs: 100,
})
.pipe(Effect.forkScoped);
yield* Deferred.await(requestReceived);
yield* TestClock.adjust("101 millis");

expect(invocation.pollUnsafe()).toBeUndefined();
yield* Deferred.succeed(releaseStarted, undefined);
expect(yield* Fiber.join(invocation)).toEqual({ loading: false });
}),
).pipe(Effect.provide(TestClock.layer())),
);

it.effect("keeps response grace for hosts that do not report navigation start", () =>
Effect.scoped(
Effect.gen(function* () {
const broker = yield* makeBroker;
const requestReceived = yield* Deferred.make<void>();
const releaseResponse = yield* Deferred.make<void>();
const requests = requestsFrom(yield* broker.connect(makeHost()));
yield* Stream.runForEach(requests, (request) =>
Effect.gen(function* () {
yield* Deferred.succeed(requestReceived, undefined);
yield* Deferred.await(releaseResponse);
yield* broker.respond({
clientId: "client-1",
connectionId: request.connectionId,
requestId: request.requestId,
ok: true,
result: { loading: false },
});
}),
).pipe(Effect.forkScoped);
yield* Effect.yieldNow;

const invocation = yield* broker
.invoke<{ loading: boolean }>({
scope,
operation: "navigate",
input: { url: "https://example.com", timeoutMs: 100 },
timeoutMs: 100,
})
.pipe(Effect.forkScoped);
yield* Deferred.await(requestReceived);
yield* TestClock.adjust("1.1 seconds");

expect(invocation.pollUnsafe()).toBeUndefined();
yield* Deferred.succeed(releaseResponse, undefined);
expect(yield* Fiber.join(invocation)).toEqual({ loading: false });
}),
).pipe(Effect.provide(TestClock.layer())),
);

it.effect("targets multiple tabs explicitly while retaining a default tab", () =>
Effect.scoped(
Effect.gen(function* () {
Expand Down Expand Up @@ -508,6 +643,36 @@ it.effect("distinguishes malformed remote failures", () =>
),
);

it.effect("does not swallow a failed navigation-start response", () =>
Effect.scoped(
Effect.gen(function* () {
const broker = yield* makeBroker;
const requests = requestsFrom(yield* broker.connect(makeHost()));
yield* Stream.runForEach(requests, (request) =>
broker.respond({
clientId: "client-1",
connectionId: request.connectionId,
requestId: request.requestId,
phase: "started",
ok: false,
}),
).pipe(Effect.forkScoped);
yield* Effect.yieldNow;

const error = yield* broker
.invoke<void>({
scope,
operation: "navigate",
input: { url: "https://example.com" },
timeoutMs: 2_000,
})
.pipe(Effect.flip);

expect(error).toBeInstanceOf(PreviewAutomationMalformedResponseError);
}),
),
);

it.effect("rejects calls when no connected host exists", () =>
Effect.gen(function* () {
const broker = yield* makeBroker;
Expand Down
43 changes: 41 additions & 2 deletions apps/server/src/mcp/PreviewAutomationBroker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import type * as Cause from "effect/Cause";
import * as Crypto from "effect/Crypto";
import * as Deferred from "effect/Deferred";
import * as Effect from "effect/Effect";
import * as Exit from "effect/Exit";
import * as Layer from "effect/Layer";
import * as Option from "effect/Option";
import * as Queue from "effect/Queue";
Expand Down Expand Up @@ -80,6 +81,7 @@ interface ClientConnection {

interface PendingRequest {
readonly queue: ClientConnection["queue"];
readonly started: Deferred.Deferred<void>;
readonly deferred: Deferred.Deferred<unknown, PreviewAutomationError>;
readonly context: PreviewAutomationRequestErrorContext;
}
Expand Down Expand Up @@ -123,6 +125,14 @@ interface BrokerState {
readonly focusSequence: number;
}

const PREVIEW_AUTOMATION_RESPONSE_GRACE_MS = 1_000;

const initialResponseTimeoutMs = (
operation: PreviewAutomationOperation,
timeoutMs: number,
): number =>
operation === "navigate" ? timeoutMs * 2 + PREVIEW_AUTOMATION_RESPONSE_GRACE_MS : timeoutMs;

const removeConnectionFromState = (
current: BrokerState,
clientId: string,
Expand Down Expand Up @@ -453,11 +463,18 @@ export const make = Effect.gen(function* PreviewAutomationBrokerMake() {
) {
return [undefined, current] as const;
}
if (response.phase === "started" && response.ok) {
return [entry, current] as const;
}
const next = new Map(current.pending);
next.delete(response.requestId);
return [entry, { ...current, pending: next }] as const;
});
if (!pending) return;
if (response.phase === "started" && response.ok) {
yield* Deferred.succeed(pending.started, undefined);
return;
}
if (response.ok) {
yield* Deferred.succeed(pending.deferred, response.result);
} else {
Expand All @@ -474,6 +491,7 @@ export const make = Effect.gen(function* PreviewAutomationBrokerMake() {
input: Parameters<PreviewAutomationBroker["Service"]["invoke"]>[0],
): Effect.fn.Return<A, PreviewAutomationError> {
const timeoutMs = input.timeoutMs ?? 15_000;
const started = yield* Deferred.make<void>();
const deferred = yield* Deferred.make<unknown, PreviewAutomationError>();
const route = yield* SynchronizedRef.modify(state, (current) => {
const assignments = new Map(
Expand Down Expand Up @@ -556,7 +574,7 @@ export const make = Effect.gen(function* PreviewAutomationBrokerMake() {
...selectorDiagnostics,
};
const pending = new Map(current.pending);
pending.set(requestId, { queue: connection.queue, deferred, context });
pending.set(requestId, { queue: connection.queue, started, deferred, context });
return [
{ connection, requestId, requestContext: context, requestSequence },
{ ...current, assignments, pending, requestSequence: current.requestSequence + 1 },
Expand Down Expand Up @@ -597,6 +615,7 @@ export const make = Effect.gen(function* PreviewAutomationBrokerMake() {
threadId: input.scope.threadId,
tabId: requestContext.tabId,
tabIdExplicit: input.tabId !== undefined,
supportsStartedResponse: true,
operation: input.operation,
input: input.input,
timeoutMs,
Expand All @@ -610,7 +629,27 @@ export const make = Effect.gen(function* PreviewAutomationBrokerMake() {
}
return yield* new PreviewAutomationRequestQueueClosedError(requestContext);
}
const result = yield* Deferred.await(deferred).pipe(Effect.timeoutOption(timeoutMs));
const first = yield* Effect.raceFirst(
Deferred.await(started).pipe(Effect.as({ _tag: "Started" as const })),
Deferred.await(deferred).pipe(
Effect.exit,
Effect.map((exit) => ({ _tag: "Completed" as const, exit })),
),
).pipe(Effect.timeoutOption(initialResponseTimeoutMs(input.operation, timeoutMs)));
if (Option.isNone(first)) {
yield* disconnect(connection.clientId, connection.queue, true);
return yield* new PreviewAutomationTimeoutError(requestContext);
}
if (first.value._tag === "Completed") {
if (Exit.isFailure(first.value.exit)) {
return yield* Effect.failCause(first.value.exit.cause);
}
return first.value.exit.value as A;
}

const result = yield* Deferred.await(deferred).pipe(
Effect.timeoutOption(timeoutMs + PREVIEW_AUTOMATION_RESPONSE_GRACE_MS),
);
return yield* Option.match(result, {
onNone: () =>
Effect.gen(function* () {
Expand Down
21 changes: 20 additions & 1 deletion apps/server/src/mcp/toolkits/preview/handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ import {
parseThreadSegmentFromAttachmentId,
} from "../../../attachmentStore.ts";
import * as ServerConfig from "../../../config.ts";
import { claimPreviewRecording, normalizePreviewOpenInput } from "./handlers.ts";
import {
claimPreviewRecording,
normalizePreviewNavigateInput,
normalizePreviewOpenInput,
} from "./handlers.ts";

describe("normalizePreviewOpenInput", () => {
it("leaves an unstated visibility for the client preference to decide", () => {
Expand Down Expand Up @@ -42,6 +46,21 @@ describe("normalizePreviewOpenInput", () => {
});
});

describe("normalizePreviewNavigateInput", () => {
it("makes the default navigation budget explicit for the browser host", () => {
expect(normalizePreviewNavigateInput({ url: "https://example.com" })).toEqual({
url: "https://example.com",
timeoutMs: 15_000,
});
expect(normalizePreviewNavigateInput({ url: "https://example.com", timeoutMs: 2_500 })).toEqual(
{
url: "https://example.com",
timeoutMs: 2_500,
},
);
});
});

describe("claimPreviewRecording", () => {
it.effect("overlapping and repeated claims return the same retained recording", () =>
Effect.gen(function* () {
Expand Down
16 changes: 14 additions & 2 deletions apps/server/src/mcp/toolkits/preview/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
type ToolActivityIcon,
type ThreadId,
type PreviewAutomationOperation,
type PreviewAutomationNavigateInput,
type PreviewAutomationOpenInput,
type PreviewAutomationRecordingStatus,
type PreviewAutomationResizeResult,
Expand All @@ -31,6 +32,8 @@ import * as McpInvocationContext from "../../McpInvocationContext.ts";
import * as PreviewAutomationBroker from "../../PreviewAutomationBroker.ts";
import { PreviewSnapshotToolkit, PreviewStandardToolkit, PreviewToolkit } from "./tools.ts";

const DEFAULT_PREVIEW_NAVIGATION_TIMEOUT_MS = 15_000;

/**
* Collapses the `show` alias onto `open` and defaults tab reuse.
*
Expand All @@ -50,6 +53,13 @@ export function normalizePreviewOpenInput(
};
}

/** Makes the navigation budget explicit so the host can report when navigation actually starts. */
export function normalizePreviewNavigateInput(
input: PreviewAutomationNavigateInput,
): PreviewAutomationNavigateInput & { readonly timeoutMs: number } {
return { ...input, timeoutMs: input.timeoutMs ?? DEFAULT_PREVIEW_NAVIGATION_TIMEOUT_MS };
}

const invoke = Effect.fn("PreviewToolkit.invoke")(function* <A>(
operation: PreviewAutomationOperation,
input: unknown,
Expand Down Expand Up @@ -191,8 +201,10 @@ const handlers = {
preview_status: (input) => invokeTargeted<PreviewAutomationStatus>("status", input ?? {}),
preview_open: (input) =>
invokeTargeted<PreviewAutomationStatus>("open", normalizePreviewOpenInput(input)),
preview_navigate: (input) =>
invokeTargeted<PreviewAutomationStatus>("navigate", input, input.timeoutMs),
preview_navigate: (input) => {
const normalized = normalizePreviewNavigateInput(input);
return invokeTargeted<PreviewAutomationStatus>("navigate", normalized, normalized.timeoutMs);
},
preview_resize: (input) =>
invokeTargeted<PreviewAutomationResizeResult>("resize", input, input.timeoutMs),
preview_set_appearance: (input) =>
Expand Down
6 changes: 5 additions & 1 deletion apps/web/src/components/preview/PreviewAutomationHosts.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,10 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId })
const presentationSuppressedRuntimeTabsRef = useRef(new Map<string, Set<string>>());

const handleRequest = useCallback(
async (request: PreviewAutomationRequest): Promise<unknown> => {
async (
request: PreviewAutomationRequest,
controls: { readonly notifyStarted: () => Promise<void> },
): Promise<unknown> => {
// Session sync and tab creation consume the same budget as overlay registration.
const hostDeadlineMs = Date.now() + resolveHostWaitBudgetMs(request.timeoutMs);
const threadRef: ScopedThreadRef = {
Expand Down Expand Up @@ -579,6 +582,7 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId })
url: input.url!,
},
);
await controls.notifyStarted();
await ready.bridge.navigate(ready.runtimeTabId, resolution.resolvedUrl);
await waitForNavigationReadiness(
threadRef,
Expand Down
Loading
Loading