Skip to content

Commit abbab35

Browse files
rookmaterookmate
committed
fix(server): streamed clicks no longer wait behind stale hovers
1 parent 09bb9f1 commit abbab35

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
@@ -766,6 +766,133 @@ it.live(
766766
).pipe(Effect.provide(layer)),
767767
);
768768

769+
it.live.each([
770+
{
771+
name: "hover burst before a click",
772+
inputs: [
773+
...Array.from({ length: 120 }, (_, x) => ({
774+
type: "mouse",
775+
action: "move",
776+
x,
777+
buttons: 0,
778+
button: "none",
779+
})),
780+
{ type: "mouse", action: "down", x: 119, buttons: 1, button: "left" },
781+
{ type: "mouse", action: "up", x: 119, buttons: 0, button: "left" },
782+
],
783+
expected: [
784+
{ type: "mouseMoved", x: -1 },
785+
{ type: "mouseMoved", x: 119 },
786+
{ type: "mousePressed", x: 119 },
787+
{ type: "mouseReleased", x: 119 },
788+
],
789+
},
790+
{
791+
name: "modifier changes and drag points",
792+
inputs: [
793+
{ type: "mouse", action: "move", x: 1, buttons: 0, button: "none" },
794+
{ type: "mouse", action: "move", x: 2, buttons: 0, button: "none", modifiers: 2 },
795+
{ type: "mouse", action: "move", x: 3, buttons: 0, button: "none", modifiers: 2 },
796+
{ type: "mouse", action: "down", x: 3, buttons: 1, button: "left" },
797+
{ type: "mouse", action: "move", x: 4, buttons: 1, button: "left" },
798+
{ type: "mouse", action: "move", x: 5, buttons: 1, button: "left" },
799+
{ type: "mouse", action: "up", x: 5, buttons: 0, button: "left" },
800+
],
801+
expected: [
802+
{ type: "mouseMoved", x: -1 },
803+
{ type: "mouseMoved", x: 1, modifiers: 0 },
804+
{ type: "mouseMoved", x: 3, modifiers: 2 },
805+
{ type: "mousePressed", x: 3 },
806+
{ type: "mouseMoved", x: 4 },
807+
{ type: "mouseMoved", x: 5 },
808+
{ type: "mouseReleased", x: 5 },
809+
],
810+
},
811+
{
812+
name: "wheel and keyboard boundaries",
813+
inputs: [
814+
{ type: "mouse", action: "move", x: 1, buttons: 0, button: "none" },
815+
{ type: "wheel", x: 1, deltaY: 20 },
816+
{ type: "wheel", x: 1, deltaY: 30 },
817+
{ type: "mouse", action: "move", x: 2, buttons: 0, button: "none" },
818+
{ type: "key", action: "down", key: "a", code: "KeyA", text: "a" },
819+
{ type: "mouse", action: "move", x: 3, buttons: 0, button: "none" },
820+
],
821+
expected: [
822+
{ type: "mouseMoved", x: -1 },
823+
{ type: "mouseMoved", x: 1 },
824+
{ type: "mouseWheel", deltaY: 20 },
825+
{ type: "mouseWheel", deltaY: 30 },
826+
{ type: "mouseMoved", x: 2 },
827+
{ type: "keyDown", key: "a" },
828+
{ type: "mouseMoved", x: 3 },
829+
],
830+
},
831+
{
832+
name: "text, navigation and resize boundaries",
833+
inputs: [
834+
{ type: "mouse", action: "move", x: 1, buttons: 0, button: "none" },
835+
{ type: "text", text: "typed" },
836+
{ type: "mouse", action: "move", x: 2, buttons: 0, button: "none" },
837+
{ type: "navigate", url: "http://localhost:5173/next" },
838+
{ type: "mouse", action: "move", x: 3, buttons: 0, button: "none" },
839+
{ type: "resize", width: 640, height: 480 },
840+
{ type: "mouse", action: "move", x: 4, buttons: 0, button: "none" },
841+
],
842+
expected: [
843+
{ type: "mouseMoved", x: -1 },
844+
{ type: "mouseMoved", x: 1 },
845+
{ text: "typed" },
846+
{ type: "mouseMoved", x: 2 },
847+
{ type: "mouseMoved", x: 3 },
848+
{ type: "mouseMoved", x: 4 },
849+
],
850+
},
851+
{
852+
name: "ambiguous button fields",
853+
inputs: [
854+
{ type: "mouse", action: "move", x: 1, button: "none" },
855+
{ type: "mouse", action: "move", x: 2, button: "none" },
856+
{ type: "mouse", action: "move", x: 3, buttons: 0 },
857+
{ type: "mouse", action: "move", x: 4, buttons: 0 },
858+
],
859+
expected: [-1, 1, 2, 3, 4].map((x) => ({ type: "mouseMoved", x })),
860+
},
861+
])("orders streamed input correctly for $name", (testCase) =>
862+
Effect.scoped(
863+
Effect.gen(function* () {
864+
const { browser, tabId } = yield* ready;
865+
const viewer = yield* browser.attachViewer(viewerInput(tabId, true));
866+
yield* viewer.input({ type: "takeControl" });
867+
const session = contexts[0]!.sessions.at(-1)!;
868+
const send = session.send.getMockImplementation()!;
869+
const started = Promise.withResolvers<void>();
870+
const finish = Promise.withResolvers<void>();
871+
yield* Effect.addFinalizer(() => Effect.sync(() => finish.resolve()));
872+
session.send.mockClear();
873+
session.send.mockImplementationOnce(async (method, input) => {
874+
started.resolve();
875+
await finish.promise;
876+
return send(method, input);
877+
});
878+
const running = yield* viewer
879+
.input({ type: "mouse", action: "move", x: -1, button: "none", buttons: 0 })
880+
.pipe(Effect.forkScoped({ startImmediately: true }));
881+
yield* Effect.promise(() => started.promise);
882+
const waiting = yield* Effect.forEach(testCase.inputs, (input) =>
883+
viewer.input(input).pipe(Effect.forkScoped({ startImmediately: true })),
884+
);
885+
finish.resolve();
886+
yield* Effect.forEach([running, ...waiting], Fiber.join);
887+
const dispatched = session.send.mock.calls.flatMap(([method, input]) =>
888+
method.startsWith("Input.") ? [input] : [],
889+
);
890+
expect(dispatched).toHaveLength(testCase.expected.length);
891+
expect(dispatched).toMatchObject(testCase.expected);
892+
}),
893+
).pipe(Effect.provide(layer)),
894+
);
895+
769896
it.live("only the controlling viewer's copy and cut keys reach its clipboard", () =>
770897
Effect.scoped(
771898
Effect.gen(function* () {

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2440,9 +2440,17 @@ const make = Effect.gen(function* () {
24402440
: {}),
24412441
});
24422442
} else {
2443-
await tab.control.human(viewer.id, () =>
2444-
dispatchViewerInput(tab, session, viewer, message),
2445-
);
2443+
const dispatch = () => dispatchViewerInput(tab, session, viewer, message);
2444+
if (
2445+
message.type === "mouse" &&
2446+
message.action === "move" &&
2447+
message.button === "none" &&
2448+
message.buttons === 0
2449+
) {
2450+
await tab.control.humanLatest(viewer.id, `hover:${modifiersOf(message)}`, dispatch);
2451+
} else {
2452+
await tab.control.human(viewer.id, dispatch);
2453+
}
24462454
}
24472455
} catch {
24482456
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)