From df5dbea4310a03031ce72aa7222bfb68b0480fc6 Mon Sep 17 00:00:00 2001 From: Adam Shiervani Date: Fri, 1 May 2026 14:48:25 +0200 Subject: [PATCH] fix(keyboard): keep modifiers out of auto-release (#1438) * fix(keyboard): keep modifiers out of auto-release Prevent per-key auto-release from dropping held modifiers during jitter while keeping explicit cleanup paths covered by E2E tests. * fix(keyboard): keep modifiers out of auto-release Prevent per-key auto-release from dropping held modifiers during jitter while keeping explicit cleanup paths covered by E2E tests. * chore(keyboard): trim autorelease comments Keep comments focused on keyboard behavior and remove branch-specific narrative from the tests. * fix(keyboard): reset keepalive timing on key state changes Reset session keepalive timing on every keyboard state change so stale gaps do not poison later holds under modifiers. --- cloud.go | 2 + internal/usbgadget/hid_keyboard.go | 44 +-- internal/usbgadget/usbgadget.go | 1 - ui/e2e/helpers.ts | 11 + ui/e2e/remote-agent/ra-all.spec.ts | 540 ++++++++++++++++++++++++++--- ui/src/hooks/useKeyboard.ts | 14 + ui/src/routes/devices.$id.tsx | 5 +- ui/src/test/testHooks.ts | 16 + usb.go | 5 - web.go | 2 + 10 files changed, 552 insertions(+), 88 deletions(-) diff --git a/cloud.go b/cloud.go index 9a35256c..19ac9557 100644 --- a/cloud.go +++ b/cloud.go @@ -475,6 +475,8 @@ func handleSessionRequest( } if currentSession != nil { writeJSONRPCEvent("otherSessionConnected", nil, currentSession) + gadget.CancelAllAutoReleaseTimers() + _ = rpcKeyboardReport(0, keyboardClearStateKeys) peerConn := currentSession.peerConnection go func() { time.Sleep(1 * time.Second) diff --git a/internal/usbgadget/hid_keyboard.go b/internal/usbgadget/hid_keyboard.go index 5b114be8..0d185b6e 100644 --- a/internal/usbgadget/hid_keyboard.go +++ b/internal/usbgadget/hid_keyboard.go @@ -157,10 +157,6 @@ func (u *UsbGadget) SetOnKeysDownChange(f func(state KeysDownState)) { u.onKeysDownChange = &f } -func (u *UsbGadget) SetOnKeepAliveReset(f func()) { - u.onKeepAliveReset = &f -} - // DefaultAutoReleaseDuration is the default duration for auto-release of a key. const DefaultAutoReleaseDuration = 100 * time.Millisecond @@ -188,11 +184,6 @@ func (u *UsbGadget) cancelAutoRelease(key byte) { timer.Stop() u.kbdAutoReleaseTimers[key] = nil delete(u.kbdAutoReleaseTimers, key) - - // Reset keep-alive timing when key is released - if u.onKeepAliveReset != nil { - (*u.onKeepAliveReset)() - } } } @@ -239,21 +230,13 @@ func (u *UsbGadget) performAutoRelease(key byte) { delete(u.kbdAutoReleaseTimers, key) u.kbdAutoReleaseLock.Unlock() - // Skip if already released + // Timers are only scheduled for non-modifier keys. state := u.GetKeysDownState() alreadyReleased := true - - if mask, exists := KeyCodeToMaskMap[key]; exists { - // Modifier keys are tracked in state.Modifier bitmask, not in state.Keys - if state.Modifier&mask != 0 { + for i := range state.Keys { + if state.Keys[i] == key { alreadyReleased = false - } - } else { - for i := range state.Keys { - if state.Keys[i] == key { - alreadyReleased = false - break - } + break } } @@ -598,14 +581,19 @@ func (u *UsbGadget) KeypressReport(key byte, press bool) error { if err != nil && !IsHIDTemporarilyUnavailableError(err) { u.log.Warn().Uint8("key", key).Bool("press", press).Msg("failed to report key") } - isRolledOver := state.Keys[0] == hidErrorRollOver - if isRolledOver { - u.cancelAutoRelease(key) - } else if press { - u.scheduleAutoRelease(key) - } else { - u.cancelAutoRelease(key) + isRolledOver := state.Keys[0] == hidErrorRollOver + _, isModifier := KeyCodeToMaskMap[key] + + // Modifiers are tracked separately from the key buffer and must only be + // released by explicit state clears or matching key-up reports. + if !isModifier { + switch { + case isRolledOver, !press: + u.cancelAutoRelease(key) + default: + u.scheduleAutoRelease(key) + } } return err diff --git a/internal/usbgadget/usbgadget.go b/internal/usbgadget/usbgadget.go index 1d73cdf3..5e765245 100644 --- a/internal/usbgadget/usbgadget.go +++ b/internal/usbgadget/usbgadget.go @@ -90,7 +90,6 @@ type UsbGadget struct { onKeyboardStateChange *func(state KeyboardState) onKeysDownChange *func(state KeysDownState) - onKeepAliveReset *func() log *zerolog.Logger diff --git a/ui/e2e/helpers.ts b/ui/e2e/helpers.ts index 537f2637..1e6f3872 100644 --- a/ui/e2e/helpers.ts +++ b/ui/e2e/helpers.ts @@ -80,6 +80,17 @@ export async function sendKeypress(page: Page, keyCode: number, press: boolean): ); } +/** + * Temporarily pause browser keypress keepalives while preserving held keys. + */ +export async function pauseKeepAlive(page: Page, ms: number): Promise { + await page.evaluate(durationMs => { + const hooks = window.__kvmTestHooks; + if (!hooks) throw new Error("Test hooks not available"); + hooks.pauseKeepAlive(durationMs); + }, ms); +} + export async function tapKey(page: Page, keyCode: number, holdMs = 20): Promise { await sendKeypress(page, keyCode, true); await page.waitForTimeout(holdMs); diff --git a/ui/e2e/remote-agent/ra-all.spec.ts b/ui/e2e/remote-agent/ra-all.spec.ts index beb648ab..7e696089 100644 --- a/ui/e2e/remote-agent/ra-all.spec.ts +++ b/ui/e2e/remote-agent/ra-all.spec.ts @@ -12,6 +12,8 @@ import { HID_KEY, SSH_OPTS, callJsonRpc, + getKeysDownState, + pauseKeepAlive, sendKeypress, tapKey, waitForWebRTCReady, @@ -633,31 +635,108 @@ test.describe("Remote Host Agent", () => { // KEYBOARD: MODIFIER AUTO-RELEASE // ═══════════════════════════════════════════ - test("keyboard: modifier keys auto-release after timeout", async () => { - test.setTimeout(15_000); + test("keyboard: modifiers do not participate in per-key auto-release (10s lone hold)", async () => { + test.setTimeout(60_000); const modifiers = [ - { hid: 0xe0, linux: KEY.LEFT_CTRL, label: "LeftCtrl" }, - { hid: 0xe1, linux: KEY.LEFT_SHIFT, label: "LeftShift" }, - { hid: 0xe2, linux: KEY.LEFT_ALT, label: "LeftAlt" }, + { hid: 0xe0, linux: KEY.LEFT_CTRL, label: "LeftCtrl", maskBit: 0x01 }, + { hid: 0xe1, linux: KEY.LEFT_SHIFT, label: "LeftShift", maskBit: 0x02 }, + { hid: 0xe2, linux: KEY.LEFT_ALT, label: "LeftAlt", maskBit: 0x04 }, ]; - for (const { hid, linux, label } of modifiers) { + for (const { hid, linux, label, maskBit } of modifiers) { await agent!.clearKeyboardEvents(); - // Call keypressReport directly via JSON-RPC to bypass the browser's - // keepalive timer, which would otherwise extend the auto-release indefinitely. - await callJsonRpc(sharedPage, "keypressReport", { key: hid, press: true }); + await sendKeypress(sharedPage, hid, true); + + const SAMPLES = 20; + const SAMPLE_INTERVAL = 500; + for (let i = 0; i < SAMPLES; i++) { + await new Promise(r => setTimeout(r, SAMPLE_INTERVAL)); + + const state = await getKeysDownState(sharedPage); + expect( + state?.modifier ?? 0, + `${label} bit should be set on sample ${i + 1}/${SAMPLES} (t=${(i + 1) * SAMPLE_INTERVAL}ms)`, + ).toBe(maskBit); + + const events = await agent!.getKeyboardEvents(); + const releases = events.filter(ev => ev.code === linux && ev.type === "key_release"); + expect(releases.length, `${label} must not auto-release (sample ${i + 1}/${SAMPLES})`).toBe( + 0, + ); + } + + const releaseStart = Date.now(); + await sendKeypress(sharedPage, hid, false); + await new Promise(r => setTimeout(r, 200)); + + const finalEvents = await agent!.getKeyboardEvents(); + const presses = finalEvents.filter(ev => ev.code === linux && ev.type === "key_press"); + const releases = finalEvents.filter(ev => ev.code === linux && ev.type === "key_release"); + + expect(presses.length, `${label} should have exactly 1 press`).toBe(1); + expect(releases.length, `${label} should have exactly 1 release`).toBe(1); + + const releaseLatency = releases[0].time_ms - presses[0].time_ms; + expect( + releaseLatency, + `${label} release should occur after the full 10s hold`, + ).toBeGreaterThan(SAMPLES * SAMPLE_INTERVAL - 1000); + expect(Date.now() - releaseStart).toBeLessThan(2000); + } + }); + + test("keyboard: modifier does not auto-release without browser keepalives", async () => { + await agent!.clearKeyboardEvents(); + + try { + await callJsonRpc(sharedPage, "keypressReport", { key: 0xe1, press: true }); + + await expect + .poll( + async () => { + const s = (await callJsonRpc(sharedPage, "getKeyDownState")) as { + modifier: number; + keys: number[]; + }; + return s.modifier === 0x02 && s.keys.every((k: number) => k === 0); + }, + { + message: "LeftShift should be held after direct keypressReport", + timeout: 5000, + intervals: [100, 200, 500], + }, + ) + .toBe(true); + + await expect + .poll( + async () => { + const events = await agent!.getKeyboardEvents(); + return events.some(ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_press"); + }, + { + message: "Host should see LeftShift press", + timeout: 5000, + intervals: [100, 200, 500], + }, + ) + .toBe(true); + await new Promise(r => setTimeout(r, 300)); - const events = await agent!.getKeyboardEvents(); - const presses = events.filter(ev => ev.code === linux && ev.type === "key_press"); - const releases = events.filter(ev => ev.code === linux && ev.type === "key_release"); + const state = (await callJsonRpc(sharedPage, "getKeyDownState")) as { + modifier: number; + keys: number[]; + }; + expect(state.modifier, "LeftShift should still be held without keepalives").toBe(0x02); - expect(presses.length, `${label} press should be received`).toBeGreaterThanOrEqual(1); - expect(releases.length, `${label} should auto-release after timeout`).toBeGreaterThanOrEqual( - 1, - ); + const events = await agent!.getKeyboardEvents(); + const releases = events.filter(ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_release"); + expect(releases.length, "LeftShift must not auto-release without keepalives").toBe(0); + } finally { + await callJsonRpc(sharedPage, "keypressReport", { key: 0xe1, press: false }).catch(() => {}); } }); @@ -741,6 +820,51 @@ test.describe("Remote Host Agent", () => { expect(stuckPresses.length, "No stuck keys after re-focus").toBe(0); }); + test("keepalive: window blur clears lone modifier", async () => { + await agent!.clearKeyboardEvents(); + + await sendKeypress(sharedPage, 0xe1, true); + await expect + .poll( + async () => { + const state = await getKeysDownState(sharedPage); + return state?.modifier === 0x02; + }, + { + message: "LeftShift should be held before blur", + timeout: 5000, + intervals: [100, 200, 500], + }, + ) + .toBe(true); + + await sharedPage.evaluate(() => { + const target = globalThis as typeof globalThis & { + dispatchEvent: (event: Event) => boolean; + }; + target.dispatchEvent(new Event("blur")); + }); + + await expect + .poll( + async () => { + const state = await getKeysDownState(sharedPage); + const events = await agent!.getKeyboardEvents(); + return ( + state?.modifier === 0 && + state.keys.every((k: number) => k === 0) && + events.some(ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_release") + ); + }, + { + message: "Window blur should clear LeftShift", + timeout: 5000, + intervals: [100, 200, 500], + }, + ) + .toBe(true); + }); + test("keepalive: arrow key held for 500ms is not prematurely released", async () => { // Regression: arrow keys would release intermittently during hold when browser // setInterval jitter exceeded the old tolerance window. @@ -1067,6 +1191,214 @@ test.describe("Remote Host Agent", () => { ); }); + // ═══════════════════════════════════════════ + // KEYBOARD: ISSUE #1428 REGRESSION TESTS + // ═══════════════════════════════════════════ + + test("regression #1428: modifier survives induced keepalive gap mid-chord", async () => { + await agent!.clearKeyboardEvents(); + + await sendKeypress(sharedPage, 0xe1, true); // Shift down + await new Promise(r => setTimeout(r, 80)); + + // Exceeds the per-key auto-release deadline but stays below the session + // replacement/ICE cleanup paths. + await pauseKeepAlive(sharedPage, 250); + + await new Promise(r => setTimeout(r, 50)); + await sendKeypress(sharedPage, 0x04, true); // A down + await new Promise(r => setTimeout(r, 30)); + await sendKeypress(sharedPage, 0x04, false); // A up + + await new Promise(r => setTimeout(r, 250)); + + const events = await agent!.getKeyboardEvents(); + const shiftPressesPreRelease = events.filter( + ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_press", + ); + const shiftReleasesPreRelease = events.filter( + ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_release", + ); + expect(shiftPressesPreRelease.length, "Shift pressed exactly once").toBe(1); + expect( + shiftReleasesPreRelease.length, + "Shift must NOT have been auto-released during the keepalive gap (#1428)", + ).toBe(0); + + const aPresses = events.filter(ev => ev.code === KEY.A && ev.type === "key_press"); + expect(aPresses.length, "A should have been pressed inside the gap").toBeGreaterThanOrEqual(1); + expect(aPresses[0].time_ms).toBeGreaterThan(shiftPressesPreRelease[0].time_ms); + + await sendKeypress(sharedPage, 0xe1, false); + await new Promise(r => setTimeout(r, 200)); + + const finalEvents = await agent!.getKeyboardEvents(); + const shiftReleases = finalEvents.filter( + ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_release", + ); + expect(shiftReleases.length, "Shift should have exactly 1 release (the explicit one)").toBe(1); + }); + + test("regression #1428: auto-released key under modifier does not poison next chord", async () => { + test.setTimeout(15_000); + await agent!.clearKeyboardEvents(); + + await sendKeypress(sharedPage, 0xe1, true); // Shift down + await new Promise(r => setTimeout(r, 80)); + await sendKeypress(sharedPage, 0x04, true); // A down + await new Promise(r => setTimeout(r, 80)); + await pauseKeepAlive(sharedPage, 5000); + await new Promise(r => setTimeout(r, 300)); + + await expect + .poll( + async () => { + const state = await getKeysDownState(sharedPage); + return state?.modifier === 0x02 && state.keys.every((k: number) => k === 0); + }, + { + message: "A should auto-release while LeftShift remains held", + timeout: 5000, + intervals: [100, 200, 500], + }, + ) + .toBe(true); + + await new Promise(r => setTimeout(r, 3000)); + + await agent!.clearKeyboardEvents(); + const bPressStart = Date.now(); + await sendKeypress(sharedPage, 0x05, true); // B down + + for (const t of [50, 150, 300, 450]) { + await new Promise(r => setTimeout(r, Math.max(0, t - (Date.now() - bPressStart)))); + const state = await getKeysDownState(sharedPage); + expect( + state?.modifier === 0x02 && state.keys.includes(0x05), + `B should still be held with LeftShift at t+${t}ms`, + ).toBe(true); + } + + await sendKeypress(sharedPage, 0x05, false); + await sendKeypress(sharedPage, 0x04, false); + await sendKeypress(sharedPage, 0xe1, false); + await new Promise(r => setTimeout(r, 200)); + + const events = await agent!.getKeyboardEvents(); + const bPresses = events.filter(ev => ev.code === KEY.B && ev.type === "key_press"); + const bReleases = events.filter(ev => ev.code === KEY.B && ev.type === "key_release"); + const shiftReleases = events.filter( + ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_release", + ); + + expect(bPresses.length, "B pressed exactly once").toBe(1); + expect(bReleases.length, "B should have exactly one release").toBe(1); + expect(shiftReleases.length, "LeftShift should release only explicitly").toBe(1); + + const holdDuration = bReleases[0].time_ms - bPresses[0].time_ms; + expect( + holdDuration, + "B should not auto-release at ~100ms under LeftShift", + ).toBeGreaterThanOrEqual(400); + }); + + test("regression #1428: lone-modifier hold does not poison next hold's auto-release", async () => { + test.setTimeout(15_000); + await agent!.clearKeyboardEvents(); + + await sendKeypress(sharedPage, 0xe1, true); + await new Promise(r => setTimeout(r, 1000)); + await sendKeypress(sharedPage, 0xe1, false); + + await new Promise(r => setTimeout(r, 3000)); + + await agent!.clearKeyboardEvents(); + const aPressStart = Date.now(); + await sendKeypress(sharedPage, 0x04, true); + + for (const t of [50, 150, 300, 450]) { + await new Promise(r => setTimeout(r, t - (Date.now() - aPressStart))); + const state = await getKeysDownState(sharedPage); + expect( + state?.keys?.includes(0x04) ?? false, + `'a' should still be held in keysDownState at t+${t}ms (cross-hold reset working)`, + ).toBe(true); + } + + await sendKeypress(sharedPage, 0x04, false); + await new Promise(r => setTimeout(r, 200)); + + const events = await agent!.getKeyboardEvents(); + const aPresses = events.filter(ev => ev.code === KEY.A && ev.type === "key_press"); + const aReleases = events.filter(ev => ev.code === KEY.A && ev.type === "key_release"); + expect(aPresses.length, "A pressed exactly once").toBe(1); + expect(aReleases.length, "A should have exactly one release (no premature auto-release)").toBe( + 1, + ); + + const holdDuration = aReleases[0].time_ms - aPresses[0].time_ms; + expect( + holdDuration, + "A should be held for ~500ms — premature release at ~100ms means cross-hold reset is broken", + ).toBeGreaterThanOrEqual(400); + }); + + test("regression #1428: auto-released key does not poison next hold's auto-release", async () => { + test.setTimeout(15_000); + await agent!.clearKeyboardEvents(); + + await sendKeypress(sharedPage, 0x04, true); + await new Promise(r => setTimeout(r, 80)); + await pauseKeepAlive(sharedPage, 5000); + await new Promise(r => setTimeout(r, 300)); + + await expect + .poll( + async () => { + const state = await getKeysDownState(sharedPage); + return state?.modifier === 0 && state.keys.every((k: number) => k === 0); + }, + { + message: "A should auto-release to an empty device state", + timeout: 5000, + intervals: [100, 200, 500], + }, + ) + .toBe(true); + + await new Promise(r => setTimeout(r, 3000)); + + await agent!.clearKeyboardEvents(); + const bPressStart = Date.now(); + await sendKeypress(sharedPage, 0x05, true); + + // Keepalive timing must reset when auto-release empties the keyboard state. + for (const t of [50, 150, 300, 450]) { + await new Promise(r => setTimeout(r, Math.max(0, t - (Date.now() - bPressStart)))); + const state = await getKeysDownState(sharedPage); + expect( + state?.keys?.includes(0x05) ?? false, + `B should still be held in keysDownState at t+${t}ms after auto-release reset`, + ).toBe(true); + } + + await sendKeypress(sharedPage, 0x05, false); + await sendKeypress(sharedPage, 0x04, false); + await new Promise(r => setTimeout(r, 200)); + + const events = await agent!.getKeyboardEvents(); + const bPresses = events.filter(ev => ev.code === KEY.B && ev.type === "key_press"); + const bReleases = events.filter(ev => ev.code === KEY.B && ev.type === "key_release"); + expect(bPresses.length, "B pressed exactly once").toBe(1); + expect(bReleases.length, "B should have exactly one release").toBe(1); + + const holdDuration = bReleases[0].time_ms - bPresses[0].time_ms; + expect( + holdDuration, + "B should be held for ~500ms, not auto-release at ~100ms from stale jitter state", + ).toBeGreaterThanOrEqual(400); + }); + // ═══════════════════════════════════════════ // KEYBOARD: KEYS RELEASED ON DISCONNECT // ═══════════════════════════════════════════ @@ -1074,56 +1406,81 @@ test.describe("Remote Host Agent", () => { test("keyboard: all keys released when WebRTC session disconnects", async ({ browser }) => { test.setTimeout(30_000); - // Opening a new page takes over currentSession (single-session device), - // kicking sharedPage. We'll reconnect sharedPage at the end. const freshPage = await browser.newPage(); await freshPage.goto("/", { waitUntil: "networkidle" }); await waitForWebRTCReady(freshPage); await agent!.clearKeyboardEvents(); - // Hold down a modifier (LeftShift) and a regular key (Space) without releasing await sendKeypress(freshPage, 0xe1, true); await new Promise(r => setTimeout(r, 20)); await sendKeypress(freshPage, HID_KEY.SPACE, true); - await new Promise(r => setTimeout(r, 50)); - // Verify the host received the presses before we disconnect - const preEvents = await agent!.getKeyboardEvents(); - const shiftPresses = preEvents.filter( - ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_press", - ); - expect(shiftPresses.length, "Host should see LeftShift press").toBeGreaterThanOrEqual(1); + await expect + .poll( + async () => { + const state = await getKeysDownState(freshPage); + return state?.modifier === 0x02 && state.keys.includes(HID_KEY.SPACE); + }, + { + message: "LeftShift and Space should be held before disconnect", + timeout: 5000, + intervals: [100, 200, 500], + }, + ) + .toBe(true); + + await expect + .poll( + async () => { + const events = await agent!.getKeyboardEvents(); + return ( + events.some(ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_press") && + events.some(ev => ev.code === KEY.SPACE && ev.type === "key_press") + ); + }, + { + message: "Host should see LeftShift and Space presses", + timeout: 5000, + intervals: [100, 200, 500], + }, + ) + .toBe(true); + + // Close the peer directly so browser blur/page-unload cleanup cannot satisfy the test. + await freshPage.evaluate(() => { + const peerConnection = ( + globalThis as typeof globalThis & { + __kvmTestHooks?: { _getPeerConnection?: () => { close: () => void } | null }; + } + ).__kvmTestHooks?._getPeerConnection?.(); + if (!peerConnection) throw new Error("Peer connection not available"); + peerConnection.close(); + }); + + await expect + .poll( + async () => { + const events = await agent!.getKeyboardEvents(); + return ( + events.some(ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_release") && + events.some(ev => ev.code === KEY.SPACE && ev.type === "key_release") + ); + }, + { + message: "Host should see LeftShift and Space releases after disconnect", + timeout: 5000, + intervals: [100, 200, 500], + }, + ) + .toBe(true); - // Close the page to sever the WebRTC session, triggering the all-keys-up report await freshPage.close(); - await new Promise(r => setTimeout(r, 1000)); - // Verify the host received releases for both keys - const allEvents = await agent!.getKeyboardEvents(); - const shiftReleases = allEvents.filter( - ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_release", - ); - const spaceReleases = allEvents.filter( - ev => ev.code === KEY.SPACE && ev.type === "key_release", - ); - - expect( - shiftReleases.length, - "Host should see LeftShift release after disconnect", - ).toBeGreaterThanOrEqual(1); - expect( - spaceReleases.length, - "Host should see Space release after disconnect", - ).toBeGreaterThanOrEqual(1); - - // Reconnect sharedPage so subsequent tests can use it await sharedPage.goto("/", { waitUntil: "networkidle" }); await waitForWebRTCReady(sharedPage); await waitForRpcReady(sharedPage); - // Verify device-side keys-down state is clear by querying the device - // directly via JSON-RPC (bypasses Zustand store / hidRpc timing) await expect .poll( async () => { @@ -1142,6 +1499,87 @@ test.describe("Remote Host Agent", () => { .toBe(true); }); + test("keyboard: held modifier released when WebRTC session is replaced", async ({ browser }) => { + test.setTimeout(30_000); + + const oldPage = await browser.newPage(); + let replacementPage: Page | null = null; + + try { + await oldPage.goto("/", { waitUntil: "networkidle" }); + await waitForWebRTCReady(oldPage); + await waitForRpcReady(oldPage); + + await agent!.clearKeyboardEvents(); + await callJsonRpc(oldPage, "keypressReport", { key: 0xe1, press: true }); + + await expect + .poll( + async () => { + const s = (await callJsonRpc(oldPage, "getKeyDownState")) as { + modifier: number; + keys: number[]; + }; + return s.modifier === 0x02 && s.keys.every((k: number) => k === 0); + }, + { + message: "Old session should hold LeftShift before replacement", + timeout: 5000, + intervals: [100, 200, 500], + }, + ) + .toBe(true); + + replacementPage = await browser.newPage(); + await replacementPage.goto("/", { waitUntil: "networkidle" }); + await waitForWebRTCReady(replacementPage); + await waitForRpcReady(replacementPage); + + await expect + .poll( + async () => { + const events = await agent!.getKeyboardEvents(); + return events.some(ev => ev.code === KEY.LEFT_SHIFT && ev.type === "key_release"); + }, + { + message: "Replacing the session should release LeftShift", + timeout: 5000, + intervals: [200, 500], + }, + ) + .toBe(true); + + await expect + .poll( + async () => { + const s = (await callJsonRpc(replacementPage!, "getKeyDownState")) as { + modifier: number; + keys: number[]; + }; + return s.modifier === 0 && s.keys.every((k: number) => k === 0); + }, + { + message: "Keyboard state should be clear after session replacement", + timeout: 5000, + intervals: [200, 500], + }, + ) + .toBe(true); + } finally { + if (replacementPage) { + await callJsonRpc(replacementPage, "keypressReport", { key: 0xe1, press: false }).catch( + () => {}, + ); + await replacementPage.close().catch(() => {}); + } + await oldPage.close().catch(() => {}); + + await sharedPage.goto("/", { waitUntil: "networkidle" }); + await waitForWebRTCReady(sharedPage); + await waitForRpcReady(sharedPage); + } + }); + // ═══════════════════════════════════════════ // MOUSE // ═══════════════════════════════════════════ @@ -1645,9 +2083,7 @@ test.describe("Remote Host Agent", () => { } // Verify keyboard works before test - const preEvents = await agent!.expectKeyPress(KEY.SPACE, async () => { - await tapKey(sharedPage, HID_KEY.SPACE); - }); + const preEvents = await waitForKeyboardReady(agent!, sharedPage); expect(preEvents.length, "keyboard should work before EBUSY test").toBeGreaterThan(0); // Mount ISO as CDROM diff --git a/ui/src/hooks/useKeyboard.ts b/ui/src/hooks/useKeyboard.ts index 367d162f..f9c3dae9 100644 --- a/ui/src/hooks/useKeyboard.ts +++ b/ui/src/hooks/useKeyboard.ts @@ -138,6 +138,19 @@ export default function useKeyboard() { }, KEEPALIVE_INTERVAL); }, [cancelKeepAlive]); + // Test hook: pause keepalives while preserving the held-key set. + const pauseKeepAlive = useCallback( + (ms: number) => { + cancelKeepAlive(); + window.setTimeout(() => { + if (heldKeysRef.current.size > 0) { + scheduleKeepAlive(); + } + }, ms); + }, + [cancelKeepAlive, scheduleKeepAlive], + ); + // resetKeyboardState is used to reset the keyboard state to no keys pressed and no modifiers. // This is useful for macros, in case of client-side rollover, and when the browser loses focus const resetKeyboardState = useCallback(async () => { @@ -400,5 +413,6 @@ export default function useKeyboard() { executeMacro, cleanup, cancelExecuteMacro, + pauseKeepAlive, }; } diff --git a/ui/src/routes/devices.$id.tsx b/ui/src/routes/devices.$id.tsx index d5668a14..81b710d5 100644 --- a/ui/src/routes/devices.$id.tsx +++ b/ui/src/routes/devices.$id.tsx @@ -713,7 +713,7 @@ export default function KvmIdRoute() { const { setFailsafeMode } = useFailsafeModeStore(); // Keyboard handler for E2E tests - const { handleKeyPress } = useKeyboard(); + const { handleKeyPress, pauseKeepAlive } = useKeyboard(); // Mouse handler for E2E tests const { reportAbsMouseEvent, rpcHidReady } = useHidRpc(); @@ -927,6 +927,7 @@ export default function KvmIdRoute() { useEffect(() => { registerTestHandlers({ handleKeyPress, + pauseKeepAlive, handleAbsMouseMove, getKeyboardLedState: () => useHidStore.getState().keyboardLedState, getKeysDownState: () => useHidStore.getState().keysDownState, @@ -940,7 +941,7 @@ export default function KvmIdRoute() { getPeerConnection: () => useRTCStore.getState().peerConnection, }); return cleanupTestHooks; - }, [handleKeyPress, handleAbsMouseMove]); + }, [handleKeyPress, pauseKeepAlive, handleAbsMouseMove]); const outlet = useOutlet(); const onModalClose = useCallback(() => { diff --git a/ui/src/test/testHooks.ts b/ui/src/test/testHooks.ts index 2253f69b..aaafe287 100644 --- a/ui/src/test/testHooks.ts +++ b/ui/src/test/testHooks.ts @@ -13,6 +13,7 @@ import { KeyboardLedState, KeysDownState } from "@/hooks/stores"; /** Internal handlers set by React components (prefixed with _ to indicate internal use) */ interface TestHooksInternal { _handleKeyPress?: (key: number, press: boolean) => void; + _pauseKeepAlive?: (ms: number) => void; _handleAbsMouseMove?: (x: number, y: number, buttons: number) => void; _getKeyboardLedState?: () => KeyboardLedState; _getKeysDownState?: () => KeysDownState; @@ -30,6 +31,10 @@ export interface KvmTestHooks extends TestHooksInternal { getKeyboardLedState: () => KeyboardLedState | null; getKeysDownState: () => KeysDownState | null; sendKeypress: (key: number, press: boolean) => void; + /** + * Test-only: pause keypress keepalives while preserving held keys. + */ + pauseKeepAlive: (ms: number) => void; sendAbsMouseMove: (x: number, y: number, buttons: number) => void; sendJsonRpc: ( method: string, @@ -97,6 +102,14 @@ export function initTestHooks(): void { } }, + pauseKeepAlive: (ms: number) => { + if (hooks._pauseKeepAlive) { + hooks._pauseKeepAlive(ms); + } else { + console.warn("[E2E] pauseKeepAlive called but no handler registered"); + } + }, + sendAbsMouseMove: (x: number, y: number, buttons: number) => { if (hooks._handleAbsMouseMove) { hooks._handleAbsMouseMove(x, y, buttons); @@ -321,6 +334,7 @@ export function initTestHooks(): void { */ export function registerTestHandlers(handlers: { handleKeyPress: (key: number, press: boolean) => void; + pauseKeepAlive: (ms: number) => void; handleAbsMouseMove: (x: number, y: number, buttons: number) => void; getKeyboardLedState: () => KeyboardLedState; getKeysDownState: () => KeysDownState; @@ -336,6 +350,7 @@ export function registerTestHandlers(handlers: { if (!window.__kvmTestHooks) return; window.__kvmTestHooks._handleKeyPress = handlers.handleKeyPress; + window.__kvmTestHooks._pauseKeepAlive = handlers.pauseKeepAlive; window.__kvmTestHooks._handleAbsMouseMove = handlers.handleAbsMouseMove; window.__kvmTestHooks._getKeyboardLedState = handlers.getKeyboardLedState; window.__kvmTestHooks._getKeysDownState = handlers.getKeysDownState; @@ -356,6 +371,7 @@ export function cleanupTestHooks(): void { if (!window.__kvmTestHooks) return; window.__kvmTestHooks._handleKeyPress = undefined; + window.__kvmTestHooks._pauseKeepAlive = undefined; window.__kvmTestHooks._handleAbsMouseMove = undefined; window.__kvmTestHooks._getKeyboardLedState = undefined; window.__kvmTestHooks._getKeysDownState = undefined; diff --git a/usb.go b/usb.go index 36ca9274..4f2ac245 100644 --- a/usb.go +++ b/usb.go @@ -37,11 +37,6 @@ func initUsbGadget() { gadget.SetOnKeysDownChange(func(state usbgadget.KeysDownState) { if currentSession != nil { currentSession.enqueueKeysDownState(state) - } - }) - - gadget.SetOnKeepAliveReset(func() { - if currentSession != nil { currentSession.resetKeepAliveTime() } }) diff --git a/web.go b/web.go index 4a1efe92..1e1bf796 100644 --- a/web.go +++ b/web.go @@ -259,6 +259,8 @@ func handleWebRTCSession(c *gin.Context) { } if currentSession != nil { writeJSONRPCEvent("otherSessionConnected", nil, currentSession) + gadget.CancelAllAutoReleaseTimers() + _ = rpcKeyboardReport(0, keyboardClearStateKeys) peerConn := currentSession.peerConnection go func() { time.Sleep(1 * time.Second)