From 29a00072f2101d813ad32f807c1cff059eb445ca Mon Sep 17 00:00:00 2001 From: Mr P-Tech Date: Mon, 10 Aug 2026 16:18:45 +0100 Subject: [PATCH 1/2] fix(control): bound a hold whose release was lost MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Taking a host's machine away from them needed only a dropped "up". `armHoldWatchdog` runs at the end of `trackHeldState`, which runs for every event — including mouse moves. So the 5s idle timer was reset by movement. If a button's "up" was lost while the guest kept moving the mouse, every move re-armed the timer and it never fired, and the button stayed held forever. A held button is not passive. `dispatch` reads `heldButtons.size > 0` as "dragging", and a drag is the one case where remote movement is injected instead of being tracked virtually. So the stuck hold turned every remote move into a real pointer move: the guest's mouse drove the host's, the host could not move their own cursor, and their clicks did nothing because the button was already down and everything had become a drag. Revoking control was the only escape, which is precisely what `disable()` -> `releaseAll()` does — and why it recovered without a reboot. Fixing it by not resetting the idle timer on movement would trade one bug for another: a legitimate drag longer than the timeout would be torn apart mid-drag. Both properties are needed at once, so there are now two timers. The idle timer keeps its reset-on-every-event behaviour, so an active drag is never cut short. A second, absolute timer starts once when the first button or key goes down, is never reset, and releases everything after 30s. It is the only thing that can bound a hold whose release was lost, and no real drag reaches it. Also surfaces heldButtons/heldKeys in getDiagnostics. A stuck hold is otherwise invisible: it presents as "my mouse is possessed", with nothing anywhere reporting that the injector still thinks a button is down. --- apps/desktop/src/main/ipc/input.test.ts | 6 ++ packages/remote-input/src/injector.test.ts | 119 +++++++++++++++++++++ packages/remote-input/src/injector.ts | 65 ++++++++++- packages/remote-input/src/types.ts | 10 ++ 4 files changed, 196 insertions(+), 4 deletions(-) diff --git a/apps/desktop/src/main/ipc/input.test.ts b/apps/desktop/src/main/ipc/input.test.ts index e7db45ea..6044a6ef 100644 --- a/apps/desktop/src/main/ipc/input.test.ts +++ b/apps/desktop/src/main/ipc/input.test.ts @@ -13,6 +13,8 @@ vi.mock('../input/injector', () => ({ backend: 'nut-js', backendSupported: true, stats: { received: 0, injected: 0, rejected: 0, errors: 0 }, + heldButtons: 0, + heldKeys: 0, }), updateScreenSize: vi.fn(), emergencyStop: vi.fn().mockResolvedValue(undefined), @@ -99,6 +101,8 @@ describe('IPC Input Handlers', () => { backend: 'nut-js', backendSupported: true, stats: { received: 0, injected: 0, rejected: 0, errors: 0 }, + heldButtons: 0, + heldKeys: 0, }); const handler = mockIpcMainHandlers.get('input:enable')!; @@ -132,6 +136,8 @@ describe('IPC Input Handlers', () => { backend: 'nut-js', backendSupported: true, stats: { received: 1, injected: 1, rejected: 0, errors: 0 }, + heldButtons: 0, + heldKeys: 0, }); const handler = mockIpcMainHandlers.get('input:status')!; diff --git a/packages/remote-input/src/injector.test.ts b/packages/remote-input/src/injector.test.ts index 4465d476..08d4ca03 100644 --- a/packages/remote-input/src/injector.test.ts +++ b/packages/remote-input/src/injector.test.ts @@ -419,6 +419,125 @@ describe('RemoteInputInjector held-input safety', () => { vi.useRealTimers(); } }); + + // The failure that took a host's machine away from them. + // + // A lost "up" leaves the button held. The guest carries on moving the mouse, + // every move resets the idle watchdog, and it never fires. A held button + // makes dispatch treat movement as a drag and inject it, so the guest's + // pointer starts driving the host's and the host cannot use their own + // machine until control is revoked. The absolute timer is the only thing + // that ends this, so it must not be resettable by incoming movement. + it('releases a hold whose "up" was lost even while moves keep arriving', async () => { + vi.useFakeTimers(); + try { + const backend = fakeBackend(); + const injector = new RemoteInputInjector({ + selection: { kind: 'nut-js', platform: 'linux', displayServer: 'x11' }, + createBackend: () => backend, + logger: silentLogger, + virtualCursor: false, + holdTimeoutMs: 1000, + maxHoldMs: 5000, + }); + injector.enable(); + + await injector.inject(down()); + + // Movement never stops, so the idle timer is reset over and over and + // would on its own keep the button held indefinitely. + for (let i = 0; i < 20; i += 1) { + await vi.advanceTimersByTimeAsync(400); + await injector.inject({ type: 'mouse', action: 'move', x: 0.5, y: 0.5 }); + } + + const ups = vi + .mocked(backend.inject) + .mock.calls.filter(([e]) => 'action' in e && e.action === 'up'); + expect(ups.length).toBeGreaterThan(0); + expect(injector.getDiagnostics().heldButtons).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + // The absolute timer must not turn into a guillotine for real drags either: + // it starts once per hold and is not restarted by each new press. + it('does not restart the absolute timer on every press', async () => { + vi.useFakeTimers(); + try { + const backend = fakeBackend(); + const injector = new RemoteInputInjector({ + selection: { kind: 'nut-js', platform: 'linux', displayServer: 'x11' }, + createBackend: () => backend, + logger: silentLogger, + virtualCursor: false, + holdTimeoutMs: 1000, + maxHoldMs: 5000, + }); + injector.enable(); + + await injector.inject(down()); + for (let i = 0; i < 10; i += 1) { + await vi.advanceTimersByTimeAsync(400); + await injector.inject(down()); + } + + // 4s of presses is inside both windows, so nothing has been released. + expect(injector.getDiagnostics().heldButtons).toBe(1); + + await vi.advanceTimersByTimeAsync(2000); + expect(injector.getDiagnostics().heldButtons).toBe(0); + } finally { + vi.useRealTimers(); + } + }); + + // A clean press/release must leave nothing armed, or the next hold inherits + // a timer that is already partway through its window. + it('clears the absolute timer when a button is released normally', async () => { + vi.useFakeTimers(); + try { + const backend = fakeBackend(); + const injector = new RemoteInputInjector({ + selection: { kind: 'nut-js', platform: 'linux', displayServer: 'x11' }, + createBackend: () => backend, + logger: silentLogger, + virtualCursor: false, + holdTimeoutMs: 1000, + maxHoldMs: 5000, + }); + injector.enable(); + + await injector.inject(down()); + await vi.advanceTimersByTimeAsync(4000); + await injector.inject({ + type: 'mouse', + action: 'up', + button: 'left', + x: 0.5, + y: 0.5, + }); + vi.mocked(backend.inject).mockClear(); + + // A fresh hold, kept alive so only the absolute timer is under test. + // It gets a full 5s window; had it inherited the previous hold's timer + // there would be 1s left and the button would be released during this. + await injector.inject(down()); + for (let i = 0; i < 10; i += 1) { + await vi.advanceTimersByTimeAsync(400); + await injector.inject({ type: 'mouse', action: 'move', x: 0.5, y: 0.5 }); + } + + const ups = vi + .mocked(backend.inject) + .mock.calls.filter(([e]) => 'action' in e && e.action === 'up'); + expect(ups).toHaveLength(0); + expect(injector.getDiagnostics().heldButtons).toBe(1); + } finally { + vi.useRealTimers(); + } + }); }); // Remote mouse movement must never spend the local pointer — that is what diff --git a/packages/remote-input/src/injector.ts b/packages/remote-input/src/injector.ts index 73f461c2..12f5416a 100644 --- a/packages/remote-input/src/injector.ts +++ b/packages/remote-input/src/injector.ts @@ -33,8 +33,22 @@ export interface RemoteInputInjectorOptions { /** * How long a button or key may stay held with no further input before it * is force-released. Guards against a viewer dropping mid-drag. Default 5s. + * + * Reset by every event, so an active drag is never cut short. */ holdTimeoutMs?: number; + /** + * How long a button or key may stay held in total, however much input keeps + * arriving. Default 30s. + * + * `holdTimeoutMs` alone cannot bound a hold whose release was lost: the + * guest carries on moving the mouse, every move resets the idle timer, and + * the button stays down forever. A held button makes `dispatch` treat all + * movement as a drag and inject it, so the guest ends up driving the host's + * pointer and the host cannot use their own machine. This is the backstop, + * and it is deliberately never reset. + */ + maxHoldMs?: number; /** * Keep the local and remote cursors independent (default true). * @@ -77,7 +91,9 @@ export class RemoteInputInjector { private readonly heldButtons = new Set(); private readonly heldKeys = new Set(); private holdWatchdog: ReturnType | null = null; + private maxHoldWatchdog: ReturnType | null = null; private readonly holdTimeoutMs: number; + private readonly maxHoldMs: number; // Two-cursor mode: remote movement never moves the local pointer, so both // people keep a usable cursor at the same time. @@ -99,6 +115,7 @@ export class RemoteInputInjector { this.makeBackend = options.createBackend ?? createInputBackend; this.rateLimiter = new InputRateLimiter(options.maxEventsPerSecond ?? 1000); this.holdTimeoutMs = options.holdTimeoutMs ?? 5000; + this.maxHoldMs = options.maxHoldMs ?? 30_000; this.virtualCursor = options.virtualCursor ?? true; this.edgeMarginPx = Math.max(0, options.edgeMarginPx ?? 0); this.onRejected = options.onRejected; @@ -198,7 +215,10 @@ export class RemoteInputInjector { const keys = [...this.heldKeys]; this.heldButtons.clear(); this.heldKeys.clear(); + // Both timers: the hold is over, and a stale absolute timer would fire + // partway through the next one. this.clearHoldWatchdog(); + this.clearMaxHoldWatchdog(); if (buttons.length === 0 && keys.length === 0) return; @@ -359,19 +379,53 @@ export class RemoteInputInjector { } /** - * While something is held, arm a timer to release it if no further input - * arrives. A viewer whose connection drops mid-drag would otherwise leave - * the button down indefinitely. + * Arm both hold watchdogs. Two are needed, and one alone is a trap. + * + * The idle timer catches a viewer who disappears mid-drag: no further input + * of any kind arrives, so nothing would ever release the button. It is reset + * by every event, because a drag that is still receiving movement is alive + * and must not be torn apart at an arbitrary deadline. + * + * That reset is exactly what makes the idle timer insufficient on its own. + * If a button's "up" is lost while the guest keeps moving the mouse, every + * move re-arms the idle timer and it never fires. The button stays held, so + * `dispatch` treats all subsequent movement as a drag and injects it — the + * guest's pointer starts driving the host's, and the host cannot use their + * own machine until control is revoked. + * + * So the absolute timer runs from the moment the first button or key went + * down and is never reset. It is the only thing that bounds a hold whose + * release was lost, and it is generous enough that no real drag reaches it. */ private armHoldWatchdog(): void { this.clearHoldWatchdog(); - if (this.heldButtons.size === 0 && this.heldKeys.size === 0) return; + + if (this.heldButtons.size === 0 && this.heldKeys.size === 0) { + this.clearMaxHoldWatchdog(); + return; + } this.holdWatchdog = setTimeout(() => { void this.releaseAll('no input while a button or key was held'); }, this.holdTimeoutMs); // Never keep a Node process alive just for this timer. this.holdWatchdog.unref(); + + // Started once per hold, then left alone: re-arming it here would + // reintroduce the very stall this timer exists to break. + if (this.maxHoldWatchdog === null) { + this.maxHoldWatchdog = setTimeout(() => { + void this.releaseAll('held past the maximum hold duration'); + }, this.maxHoldMs); + this.maxHoldWatchdog.unref(); + } + } + + private clearMaxHoldWatchdog(): void { + if (this.maxHoldWatchdog !== null) { + clearTimeout(this.maxHoldWatchdog); + this.maxHoldWatchdog = null; + } } private trackHeldState(event: InputEvent): void { @@ -483,6 +537,7 @@ export class RemoteInputInjector { if (this.pendingInject) await this.pendingInject; await this.releaseAll('shutting down'); this.clearHoldWatchdog(); + this.clearMaxHoldWatchdog(); // The last chance to give the host their mouse back. // @@ -510,6 +565,8 @@ export class RemoteInputInjector { backend: backend.name, backendSupported: backend.supported, stats: { ...this.stats }, + heldButtons: this.heldButtons.size, + heldKeys: this.heldKeys.size, }; if (backend.reason !== undefined) diagnostics.reason = backend.reason; diff --git a/packages/remote-input/src/types.ts b/packages/remote-input/src/types.ts index d77081b5..ae9d1a92 100644 --- a/packages/remote-input/src/types.ts +++ b/packages/remote-input/src/types.ts @@ -131,4 +131,14 @@ export interface InputDiagnostics { reason?: string; details?: Record; stats: InputStats; + /** + * How many buttons and keys the injector believes are held right now. + * + * Worth surfacing because a stuck hold is not a quiet failure: a held button + * makes every remote move inject as a drag, so the guest's pointer drives + * the host's and the host loses their machine. Non-zero here while nobody is + * pressing anything is that state, and is otherwise invisible. + */ + heldButtons: number; + heldKeys: number; } From 4ee67517db3c9bad7fcaddde5e2964ea64ee1dde Mon Sep 17 00:00:00 2001 From: Mr P-Tech Date: Mon, 10 Aug 2026 16:33:21 +0100 Subject: [PATCH 2/2] fix(linux): unbundle dbus-next, and skip the desktop overlay on Wayland MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both diagnosed from documentation and build output rather than inference. **dbus-next could never load in a packaged build.** Its `require('x11')` sits inside `getDbusAddressFromWindowSelection`, a function `connection.js` never calls — it only imports `getDbusAddressFromFs`. Lazy, and dead. But rollup's commonjs plugin hoisted it to module scope, line 11 of the chunk, beside the node builtins: const path$1 = require("path"); const require$$1$2 = require("os"); require("x11"); <-- hoisted out of a function that never runs `x11` is a dependency of nothing here — not of dbus-next (it is not even in its optionalDependencies), not of remote-input, and it ships nowhere. So loading the chunk threw `Cannot find module 'x11'`, KWin cursor reporting was never available in any packaged build on any Linux host, `getCursorPosition()` always returned null, and `restoreLocalPointer()` was permanently a no-op. dbus-next was only bundled because remote-input is excluded from externalization and drags its dependencies in with it. Declaring dbus-next in the desktop app makes externalizeDepsPlugin leave it alone and electron-builder ship it, so the lazy require stays lazy. The chunk is gone and the require with it; the module is now a genuine runtime `import("dbus-next")`. **The desktop cursor overlay is unsupported on Wayland.** Electron documents `showInactive()` — the call that shows this window *without* taking focus — as "Not supported on Wayland (Linux)", along with `setPosition()`, and notes that positioning, moving, focusing and blurring generally are not possible there without user input. The `level` argument to `setAlwaysOnTop` is documented macOS/Windows only. This window is fullscreen and always-on-top over the host's real desktop, and the file's own header already warned that one which failed to be inert "would lock the user out of their own desktop". Building it out of operations the platform documents as unsupported is not a risk worth carrying, so it is not created on Wayland. The in-app cursor still draws inside the PairUX window, so the guest's pointer stays visible over the video; only the desktop-wide overlay is given up. --- apps/desktop/package.json | 1 + .../src/main/overlay/cursorOverlay.test.ts | 26 ++++++++++++++ .../desktop/src/main/overlay/cursorOverlay.ts | 34 ++++++++++++++++++- pnpm-lock.yaml | 33 ++++++------------ 4 files changed, 71 insertions(+), 23 deletions(-) create mode 100644 apps/desktop/src/main/overlay/cursorOverlay.test.ts diff --git a/apps/desktop/package.json b/apps/desktop/package.json index f8c06f82..625a863d 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -31,6 +31,7 @@ "@radix-ui/react-label": "^2.1.8", "@radix-ui/react-slot": "^1.2.4", "class-variance-authority": "^0.7.1", + "dbus-next": "^0.10.2", "dotenv": "^17.2.3", "linkify-react": "^4.3.2", "linkifyjs": "^4.3.2", diff --git a/apps/desktop/src/main/overlay/cursorOverlay.test.ts b/apps/desktop/src/main/overlay/cursorOverlay.test.ts new file mode 100644 index 00000000..bc8112bc --- /dev/null +++ b/apps/desktop/src/main/overlay/cursorOverlay.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest'; +import { canShowDesktopOverlay } from './cursorOverlay'; + +// The overlay is a fullscreen, always-on-top window over the host's real +// desktop. If it fails to be inert it takes input the host cannot get back, +// which is the most damaging failure this app has. So the question is not +// "does click-through usually work" but "can we show it without taking focus +// at all" — and on Wayland Electron documents showInactive() as unsupported. +describe('canShowDesktopOverlay', () => { + it('refuses Wayland, where showInactive is unsupported', () => { + expect(canShowDesktopOverlay('wayland')).toBe(false); + }); + + it('allows the display servers whose window APIs Electron supports', () => { + expect(canShowDesktopOverlay('x11')).toBe(true); + expect(canShowDesktopOverlay('macos')).toBe(true); + expect(canShowDesktopOverlay('windows')).toBe(true); + }); + + // An unknown display server is only reported on Linux when neither + // WAYLAND_DISPLAY nor DISPLAY is set — a headless or unusual session rather + // than a compositor known to mishandle the window. + it('allows an unknown display server', () => { + expect(canShowDesktopOverlay('unknown')).toBe(true); + }); +}); diff --git a/apps/desktop/src/main/overlay/cursorOverlay.ts b/apps/desktop/src/main/overlay/cursorOverlay.ts index a18bb18d..d9fccbef 100644 --- a/apps/desktop/src/main/overlay/cursorOverlay.ts +++ b/apps/desktop/src/main/overlay/cursorOverlay.ts @@ -13,12 +13,44 @@ */ import { BrowserWindow, screen } from 'electron'; +import { detectDisplayServer } from '../platform'; let overlay: BrowserWindow | null = null; +/** + * Whether a desktop-wide overlay can be shown safely on this display server. + * + * False on Wayland, because Electron documents the operations this window is + * built out of as unsupported there: + * + * - `showInactive()` — "Not supported on Wayland (Linux)". This is the one + * that matters. It is how the overlay appears *without* taking focus; with + * it unavailable there is no way to put a fullscreen always-on-top window + * on screen and be sure it has not grabbed the user's input. + * - `setPosition()` — "Not supported on Wayland (Linux)", and `getBounds()` + * reports `{ x: 0, y: 0 }`, so the window cannot be reliably placed. + * - the `level` argument to `setAlwaysOnTop` is documented macOS/Windows only. + * + * More generally: "On Wayland (Linux) it is generally not possible to + * programmatically resize windows after creation, or to position, move, focus, + * or blur windows without user input." + * + * The failure mode if we show it anyway is the worst one this app has — a + * fullscreen window over the host's desktop that takes input they cannot get + * back. The in-app cursor still draws inside the PairUX window, so the guest's + * pointer stays visible where the video is; only the desktop-wide overlay is + * given up. + * + * https://www.electronjs.org/docs/latest/api/browser-window + */ +export function canShowDesktopOverlay(displayServer: string): boolean { + return displayServer !== 'wayland'; +} + /** Escape hatch, in case a compositor mishandles a click-through window. */ function isDisabled(): boolean { - return process.env.PAIRUX_DISABLE_CURSOR_OVERLAY === '1'; + if (process.env.PAIRUX_DISABLE_CURSOR_OVERLAY === '1') return true; + return !canShowDesktopOverlay(detectDisplayServer()); } function buildHtml(): string { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ee590906..5c77bc38 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -98,6 +98,9 @@ importers: class-variance-authority: specifier: ^0.7.1 version: 0.7.1 + dbus-next: + specifier: ^0.10.2 + version: 0.10.2 dotenv: specifier: ^17.2.3 version: 17.2.3 @@ -11552,8 +11555,7 @@ snapshots: '@nodelib/fs.scandir': 2.1.5 fastq: 1.20.1 - '@nornagon/put@0.0.8': - optional: true + '@nornagon/put@0.0.8': {} '@npmcli/agent@3.0.0': dependencies: @@ -13752,7 +13754,6 @@ snapshots: xml2js: 0.4.23 optionalDependencies: usocket: 0.3.0 - optional: true debug@2.6.9: dependencies: @@ -13909,8 +13910,7 @@ snapshots: es-errors: 1.3.0 gopd: 1.2.0 - duplexer@0.1.2: - optional: true + duplexer@0.1.2: {} eastasianwidth@0.2.0: {} @@ -14333,7 +14333,6 @@ snapshots: split: 0.3.3 stream-combiner: 0.0.4 through: 2.3.8 - optional: true event-target-shim@5.0.1: {} @@ -14770,8 +14769,7 @@ snapshots: fresh@0.5.2: {} - from@0.1.7: - optional: true + from@0.1.7: {} fs-constants@1.0.0: {} @@ -15060,8 +15058,7 @@ snapshots: dependencies: hermes-estree: 0.25.1 - hexy@0.2.11: - optional: true + hexy@0.2.11: {} hoist-non-react-statics@3.3.2: dependencies: @@ -15587,8 +15584,7 @@ snapshots: dependencies: argparse: 2.0.1 - jsbi@2.0.5: - optional: true + jsbi@2.0.5: {} jsbn@0.1.1: optional: true @@ -15927,8 +15923,7 @@ snapshots: loglevel@1.9.2: {} - long@4.0.0: - optional: true + long@4.0.0: {} loose-envify@1.4.0: dependencies: @@ -16019,8 +16014,7 @@ snapshots: map-obj@5.0.0: {} - map-stream@0.1.0: - optional: true + map-stream@0.1.0: {} marked@18.0.5: {} @@ -16770,7 +16764,6 @@ snapshots: pause-stream@0.0.11: dependencies: through: 2.3.8 - optional: true pe-library@0.4.1: {} @@ -17782,7 +17775,6 @@ snapshots: split@0.3.3: dependencies: through: 2.3.8 - optional: true sprintf-js@1.0.3: {} @@ -17844,7 +17836,6 @@ snapshots: stream-combiner@0.0.4: dependencies: duplexer: 0.1.2 - optional: true strict-uri-encode@2.0.0: {} @@ -18131,8 +18122,7 @@ snapshots: throat@5.0.0: {} - through@2.3.8: - optional: true + through@2.3.8: {} timm@1.7.1: {} @@ -18888,7 +18878,6 @@ snapshots: dependencies: sax: 1.6.0 xmlbuilder: 11.0.1 - optional: true xml2js@0.5.0: dependencies: