fix(cli): replace unsafe WebSocket double cast with typed adapter

diff --git a/ui/cli/src/client/lifecycle.ts b/ui/cli/src/client/lifecycle.ts
index c7a7b24398d6d5e3ce0f1dc287f6da016edc621f..28ef4f202d8640394e8f8438e0457ef47f8c1a14 100644 (file)
--- a/ui/cli/src/client/lifecycle.ts
+++ b/ui/cli/src/client/lifecycle.ts
@@ -50,6 +50,9 @@ export const executeCommand = async (options: ExecuteOptions): Promise<void> =>
   activeSpinner = spinner ?? undefined
   activeClient = client
 
+  const budget = timeoutMs ?? UI_WEBSOCKET_REQUEST_TIMEOUT_MS
+  const startTime = Date.now()
+
   let connectTimeoutId: ReturnType<typeof setTimeout> | undefined
   try {
     const connectPromise = client.connect()
@@ -59,7 +62,7 @@ export const executeCommand = async (options: ExecuteOptions): Promise<void> =>
       new Promise<never>((_resolve, reject) => {
         connectTimeoutId = setTimeout(() => {
           reject(new Error(`Connection to ${url} timed out`))
-        }, timeoutMs ?? UI_WEBSOCKET_REQUEST_TIMEOUT_MS)
+        }, budget)
       }),
     ])
   } catch (error: unknown) {
@@ -70,11 +73,18 @@ export const executeCommand = async (options: ExecuteOptions): Promise<void> =>
     clearTimeout(connectTimeoutId)
   }
 
