Fix/remote supervision hardening

src/remote-device/desktop-commander-integration.ts

@@ -90,18 +90,24 @@ export class DesktopCommanderIntegration {
                 }
             );
 
-            // Connect to Desktop Commander
-            console.debug('[DEBUG] Connecting MCP client to transport');
-            await this.mcpClient.connect(this.mcpTransport);
-            this.isReady = true;
-
             // Supervise the local half. Without these, a child crash is silent:
             // the SDK clears its transport and every subsequent call throws
             // "Not connected" with nothing tying it back to the death.
+            // Attached BEFORE connect(): the SDK chains handlers that already
+            // exist on the transport, so its own close handling — which rejects
+            // in-flight requests with "Connection closed" immediately — keeps
+            // running. Assigned after connect() they would REPLACE the SDK's
+            // handler and an in-flight call at child death would hang until
+            // the 60s request timeout instead of failing fast.
             this.mcpTransport.onclose = () => this.handleLocalDisconnect('stdio transport closed');
             this.mcpTransport.onerror = (err: Error) =>
                 this.handleLocalDisconnect(`stdio transport error: ${err?.message ?? String(err)}`);
 
+            // Connect to Desktop Commander
+            console.debug('[DEBUG] Connecting MCP client to transport');
+            await this.mcpClient.connect(this.mcpTransport);
+            this.isReady = true;
+
             console.log(' - 🔌 Connected to Desktop Commander MCP');
             console.debug('[DEBUG] Desktop Commander MCP connection successful');
 
