Skip to content

Commit 393ebee

Browse files
rookmaterookmate
committed
fix(server): streamed clicks no longer wait behind stale hovers
1 parent d14ace8 commit 393ebee

4 files changed

Lines changed: 348 additions & 3 deletions

File tree

‎apps/server/src/preview/ServerBrowser.test.ts‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -842,6 +842,133 @@ it.live(
842842
).pipe(Effect.provide(layer)),
843843
);
844844

845+
it.effect.each([
846+
{
847+
name: "hover burst before a click",
848+
inputs: [
849+
...Array.from({ length: 120 }, (_, x) => ({
850+
type: "mouse",
851+
action: "move",
852+
x,
853+
buttons: 0,
854+
button: "none",
855+
})),
856+
{ type: "mouse", action: "down", x: 119, buttons: 1, button: "left" },
857+
{ type: "mouse", action: "up", x: 119, buttons: 0, button: "left" },
858+
],
859+
expected: [
860+
{ type: "mouseMoved", x: -1 },
861+
{ type: "mouseMoved", x: 119 },
862+
{ type: "mousePressed", x: 119 },
863+
{ type: "mouseReleased", x: 119 },
864+
],
865+
},
866+
{
867+
name: "modifier changes and drag points",
868+
inputs: [
869+
{ type: "mouse", action: "move", x: 1, buttons: 0, button: "none" },
870+
{ type: "mouse", action: "move", x: 2, buttons: 0, button: "none", modifiers: 2 },
871+
{ type: "mouse", action: "move", x: 3, buttons: 0, button: "none", modifiers: 2 },
872+
{ type: "mouse", action: "down", x: 3, buttons: 1, button: "left" },
873+
{ type: "mouse", action: "move", x: 4, buttons: 1, button: "left" },
874+
{ type: "mouse", action: "move", x: 5, buttons: 1, button: "left" },
875+
{ type: "mouse", action: "up", x: 5, buttons: 0, button: "left" },
876+
],
877+
expected: [
878+
{ type: "mouseMoved", x: -1 },
879+
{ type: "mouseMoved", x: 1, modifiers: 0 },
880+
{ type: "mouseMoved", x: 3, modifiers: 2 },
881+
{ type: "mousePressed", x: 3 },
882+
{ type: "mouseMoved", x: 4 },
883+
{ type: "mouseMoved", x: 5 },
884+
{ type: "mouseReleased", x: 5 },
885+
],
886+
},
887+
{
888+
name: "wheel and keyboard boundaries",
889+
inputs: [
890+
{ type: "mouse", action: "move", x: 1, buttons: 0, button: "none" },
891+
{ type: "wheel", x: 1, deltaY: 20 },
892+
{ type: "wheel", x: 1, deltaY: 30 },
893+
{ type: "mouse", action: "move", x: 2, buttons: 0, button: "none" },
894+
{ type: "key", action: "down", key: "a", code: "KeyA", text: "a" },
895+
{ type: "mouse", action: "move", x: 3, buttons: 0, button: "none" },
896+
],
897+
expected: [
898+
{ type: "mouseMoved", x: -1 },
899+
{ type: "mouseMoved", x: 1 },
900+
{ type: "mouseWheel", deltaY: 20 },
901+
{ type: "mouseWheel", deltaY: 30 },
902+
{ type: "mouseMoved", x: 2 },
903+
{ type: "keyDown", key: "a" },
904+
{ type: "mouseMoved", x: 3 },
905+
],
906+
},
907+
{
908+
name: "text, navigation and resize boundaries",
909+
inputs: [
910+
{ type: "mouse", action: "move", x: 1, buttons: 0, button: "none" },
911+
{ type: "text", text: "typed" },
912+
{ type: "mouse", action: "move", x: 2, buttons: 0, button: "none" },
913+
{ type: "navigate", url: "http://localhost:5173/next" },
914+
{ type: "mouse", action: "move", x: 3, buttons: 0, button: "none" },
915+
{ type: "resize", width: 640, height: 480 },
916+
{ type: "mouse", action: "move", x: 4, buttons: 0, button: "none" },
917+
],
918+
expected: [
919+
{ type: "mouseMoved", x: -1 },
920+
{ type: "mouseMoved", x: 1 },
921+
{ text: "typed" },
922+
{ type: "mouseMoved", x: 2 },
923+
{ type: "mouseMoved", x: 3 },
924+
{ type: "mouseMoved", x: 4 },
925+
],
926+
},
927+
{
928+
name: "ambiguous button fields",
929+
inputs: [
930+
{ type: "mouse", action: "move", x: 1, button: "none" },
931+
{ type: "mouse", action: "move", x: 2, button: "none" },
932+
{ type: "mouse", action: "move", x: 3, buttons: 0 },
933+
{ type: "mouse", action: "move", x: 4, buttons: 0 },
934+
],
935+
expected: [-1, 1, 2, 3, 4].map((x) => ({ type: "mouseMoved", x })),
936+
},
937+
])("orders streamed input correctly for $name", (testCase) =>
938+
Effect.scoped(
939+
Effect.gen(function* () {
940+
const { browser, tabId } = yield* ready;
941+
const viewer = yield* browser.attachViewer(viewerInput(tabId, true));
942+
yield* viewer.input({ type: "takeControl" });
943+
const session = contexts[0]!.sessions.at(-1)!;
944+
const send = session.send.getMockImplementation()!;
945+
const started = Promise.withResolvers<void>();
946+
const finish = Promise.withResolvers<void>();
947+
yield* Effect.addFinalizer(() => Effect.sync(() => finish.resolve()));
948+
session.send.mockClear();
949+
session.send.mockImplementationOnce(async (method, input) => {
950+
started.resolve();
951+
await finish.promise;
952+
return send(method, input);
953+
});
954+
const running = yield* viewer
955+
.input({ type: "mouse", action: "move", x: -1, button: "none", buttons: 0 })
956+
.pipe(Effect.forkScoped({ startImmediately: true }));
957+
yield* Effect.promise(() => started.promise);
958+
const waiting = yield* Effect.forEach(testCase.inputs, (input) =>
959+
viewer.input(input).pipe(Effect.forkScoped({ startImmediately: true })),
960+
);
961+
finish.resolve();
962+
yield* Effect.forEach([running, ...waiting], Fiber.join);
963+
const dispatched = session.send.mock.calls.flatMap(([method, input]) =>
964+
method.startsWith("Input.") ? [input] : [],
965+
);
966+
expect(dispatched).toHaveLength(testCase.expected.length);
967+
expect(dispatched).toMatchObject(testCase.expected);
968+
}),
969+
).pipe(Effect.provide(layer)),
970+
);
971+
845972
it.live("only the controlling viewer's copy and cut keys reach its clipboard", () =>
846973
Effect.scoped(
847974
Effect.gen(function* () {

‎apps/server/src/preview/ServerBrowser.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2445,9 +2445,17 @@ const make = Effect.gen(function* () {
24452445
: {}),
24462446
});
24472447
} else {
2448-
await tab.control.human(viewer.id, () =>
2449-
dispatchViewerInput(tab, session, viewer, message),
2450-
);
2448+
const dispatch = () => dispatchViewerInput(tab, session, viewer, message);
2449+
if (
2450+
message.type === "mouse" &&
2451+
message.action === "move" &&
2452+
message.button === "none" &&
2453+
message.buttons === 0
2454+
) {
2455+
await tab.control.humanLatest(viewer.id, `hover:${modifiersOf(message)}`, dispatch);
2456+
} else {
2457+
await tab.control.human(viewer.id, dispatch);
2458+
}
24512459
}
24522460
} catch {
24532461
broadcastControl(tab);

‎apps/server/src/preview/SessionControl.test.ts‎

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,193 @@ import { describe, expect, it } from "vite-plus/test";
33
import { BrowserControlInterrupted, SessionControl } from "./SessionControl.ts";
44

55
describe("SessionControl", () => {
6+
it("replaces waiting hover work without changing the running action or delaying the following click", async () => {
7+
const control = new SessionControl("agent");
8+
await control.take("viewer");
9+
const started = Promise.withResolvers<void>();
10+
const finish = Promise.withResolvers<void>();
11+
const events: Array<number | string> = [];
12+
const running = control.humanLatest("viewer", "hover:0", async () => {
13+
events.push("running");
14+
started.resolve();
15+
await finish.promise;
16+
});
17+
await started.promise;
18+
const moves = Array.from({ length: 120 }, (_, position) =>
19+
control.humanLatest("viewer", "hover:0", async () => {
20+
events.push(position);
21+
}),
22+
);
23+
const click = control.human("viewer", async () => {
24+
events.push("click");
25+
});
26+
finish.resolve();
27+
await Promise.all([running, ...moves, click]);
28+
expect(events).toEqual(["running", 119, "click"]);
29+
});
30+
31+
it.each(["human", "system", "modifiers"] as const)(
32+
"does not merge waiting hover work across a %s action",
33+
async (barrier) => {
34+
const control = new SessionControl("agent");
35+
await control.take("viewer");
36+
const finish = Promise.withResolvers<void>();
37+
const events: string[] = [];
38+
const running = control.human("viewer", () => finish.promise);
39+
const before = control.humanLatest("viewer", "hover:0", async () => {
40+
events.push("before");
41+
});
42+
const recordBarrier = async () => {
43+
events.push("barrier");
44+
};
45+
const between =
46+
barrier === "modifiers"
47+
? control.humanLatest("viewer", "hover:2", recordBarrier)
48+
: barrier === "system"
49+
? control.system(recordBarrier)
50+
: control.human("viewer", recordBarrier);
51+
const after = control.humanLatest("viewer", "hover:0", async () => {
52+
events.push("after");
53+
});
54+
finish.resolve();
55+
await Promise.all([running, before, between, after]);
56+
expect(events).toEqual(["before", "barrier", "after"]);
57+
},
58+
);
59+
60+
it("does not let another viewer replace or split the owner's waiting hover", async () => {
61+
const control = new SessionControl("agent");
62+
await control.take("viewer");
63+
const finish = Promise.withResolvers<void>();
64+
const events: string[] = [];
65+
const running = control.human("viewer", () => finish.promise);
66+
const stale = control.humanLatest("viewer", "hover", async () => {
67+
events.push("stale");
68+
});
69+
await expect(
70+
control.humanLatest("other", "hover", async () => {
71+
events.push("other");
72+
}),
73+
).rejects.toThrow(BrowserControlInterrupted);
74+
const latest = control.humanLatest("viewer", "hover", async () => {
75+
events.push("latest");
76+
});
77+
finish.resolve();
78+
await Promise.all([running, stale, latest]);
79+
expect(events).toEqual(["latest"]);
80+
});
81+
82+
it.each(["release", "disconnect"] as const)(
83+
"does not revive waiting hover work after %s and retaking control",
84+
async (handoff) => {
85+
const control = new SessionControl("agent");
86+
await control.take("viewer");
87+
const started = Promise.withResolvers<void>();
88+
const finish = Promise.withResolvers<void>();
89+
const events: string[] = [];
90+
const running = control.human("viewer", () => {
91+
started.resolve();
92+
return finish.promise;
93+
});
94+
await started.promise;
95+
const stale = expect(
96+
control.humanLatest("viewer", "hover", async () => {
97+
events.push("stale");
98+
}),
99+
).rejects.toThrow(BrowserControlInterrupted);
100+
const released = control[handoff]("viewer");
101+
const retaken = control.take("viewer");
102+
const fresh = control.humanLatest("viewer", "hover", async () => {
103+
events.push("fresh");
104+
});
105+
const latest = control.humanLatest("viewer", "hover", async () => {
106+
events.push("latest");
107+
});
108+
finish.resolve();
109+
await Promise.all([running, stale, released, retaken, fresh, latest]);
110+
expect(events).toEqual(["latest"]);
111+
},
112+
);
113+
114+
it("rejects every caller of a merged hover when the tab closes", async () => {
115+
const control = new SessionControl("agent");
116+
await control.take("viewer");
117+
const started = Promise.withResolvers<void>();
118+
const finish = Promise.withResolvers<void>();
119+
const events: string[] = [];
120+
const running = control.human("viewer", () => {
121+
started.resolve();
122+
return finish.promise;
123+
});
124+
await started.promise;
125+
const waiting = [1, 2].map((position) =>
126+
expect(
127+
control.humanLatest("viewer", "hover", async () => {
128+
events.push(`hover ${position}`);
129+
}),
130+
).rejects.toThrow("closed"),
131+
);
132+
const closed = control.close();
133+
await expect(
134+
control.humanLatest("viewer", "hover", async () => {
135+
events.push("after close");
136+
}),
137+
).rejects.toThrow("closed");
138+
finish.resolve();
139+
await Promise.all([running, ...waiting, closed]);
140+
expect(events).toEqual([]);
141+
});
142+
143+
it("shares a merged dispatch failure and accepts fresh hover work afterwards", async () => {
144+
const control = new SessionControl("agent");
145+
await control.take("viewer");
146+
const finish = Promise.withResolvers<void>();
147+
const events: string[] = [];
148+
const running = control.human("viewer", () => finish.promise);
149+
const first = expect(
150+
control.humanLatest("viewer", "hover", async () => {
151+
events.push("superseded");
152+
}),
153+
).rejects.toThrow("dispatch failed");
154+
const last = expect(
155+
control.humanLatest("viewer", "hover", async () => {
156+
throw new Error("dispatch failed");
157+
}),
158+
).rejects.toThrow("dispatch failed");
159+
finish.resolve();
160+
await Promise.all([running, first, last]);
161+
await control.humanLatest("viewer", "hover", async () => {
162+
events.push("recovered");
163+
});
164+
expect(events).toEqual(["recovered"]);
165+
});
166+
167+
it("waits for tracked navigation before dispatching the latest human hover", async () => {
168+
const control = new SessionControl("agent");
169+
await control.take("viewer");
170+
const committed = Promise.withResolvers<void>();
171+
const events: string[] = [];
172+
const navigation = control.human("viewer", async () => {
173+
control.track(
174+
committed.promise.then(() => {
175+
events.push("navigation committed");
176+
}),
177+
);
178+
return "started";
179+
});
180+
const first = control.humanLatest("viewer", "hover", async () => {
181+
events.push("superseded");
182+
});
183+
const latest = control.humanLatest("viewer", "hover", async () => {
184+
events.push("latest");
185+
});
186+
await expect(navigation).resolves.toBe("started");
187+
expect(events).toEqual([]);
188+
committed.resolve();
189+
await Promise.all([first, latest]);
190+
expect(events).toEqual(["navigation committed", "latest"]);
191+
});
192+
6193
it("returns an action result before tracked navigation but drains it before already queued actions", async () => {
7194
const control = new SessionControl("agent");
8195
const committed = Promise.withResolvers<void>();

‎apps/server/src/preview/SessionControl.ts‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ export class SessionControl {
2222
readonly agentId: string | null;
2323
private readonly onGenerationChange: () => void;
2424
private tail: Promise<unknown> = Promise.resolve();
25+
private latestHuman: {
26+
entry: { viewerId: string; key: string; run: () => Promise<void> };
27+
result: Promise<void>;
28+
} | null = null;
2529
private pending: Promise<void> = Promise.resolve();
2630
private readonly handoff = new Set<Promise<void>>();
2731
private owner: string | null = null;
@@ -42,6 +46,7 @@ export class SessionControl {
4246
}
4347

4448
private enqueue<A>(run: () => Promise<A>): Promise<A> {
49+
this.latestHuman = null;
4550
const result = this.tail.then(run);
4651
this.tail = result.then(
4752
() => this.pending,
@@ -68,6 +73,7 @@ export class SessionControl {
6873
}
6974

7075
private changeOwner(owner: string | null) {
76+
this.latestHuman = null;
7177
this.owner = owner;
7278
this.epoch++;
7379
this.onGenerationChange();
@@ -131,6 +137,23 @@ export class SessionControl {
131137
return this.action(() => this.owner === viewerId, run);
132138
}
133139

140+
/** Replace only adjacent waiting work; the running action and intervening actions keep their order. */
141+
humanLatest(viewerId: string, key: string, run: () => Promise<void>): Promise<void> {
142+
if (this.closed || this.owner !== viewerId) return this.human(viewerId, run);
143+
const latest = this.latestHuman;
144+
if (latest?.entry.viewerId === viewerId && latest.entry.key === key) {
145+
latest.entry.run = run;
146+
return latest.result;
147+
}
148+
const entry = { viewerId, key, run };
149+
const result = this.human(viewerId, async () => {
150+
if (this.latestHuman?.entry === entry) this.latestHuman = null;
151+
await entry.run();
152+
});
153+
this.latestHuman = { entry, result };
154+
return result;
155+
}
156+
134157
async take(viewerId: string) {
135158
this.assertOpen();
136159
if (this.owner !== null && this.owner !== viewerId) {

0 commit comments

Comments
 (0)