+  const remaining = budget - (Date.now() - startTime)
+  if (remaining <= 0) {
+    spinner?.fail()
+    client.disconnect()
+    throw new ConnectionError(url, new Error('Connection consumed entire timeout budget'))
+  }
+
   try {
     if (spinner != null) {
       spinner.text = `Sending ${procedureName}...`
     }
-    const response: ResponsePayload = await client.sendRequest(procedureName, payload)
+    const response: ResponsePayload = await client.sendRequest(procedureName, payload, remaining)
     spinner?.stop()
     formatter.output(response)
   } catch (error: unknown) {

diff --git a/ui/cli/src/client/ws-adapter.ts b/ui/cli/src/client/ws-adapter.ts
new file mode 100644 (file)
index 0000000..6a8cea0
--- /dev/null
+++ b/ui/cli/src/client/ws-adapter.ts
@@ -0,0 +1,101 @@
+import { Buffer } from 'node:buffer'
+
+import type { WebSocketLike } from 'ui-common'
+import { WebSocketReadyState } from 'ui-common'
+import { WebSocket as WsWebSocket } from 'ws'
+
+const toDataString = (data: WsWebSocket.Data): string => {
+  if (Buffer.isBuffer(data)) {
+    return data.toString('utf-8')
+  }
+  if (data instanceof ArrayBuffer) {
+    return Buffer.from(data).toString('utf-8')
+  }
+  if (Array.isArray(data)) {
+    return Buffer.concat(data as Buffer[]).toString('utf-8')
+  }
+  return data
+}
+
+export const createWsAdapter = (ws: WsWebSocket): WebSocketLike => {
+  let onmessageCallback: ((event: { data: string }) => void) | null = null
+  let onerrorCallback: ((event: { error: unknown; message: string }) => void) | null = null
+  let oncloseCallback: ((event: { code: number; reason: string }) => void) | null = null
+  let onopenCallback: (() => void) | null = null
+
+  ws.onmessage = (event) => {
+    if (onmessageCallback != null) {
+      const data = toDataString(event.data)
+      onmessageCallback({ data })
+    }
+  }
+
+  ws.onerror = (event) => {
+    if (onerrorCallback != null) {
+      let error: Error
+      let message: string
+      if (event instanceof Error) {
+        error = event
+        message = event.message
+      } else {
+        message = typeof event === 'string' ? event : 'Unknown error'
+        error = new Error(message)
+      }
+      onerrorCallback({ error, message })
+    }
+  }
+
+  ws.onclose = (event) => {
+    if (oncloseCallback != null) {
+      oncloseCallback({ code: event.code, reason: event.reason })
+    }
+  }
+
+  ws.onopen = () => {
+    if (onopenCallback != null) {
+      onopenCallback()
+    }
+  }
+
+  return {
+    get onclose (): ((event: { code: number; reason: string }) => void) | null {
+      return oncloseCallback
+    },
+    set onclose (callback: ((event: { code: number; reason: string }) => void) | null) {
+      oncloseCallback = callback
+    },
+
+    get onerror (): ((event: { error: unknown; message: string }) => void) | null {
+      return onerrorCallback
+    },
+    set onerror (callback: ((event: { error: unknown; message: string }) => void) | null) {
+      onerrorCallback = callback
+    },
+
+    get onmessage (): ((event: { data: string }) => void) | null {
+      return onmessageCallback
+    },
+    set onmessage (callback: ((event: { data: string }) => void) | null) {
+      onmessageCallback = callback
+    },
+
+    get onopen (): (() => void) | null {
+      return onopenCallback
+    },
+    set onopen (callback: (() => void) | null) {
+      onopenCallback = callback
+    },
+
+    close (code?: number, reason?: string): void {
+      ws.close(code, reason)
+    },
+
+    get readyState (): WebSocketReadyState {
+      return ws.readyState as WebSocketReadyState
+    },
+
+    send (data: string): void {
+      ws.send(data)
+    },
+  }
+}

diff --git a/ui/cli/tests/ws-adapter.test.ts b/ui/cli/tests/ws-adapter.test.ts
new file mode 100644 (file)
index 0000000..106f815
--- /dev/null
+++ b/ui/cli/tests/ws-adapter.test.ts
@@ -0,0 +1,349 @@
+import type { WebSocket } from 'ws'
+
+import assert from 'node:assert'
+import { Buffer } from 'node:buffer'
+import { describe, it } from 'node:test'
+import { WebSocketReadyState } from 'ui-common'
+
+import { createWsAdapter } from '../src/client/ws-adapter.js'
+
+interface MockWs {
+  close: (code?: number, reason?: string) => void
+  onclose: ((event: { code: number; reason: string }) => void) | null
+  onerror: ((event: unknown) => void) | null
+  onmessage: ((event: { data: unknown }) => void) | null
+  onopen: (() => void) | null
+  readyState: WebSocketReadyState
+  send: (data: string) => void
+}
+
+await describe('WS Adapter', async () => {
+  await it('should convert Buffer data to string in onmessage', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    let receivedData: string | undefined
+    adapter.onmessage = (event) => {
+      receivedData = event.data
+    }
+
+    const bufferData = Buffer.from('hello', 'utf-8')
+    mockWs.onmessage?.({ data: bufferData })
+
+    assert.strictEqual(receivedData, 'hello')
+  })
+
+  await it('should convert ArrayBuffer data to string in onmessage', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    let receivedData: string | undefined
+    adapter.onmessage = (event) => {
+      receivedData = event.data
+    }
+
+    const arrayBuffer = new TextEncoder().encode('world').buffer
+    mockWs.onmessage?.({ data: arrayBuffer })
+
+    assert.strictEqual(receivedData, 'world')
+  })
+
+  await it('should convert Buffer[] data to string in onmessage', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    let receivedData: string | undefined
+    adapter.onmessage = (event) => {
+      receivedData = event.data
+    }
+
+    const bufferArray = [Buffer.from('hello'), Buffer.from(' '), Buffer.from('world')]
+    mockWs.onmessage?.({ data: bufferArray })
+
+    assert.strictEqual(receivedData, 'hello world')
+  })
+
+  await it('should pass through string data in onmessage', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    let receivedData: string | undefined
+    adapter.onmessage = (event) => {
+      receivedData = event.data
+    }
+
+    mockWs.onmessage?.({ data: 'already a string' })
+
+    assert.strictEqual(receivedData, 'already a string')
+  })
+
+  await it('should delegate readyState getter to ws', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.CONNECTING,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    assert.strictEqual(adapter.readyState, WebSocketReadyState.CONNECTING)
+  })
+
+  await it('should delegate send() to ws', () => {
+    let sentData: string | undefined
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: (data: string) => {
+        sentData = data
+      },
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+    adapter.send('test message')
+
+    assert.strictEqual(sentData, 'test message')
+  })
+
+  await it('should delegate close() to ws', () => {
+    let closeCode: number | undefined
+    let closeReason: string | undefined
+    const mockWs: MockWs = {
+      close: (code?: number, reason?: string) => {
+        closeCode = code
+        closeReason = reason
+      },
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+    adapter.close(1000, 'normal closure')
+
+    assert.strictEqual(closeCode, 1000)
+    assert.strictEqual(closeReason, 'normal closure')
+  })
+
+  await it('should forward onerror event with error shape', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    let receivedError: unknown
+    let receivedMessage: string | undefined
+    adapter.onerror = (event) => {
+      receivedError = event.error
+      receivedMessage = event.message
+    }
+
+    const testError = new Error('connection failed')
+    mockWs.onerror?.(testError)
+
+    assert.ok(receivedError instanceof Error)
+    // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
+    const error = receivedError as Error
+    assert.strictEqual(error.message, 'connection failed')
+    assert.strictEqual(receivedMessage, 'connection failed')
+  })
+
+  await it('should forward onclose event with code and reason', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.CLOSED,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    let receivedCode: number | undefined
+    let receivedReason: string | undefined
+    adapter.onclose = (event) => {
+      receivedCode = event.code
+      receivedReason = event.reason
+    }
+
+    mockWs.onclose?.({ code: 1000, reason: 'normal closure' })
+
+    assert.strictEqual(receivedCode, 1000)
+    assert.strictEqual(receivedReason, 'normal closure')
+  })
+
+  await it('should forward onopen event', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    let openCalled = false
+    adapter.onopen = () => {
+      openCalled = true
+    }
+
+    mockWs.onopen?.()
+
+    assert.strictEqual(openCalled, true)
+  })
+
+  await it('should have getter and setter for onmessage', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    const callback = (event: { data: string }): undefined => {
+      // eslint-disable-next-line no-void
+      void event
+      return undefined
+    }
+    adapter.onmessage = callback
+    assert.strictEqual(adapter.onmessage, callback)
+
+    adapter.onmessage = null
+    assert.strictEqual(adapter.onmessage, null)
+  })
+
+  await it('should have getter and setter for onerror', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    const callback = (event: { error: unknown; message: string }): undefined => {
+      // eslint-disable-next-line no-void
+      void event
+      return undefined
+    }
+    adapter.onerror = callback
+    assert.strictEqual(adapter.onerror, callback)
+
+    adapter.onerror = null
+    assert.strictEqual(adapter.onerror, null)
+  })
+
+  await it('should have getter and setter for onclose', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    const callback = (event: { code: number; reason: string }): undefined => {
+      // eslint-disable-next-line no-void
+      void event
+      return undefined
+    }
+    adapter.onclose = callback
+    assert.strictEqual(adapter.onclose, callback)
+
+    adapter.onclose = null
+    assert.strictEqual(adapter.onclose, null)
+  })
+
+  await it('should have getter and setter for onopen', () => {
+    const mockWs: MockWs = {
+      close: () => undefined,
+      onclose: null,
+      onerror: null,
+      onmessage: null,
+      onopen: null,
+      readyState: WebSocketReadyState.OPEN,
+      send: () => undefined,
+    }
+
+    const adapter = createWsAdapter(mockWs as unknown as WebSocket)
+
+    const callback = (): undefined => undefined
+    adapter.onopen = callback
+    assert.strictEqual(adapter.onopen, callback)
+
+    adapter.onopen = null
+    assert.strictEqual(adapter.onopen, null)
+  })
+})