diff --git a/src/remote-device/device.ts b/src/remote-device/device.ts index 64e856cd..cc873876 100644 --- a/src/remote-device/device.ts +++ b/src/remote-device/device.ts @@ -13,6 +13,15 @@ export interface MCPDeviceOptions { persistSession?: boolean; } +/** + * How many recently-handled call ids to remember for duplicate-delivery + * suppression. The two transports deliver a call within MILLISECONDS of each + * other, so this only has to outlive that window — 100 ids is several minutes + * of even the heaviest agent traffic, and costs ~10 KB on the user's machine + * (the device process, not the shared server). + */ +const SEEN_CALL_IDS_MAX = 100; + export class MCPDevice { private baseServerUrl: string; private remoteChannel: RemoteChannel; @@ -21,6 +30,8 @@ export class MCPDevice { private configPath: string; private persistSession: boolean; private desktop: DesktopCommanderIntegration; + /** Call ids already handled by THIS process (insertion-ordered, bounded). */ + private seenCallIds: Set = new Set(); constructor(options: MCPDeviceOptions = {}) { this.baseServerUrl = process.env.MCP_SERVER_URL || 'https://mcp.desktopcommander.app'; @@ -259,6 +270,16 @@ export class MCPDevice { // Methods moved to RemoteChannel + /** Record a handled call id, evicting the oldest once the cap is reached. */ + private rememberCallId(callId: string) { + this.seenCallIds.add(callId); + if (this.seenCallIds.size > SEEN_CALL_IDS_MAX) { + // Sets iterate in insertion order — drop the oldest entry. + const oldest = this.seenCallIds.values().next().value; + if (oldest !== undefined) this.seenCallIds.delete(oldest); + } + } + async handleNewToolCall(payload: any) { const toolCall = payload.new; // Expect toolCall to include a device_id field used to route calls to this device instance. @@ -274,9 +295,28 @@ export class MCPDevice { console.log(`🔧 Received tool call ${call_id}: ${tool_name} ${JSON.stringify(tool_args)} metadata: ${JSON.stringify(metadata)}`); + // LOCAL claim first — this is the authoritative guard against executing + // a call twice. During the transition both transports deliver every call + // to THIS SAME PROCESS, so an in-memory check is sufficient and, unlike + // the DB claim below, cannot fail open: a transient REST error made + // markCallExecuting return true for both deliveries, which could run a + // side-effecting command twice (found in review, 2026-07-24). + if (this.seenCallIds.has(call_id)) { + console.debug('[DEBUG] Duplicate delivery for call already handled here, skipping:', call_id); + return; + } + this.rememberCallId(call_id); + try { - // Update call status to executing - await this.remoteChannel.markCallExecuting(call_id); + // DB claim second — keeps the row state machine honest, gives + // cross-restart/cross-process protection, and is observable. It may + // fail open (returns true on a transient write error); the local + // guard above is what makes execution exactly-once. + const claimed = await this.remoteChannel.markCallExecuting(call_id); + if (!claimed) { + // markCallExecuting already logged the duplicate-delivery skip. + return; + } let result; @@ -309,13 +349,23 @@ export class MCPDevice { console.log(`✅ Tool call ${tool_name} completed:\r\n ${JSON.stringify(result)}`); - // Update database with result + // Update database with result, THEN ring the doorbell — the server + // fetches the row by id on the doorbell, so the write must land first. await this.remoteChannel.updateCallResult(call_id, 'completed', result); + await this.remoteChannel.notifyResult(call_id); } catch (error: any) { console.error(`❌ Tool call ${tool_name} failed:`, error.message); - await captureRemote('remote_device_tool_call_failed', { error, tool_name }); - await this.remoteChannel.updateCallResult(call_id, 'failed', null, error.message); + // The failure path must not fail: this method's promise is discarded + // at every call site, so a throw here becomes an unhandled rejection + // and takes the device process down. + try { + await captureRemote('remote_device_tool_call_failed', { error, tool_name }); + await this.remoteChannel.updateCallResult(call_id, 'failed', null, error.message); + await this.remoteChannel.notifyResult(call_id); + } catch (reportError: any) { + console.error(`❌ Could not report failure for ${call_id}:`, reportError?.message); + } } } diff --git a/src/remote-device/remote-channel.ts b/src/remote-device/remote-channel.ts index 0f42478d..1d0bcdff 100644 --- a/src/remote-device/remote-channel.ts +++ b/src/remote-device/remote-channel.ts @@ -1,5 +1,34 @@ import { createClient, SupabaseClient, Session, UserResponse, User, RealtimeChannel } from '@supabase/supabase-js'; import { captureRemote } from '../utils/capture.js'; +import { VERSION } from '../version.js'; + +const NUL_CHAR = String.fromCharCode(0); +const NUL_RE = new RegExp(NUL_CHAR, 'g'); + +/** + * Strip NUL characters (U+0000) from strings and object keys — Postgres rejects + * them in jsonb and text (22P05). Walks the structure rather than + * round-tripping JSON, which would also match escape text in legitimate content. + */ +export function stripNullBytes(value: T): T { + if (typeof value === 'string') { + return (value.includes(NUL_CHAR) ? value.replace(NUL_RE, '') : value) as T; + } + if (Array.isArray(value)) { + return value.map((item) => stripNullBytes(item)) as T; + } + if (value && typeof value === 'object') { + // Plain objects only — leave Date/Buffer/etc. untouched. + const proto = Object.getPrototypeOf(value); + if (proto !== Object.prototype && proto !== null) return value; + const out: Record = {}; + for (const [k, v] of Object.entries(value as Record)) { + out[k.includes(NUL_CHAR) ? k.replace(NUL_RE, '') : k] = stripNullBytes(v); + } + return out as T; + } + return value; +} export interface AuthSession { @@ -16,29 +45,66 @@ interface DeviceData { last_seen: string; } -const HEARTBEAT_INTERVAL = 15000; -// Cap a single channel recreate so a hung await can't pin the re-entrancy guard -// true (which would silently disable the connection watchdog). -const RECREATE_TIMEOUT_MS = 30000; -// Max time the channel may sit CONTINUOUSLY in 'joining' before we force a recreate. -// 'joining' is normally healthy (we let realtime-js's rejoin backoff converge), but on a -// HALF-OPEN socket (readyState OPEN yet dead) realtime-js parks the channel in 'joining' -// forever and never reconnects the socket — the device then wedges offline silently with -// no recreate firing. realtime-js's join push times out in ~10s, so a genuine join -// resolves/errors well within this window; 3 health ticks of unbroken 'joining' means the -// state machine has stalled and only a fresh socket (via recreate) recovers it. +// last_seen cadences. The server tiers its sweep on the transport_broadcast_v1 +// flag, so each must fit its tier's threshold in the server's constants.ts: +// capable -> 15 min, unflagged -> 45s. +const CAPABLE_HEARTBEAT_INTERVAL = 5 * 60 * 1000; +const LEGACY_HEARTBEAT_INTERVAL = 15 * 1000; +// Cap on a recreate's rebuild step so a hung await can't disable the watchdog. +// Must exceed createChannel()'s worst case (~31.5s of presence retries). +const RECREATE_TIMEOUT_MS = 45000; +// Max continuous time in 'joining' before forcing a recreate — a half-open +// socket parks the channel there forever, and a genuine join settles in ~10s. const JOINING_WEDGE_TIMEOUT_MS = 30000; +// Failed recreates before withdrawing transport_broadcast_v1 — keeping it while +// unable to join makes the device undispatchable. Not lower than 3: ordinary +// half-open recovery legitimately costs 2. +const TRANSPORT_WITHDRAW_AFTER_ATTEMPTS = 3; +// Cap on the withdrawal write; it runs in a catch block RECREATE_TIMEOUT_MS +// does not cover. +const CAPABILITY_WRITE_TIMEOUT_MS = 5000; +// Cap on the shutdown session fetch, which races device.ts's 5s force-exit. +const OFFLINE_SESSION_TIMEOUT_MS = 500; +// realtime-js parks in 'disconnecting' for ~100ms after a disconnect and +// connect() early-returns for that whole window (see waitForSocketSettled). +// Bound generously — this only ever delays a recreate, which RECREATE_TIMEOUT_MS +// already covers. +const SOCKET_SETTLE_MAX_MS = 300; +const SOCKET_SETTLE_POLL_MS = 20; export class RemoteChannel { private client: SupabaseClient | null = null; private channel: RealtimeChannel | null = null; + /** Legacy listener, on its own public channel so a private-channel auth + * failure can't take both transports down. Removed at the flip (009). */ + private legacyChannel: RealtimeChannel | null = null; private heartbeatInterval: NodeJS.Timeout | null = null; private connectionCheckInterval: NodeJS.Timeout | null = null; + /** Device the heartbeat timer maintains; null = stopped, so re-arm is inert. */ + private heartbeatDeviceId: string | null = null; + // Single-slot queue keeping concurrent `status` PATCHes in order. + private statusWriteChain: Promise = Promise.resolve(); + /** Tokens from the last setSession / TOKEN_REFRESHED, for setOffline(). */ + private lastKnownSession: { access_token: string; refresh_token: string | null } | null = null; + /** Set by unsubscribe(): suppresses status/heartbeat writes so they can't + * land after setOffline()'s durable write. */ + private shuttingDown = false; // Store subscription parameters for channel recreation private deviceId: string | null = null; + private deviceName: string | null = null; private onToolCall: ((payload: any) => void) | null = null; + // Guard so setSession being called twice can't stack auth listeners. + private authListenerRegistered = false; + /** False when presence publishing failed on an otherwise healthy channel; + * the health check retries, since SUBSCRIBED won't fire again. */ + private presenceTracked = false; + /** Last capability value written (null = never), to avoid redundant writes. */ + private transportCapableWritten: boolean | null = null; + /** Re-entrancy guard: on a wedged socket each track() buffers for the full + * 10s push timeout, so 10s health ticks would stack pushes. */ + private isTrackingPresence = false; // Track last device status to prevent duplicate log messages private lastDeviceStatus: 'online' | 'offline' = 'offline'; @@ -46,10 +112,9 @@ export class RemoteChannel { // Track last channel state for debug logging private lastChannelState: string | null = null; - // Reconnect diagnostics + guard (see connState() / recreateChannel()) - private reconnectAttempt = 0; // recreateChannel() attempts since last success - private isRecreatingChannel = false; // a recreate is in flight (re-entrancy guard) - private joiningSince: number | null = null; // ts the channel entered an unbroken 'joining' run; null when not joining + private reconnectAttempt = 0; // recreates since the last success + private isRecreatingChannel = false; // re-entrancy guard + private joiningSince: number | null = null; // start of an unbroken 'joining' run private _user: User | null = null; get user(): User | null { return this._user; } @@ -91,6 +156,31 @@ export class RemoteChannel { this._user = user; console.debug('[DEBUG] Session set successfully, user:', user.email); + // Push the CURRENT token, not the one we were handed: setSession() + // refreshes internally, and the stale parameter would overwrite it. + const { data: { session: currentSession } } = await this.client.auth.getSession(); + const realtimeToken = currentSession?.access_token ?? session.access_token; + this.client.realtime.setAuth(realtimeToken); + // Cached for setOffline(), which can't afford to wait on getSession(). + this.lastKnownSession = { + access_token: realtimeToken, + refresh_token: currentSession?.refresh_token ?? session.refresh_token ?? null, + }; + console.debug('[DEBUG] Realtime socket authorized with current session JWT'); + if (!this.authListenerRegistered) { + this.authListenerRegistered = true; + this.client.auth.onAuthStateChange((event, newSession) => { + if (event === 'TOKEN_REFRESHED' && newSession?.access_token && this.client) { + console.debug('[DEBUG] Token refreshed — re-authorizing realtime socket'); + this.client.realtime.setAuth(newSession.access_token); + this.lastKnownSession = { + access_token: newSession.access_token, + refresh_token: newSession.refresh_token ?? this.lastKnownSession?.refresh_token ?? null, + }; + } + }); + } + return { error }; } @@ -164,15 +254,18 @@ export class RemoteChannel { if (existingDevice) { console.debug('[DEBUG] Updating device status to online'); + // transport_broadcast_v1 is NOT set here: the server treats it as + // binding, so it is written only once presence is proven. await this.updateDevice(existingDevice.id, { status: 'online', last_seen: new Date().toISOString(), - capabilities: {}, // TODO: Capabilities are not yet implemented; keep this empty object for schema compatibility until device capabilities are defined and stored. + capabilities: this.capabilitiesPayload(false), device_name: deviceName }); // Store parameters for channel recreation this.deviceId = existingDevice.id; + this.deviceName = deviceName; this.onToolCall = onToolCall; console.debug(`⏳ Subscribing to tool call channel...`); @@ -180,7 +273,9 @@ export class RemoteChannel { // Create and subscribe to the channel console.debug('[DEBUG] Calling createChannel()'); - // ! Ignore silently in Initialization to reconnect after + // Independent safety net for the doorbell transport. + this.createLegacyChannel(); + await this.createChannel().catch((error) => { console.debug(`[DEBUG] Failed to create channel, will retry after socket reconnect: ${error?.message || error} — ${this.connState()}`); }); @@ -193,18 +288,112 @@ export class RemoteChannel { } /** - * Create and subscribe to the channel. - * This is used for both initial subscription and recreation after socket reconnects. + * Publish presence, retrying a non-'ok' result — track() resolves with a + * status rather than rejecting, and absent presence reads as offline on the + * server. `presenceTracked` lets the health check retry later. */ - private createChannel(): Promise { - return new Promise((resolve, reject) => { - if (!this.client || !this.user?.id || !this.onToolCall) { - console.debug('[DEBUG] createChannel() failed - missing prerequisites'); - return reject(new Error('Client not initialized or missing subscription parameters')); + private async trackPresenceWithRetry(recovered: number, attempts = 3): Promise { + if (this.isTrackingPresence) return; // never stack pushes on a wedged socket + this.isTrackingPresence = true; + try { + await this.trackPresenceInner(recovered, attempts); + } finally { + this.isTrackingPresence = false; + } + } + + private async trackPresenceInner(recovered: number, attempts: number): Promise { + for (let attempt = 1; attempt <= attempts; attempt++) { + if (!this.channel || this.channel.state !== 'joined') return; + let status: string; + try { + status = await this.channel.track({ + device_id: this.deviceId, + device_name: this.deviceName, + app_version: VERSION, + platform: process.platform + }); + } catch (trackErr: any) { + status = `threw: ${trackErr?.message}`; + } + + if (status === 'ok') { + this.presenceTracked = true; + console.log(`👋 Presence tracked (device ${this.deviceId} visible as online)`); + // Reconnect attempts preceding this join (0 on a first join). + captureRemote('remote_channel_presence_tracked', { recoveredAfterAttempts: recovered }).catch(() => { }); + // Proven end-to-end (joined AND presence published) — only now + // may the server treat our presence as authoritative. + await this.setTransportCapable(true); + return; + } + + console.error(`❌ Presence track not acknowledged (${status}) — attempt ${attempt}/${attempts}`); + if (attempt < attempts) await this.sleep(500 * attempt); + } + + this.presenceTracked = false; + console.error('❌ Presence track failed after retries — reverting to the legacy transport tier'); + captureRemote('remote_channel_presence_track_error', { attempts }).catch(() => { }); + // Withdraw: a stale flag with no presence makes the server refuse to + // dispatch at all. The legacy tier keeps the device usable. + await this.setTransportCapable(false); + } + + /** + * The complete `capabilities` JSONB value. One place only: every write + * replaces the whole column, so a second literal would silently drop keys. + */ + private capabilitiesPayload(broadcastCapable: boolean): Record { + return { + app_version: VERSION, + ...(broadcastCapable ? { transport_broadcast_v1: true } : {}) + }; + } + + /** + * Advertise (or withdraw) the broadcast capability. Only true while genuinely + * reachable that way — the server uses it to pick a transport, to read absent + * presence as offline, and to choose the sweep tier, so every change must + * re-arm the heartbeat. + */ + private async setTransportCapable(capable: boolean): Promise { + if (!this.client || !this.deviceId) return; + if (this.transportCapableWritten === capable) return; // no redundant writes + try { + const capabilities = this.capabilitiesPayload(capable); + const { error } = await this.client + .from('mcp_devices') + .update({ capabilities }) + .eq('id', this.deviceId); + if (error) { + console.error('[DEBUG] Failed to update transport capability:', error.message); + return; } + this.transportCapableWritten = capable; + console.debug(`[DEBUG] Transport capability set to ${capable ? 'broadcast_v1' : 'legacy'}`); + // Tier changed — move last_seen onto the cadence that tier's sweep + // threshold expects (no-op if the heartbeat hasn't started yet). + this.scheduleHeartbeat(); + // last_seen may already be past the 45s threshold now judging us, + // so write once immediately rather than waiting out the interval. + if (!capable && this.heartbeatDeviceId) { + this.updateHeartbeat(this.heartbeatDeviceId).catch(() => { /* logged inside */ }); + } + } catch (error: any) { + console.error('[DEBUG] Transport capability update threw:', error?.message); + } + } - console.debug('[DEBUG] Creating channel: device_tool_call_queue'); - this.channel = this.client.channel('device_tool_call_queue') + /** + * Legacy postgres_changes listener on its own public channel. Best-effort: + * failures are logged, never thrown. Removed at the flip (009). + */ + private createLegacyChannel(): void { + if (!this.client || !this.user?.id) return; + try { + this.legacyChannel = this.client + .channel('device_tool_call_queue') .on( 'postgres_changes' as any, { @@ -215,9 +404,57 @@ export class RemoteChannel { }, (payload: any) => { console.debug('[DEBUG] Realtime event received, payload:', payload?.new?.id); - if (this.onToolCall) { - this.onToolCall(payload); - } + this.dispatchToolCall(payload); + } + ) + .subscribe((status: string) => { + console.debug(`[DEBUG] Legacy channel status: ${status}`); + }); + } catch (error: any) { + console.debug('[DEBUG] Legacy channel subscribe failed (doorbell path unaffected):', error?.message); + } + } + + /** Tear down the legacy channel (best effort). */ + private async removeLegacyChannel(): Promise { + if (!this.legacyChannel || !this.client) return; + try { + await this.client.removeChannel(this.legacyChannel); + } catch { /* best effort */ } + this.legacyChannel = null; + } + + /** Create and subscribe the private channel (initial join and recreation). */ + private createChannel(): Promise { + return new Promise((resolve, reject) => { + if (!this.client || !this.user?.id || !this.onToolCall || !this.deviceId) { + // deviceId is the presence KEY; a null key gets a random one and + // the server's lookup by device id silently misses. + console.debug('[DEBUG] createChannel() failed - missing prerequisites'); + return reject(new Error('Client not initialized or missing subscription parameters')); + } + + // Private per-user channel: new_call doorbells + this device's + // Presence, keyed by device id. + const channelName = `user:${this.user.id}`; + console.debug(`[DEBUG] Creating channel: ${channelName}`); + this.channel = this.client.channel(channelName, { + // ack: true — without it send() resolves 'ok' once the frame hits + // the socket, making notifyResult's status check dead code. + config: { + private: true, + broadcast: { ack: true }, + // Non-null: the guard above rejects when !deviceId. + presence: { key: this.deviceId, enabled: true } + } + }) + .on( + 'broadcast', + { event: 'new_call' }, + ({ payload }: any) => { + this.onDoorbell(payload).catch((e: any) => { + console.error('[DEBUG] Doorbell handling failed:', e?.message); + }); } ) .subscribe((status: string, err: any) => { @@ -228,36 +465,143 @@ 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); - }); - } - resolve(); + // Update device status on successful connection (queued, so + // it can't be overtaken by a teardown's status write). + this.queueStatusWrite('online'); + // Presence is the live signal dispatch reads, so resolve + // only once it lands — otherwise registerDevice() reports + // "Device ready" while still undispatchable. + this.trackPresenceWithRetry(recovered) + .catch(() => { /* logged inside */ }) + .finally(() => 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.presenceTracked = false; + this.syncReachabilityStatus(); + // Fires on ordinary network faults too — filter on the + // error text to isolate an 008 misconfiguration. 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.syncReachabilityStatus(); 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. + // forever (which would wedge the re-entrancy guard / watchdog). console.warn(`⚠️ Channel closed — ${this.connState()}`); - this.setOnlineStatus(this.deviceId!, 'offline'); + this.syncReachabilityStatus(); reject(new Error('Tool call channel closed during subscribe')); } }); }); } + /** Hand a call to device.ts, observing the rejection — the handler is async + * and an unhandled rejection terminates the process. */ + private dispatchToolCall(payload: any): void { + try { + const maybePromise = this.onToolCall?.(payload) as unknown; + if (maybePromise instanceof Promise) { + maybePromise.catch((e: any) => { + console.error('[DEBUG] Tool call handler rejected:', e?.message); + }); + } + } catch (e: any) { + console.error('[DEBUG] Tool call handler threw:', e?.message); + } + } + + /** + * Handle a 'new_call' doorbell. It carries ids only; the row is fetched by + * primary key and fed through the same handler as a postgres_changes + * payload, so device.ts stays transport-agnostic. + */ + private async onDoorbell(payload: any): Promise { + const callId = payload?.call_id; + if (!callId) return; + if (payload?.device_id && payload.device_id !== this.deviceId) { + console.debug('[DEBUG] Ignoring doorbell for different device'); + return; + } + + // Not a telemetry event on purpose: ~126k/day in prod. Transport usage + // is already segmentable server-side via metadata.transport. + console.debug('[DEBUG] Doorbell received for call:', callId); + + if (!this.client) return; + + // Retry on transient failures (a REST blip while the socket stays + // healthy). Post-flip this fetch is the only way we learn about a call, + // so a hiccup must not cost a 5-minute timeout. + let row: any = null; + let lastError: any = null; + for (const delayMs of [0, 500, 1500]) { + if (delayMs > 0) await this.sleep(delayMs); + const { data, error } = await this.client + .from('mcp_remote_calls') + .select('*') + .eq('id', callId) + .maybeSingle(); + if (!error) { + row = data; + lastError = null; + break; + } + lastError = error; + console.debug(`[DEBUG] Doorbell row fetch attempt failed for ${callId}: ${error.message} — retrying`); + } + + if (lastError) { + console.error(`[DEBUG] Doorbell row fetch failed for ${callId} after retries:`, lastError.message); + await captureRemote('remote_channel_doorbell_fetch_error', { error: lastError }); + return; + } + if (!row) { + // Already claimed and deleted, or cleanup raced delivery. Not + // retried: the row is always inserted before the doorbell is sent. + await captureRemote('remote_channel_doorbell_row_missing', { call_id: callId }); + return; + } + // Optimization, not a guard — saves a hop when the legacy path already + // claimed this. Exactly-once lives in device.ts (seenCallIds + DB claim). + if (row.status !== 'pending') { + console.debug('[DEBUG] Doorbell call already claimed via legacy path:', callId); + return; + } + + // Same payload shape as postgres_changes ({ new: row }). + this.dispatchToolCall({ new: row }); + } + + /** + * Tell the server a result row is written. Fire-and-forget: a failed send + * just falls back to the server's 10s recovery poll. MUST run only after + * updateCallResult() resolves, so the server's fetch-by-id sees a terminal row. + */ + async notifyResult(callId: string): Promise { + if (!this.channel || this.channel.state !== 'joined') { + console.debug('[DEBUG] Result doorbell skipped — channel not joined (recovery poll covers)'); + return; + } + try { + // realtime-js send() RESOLVES with 'ok' | 'timed out' | 'error' — + // it does not reject, so check the status or failures are invisible. + const result = await this.channel.send({ type: 'broadcast', event: 'result', payload: { call_id: callId } }); + if (result === 'ok') { + console.debug('[DEBUG] Result doorbell sent:', callId); + } else { + console.debug(`[DEBUG] Result doorbell not acknowledged (${result}) — recovery poll covers:`, callId); + captureRemote('remote_channel_result_doorbell_send_failed', { result }).catch(() => { }); + } + } catch (error: any) { + console.debug('[DEBUG] Result doorbell send failed (recovery poll covers):', error?.message); + captureRemote('remote_channel_result_doorbell_send_failed', { error: error?.message }).catch(() => { }); + } + } + /** * Compact connection state for logs — e.g. "socket=open(1) ch=errored attempt=3". * readyState 1=OPEN (a 1 while joins keep failing = a half-open socket being reused), @@ -291,16 +635,21 @@ export class RemoteChannel { // 'joined' = healthy. Clear the joining-overstay timer. if (state === 'joined') { this.joiningSince = null; + // Self-heal a failed presence publish: the channel is up, so nothing + // else will ever retry (SUBSCRIBED won't fire again), and without + // presence the server reports this healthy device as offline. + if (!this.presenceTracked && this.deviceId && !this.isTrackingPresence) { + console.debug('[DEBUG] Channel joined but presence not tracked — retrying track()'); + this.trackPresenceWithRetry(0, 1).catch(() => { /* logged inside */ }); + } return; } - // 'joining' = transitional — normally let realtime-js's own rejoin backoff converge - // instead of tearing the channel down mid-join (recreating on every non-joined state - // amputates that backoff). BUT bound it: on a half-open socket realtime-js can park - // the channel in 'joining' indefinitely without ever reconnecting the socket, so the - // recreate below would never fire and the device wedges offline silently. If 'joining' - // overstays JOINING_WEDGE_TIMEOUT_MS unbroken, force a recreate — the only path that - // disconnect()s the dead socket. (connState() in the log shows the half-open socket.) + // 'joining' is transitional — let realtime-js's rejoin backoff converge + // rather than tearing the channel down mid-join. But bound it: a + // half-open socket parks the channel here indefinitely, so past + // JOINING_WEDGE_TIMEOUT_MS force a recreate, the only path that + // disconnect()s the dead socket. if (state === 'joining') { const now = Date.now(); if (this.joiningSince === null) this.joiningSince = now; @@ -325,6 +674,30 @@ export class RemoteChannel { * can't leave isRecreatingChannel stuck true and disable the watchdog. Mirrors * closeWithTimeout() in desktop-commander-integration.ts. */ + private sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); + } + + /** + * Block until realtime-js has left the 'disconnecting' state it enters on + * disconnect(), so the next subscribe() actually dials a socket instead of + * hitting connect()'s early return. Bounded either way — worst case we cost + * a recreate SOCKET_SETTLE_MAX_MS. + */ + private async waitForSocketSettled(): Promise { + const realtime = (this.client as any)?.realtime; + // No predicate to poll (older/newer client): wait out the internal + // fallback timer blind rather than guess at the state. + if (typeof realtime?.isDisconnecting !== 'function') { + await this.sleep(SOCKET_SETTLE_MAX_MS); + return; + } + const deadline = Date.now() + SOCKET_SETTLE_MAX_MS; + while (realtime.isDisconnecting() && Date.now() < deadline) { + await this.sleep(SOCKET_SETTLE_POLL_MS); + } + } + private async withTimeout(op: () => Promise, ms: number, name: string): Promise { let timer: NodeJS.Timeout | undefined; try { @@ -362,48 +735,119 @@ export class RemoteChannel { console.log(`🔄 Recreating channel... (attempt ${this.reconnectAttempt}) — ${this.connState()}`); try { + // Jittered backoff so a fleet-wide event doesn't stampede every + // device into reconnecting at once. ~1-3s rising to ~15-45s. + const backoffMs = Math.min(30_000, 1000 * 2 ** Math.min(this.reconnectAttempt, 5)) * (0.5 + Math.random()); + console.debug(`[DEBUG] Reconnect backoff: ${Math.round(backoffMs)}ms`); + await this.sleep(backoffMs); + + // realtime-js runs its own rejoin timer, and the backoff above gives + // it a window to win: the old channel can come back 'joined' while we + // slept. Destroying a healthy channel would cause a pointless outage + // cycle — bail out instead (observed live on staging, 2026-07-23). + if (this.channel?.state === 'joined') { + console.log(`✅ Channel self-healed during backoff — skipping recreate — ${this.connState()}`); + return; // finally-block below clears the re-entrancy guard + } + // Cap the whole recreate: a never-settling await (e.g. a subscribe that only // ever emits CLOSED) must not pin isRecreatingChannel=true and silently disable // the 10s watchdog. On timeout we reject -> catch -> finally clears the guard. await this.withTimeout(async () => { - // Destroy old channel — AWAIT it so the channel registry empties before we - // rebuild. (The un-awaited version raced the synchronous new-channel push, so - // realtime-js never tore the socket down and a half-open one got reused.) + // Await it so the channel registry empties before we rebuild — + // otherwise realtime-js never tears the socket down and a + // half-open one gets reused. if (this.channel) { console.debug('[DEBUG] Destroying old channel'); await this.client!.removeChannel(this.channel); this.channel = null; } + // Rebuild the legacy channel too: it shares the socket, so a + // socket-level wedge takes it down with the private channel. + await this.removeLegacyChannel(); // FIX (core): force a brand-new WebSocket. After idle / wifi-loss the socket can // be HALF-OPEN (readyState OPEN but dead); reusing it made every join TIME_OUT // forever. disconnect() drops it so the next subscribe() dials a fresh one. try { await (this.client as any).realtime?.disconnect?.(); } catch { /* best effort */ } + // ...but disconnect() is not synchronous from connect()'s point + // of view: it parks _connectionState in 'disconnecting' and + // _teardownConnection() nulls the conn.onclose that would clear + // it, so only an internal ~100ms fallback timer does. connect() + // early-returns for that whole window, so rebuilding here makes + // subscribe()'s socket.connect() a silent no-op and BOTH + // channels sit in 'joining' until the 10s join timeout — the + // wasted-first-recreate that left the device dark on the legacy + // channel too. Wait for the state to settle before rebuilding. + await this.waitForSocketSettled(); + console.debug('[DEBUG] Calling createChannel() for recreation'); + // Rebuild the legacy safety net FIRST and unconditionally: if + // createChannel() throws or exceeds RECREATE_TIMEOUT_MS, anything + // after it is skipped, which used to leave the fallback dead for + // the entire duration of a private-channel outage — every + // subsequent health tick repeating the same teardown. + this.createLegacyChannel(); await this.createChannel(); }, RECREATE_TIMEOUT_MS, 'recreateChannel'); } catch (err: any) { captureRemote('remote_channel_recreate_error', { errMsg: err?.message, attempt: this.reconnectAttempt }); console.debug(`[DEBUG] Channel recreation failed: ${err?.message} — ${this.connState()}`); + // Sustained failure: stop promising a transport we can't deliver, or + // the server's presence overlay reports this device offline + // authoritatively and overrides `status`. + if (this.reconnectAttempt >= TRANSPORT_WITHDRAW_AFTER_ATTEMPTS) { + // Bounded, in its own try: this catch block is outside + // RECREATE_TIMEOUT_MS, so a hanging PATCH would pin + // isRecreatingChannel and disable the watchdog. + try { + await this.withTimeout( + () => this.setTransportCapable(false), + CAPABILITY_WRITE_TIMEOUT_MS, + 'withdrawTransportCapability' + ); + } catch (withdrawErr: any) { + // The next failed recreate retries; the flag only advances + // on a confirmed write, so nothing is lost. + console.debug(`[DEBUG] Capability withdrawal did not complete: ${withdrawErr?.message}`); + } + } } finally { this.isRecreatingChannel = false; } } - async markCallExecuting(callId: string) { + /** + * Claim a call. True only when THIS update flipped the row pending -> + * executing, which is what makes dual delivery safe across processes. + * .eq('status','pending') makes it conditional; .select('id') makes the + * result observable. On a transient DB error it returns true (execute + * anyway), matching prior behaviour — so device.ts's in-memory guard is what + * actually guarantees exactly-once within a process. + */ + async markCallExecuting(callId: string): Promise { if (!this.client) throw new Error('Client not initialized'); - const { error } = await this.client + const { data, error } = await this.client .from('mcp_remote_calls') .update({ status: 'executing' }) - .eq('id', callId); + .eq('id', callId) + .eq('status', 'pending') + .select('id'); if (error) { console.error('[DEBUG] Failed to mark call executing:', error.message); await captureRemote('remote_channel_mark_call_executing_error', { error }); - } else { + return true; // preserve legacy behavior: execution proceeds despite the write error + } + + const claimed = !!data && data.length > 0; + if (claimed) { console.debug('[DEBUG] Call marked executing:', callId); + } else { + console.debug('[DEBUG] Call already claimed (duplicate delivery), skipping:', callId); } + return claimed; } async updateCallResult(callId: string, status: string, result: any = null, errorMessage: string | null = null) { @@ -413,11 +857,27 @@ export class RemoteChannel { completed_at: new Date().toISOString() }; - if (result !== null) updateData.result = result; - if (errorMessage !== null) updateData.error_message = errorMessage; - - console.debug('[DEBUG] Updating call result:', updateData); - const { data, error } = await this.client + // Strip NUL (U+0000) before it reaches the jsonb `result` column. + // jsonb cannot store and rejects the whole write (Postgres 22P05), + // which otherwise leaves the call stuck 'executing' → the user waits out + // a 5-minute timeout for a tool that actually ran. Common with binary + // file reads / process output. error_message is text, so it's exempt. + if (result !== null) updateData.result = stripNullBytes(result); + // Postgres `text` rejects NUL too (not just jsonb) — a NUL-bearing error + // message would fail this terminal write, and because result === null the + // fallback below wouldn't fire, stranding the call until the 5-min timeout. + if (errorMessage !== null) updateData.error_message = stripNullBytes(errorMessage); + + // Gated: the size is only knowable by serializing, and results reach + // 13 MB — doing that eagerly for a log line would cost more than the + // rest of this function. + if (process.env.DEBUG_MODE === 'true') { + console.debug( + `[DEBUG] Updating call result: ${callId} status=${status}` + + (result !== null ? ` resultBytes=~${JSON.stringify(updateData.result)?.length ?? 0}` : '') + ); + } + const { error } = await this.client .from('mcp_remote_calls') .update(updateData) .eq('id', callId); @@ -425,24 +885,114 @@ export class RemoteChannel { if (error) { console.error('[DEBUG] Failed to update call result:', error.message); await captureRemote('remote_channel_update_call_result_error', { error }); + + // Fail-fast fallback: if the RESULT write failed (sanitize should + // prevent the NUL case, but any unstorable payload lands here), + // record a terminal 'failed' with a text-only message so the user + // gets an immediate, honest error instead of a 5-minute phantom + // timeout. Guard against infinite recursion (only for result writes). + if (result !== null && status !== 'failed') { + await this.updateCallResult( + callId, + 'failed', + null, + `Result could not be stored (${error.message})` + ); + } } else { - console.debug('[DEBUG] Call result updated successfully:', data); + // (an UPDATE without .select() returns no row data — log the id) + console.debug('[DEBUG] Call result updated successfully:', callId); + } + } + + /** + * Reachable by SOME transport — the private channel or, during the + * transition, the independent legacy one. Gates the heartbeat and `status`: + * asking only about the private channel starves last_seen for a device whose + * legacy channel is fine, and the 45s sweep then blacks it out. + * Collapses to a single check at the flip (009). + */ + private isReachable(): boolean { + return this.channel?.state === 'joined' || this.legacyChannel?.state === 'joined'; + } + + /** + * Set `status` from actual reachability. `status` is transport-agnostic (the + * server filters on it), so it must not follow one channel's health — the + * private channel's error path re-fires on every rejoin and would oscillate + * the row against the heartbeat. Same predicate as the heartbeat gate. + */ + private syncReachabilityStatus(): void { + this.queueStatusWrite(this.isReachable() ? 'online' : 'offline'); + } + + /** + * Serialize the channel-callback status writes. They fire from un-awaited + * callbacks, and inside recreateChannel() a teardown's 'offline' and the + * fresh join's 'online' land ~100-300ms apart — unordered, 'offline' can win + * and leave a healthy device undispatchable until the next heartbeat. + * + * Not the single writer: updateHeartbeat, registerDevice and setOffline's + * subprocess write status directly, so this is not total ordering. + */ + private queueStatusWrite(status: 'online' | 'offline'): void { + // After teardown begins, setOffline() owns the final status write. + if (this.shuttingDown) { + console.debug(`[DEBUG] Status write '${status}' suppressed — teardown in progress`); + return; } + this.statusWriteChain = this.statusWriteChain + .then(() => (this.deviceId ? this.setOnlineStatus(this.deviceId, status) : undefined)) + .catch((e: any) => { + console.error('[DEBUG] Status write failed:', e?.message); + }); + } + + /** + * Heartbeat cadence for the tier this device is CURRENTLY in. Follows the + * capability flag (what the server actually tiers its sweep on), not the + * build — see LEGACY_HEARTBEAT_INTERVAL. + */ + private heartbeatIntervalMs(): number { + return this.transportCapableWritten === true + ? CAPABLE_HEARTBEAT_INTERVAL + : LEGACY_HEARTBEAT_INTERVAL; } async updateHeartbeat(deviceId: string) { if (!this.client) return; + // This write asserts status:'online' too, so it MUST respect the + // shutdown gate — otherwise a heartbeat firing (or in flight) as SIGINT + // lands can be applied after setOffline()'s subprocess write and leave + // an exited process marked online with a fresh last_seen, which for a + // capable device the sweep then cannot age out for a full tier window. + if (this.shuttingDown) { + console.debug('[DEBUG] Skipping heartbeat write — shutting down'); + return; + } try { + // Skip the write entirely when no transport is up. Bumping last_seen + // on a deaf device would keep its row perpetually young, so the + // server's staleness sweep could never age it out and correct a + // stale 'online' — and whenever presence is unavailable (kill + // switch, wedged socket) that stale row is exactly what dispatch + // falls back to. Staying silent lets the sweep do its job. + if (!this.isReachable()) { + console.debug('[DEBUG] Skipping heartbeat write — no transport joined; letting the row age out'); + return; + } + const { error } = await this.client .from('mcp_devices') - .update({ last_seen: new Date().toISOString() }) + .update({ last_seen: new Date().toISOString(), status: 'online' }) .eq('id', deviceId); if (error) { console.error('[DEBUG] Heartbeat update failed:', error.message); await captureRemote('remote_channel_heartbeat_error', { error }); + } else { + console.debug('[DEBUG] last_seen bookkeeping write ok:', deviceId); } - // console.log(`🔌 Heartbeat sent for device: ${deviceId}`); } catch (error: any) { console.error('Heartbeat failed:', error.message); await captureRemote('remote_channel_heartbeat_error', { error }); @@ -451,20 +1001,38 @@ export class RemoteChannel { startHeartbeat(deviceId: string) { console.debug('[DEBUG] Starting heartbeat for device:', deviceId); + this.heartbeatDeviceId = deviceId; this.connectionCheckInterval = setInterval(() => { this.checkConnectionHealth(); }, 10000); - // Update last_seen every 15 seconds - this.heartbeatInterval = setInterval(async () => { - await this.updateHeartbeat(deviceId); - }, HEARTBEAT_INTERVAL); - console.debug('[DEBUG] Heartbeat intervals set - connectionCheck: 10s, heartbeat: 15s'); + // Bookkeeping last_seen write. Self-rescheduling rather than a fixed + // setInterval so the cadence can follow the tier: a device that + // withdraws the capability flag must fall back to the fast legacy + // cadence immediately, not 30 minutes later. + this.scheduleHeartbeat(); + console.debug(`[DEBUG] Heartbeat started - connectionCheck: 10s, last_seen: ${this.heartbeatIntervalMs()}ms`); + } + + /** Arm (or re-arm) the last_seen timer at the current tier's cadence. */ + private scheduleHeartbeat(): void { + if (this.heartbeatInterval) { + clearTimeout(this.heartbeatInterval); + this.heartbeatInterval = null; + } + if (!this.heartbeatDeviceId) return; + this.heartbeatInterval = setTimeout(async () => { + if (this.heartbeatDeviceId) { + await this.updateHeartbeat(this.heartbeatDeviceId); + } + this.scheduleHeartbeat(); // re-read the tier every tick + }, this.heartbeatIntervalMs()); } stopHeartbeat() { + this.heartbeatDeviceId = null; if (this.heartbeatInterval) { - clearInterval(this.heartbeatInterval); + clearTimeout(this.heartbeatInterval); this.heartbeatInterval = null; } if (this.connectionCheckInterval) { @@ -510,14 +1078,27 @@ export class RemoteChannel { console.debug('[DEBUG] setOffline() initiating blocking update for device:', deviceId); try { - // Get current session for the subprocess - const { data: sessionData } = await this.client.auth.getSession(); - - if (!sessionData?.session?.access_token) { + // Session for the subprocess — bounded, with a cached fallback. + // getSession() is not a cheap read: it takes a lock (10s acquire + // timeout) and refreshes when the token is within ~90s of expiry, + // POSTing /token with its own ~30s retry budget. On a just-woken + // machine that outlasts device.ts's 5s force-exit, and then spawnSync + // never runs and the offline write is lost. The subprocess calls + // setSession() itself, so a slightly stale access_token is fine. + const live = await Promise.race([ + this.client.auth.getSession().then((r) => r.data?.session ?? null), + this.sleep(OFFLINE_SESSION_TIMEOUT_MS).then(() => null), + ]).catch(() => null); + const session = live ?? this.lastKnownSession; + + if (!session?.access_token) { console.error('❌ No valid session for offline update'); console.debug('[DEBUG] Session data missing or invalid'); return; } + if (!live) { + console.debug('[DEBUG] getSession() slow/failed — using last known session tokens'); + } // Get Supabase config from client const supabaseUrl = (this.client as any).supabaseUrl; @@ -547,8 +1128,8 @@ export class RemoteChannel { deviceId, supabaseUrl, supabaseKey, - sessionData.session.access_token, - sessionData.session.refresh_token || '' + session.access_token, + session.refresh_token || '' ], { timeout: 3000, stdio: 'pipe', // Capture output to prevent blocking @@ -587,8 +1168,35 @@ export class RemoteChannel { } async unsubscribe() { + // setOffline()'s durable write is the final word on `status` from here, + // so stop the heartbeat and the channel callbacks from racing it. The + // races that matter: a heartbeat tick firing as the signal arrives, and + // SIGINT during recreateChannel()'s backoff, where the later join's + // SUBSCRIBED would queue 'online' after the durable write. + this.shuttingDown = true; + // Budget against device.ts's 5s force-exit, worst case: + // 250 drain + 3x300 leave + 500 session + 3000 spawnSync = 4650ms. + // In practice only the untrack bound binds — removeChannel/unsubscribe + // set state='leaving' first, so their leave push resolves inline. + const LEAVE_BOUND_MS = 300; + // Drain queued channel-callback writes. Can't drain an in-flight + // heartbeat PATCH (it doesn't use the chain), but the gate above stops + // any new one and an in-flight one started earlier. + await Promise.race([this.statusWriteChain, this.sleep(250)]); + await Promise.race([this.removeLegacyChannel(), this.sleep(LEAVE_BOUND_MS)]); if (this.channel) { - await this.channel.unsubscribe(); + // Leave presence on the graceful path (socket close covers the abrupt + // one). Bounded: a half-open socket still reports 'joined', so the + // push just buffers and would settle via realtime-js's 10s timeout. + try { + await Promise.race([ + this.channel.untrack(), + this.sleep(LEAVE_BOUND_MS), + ]); + console.debug('[DEBUG] Presence untrack attempted (bounded)'); + } catch { /* best effort */ } + // Bounded as insurance; unsubscribe() resolves inline in practice. + await Promise.race([this.channel.unsubscribe(), this.sleep(LEAVE_BOUND_MS)]); this.channel = null; console.log('✓ Unsubscribed from tool call channel'); } diff --git a/src/remote-device/scripts/blocking-offline-update.js b/src/remote-device/scripts/blocking-offline-update.js index 40f03ca2..2f113f01 100644 --- a/src/remote-device/scripts/blocking-offline-update.js +++ b/src/remote-device/scripts/blocking-offline-update.js @@ -41,10 +41,12 @@ try { process.exit(3); // Exit code 3 for auth error } - // Update device status to offline + // Update device status to offline, stamping the exact shutdown moment so + // "last seen X ago" is precise for clean shutdowns (the periodic + // bookkeeping write only runs on the slow capable cadence). const { error } = await client .from('mcp_devices') - .update({ status: 'offline' }) + .update({ status: 'offline', last_seen: new Date().toISOString() }) .eq('id', deviceId); clearTimeout(timeoutHandle); diff --git a/src/utils/toolHistory.ts b/src/utils/toolHistory.ts index 09366744..6c9d5488 100644 --- a/src/utils/toolHistory.ts +++ b/src/utils/toolHistory.ts @@ -32,6 +32,25 @@ function formatLocalTimestamp(isoTimestamp: string): string { class ToolHistory { private history: ToolCallRecord[] = []; private readonly MAX_ENTRIES = 1000; + /** + * Cap on the output kept per entry. Entries hold the FULL ServerResult, and + * get_recent_tool_calls serialises them straight back out, so without a cap + * the history is unbounded in two compounding ways: + * + * - a single large output (a big read_file, a wide list_directory) makes + * every later history dump that includes it large too, and + * - any tool whose output happens to CONTAIN a history dump — e.g. + * `cat`-ing a file a previous dump was written to — nests the whole + * history inside itself, and each nesting level roughly doubles the JSON + * escaping. Observed 2026-07-27: 83KB of arguments on disk produced a + * 1.89MB in-memory dump this way. + * + * Excluding more tool names cannot fix that, because the nesting arrives + * through ordinary tools. Capping the stored output does, and a preview is + * all this history is for — it is a "what happened recently" aid, not a + * result cache. + */ + private readonly MAX_STORED_OUTPUT_BYTES = 4 * 1024; private readonly MAX_HISTORY_FILE_SIZE_BYTES = 5 * 1024 * 1024; // When the file exceeds the cap we trim it down to this target instead of // all the way to zero, so a single overflow doesn't cause every subsequent @@ -87,8 +106,14 @@ class ToolHistory { } } - // Keep only last 1000 entries - this.history = records.slice(-this.MAX_ENTRIES); + // Keep only last 1000 entries, and cap on the way IN as well as on the + // way out. Entries written before the cap existed are still on disk and + // are well under the whole-file trim threshold, so without this the cap + // does nothing for anyone upgrading with an existing history file — i.e. + // for everyone it was written for. + this.history = records + .slice(-this.MAX_ENTRIES) + .map(record => ({ ...record, output: this.capOutput(record.output) })); // If file is getting too large, trim it if (lines.length > this.MAX_ENTRIES * 2) { @@ -214,17 +239,44 @@ class ToolHistory { /** * Add a tool call to history */ + /** + * Replace an oversized output with a short marker. Keeps the record shape + * ({ content: [...] }) so readers and formatters need no special case. + */ + private capOutput(output: ServerResult): ServerResult { + let size: number; + try { + size = JSON.stringify(output)?.length ?? 0; + } catch { + // Circular or otherwise unserialisable — it could never be returned to a + // client anyway, so don't retain it. + size = Number.POSITIVE_INFINITY; + } + if (size <= this.MAX_STORED_OUTPUT_BYTES) return output; + + const shown = Number.isFinite(size) ? `${size} bytes` : 'unserialisable'; + return { + ...(output as any), + content: [ + { + type: 'text', + text: `[output omitted from history: ${shown}, over the ${this.MAX_STORED_OUTPUT_BYTES}-byte cap]`, + }, + ], + } as ServerResult; + } + addCall( - toolName: string, - args: any, - output: ServerResult, + toolName: string, + args: any, + output: ServerResult, duration?: number ): void { const record: ToolCallRecord = { timestamp: new Date().toISOString(), toolName, arguments: args, - output, + output: this.capOutput(output), duration }; diff --git a/test/test-line-count.js b/test/test-line-count.js index e467ffea..f6245eab 100644 --- a/test/test-line-count.js +++ b/test/test-line-count.js @@ -10,13 +10,25 @@ const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename); const TEST_DIR = join(__dirname, 'test_output'); -// Ensure test dir is allowed -await configManager.setValue('allowedDirectories', [TEST_DIR]); +// Saved before the test narrows allowedDirectories, restored in teardown(). +// Without this the test leaks its own directory into the user's real +// ~/.claude-server-commander/config.json and every later filesystem call — +// in this process or any other — is refused outside test_output. +let originalConfig = null; async function setup() { + originalConfig = await configManager.getConfig(); + // Ensure test dir is allowed + await configManager.setValue('allowedDirectories', [TEST_DIR]); await fs.mkdir(TEST_DIR, { recursive: true }); } +async function teardown() { + if (originalConfig) { + await configManager.updateConfig(originalConfig); + } +} + async function createTestFile(name, content) { const filePath = join(TEST_DIR, name); await fs.writeFile(filePath, content, 'utf8'); @@ -111,10 +123,19 @@ async function testLineCount() { } console.log(`\n${passed} passed, ${failed} failed out of ${passed + failed} tests`); - if (failed > 0) process.exit(1); + return failed; } -setup().then(testLineCount).catch(err => { +// teardown must run on the failure path too, or a failing assertion leaves the +// user's allowedDirectories pinned to test_output. +let exitCode = 0; +try { + await setup(); + exitCode = (await testLineCount()) > 0 ? 1 : 0; +} catch (err) { console.error('Test error:', err); - process.exit(1); -}); + exitCode = 1; +} finally { + await teardown(); +} +process.exit(exitCode); diff --git a/test/test-remote-channel-reconnect.js b/test/test-remote-channel-reconnect.js index fa939360..bb1faeb9 100644 --- a/test/test-remote-channel-reconnect.js +++ b/test/test-remote-channel-reconnect.js @@ -60,6 +60,18 @@ class FakeChannel { }); return this; } + // Presence (added with the Broadcast/Presence transport): the device track()s + // itself on SUBSCRIBED and untrack()s on a graceful unsubscribe. realtime-js + // RESOLVES these with a status string ('ok' | 'error' | 'timed out') rather + // than rejecting, so the fakes mirror that contract. + track() { + this.tracked = true; + return Promise.resolve('ok'); + } + untrack() { + this.tracked = false; + return Promise.resolve('ok'); + } unsubscribe() { this.state = 'leaving'; return Promise.resolve({ error: null }); @@ -154,6 +166,17 @@ function makeRemoteChannel() { rc._user = { id: 'user-1', email: 'tester@example.com' }; rc.onToolCall = () => {}; rc.deviceId = 'device-1'; + rc.deviceName = 'test-device'; + // recreateChannel() sleeps a jittered backoff before rebuilding so a fleet-wide + // event doesn't stampede reconnects. These tests are about WHETHER the wedge + // recovers, not how long it waits — stub the sleep so the suite stays + // sub-second, but record the requested delays so the formula can be asserted + // (see "reconnect backoff grows and stays bounded" below). + rc.sleptMs = []; + rc.sleep = (ms) => { + rc.sleptMs.push(ms); + return Promise.resolve(); + }; return { rc, client }; } @@ -325,6 +348,52 @@ async function main() { ); }); + // The jittered backoff exists so a fleet-wide event (server deploy, Supabase + // blip) doesn't stampede every device into reconnecting at the same instant. + // Assert the shape rather than exact values: it must GROW with consecutive + // attempts and stay BOUNDED so a device can't disappear for minutes. + await test('reconnect backoff grows with attempts and stays bounded', async () => { + const { rc, client } = makeRemoteChannel(); + await withQuietLogs(async () => { + await goHalfOpen(rc, client); + // Model a PERSISTENT outage: rebuilding the socket doesn't help, so every + // recreate fails and reconnectAttempt actually climbs. The default fake + // heals on rebuildSocket(), which meant every recreate SUCCEEDED, the + // counter reset to 0, and all samples came from the attempt-1 + // distribution — making a "grows" assertion a coin flip (~10% flake, + // measured over 30 runs) and never exercising the cap at all. + client.realtime.rebuildSocket = function () { + this.rebuilds++; + this.conn = { readyState: 1 }; + this.socketDead = true; // still dead after the rebuild + }; + client.realtime.socketDead = true; + for (let i = 0; i < 7; i++) { + await rc.recreateChannel(); + if (rc.channel) rc.channel.state = 'errored'; + } + }); + + assert.ok(rc.sleptMs.length >= 6, `expected several backoff sleeps, got ${rc.sleptMs.length}`); + // Formula: min(30_000, 1000 * 2**min(attempt,5)) * (0.5 + random()) + // -> hard ceiling is 30_000 * 1.5 = 45_000 ms. + assert.ok( + rc.sleptMs.every((ms) => ms > 0 && ms <= 45_000), + `every backoff must be positive and <= 45s: ${JSON.stringify(rc.sleptMs)}` + ); + // With the counter climbing, late attempts draw from a strictly higher + // range than the first: attempt 1 tops out at 3_000, attempt 5+ starts at + // 15_000 — so this cannot flake on jitter. + assert.ok( + rc.sleptMs[0] <= 3_000, + `first backoff should be the attempt-1 range: ${rc.sleptMs[0]}` + ); + assert.ok( + Math.max(...rc.sleptMs.slice(-2)) >= 15_000, + `late backoffs should reach the capped range: ${JSON.stringify(rc.sleptMs)}` + ); + }); + console.log( `\n${failures ? '🔴' : '✅'} remote-channel reconnect: ${failures} failing test(s).` ); diff --git a/test/test-remote-transport.js b/test/test-remote-transport.js new file mode 100644 index 00000000..d6be394a --- /dev/null +++ b/test/test-remote-transport.js @@ -0,0 +1,571 @@ +#!/usr/bin/env node + +/** + * Remote transport tests (Broadcast/Presence + legacy fallback). + * + * Sections: + * 1. Exactly-once execution under dual delivery + * 2. Doorbell routing and row fetch + * 3. Result write ordering + * 4. Heartbeat cadence tiers + * 5. Reachability and status writes + * 6. Capability withdrawal + * 7. Shutdown + * + * Run: npm run build && node test/test-remote-transport.js + */ + +import { MCPDevice } from '../dist/remote-device/device.js'; +import { RemoteChannel } from '../dist/remote-device/remote-channel.js'; + +// Server-side thresholds this device must fit inside. Hand-copied from +// remote-dc-mcp/src/server/constants.ts — the repos ship separately and nothing +// enforces the copy, so change both together. +const SERVER_LEGACY_OFFLINE_TIMEOUT_MS = 45 * 1000; +const SERVER_CAPABLE_OFFLINE_TIMEOUT_MS = 15 * 60 * 1000; + +const DEVICE_ID = 'device-1'; +const OTHER_DEVICE = 'device-2'; + +process.env.DESKTOP_COMMANDER_DISABLE_TELEMETRY = '1'; + +let failures = 0; +async function test(name, fn) { + try { + await fn(); + console.log(`✅ PASS ${name}`); + } catch (error) { + failures++; + console.error(`🔴 FAIL ${name}\n ${error.message}`); + } +} +function assert(condition, message) { + if (!condition) throw new Error(message); +} + +const makeChannelState = (state) => ({ state }); + +// Captured before any test runs: the heartbeat re-arm test monkeypatches +// globalThis.setTimeout to never fire, and the fake client's write completion +// must not silently depend on that. +const realSetTimeout = globalThis.setTimeout; + +/** MCPDevice with the network and desktop edges stubbed. */ +function makeDevice({ claimResults = [] } = {}) { + const device = new MCPDevice(); + const executed = []; + const claims = [...claimResults]; + + device.deviceId = DEVICE_ID; + device.desktop = { + callClientTool: async (toolName, args) => { + executed.push({ toolName, args }); + return { content: [{ type: 'text', text: 'ok' }] }; + }, + }; + device.remoteChannel = { + // Default: first delivery claims, later ones lose. Override to model a + // transient DB error, which makes the claim return true (fail open). + markCallExecuting: async () => (claims.length ? claims.shift() : true), + updateCallResult: async () => {}, + notifyResult: async () => {}, + }; + return { device, executed }; +} + +/** + * Supabase client fake covering both shapes the device uses: + * `update(...).eq(...)` (awaited) and `select(...).eq(...).maybeSingle()`. + * Records every mcp_devices write in `writes`. + */ +function makeFakeClient({ row = null, failFetches = 0, writeLatencies = [] } = {}) { + const writes = []; + // Recorded when a write COMPLETES, not when it is issued. `writes` alone + // cannot test ordering: setOnlineStatus evaluates .update() synchronously + // before its only await, so issue order holds with or without the + // statusWriteChain serialisation. + const completions = []; + let fetchAttempts = 0; + let pendingWrite = null; + + const result = () => { + const p = Promise.resolve({ data: null, error: null }); + p.maybeSingle = async () => { + fetchAttempts++; + if (fetchAttempts <= failFetches) { + return { data: null, error: { message: 'fetch failed' } }; + } + return { data: row, error: null }; + }; + p.eq = () => result(); + p.select = () => result(); + return p; + }; + + const chain = { + update: (payload) => { + writes.push(payload); + // Per-write completion latency, so a test can make an earlier write land + // LATER than a later one — the only way to observe serialisation. + pendingWrite = { + payload, + delay: writeLatencies.length ? writeLatencies.shift() : 0, + }; + return chain; + }, + select: () => chain, + insert: () => chain, + eq: () => { + if (!pendingWrite) return result(); + const { payload, delay } = pendingWrite; + pendingWrite = null; + const p = new Promise((resolve) => { + const settle = () => { + completions.push(payload); + resolve({ data: null, error: null }); + }; + // Only defer when a test actually asked for latency, so every other + // test keeps the original resolve-immediately semantics. + if (delay > 0) realSetTimeout(settle, delay); + else settle(); + }); + // markCallExecuting chains .eq().eq().select() off a single update(), so + // this must stay chainable exactly like result() does — returning a bare + // promise leaves that chain hanging forever. + p.eq = () => p; + p.select = () => p; + p.maybeSingle = async () => ({ data: null, error: null }); + return p; + }, + }; + + return { + writes, + completions, + attempts: () => fetchAttempts, + // Required by recreateChannel(); without them it dies on a TypeError before + // reaching anything the recreate tests stub. + removeChannel: () => Promise.resolve('ok'), + // isDisconnecting models a client that has already settled, so + // waitForSocketSettled() polls once and returns. NOTE: this fake has no + // real connection state, so it cannot observe whether a new socket was + // actually dialled — the recreate tests verify sequencing, not transport. + realtime: { disconnect: () => Promise.resolve(), isDisconnecting: () => false }, + from: () => chain, + }; +} + +function makeRemoteChannel(opts = {}) { + const rc = new RemoteChannel(); + const client = makeFakeClient(opts); + rc.client = client; // private in TS, plain property at runtime + rc._user = { id: 'user-1', email: 'tester@example.com' }; + rc.deviceId = DEVICE_ID; + rc.deviceName = 'test-device'; + rc.onToolCall = () => {}; + return { rc, client }; +} + +const payloadFor = (id, deviceId = DEVICE_ID) => ({ + new: { + id, + tool_name: 'start_process', + tool_args: { command: 'echo hi' }, + device_id: deviceId, + metadata: {}, + }, +}); + +// --- 1. Exactly-once under dual delivery ------------------------------------ +// Both transports deliver every call during the transition. The DB claim fails +// OPEN on a transient error, so the in-memory guard is the real guarantee. + +await test('dual delivery of the same call executes the tool exactly once', async () => { + const { device, executed } = makeDevice(); + await device.handleNewToolCall(payloadFor('call-a')); + await device.handleNewToolCall(payloadFor('call-a')); // the other transport + assert(executed.length === 1, `expected 1 execution, got ${executed.length}`); +}); + +await test('exactly-once holds when the DB claim fails OPEN for both deliveries', async () => { + const { device, executed } = makeDevice({ claimResults: [true, true] }); + await device.handleNewToolCall(payloadFor('call-b')); + await device.handleNewToolCall(payloadFor('call-b')); + assert(executed.length === 1, `fail-open claim double-executed: ${executed.length} runs`); +}); + +await test('a lost DB claim (another process won) skips execution', async () => { + const { device, executed } = makeDevice({ claimResults: [false] }); + await device.handleNewToolCall(payloadFor('call-c')); + assert(executed.length === 0, `expected no execution, got ${executed.length}`); +}); + +await test('calls for another device are ignored and do not poison the dedupe set', async () => { + const { device, executed } = makeDevice(); + await device.handleNewToolCall(payloadFor('call-d', OTHER_DEVICE)); + assert(executed.length === 0, 'must not execute another device call'); + // The device filter runs before dedupe, so our own copy must still run. + await device.handleNewToolCall(payloadFor('call-d')); + assert(executed.length === 1, 'a mismatched delivery must not suppress our own'); +}); + +await test('the seen-call-id set stays bounded', async () => { + const { device } = makeDevice(); + for (let i = 0; i < 250; i++) await device.handleNewToolCall(payloadFor(`bulk-${i}`)); + assert(device.seenCallIds.size <= 100, `set grew to ${device.seenCallIds.size}`); +}); + +// --- 2. Doorbell routing ---------------------------------------------------- + +await test('doorbell for another device is ignored without fetching', async () => { + const { rc, client } = makeRemoteChannel(); + await rc.onDoorbell({ call_id: 'x', device_id: OTHER_DEVICE }); + assert(client.attempts() === 0, 'must not even fetch the row'); +}); + +await test('doorbell delivers a pending row through the shared handler', async () => { + const row = { id: 'x', status: 'pending', tool_name: 'start_process' }; + const { rc } = makeRemoteChannel({ row }); + const delivered = []; + rc.onToolCall = (p) => delivered.push(p); + await rc.onDoorbell({ call_id: 'x', device_id: DEVICE_ID }); + assert(delivered.length === 1, 'expected one delivery'); + assert(delivered[0].new === row, 'must pass the fetched row as {new: row}'); +}); + +await test('doorbell for an already-claimed row does not re-deliver', async () => { + const { rc } = makeRemoteChannel({ row: { id: 'x', status: 'executing' } }); + const delivered = []; + rc.onToolCall = (p) => delivered.push(p); + await rc.onDoorbell({ call_id: 'x', device_id: DEVICE_ID }); + assert(delivered.length === 0, 'non-pending rows must not be re-delivered'); +}); + +await test('doorbell row fetch retries a transient failure', async () => { + const { rc, client } = makeRemoteChannel({ row: { id: 'x', status: 'pending' }, failFetches: 2 }); + const delivered = []; + rc.onToolCall = (p) => delivered.push(p); + rc.sleep = () => Promise.resolve(); + await rc.onDoorbell({ call_id: 'x', device_id: DEVICE_ID }); + assert(client.attempts() === 3, `expected 3 attempts, got ${client.attempts()}`); + assert(delivered.length === 1, 'should deliver after the retry succeeds'); +}); + +await test('doorbell with a missing row is a no-op', async () => { + const { rc } = makeRemoteChannel({ row: null }); + const delivered = []; + rc.onToolCall = (p) => delivered.push(p); + await rc.onDoorbell({ call_id: 'gone', device_id: DEVICE_ID }); + assert(delivered.length === 0, 'missing row must not deliver'); +}); + +// --- 2b. Handler rejections are observed ------------------------------------- +// handleNewToolCall is async and its promise is discarded at both call sites, so +// a rejection would be unhandled and terminate the device process. + +await test('a rejecting tool-call handler does not produce an unhandled rejection', async () => { + const { rc } = makeRemoteChannel({ row: { id: 'x', status: 'pending' } }); + rc.onToolCall = async () => { throw new Error('handler blew up'); }; + + const unhandled = []; + const onUnhandled = (e) => unhandled.push(e); + process.on('unhandledRejection', onUnhandled); + try { + await rc.onDoorbell({ call_id: 'x', device_id: DEVICE_ID }); + await new Promise((r) => setImmediate(r)); // let a rejection surface + } finally { + process.off('unhandledRejection', onUnhandled); + } + assert(unhandled.length === 0, `unhandled rejection escaped: ${unhandled[0]?.message}`); +}); + +await test('a synchronously throwing handler is contained too', async () => { + const { rc } = makeRemoteChannel({ row: { id: 'x', status: 'pending' } }); + rc.onToolCall = () => { throw new Error('sync throw'); }; + await rc.onDoorbell({ call_id: 'x', device_id: DEVICE_ID }); // must not reject +}); + +// --- 3. Result ordering ----------------------------------------------------- +// The server fetches the row by id when the doorbell arrives, so the write must +// land first or it sees a non-terminal row and waits for the recovery poll. + +await test('the result row is written BEFORE the doorbell is rung', async () => { + const order = []; + const { device, executed } = makeDevice(); + device.remoteChannel.updateCallResult = async () => { order.push('write'); }; + device.remoteChannel.notifyResult = async () => { order.push('doorbell'); }; + await device.handleNewToolCall(payloadFor('call-order')); + assert(executed.length === 1, 'tool should have run'); + assert(order.join(',') === 'write,doorbell', `expected write,doorbell — got ${order.join(',')}`); +}); + +// --- 4. Heartbeat cadence tiers --------------------------------------------- +// The server tiers its offline sweep on the capability FLAG, not the app +// version, and the flag is only set once presence is proven. So an unproven +// device is judged by the 45s legacy rule and must heartbeat fast enough to +// survive it, or it is swept offline while its legacy channel still works. + +await test('legacy tier heartbeats inside the server 45s sweep threshold', async () => { + const { rc } = makeRemoteChannel(); + rc.transportCapableWritten = null; // never written = legacy tier + const cadence = rc.heartbeatIntervalMs(); + assert( + cadence * 2 < SERVER_LEGACY_OFFLINE_TIMEOUT_MS, + `legacy cadence ${cadence}ms must allow >=2 writes inside ${SERVER_LEGACY_OFFLINE_TIMEOUT_MS}ms` + ); + rc.transportCapableWritten = false; // explicitly withdrawn + assert(rc.heartbeatIntervalMs() === cadence, 'a withdrawn capability uses the fast cadence'); +}); + +await test('capable tier heartbeats inside the server capable sweep threshold', async () => { + const { rc } = makeRemoteChannel(); + rc.transportCapableWritten = true; + const cadence = rc.heartbeatIntervalMs(); + assert( + cadence * 2 < SERVER_CAPABLE_OFFLINE_TIMEOUT_MS, + `capable cadence ${cadence}ms must allow >=2 writes inside ${SERVER_CAPABLE_OFFLINE_TIMEOUT_MS}ms` + ); + assert(cadence > SERVER_LEGACY_OFFLINE_TIMEOUT_MS, 'capable cadence is the slow one'); +}); + +await test('withdrawing the capability re-arms the heartbeat at the fast cadence', async () => { + const { rc } = makeRemoteChannel(); + rc.transportCapableWritten = true; + rc.channel = makeChannelState('joined'); + rc.startHeartbeat(DEVICE_ID); + try { + const armed = []; + const realSetTimeout = globalThis.setTimeout; + globalThis.setTimeout = (fn, ms) => { + armed.push(ms); + return realSetTimeout(() => {}, 0); // never fire + }; + try { + await rc.setTransportCapable(false); + } finally { + globalThis.setTimeout = realSetTimeout; + } + assert(armed.length > 0, 'withdrawing must re-arm the heartbeat timer'); + assert( + armed[armed.length - 1] * 2 < SERVER_LEGACY_OFFLINE_TIMEOUT_MS, + `re-armed cadence ${armed[armed.length - 1]}ms must fit the 45s sweep` + ); + } finally { + rc.stopHeartbeat(); + } +}); + +await test('stopHeartbeat halts the self-rescheduling timer', async () => { + const { rc } = makeRemoteChannel(); + rc.channel = makeChannelState('joined'); + rc.startHeartbeat(DEVICE_ID); + rc.stopHeartbeat(); + assert(rc.heartbeatInterval === null, 'timer handle cleared'); + assert(rc.heartbeatDeviceId === null, 'device id cleared so re-arm is inert'); + rc.scheduleHeartbeat(); // must be inert after stop + assert(rc.heartbeatInterval === null, 'scheduleHeartbeat after stop must not re-arm'); +}); + +// --- 5. Reachability and status writes -------------------------------------- +// `status` is what the server's device selection filters on, and it is +// transport-agnostic — so it must follow "reachable by ANY transport", never the +// private channel alone. + +await test('heartbeat writes when only the legacy channel is joined', async () => { + const { rc, client } = makeRemoteChannel(); + rc.channel = null; // private channel never joined + rc.legacyChannel = makeChannelState('joined'); // fallback is up + await rc.updateHeartbeat(DEVICE_ID); + assert(client.writes.length === 1, 'legacy-only reachable device must still write last_seen'); + assert(client.writes[0].last_seen, 'write should bump last_seen'); + assert(client.writes[0].status === 'online', 'write should assert online'); +}); + +await test('heartbeat stays silent when no transport is joined', async () => { + const { rc, client } = makeRemoteChannel(); + rc.channel = makeChannelState('errored'); + rc.legacyChannel = makeChannelState('closed'); + await rc.updateHeartbeat(DEVICE_ID); + assert(client.writes.length === 0, 'a deaf device must let the sweep age its row out'); +}); + +await test('heartbeat writes when the private channel is joined', async () => { + const { rc, client } = makeRemoteChannel(); + rc.channel = makeChannelState('joined'); + rc.legacyChannel = null; + await rc.updateHeartbeat(DEVICE_ID); + assert(client.writes.length === 1, 'private channel joined = reachable'); +}); + +await test('private-channel failure keeps status online while legacy is joined', async () => { + const { rc, client } = makeRemoteChannel(); + rc.channel = makeChannelState('errored'); + rc.legacyChannel = makeChannelState('joined'); + rc.syncReachabilityStatus(); + await rc.statusWriteChain; + assert(client.writes.length === 1, 'one status write'); + assert(client.writes[0].status === 'online', 'still reachable via legacy = online'); +}); + +await test('status goes offline when no transport is joined', async () => { + const { rc, client } = makeRemoteChannel(); + rc.channel = makeChannelState('errored'); + rc.legacyChannel = makeChannelState('closed'); + rc.syncReachabilityStatus(); + await rc.statusWriteChain; + assert(client.writes[0].status === 'offline', 'genuinely deaf device goes offline'); +}); + +await test('concurrent status writes stay ordered', async () => { + // The first write completes AFTER the second is issued. Without the + // statusWriteChain serialisation the teardown's 'offline' then lands at the + // DB after the re-join's 'online', leaving a healthy device undispatchable + // until the next heartbeat (up to 5 min on the capable tier). Assert on + // `completions`, not `writes` — see makeFakeClient. + const { rc, client } = makeRemoteChannel({ writeLatencies: [20, 0] }); + rc.channel = makeChannelState('joined'); + rc.queueStatusWrite('offline'); // teardown + rc.queueStatusWrite('online'); // immediate re-join + await rc.statusWriteChain; + // Let the deferred first write land even when the implementation does NOT + // serialise, so this fails on ORDER — the actual bug — rather than on timing. + const deadline = Date.now() + 500; + while (client.completions.length < 2 && Date.now() < deadline) { + await new Promise((r) => realSetTimeout(r, 5)); + } + assert(client.writes.length === 2, 'both writes issued'); + assert(client.completions.length === 2, 'both writes completed'); + assert( + client.completions.map((w) => w.status).join(',') === 'offline,online', + `writes must COMPLETE in issue order so the join wins, got ${client.completions + .map((w) => w.status) + .join(',')}` + ); +}); + +// --- 6. Capability withdrawal ----------------------------------------------- +// For a flagged device the server treats absent presence as authoritative +// offline and applies that overlay before selection, overriding `status`. So a +// device that cannot join the private channel must stop advertising the flag or +// it is undispatchable however healthy its legacy channel is. + +await test('sustained recreate failure withdraws the transport capability', async () => { + const { rc, client } = makeRemoteChannel(); + rc.transportCapableWritten = true; // previously proven + rc.legacyChannel = makeChannelState('joined'); + rc.sleep = () => Promise.resolve(); // skip the jittered backoff + const order = []; + rc.createChannel = () => { + order.push('private'); + return Promise.reject(new Error('Unauthorized')); + }; + rc.createLegacyChannel = () => { order.push('legacy'); }; + rc.channel = makeChannelState('errored'); + + for (let i = 0; i < 3; i++) await rc.recreateChannel(); + + // Also proves the recreate reached createChannel rather than dying earlier, + // and that the legacy net is rebuilt first and on every attempt. + assert( + order.slice(0, 2).join(',') === 'legacy,private', + `legacy net must be rebuilt first: ${JSON.stringify(order)}` + ); + assert( + order.filter((o) => o === 'legacy').length === 3, + 'legacy net must be rebuilt on every recreate attempt' + ); + assert(rc.transportCapableWritten === false, 'capability must be withdrawn'); + const capWrite = client.writes.find((w) => w.capabilities); + assert(capWrite, 'a capabilities write should have been issued'); + assert( + capWrite.capabilities.transport_broadcast_v1 === undefined, + 'the withdrawn payload must not carry the flag' + ); + assert(capWrite.capabilities.app_version !== undefined, 'app_version must survive'); +}); + +await test('a single recreate failure does not withdraw the capability', async () => { + const { rc } = makeRemoteChannel(); + rc.transportCapableWritten = true; + rc.legacyChannel = makeChannelState('joined'); + rc.sleep = () => Promise.resolve(); + rc.createChannel = () => Promise.reject(new Error('transient')); + rc.createLegacyChannel = () => {}; + rc.channel = makeChannelState('errored'); + await rc.recreateChannel(); + assert(rc.transportCapableWritten === true, 'one blip must not withdraw'); +}); + +await test('a hanging capability withdrawal cannot pin the recreate guard', async () => { + const { rc } = makeRemoteChannel(); + rc.transportCapableWritten = true; + rc.legacyChannel = makeChannelState('joined'); + rc.sleep = () => Promise.resolve(); + rc.createChannel = () => Promise.reject(new Error('Unauthorized')); + rc.createLegacyChannel = () => {}; + rc.channel = makeChannelState('errored'); + rc.setTransportCapable = () => new Promise(() => {}); // never settles + const realWithTimeout = rc.withTimeout.bind(rc); + rc.withTimeout = (op, _ms, name) => realWithTimeout(op, 20, name); + + for (let i = 0; i < 3; i++) await rc.recreateChannel(); + + assert(rc.isRecreatingChannel === false, 'the guard must be released even if the write hangs'); +}); + +// --- 7. Shutdown ------------------------------------------------------------ +// setOffline()'s durable write is the final word on status, so nothing may race +// or outlast it — device.ts force-exits 5s after the signal. + +await test('status writes are suppressed once shutting down', async () => { + const { rc, client } = makeRemoteChannel(); + rc.channel = makeChannelState('joined'); + rc.legacyChannel = makeChannelState('joined'); + rc.shuttingDown = true; + rc.syncReachabilityStatus(); + rc.queueStatusWrite('online'); + await rc.statusWriteChain; + assert(client.writes.length === 0, 'no status write after teardown starts'); +}); + +await test('heartbeat is suppressed once shutting down', async () => { + const { rc, client } = makeRemoteChannel(); + rc.channel = makeChannelState('joined'); + rc.shuttingDown = true; + await rc.updateHeartbeat(DEVICE_ID); + assert(client.writes.length === 0, 'no heartbeat write during shutdown'); +}); + +await test('unsubscribe is bounded and still clears the channel', async () => { + const { rc } = makeRemoteChannel(); + rc.legacyChannel = null; + rc.channel = { + state: 'joined', + untrack: () => new Promise(() => {}), // never settles + unsubscribe: () => new Promise(() => {}), // half-open socket + }; + rc.sleep = () => Promise.resolve(); + await rc.unsubscribe(); + assert(rc.channel === null, 'must give up on a wedged leave push and move on'); + assert(rc.shuttingDown === true, 'teardown flag set'); +}); + +await test('setOffline does not hang when getSession stalls', async () => { + const { rc } = makeRemoteChannel(); + rc.client.auth = { getSession: () => new Promise(() => {}) }; // never settles + rc.lastKnownSession = { access_token: 'cached-at', refresh_token: 'cached-rt' }; + // Missing config makes setOffline return right after the session step, so no + // subprocess is spawned. + rc.client.supabaseUrl = undefined; + rc.client.supabaseKey = undefined; + + let settled = false; + await Promise.race([ + rc.setOffline(DEVICE_ID).then(() => { settled = true; }), + new Promise((r) => setTimeout(r, 3000)), + ]); + assert(settled, 'setOffline must settle rather than block the shutdown path'); +}); + +console.log(`\n${failures ? '🔴' : '✅'} remote transport: ${failures} failing test(s).`); +process.exit(failures ? 1 : 0); diff --git a/test/test-strip-null-bytes.js b/test/test-strip-null-bytes.js new file mode 100644 index 00000000..9d1e8eba --- /dev/null +++ b/test/test-strip-null-bytes.js @@ -0,0 +1,77 @@ +#!/usr/bin/env node + +/** + * Regression tests for stripNullBytes (remote-device/remote-channel.ts). + * + * Postgres rejects a NUL (U+0000) in BOTH jsonb and text with 22P05, which + * stranded remote calls at 'executing' until the 5-minute timeout. The first + * implementation serialized to JSON and regex-replaced the ESCAPE TEXT, which + * had two failure modes found in review (2026-07-24) — both covered here: + * + * 1. SILENT CORRUPTION: content containing the six literal characters + * backslash-u-0-0-0-0 (e.g. reading a source file with that escape in it) + * had those characters deleted from the tool result, undetected. + * 2. THROW: a doubled-backslash form produced invalid JSON, so the write threw + * and the call was reported failed even though the tool had succeeded. + * + * Run: npm run build && node test/test-strip-null-bytes.js + */ + +import { stripNullBytes } from '../dist/remote-device/remote-channel.js'; + +const NUL = String.fromCharCode(0); +const BACKSLASH = String.fromCharCode(92); +let failures = 0; + +function check(name, actual, expected) { + const ok = JSON.stringify(actual) === JSON.stringify(expected); + if (ok) { + console.log(`✅ PASS ${name}`); + } else { + failures++; + console.error(`❌ FAIL ${name}\n expected: ${JSON.stringify(expected)}\n actual: ${JSON.stringify(actual)}`); + } +} + +// --- the bug it exists to fix: real NUL characters are removed --------------- +check('strips a real NUL from a string', stripNullBytes(`a${NUL}b`), 'ab'); +check( + 'strips a real NUL nested in a tool result', + stripNullBytes({ content: [{ type: 'text', text: `START${NUL}END` }] }), + { content: [{ type: 'text', text: 'STARTEND' }] } +); +check('strips real NULs from object keys', stripNullBytes({ [`k${NUL}`]: 1 }), { k: 1 }); + +// --- regression 1: literal escape TEXT must be preserved, not corrupted ------ +const literalEscape = `const re = /${BACKSLASH}u0000/g;`; +check('preserves literal backslash-u0000 text (no silent corruption)', + stripNullBytes({ text: literalEscape }), { text: literalEscape }); + +// --- regression 2: doubled backslash must not throw -------------------------- +const doubled = `x${BACKSLASH}${BACKSLASH}u0000y`; +let threw = null; +try { + check('handles doubled backslash without throwing', stripNullBytes({ text: doubled }), { text: doubled }); +} catch (e) { + threw = e; + failures++; + console.error(`❌ FAIL handles doubled backslash without throwing — threw: ${e.message}`); +} + +// --- structure / passthrough ------------------------------------------------ +check('leaves clean content untouched', stripNullBytes({ a: 'clean', b: [1, 2] }), { a: 'clean', b: [1, 2] }); +check('passes through null', stripNullBytes(null), null); +check('passes through numbers/booleans', stripNullBytes({ n: 42, b: true }), { n: 42, b: true }); +check('handles arrays of strings with NUL', stripNullBytes([`p${NUL}q`, 'clean']), ['pq', 'clean']); + +// --- the property that actually matters for Postgres ------------------------ +const out = JSON.stringify(stripNullBytes({ x: `${NUL}${NUL}`, y: literalEscape })); +if (out.includes(NUL)) { + failures++; + console.error('❌ FAIL output still contains a raw NUL character'); +} else { + console.log('✅ PASS output contains no raw NUL characters'); +} + +console.log(`\n${failures === 0 ? '✅' : '❌'} strip-null-bytes: ${failures} failing test(s).`); +process.exitCode = failures === 0 ? 0 : 1;