diff --git a/apps/desktop/src/main/ipc/input.ts b/apps/desktop/src/main/ipc/input.ts index 9b88afe..cfb2c44 100644 --- a/apps/desktop/src/main/ipc/input.ts +++ b/apps/desktop/src/main/ipc/input.ts @@ -179,11 +179,30 @@ export function registerInputHandlers(): void { return { success: true }; }); - // Cleanup on app quit - app.on('will-quit', () => { + // Cleanup on app quit. + // + // This must block. Releasing a held mouse button is asynchronous, and a + // button still down when the process exits stays down for the whole OS + // session — the host is left unable to click anything and has to reboot. + // Fire-and-forget here loses that release every time. + let quitCleanupStarted = false; + app.on('will-quit', (event) => { + if (quitCleanupStarted) return; + quitCleanupStarted = true; + event.preventDefault(); + unregisterEmergencyShortcut(); destroyRemoteCursor(); - void disposeInputInjector(); + + // Bounded, so a wedged backend cannot make the app unquittable. + const deadline = new Promise((resolve) => setTimeout(resolve, 2000)); + void Promise.race([disposeInputInjector(), deadline]) + .catch((error: unknown) => { + console.error('[IPC:Input] Cleanup on quit failed', error); + }) + .finally(() => { + app.exit(0); + }); }); console.log('[IPC:Input] Input handlers registered'); diff --git a/packages/remote-input/src/backends/nutjs.ts b/packages/remote-input/src/backends/nutjs.ts index bafabf3..64b1c0f 100644 --- a/packages/remote-input/src/backends/nutjs.ts +++ b/packages/remote-input/src/backends/nutjs.ts @@ -21,6 +21,17 @@ async function loadNut() { type NutModule = Awaited>; +/** + * How long to wait for a synthetic pointer move to be applied before acting on + * it. Roughly one frame — long enough for the window server, short enough that + * remote input still feels immediate. + */ +const POSITION_SETTLE_MS = 16; + +function settle(ms: number = POSITION_SETTLE_MS): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + let nutPromise: ReturnType | null = null; function getNut(): ReturnType { @@ -146,6 +157,17 @@ export class NutJsInputBackend implements InputBackend { const button = mapMouseButton(event.button, Button); await mouse.setPosition({ x, y }); + // Let the pointer actually arrive before pressing. + // + // autoDelayMs is 0, so without this the position change and the button + // event are issued back to back. macOS applies a synthetic move through + // the window server asynchronously, so the press can be delivered before + // the pointer has moved — the click lands wherever the pointer used to be. + // + // It also puts a real gap between press and release. Two-cursor mode + // borrows the pointer, clicks, and hands it straight back, which without + // a delay is a sub-millisecond blip that many controls simply ignore. + await settle(); switch (event.action) { case 'down': diff --git a/packages/remote-input/src/injector.test.ts b/packages/remote-input/src/injector.test.ts index 11c2f9b..4465d47 100644 --- a/packages/remote-input/src/injector.test.ts +++ b/packages/remote-input/src/injector.test.ts @@ -206,6 +206,99 @@ describe('RemoteInputInjector', () => { // a permanent drag: every click is swallowed and the machine looks frozen to // its own user. Recovering used to need a reboot, so releasing held input is // the single most important safety property of this class. +// A button left down at the OS level survives the process that pressed it. The +// host is then unable to click anything and has to reboot, so letting go can +// never depend on this injector's bookkeeping being correct. +describe('RemoteInputInjector unconditional release', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('releases at the OS level on dispose, even with nothing tracked', async () => { + const backend = fakeBackend(); + const injector = makeInjector(backend); + injector.enable(); + + await injector.dispose(); + + expect(backend.emergencyStop).toHaveBeenCalled(); + }); + + it('releases at the OS level on disable, even with nothing tracked', async () => { + const backend = fakeBackend(); + const injector = makeInjector(backend); + injector.enable(); + + injector.disable(); + // disable() is sync and finishes the release in the background. + await vi.waitFor(() => { + expect(backend.emergencyStop).toHaveBeenCalled(); + }); + }); + + // The press that escapes tracking is the one that strands the host, so the + // release must not be skipped just because heldButtons looks empty. + it('still releases when a press was never tracked', async () => { + const backend = fakeBackend(); + const injector = makeInjector(backend); + injector.enable(); + + // A press the injector does not know about, exactly as a mid-flight + // dispatch would leave it: it went straight to the backend, so + // trackHeldState never saw it and releaseAll has nothing to let go of. + await backend.inject({ type: 'mouse', action: 'down', button: 'left', x: 0.5, y: 0.5 }); + + await injector.dispose(); + + expect(backend.emergencyStop).toHaveBeenCalled(); + }); + + it('waits for an in-flight injection before releasing on dispose', async () => { + let releaseDispatch: (() => void) | undefined; + let firstCallBlocked = false; + const backend = fakeBackend({ + // Only the press hangs. The releases that follow must resolve, or the + // test deadlocks on the very cleanup it is trying to observe. + inject: vi.fn().mockImplementation(() => { + if (firstCallBlocked) return Promise.resolve(); + firstCallBlocked = true; + return new Promise((resolve) => { + releaseDispatch = resolve; + }); + }), + }); + const injector = makeInjector(backend); + injector.enable(); + + const inFlight = injector.inject({ + type: 'mouse', + action: 'down', + button: 'left', + x: 0.5, + y: 0.5, + }); + + // Let dispatch get as far as the backend, so the press is genuinely + // in-flight before dispose() is asked to clean up. + await vi.waitFor(() => { + expect(backend.inject).toHaveBeenCalled(); + }); + + const disposed = injector.dispose(); + + // Nothing may be released while the press is still being dispatched — + // that is exactly the window where trackHeldState has not run yet, and + // releasing there would leave the button down for good. + expect(backend.emergencyStop).not.toHaveBeenCalled(); + + releaseDispatch?.(); + await inFlight; + await disposed; + + expect(backend.emergencyStop).toHaveBeenCalled(); + }); +}); + describe('RemoteInputInjector held-input safety', () => { const down = (button: 'left' | 'right' | 'middle' = 'left'): InputEvent => ({ type: 'mouse', diff --git a/packages/remote-input/src/injector.ts b/packages/remote-input/src/injector.ts index 19a41f1..2b8d184 100644 --- a/packages/remote-input/src/injector.ts +++ b/packages/remote-input/src/injector.ts @@ -171,9 +171,17 @@ export class RemoteInputInjector { // Wait for any in-flight inject to finish (so trackHeldState has run), // then let go of everything. releaseAll() is deliberately not gated on // `enabled` — releasing is always safe and must never be skipped. - void (this.pendingInject ?? Promise.resolve()).then(() => - this.releaseAll('injection disabled') - ); + void (this.pendingInject ?? Promise.resolve()) + .then(() => this.releaseAll('injection disabled')) + // releaseAll only lets go of what this injector *tracked*. Follow it + // with an unconditional OS-level release so a press that ever escaped + // tracking cannot survive a control handoff — that is the difference + // between "the host takes back control" and "the host reboots". + // Releasing a button that is not pressed is a harmless no-op. + .then(() => this.backend?.emergencyStop()) + .catch((error: unknown) => { + this.logger.error('[RemoteInput] Failed to release input on disable', { error }); + }); this.logger.log('[RemoteInput] Injection disabled'); } @@ -456,9 +464,22 @@ export class RemoteInputInjector { */ async dispose(): Promise { this.enabled = false; + if (this.pendingInject) await this.pendingInject; await this.releaseAll('shutting down'); this.clearHoldWatchdog(); + // The last chance to give the host their mouse back. + // + // releaseAll is driven by tracked state, so it does nothing if a press + // ever escaped tracking — and once this process exits, a button left down + // at the OS level stays down. There is no recovery short of a reboot, so + // release unconditionally rather than trusting the bookkeeping. + try { + await this.backend?.emergencyStop(); + } catch (error) { + this.logger.error('[RemoteInput] Final input release failed', { error }); + } + try { await this.backend?.dispose?.(); } catch (error) {