@@ -203,10 +209,18 @@ export class DesktopCommanderIntegration {
         // restarts `desktop-commander remote`.
         await this.ensureReady();
 
+        // The child can die between ensureReady() resolving and the call below
+        // (handleLocalDisconnect nulls the client); surface that as a clear
+        // error instead of a TypeError on null.
+        const client = this.mcpClient;
+        if (!client) {
+            throw new Error('Local Desktop Commander MCP connection was lost while dispatching; it will be restarted on the next call');
+        }
+
         // Proxy other tools to MCP server
         try {
             console.debug('[DEBUG] Calling MCP tool:', toolName, 'args:', JSON.stringify(args).substring(0, 100));
-            const result = await this.mcpClient!.callTool({
+            const result = await client.callTool({
                 name: toolName,
                 arguments: args,
                 _meta: { remote: true, ...metadata || {} }

src/remote-device/device-status-arbiter.ts

@@ -0,0 +1,61 @@
+/**
+ * Single writer for the device's mcp_devices online/offline status.
+ *
+ * The device has two independent health signals — the Supabase Realtime
+ * channel (server connectivity) and the local Desktop Commander child
+ * (execution capability) — and previously each wrote status directly.
+ * Independent writers contradict each other: a channel resubscribe used to
+ * mark the device online while the local child was dead, advertising a device
+ * that fails every routed call.
+ *
+ * The arbiter owns the truth: online iff BOTH signals are healthy, written
+ * only on transitions — so retry storms that re-report an unchanged state
+ * (e.g. repeated TIMED_OUT during a reconnect loop) produce zero extra
+ * PATCHes to mcp_devices.
+ */
+export type DeviceHealthPart = 'channel' | 'child';
+
+export class DeviceStatusArbiter {
+    // The channel is assumed healthy until it reports otherwise: channel setup
+    // completes before supervision starts reporting, and the first
+    // CHANNEL_ERROR / TIMED_OUT / CLOSED flips it. The child must prove itself.
+    private parts: Record<DeviceHealthPart, boolean> = { channel: true, child: false };
+    // Devices register as offline; that is the last known persisted state.
+    private lastWritten: 'online' | 'offline' | null = 'offline';
+    private writeChain: Promise<void> = Promise.resolve();
+    private readonly write: (status: 'online' | 'offline') => Promise<void>;
+
+    constructor(options: { write: (status: 'online' | 'offline') => Promise<void> }) {
+        this.write = options.write;
+    }
+
+    get status(): 'online' | 'offline' {
+        return this.parts.channel && this.parts.child ? 'online' : 'offline';
+    }
+
+    report(part: DeviceHealthPart, ready: boolean) {
+        this.parts[part] = ready;
+        this.maybeWrite(false);
+    }
+
+    /**
+     * Force-write the current status. Needed once after registration: reports
+     * that arrive before a deviceId exists have their writes dropped by the
+     * device's write callback, so the persisted state must be brought up to
+     * date as soon as writes can succeed.
+     */
+    sync() {
+        this.maybeWrite(true);
+    }
+
+    private maybeWrite(force: boolean) {
+        const status = this.status;
+        if (!force && status === this.lastWritten) return;
+        this.lastWritten = status;
+        // Serialize writes so offline→online in quick succession lands in order.
+        this.writeChain = this.writeChain
+            .then(() => this.write(status))
+            .catch((error: any) =>
+                console.error(`Failed to write device status '${status}':`, error?.message ?? error));
+    }
+}

src/remote-device/device.ts

@@ -3,6 +3,7 @@
 import { RemoteChannel } from './remote-channel.js';
 import { DeviceAuthenticator } from './device-authenticator.js';
 import { DesktopCommanderIntegration } from './desktop-commander-integration.js';
+import { DeviceStatusArbiter } from './device-status-arbiter.js';
 import { fileURLToPath } from 'url';
 import os from 'os';
 import fs from 'fs/promises';
@@ -21,6 +22,10 @@ export class MCPDevice {
     private configPath: string;
     private persistSession: boolean;
     private desktop: DesktopCommanderIntegration;
+    private statusArbiter: DeviceStatusArbiter;
+    private recoveringLocalMcp: boolean = false;
+    private localRestartAttempt: number = 0;
+    private localMcpStableSince: number = 0;
 
     constructor(options: MCPDeviceOptions = {}) {
         this.baseServerUrl = process.env.MCP_SERVER_URL || 'https://mcp.desktopcommander.app';
@@ -33,6 +38,15 @@ export class MCPDevice {
         // Initialize desktop integration
         this.desktop = new DesktopCommanderIntegration();
 
+        // Single writer for mcp_devices status: online iff BOTH the remote
+        // channel and the local child are healthy, written on transitions only.
+        this.statusArbiter = new DeviceStatusArbiter({
+            write: async (status) => {
+                if (!this.deviceId) return; // pre-registration reports; sync() catches up later
+                await this.remoteChannel.setOnlineStatus(this.deviceId, status);
+            },
+        });
+
         // Graceful shutdown handlers (only set once)
         this.setupShutdownHandlers();
     }
@@ -98,13 +112,17 @@ export class MCPDevice {
             // Initialize desktop integration
             await this.desktop.initialize();
             this.desktop.onDisconnect((reason) => void this.handleLocalMcpLoss(reason));
+            this.statusArbiter.report('child', true);
 
             console.log(`⏳ Connecting to Remote MCP ${this.baseServerUrl}`);
             const { supabaseUrl, anonKey } = await this.fetchSupabaseConfig();
             console.log(`   - 🔌 Connected to Remote MCP`);
 
             // Initialize Remote Channel
             this.remoteChannel.initialize(supabaseUrl, anonKey);
+            // Route channel health through the arbiter so a channel resubscribe
+            // can't mark the device online while the local child is dead.
+            this.remoteChannel.setChannelHealthReporter((ready) => this.statusArbiter.report('channel', ready));
 
             // Load persisted configuration (deviceId, session)
             let session = await this.loadPersistedConfig();
@@ -163,6 +181,10 @@ export class MCPDevice {
                 (payload: any) => this.handleNewToolCall(payload)
             );
 
+            // Registration is done and a deviceId exists — persist the current
+            // arbiter status (earlier reports had their writes dropped).
+            this.statusArbiter.sync();
+
             console.log('✅ Device ready:');
             console.log(`   - User:         ${this.remoteChannel.user!.email}`);
             console.log(`   - Device ID:    ${this.deviceId}`);
@@ -267,23 +289,50 @@ export class MCPDevice {
      * connected" until someone restarted the process by hand.
      */
     private async handleLocalMcpLoss(reason: string) {
-        if (this.deviceId) {
-            await this.remoteChannel.setOnlineStatus(this.deviceId, 'offline')
-                .catch((e: any) => console.error('Failed to mark device offline:', e.message));
-        }
-
         // Recover proactively rather than waiting for the next tool call to
         // trigger the lazy restart: we just went offline, so no further calls
         // would be routed here and that wait would never end.
+        //
+        // Restart attempts use exponential backoff (base 2s, ceiling 5min) and
+        // never give up: a child that keeps dying is respawned ever more
+        // slowly instead of hammering mcp_devices with offline/online churn,
+        // and a transient restart failure (files mid-upgrade, resource
+        // exhaustion) self-heals on a later attempt instead of leaving the
+        // device offline until a human restarts the process. A death after a
+        // stable stretch restarts the ladder from the base delay.
+        if (this.recoveringLocalMcp || this.isShuttingDown) return;
+        this.recoveringLocalMcp = true;
         try {
-            await this.desktop.ensureReady();
-            if (this.deviceId) {
-                await this.remoteChannel.setOnlineStatus(this.deviceId, 'online');
+            this.statusArbiter.report('child', false);
+
+            const baseMs = Number(process.env.DC_LOCAL_RESTART_BACKOFF_BASE_MS) || 2000;
+            const stableMs = Number(process.env.DC_LOCAL_RESTART_STABLE_UPTIME_MS) || 60000;
+            const ceilingMs = 300000;
+            if (Date.now() - this.localMcpStableSince > stableMs) {
+                this.localRestartAttempt = 0;
             }
-            console.log('♻️  Local Desktop Commander MCP restarted; device is online again');
-        } catch (error: any) {
-            console.error(`❌ Could not restart local Desktop Commander MCP: ${error.message}`);
-            await captureRemote('remote_device_local_mcp_restart_failed', { error, reason });
+
+            while (!this.isShuttingDown) {
+                const delay = Math.min(baseMs * 2 ** this.localRestartAttempt, ceilingMs);
+                this.localRestartAttempt++;
+                // Jitter so a fleet-wide trigger doesn't synchronize retries.
+                await new Promise(r => setTimeout(r, delay + Math.random() * delay * 0.15));
+                if (this.isShuttingDown) return;
+                try {
+                    await this.desktop.ensureReady();
+                    this.localMcpStableSince = Date.now();
+                    this.statusArbiter.report('child', true);
+                    console.log(`♻️  Local Desktop Commander MCP restarted (attempt ${this.localRestartAttempt}); device is online again`);
+                    return;
+                } catch (error: any) {
+                    console.error(`❌ Restart attempt ${this.localRestartAttempt} failed: ${error.message}`);
+                    await captureRemote('remote_device_local_mcp_restart_failed', {
+                        error, reason, attempt: this.localRestartAttempt,
+                    });
+                }
+            }
+        } finally {
+            this.recoveringLocalMcp = false;
         }
     }
 

src/remote-device/remote-channel.ts

@@ -43,6 +43,28 @@ export class RemoteChannel {
     // Track last device status to prevent duplicate log messages
     private lastDeviceStatus: 'online' | 'offline' = 'offline';
 
+    // When set (by MCPDevice), channel health flows through the device status
+    // arbiter instead of writing mcp_devices directly — the arbiter combines
+    // it with local-child health so a channel resubscribe can't mark a device
+    // whose child is dead back online.
+    private channelHealthReporter: ((ready: boolean) => void) | null = null;
+
+    setChannelHealthReporter(reporter: (ready: boolean) => void) {
+        this.channelHealthReporter = reporter;
+    }
+
+    private reportChannelHealth(ready: boolean) {
+        if (this.channelHealthReporter) {
+            this.channelHealthReporter(ready);
+            return;
+        }
+        // Legacy fallback when no arbiter is wired (standalone use).
+        if (this.deviceId) {
+            this.setOnlineStatus(this.deviceId, ready ? 'online' : 'offline')
+                .catch((e: any) => console.error('Failed to update device status:', e.message));
+        }
+    }
+
     // Track last channel state for debug logging
     private lastChannelState: string | null = null;
 
@@ -228,30 +250,27 @@ export class RemoteChannel {
                         const recovered = this.reconnectAttempt;
                         this.reconnectAttempt = 0;
                         console.log(`✅ Channel subscribed${recovered > 0 ? ` (recovered after ${recovered} attempt${recovered === 1 ? '' : 's'})` : ''}`);
-                        // Update device status on successful connection
-                        if (this.deviceId) {
-                            this.setOnlineStatus(this.deviceId, 'online').catch(e => {
-                                console.error('Failed to set online status:', e.message);
-                            });
-                        }
+                        // Report channel health on successful connection (the
+                        // arbiter decides whether the device is truly online).
+                        this.reportChannelHealth(true);
                         resolve();
                     } else if (status === 'CHANNEL_ERROR') {
                         // CHANNEL_ERROR is the only status carrying a real error message.
                         console.error(`❌ Channel error: ${err?.message || 'unknown'} — ${this.connState()}`);
-                        this.setOnlineStatus(this.deviceId!, 'offline');
+                        this.reportChannelHealth(false);
                         captureRemote('remote_channel_subscription_error', { error: err?.message || 'Channel error' }).catch(() => { });
                         reject(err || new Error('Failed to initialize tool call channel subscription'));
                     } else if (status === 'TIMED_OUT') {
                         console.error(`⏱️ Channel subscription timed out, Reconnecting... — ${this.connState()}`);
-                        this.setOnlineStatus(this.deviceId!, 'offline');
+                        this.reportChannelHealth(false);
                         captureRemote('remote_channel_subscription_timeout', { attempt: this.reconnectAttempt }).catch(() => { });
                         reject(new Error('Tool call channel subscription timed out'));
                     } else if (status === 'CLOSED') {
                         // Settle the promise so an in-flight recreateChannel() can't await
                         // forever (which would wedge the re-entrancy guard / watchdog), and
                         // mark the device offline like the other degraded states.
                         console.warn(`⚠️ Channel closed — ${this.connState()}`);
-                        this.setOnlineStatus(this.deviceId!, 'offline');
+                        this.reportChannelHealth(false);
                         reject(new Error('Tool call channel closed during subscribe'));
                     }
                 });