From 2c487305d6f49485a432113e261e35e2eca5c3b4 Mon Sep 17 00:00:00 2001 From: Sinabina Date: Thu, 16 Jul 2026 17:43:58 -0700 Subject: [PATCH] fix physical iOS bridge reliability --- ios-qa/daemon/src/devicectl.ts | 55 +++++++++++-- ios-qa/daemon/src/proxy.ts | 36 +++++++-- ios-qa/daemon/src/tunnel-bootstrap.ts | 17 +++- ios-qa/daemon/test/proxy-classify.test.ts | 78 ++++++++++++++++++- ios-qa/daemon/test/tunnel-bootstrap.test.ts | 46 +++++++++-- ios-qa/templates/StateServer.swift.template | 26 ++++++- .../Sources/DebugBridgeCore/StateServer.swift | 26 ++++++- 7 files changed, 259 insertions(+), 25 deletions(-) diff --git a/ios-qa/daemon/src/devicectl.ts b/ios-qa/daemon/src/devicectl.ts index ee1696eb9..e3e5be85c 100644 --- a/ios-qa/daemon/src/devicectl.ts +++ b/ios-qa/daemon/src/devicectl.ts @@ -10,13 +10,24 @@ import { tmpdir } from 'os'; import { join } from 'path'; export interface DeviceEntry { + /** CoreDevice UUID used by `devicectl --device`. */ identifier: string; + /** Hardware UDID shown by Xcode and commonly supplied by users/CI. */ + hardwareUdid: string | null; name: string; model: string; state: string; // "connected" | "available" | "available (paired)" | ... paired: boolean; } +export type DeviceListResult = + | { ok: true; devices: DeviceEntry[] } + | { + ok: false; + error: 'devicectl_unavailable' | 'devicectl_failed' | 'devicectl_bad_response'; + detail: string; + }; + export interface SpawnImpl { (cmd: string, args: string[]): SpawnSyncReturns; } @@ -66,29 +77,59 @@ const legacyResolve6: ResolveImpl = async (hostname) => { * List devices currently known to CoreDevice. Includes connected, paired, * and pairing-in-progress devices. */ -export function listDevices(spawn: SpawnImpl = defaultSpawn): DeviceEntry[] { +export function listDevices(spawn: SpawnImpl = defaultSpawn): DeviceListResult { const tmp = join(tmpdir(), `devicectl-list-${process.pid}-${Date.now()}.json`); try { const r = spawn('xcrun', ['devicectl', 'list', 'devices', '--json-output', tmp]); - if (r.status !== 0) return []; + if (r.error) { + const code = (r.error as NodeJS.ErrnoException).code; + return { + ok: false, + error: code === 'ENOENT' ? 'devicectl_unavailable' : 'devicectl_failed', + detail: code ? `${code}: ${r.error.message}` : r.error.message, + }; + } + if (r.status !== 0) { + const stderr = r.stderr?.toString().trim(); + return { + ok: false, + error: 'devicectl_failed', + detail: stderr || `devicectl exited ${r.status ?? 'without a status'}`, + }; + } const raw = readFileSync(tmp, 'utf-8'); const obj = JSON.parse(raw); - const list = (obj.result?.devices ?? []) as Array>; - return list.map((d) => { + const list = obj?.result?.devices; + if (!Array.isArray(list)) { + return { + ok: false, + error: 'devicectl_bad_response', + detail: 'JSON response is missing result.devices[]', + }; + } + const devices = (list as Array>).map((d) => { const conn = d.connectionProperties as Record | undefined; const props = d.deviceProperties as Record | undefined; const hw = d.hardwareProperties as Record | undefined; const pairingState = String(conn?.pairingState ?? ''); + const identifier = String(d.identifier ?? ''); + if (!identifier) throw new Error('device entry is missing identifier'); return { - identifier: String(d.identifier ?? ''), + identifier, + hardwareUdid: typeof hw?.udid === 'string' && hw.udid ? hw.udid : null, name: String(props?.name ?? 'unknown'), model: String(hw?.productType ?? 'unknown'), state: String(conn?.tunnelState ?? 'unknown'), paired: pairingState === 'paired', }; }); - } catch { - return []; + return { ok: true, devices }; + } catch (err) { + return { + ok: false, + error: 'devicectl_bad_response', + detail: err instanceof Error ? err.message : String(err), + }; } finally { try { rmSync(tmp, { force: true }); } catch { /* ignore */ } } diff --git a/ios-qa/daemon/src/proxy.ts b/ios-qa/daemon/src/proxy.ts index 143188699..f530667da 100644 --- a/ios-qa/daemon/src/proxy.ts +++ b/ios-qa/daemon/src/proxy.ts @@ -13,6 +13,7 @@ const MAX_BODY = 1_048_576; // 1MB hard cap on tailnet ingress export interface DeviceTunnel { udid: string; + bundleId?: string; ipv6Addr: string; port: number; bootTokenRotated: string; // the rotated bearer the daemon uses to talk to StateServer @@ -33,6 +34,7 @@ export async function proxyToDevice(opts: { tunnel: DeviceTunnel; sessionId: string | null; agentIdentity?: string; + timeoutMs?: number; }): Promise<{ status: number; headers: Record; body: Buffer }> { const { inbound, body, tunnel, sessionId, agentIdentity } = opts; if (body.length > MAX_BODY) { @@ -46,6 +48,9 @@ export async function proxyToDevice(opts: { }; if (sessionId) headers['x-session-id'] = sessionId; if (agentIdentity) headers['x-agent-identity'] = agentIdentity; + if (tunnel.bundleId && isCoordinateMutation(inbound.method, inbound.url)) { + headers['x-gstack-expected-bundle-id'] = tunnel.bundleId; + } // Bracket IPv6 literals; pass IPv4 + hostnames bare. The CoreDevice tunnel // is always IPv6 in production, but tests inject 127.0.0.1 to talk to a @@ -54,11 +59,17 @@ export async function proxyToDevice(opts: { const isIPv6 = (tunnel.ipv6Addr.match(/:/g)?.length ?? 0) >= 2; const hostPart = isIPv6 ? `[${tunnel.ipv6Addr}]` : tunnel.ipv6Addr; const url = `http://${hostPart}:${tunnel.port}${inbound.url ?? '/'}`; - return new Promise((resolve, reject) => { + return new Promise((resolve) => { + let settled = false; + const finish = (result: { status: number; headers: Record; body: Buffer }) => { + if (settled) return; + settled = true; + resolve(result); + }; const req = httpRequest(url, { method: inbound.method, headers, - timeout: 30_000, + timeout: opts.timeoutMs ?? 30_000, }, (res) => { const chunks: Buffer[] = []; res.on('data', (c) => chunks.push(c)); @@ -67,21 +78,30 @@ export async function proxyToDevice(opts: { for (const [k, v] of Object.entries(res.headers)) { if (typeof v === 'string') respHeaders[k] = v; } - resolve({ + finish({ status: res.statusCode ?? 502, headers: respHeaders, body: Buffer.concat(chunks), }); }); + res.on('aborted', () => finish(makeError(503, 'device_disconnected'))); + }); + req.on('timeout', () => { + // Node's request timeout is advisory: without destroying the socket it + // can hang forever while a suspended app keeps the CoreDevice route but + // stops servicing HTTP. Resolve first, then destroy; the resulting error + // event is ignored by the settled guard. + finish(makeError(504, 'upstream_timeout')); + req.destroy(); }); req.on('error', (err) => { const e = err as { code?: string }; if (e.code === 'ECONNREFUSED' || e.code === 'EHOSTUNREACH') { - resolve(makeError(503, 'device_disconnected')); + finish(makeError(503, 'device_disconnected')); } else if (e.code === 'ETIMEDOUT') { - resolve(makeError(504, 'upstream_timeout')); + finish(makeError(504, 'upstream_timeout')); } else { - reject(err); + finish(makeError(502, 'upstream_error')); } }); req.write(body); @@ -89,6 +109,10 @@ export async function proxyToDevice(opts: { }); } +function isCoordinateMutation(method: string | undefined, path: string | undefined): boolean { + return method === 'POST' && path !== undefined && ['/tap', '/swipe', '/type'].includes(path.split('?')[0]!); +} + function makeError(status: number, error: string): { status: number; headers: Record; body: Buffer } { const body = Buffer.from(JSON.stringify({ error }, sanitizeReplacer)); return { diff --git a/ios-qa/daemon/src/tunnel-bootstrap.ts b/ios-qa/daemon/src/tunnel-bootstrap.ts index aa6636938..4a3165a22 100644 --- a/ios-qa/daemon/src/tunnel-bootstrap.ts +++ b/ios-qa/daemon/src/tunnel-bootstrap.ts @@ -47,6 +47,9 @@ export type BootstrapResult = | { ok: false; error: BootstrapErrorReason; detail?: string }; export type BootstrapErrorReason = + | 'device_discovery_unavailable' + | 'device_discovery_failed' + | 'device_discovery_bad_response' | 'no_devices' | 'no_paired_device' | 'device_not_found' @@ -71,12 +74,21 @@ export async function bootstrapTunnel(opts: BootstrapOptions): Promise d.identifier === opts.udid) + ? devices.find((d) => d.identifier === opts.udid || d.hardwareUdid === opts.udid) : devices.find((d) => d.paired) ?? devices[0]; if (!target) { return { ok: false, error: 'device_not_found', detail: opts.udid }; @@ -166,6 +178,7 @@ export async function bootstrapTunnel(opts: BootstrapOptions): Promise { test('healthz, screenshot, elements, snapshot are observe-tier', () => { @@ -45,3 +47,77 @@ describe('classifyRoute', () => { expect(classifyRoute('GET', '/auth/sessions').allowed).toBe(false); // loopback-only }); }); + +describe('proxyToDevice failure bounds and bundle assertions', () => { + test('a suspended/non-responsive app returns a bounded 504', async () => { + const server = createServer(() => { + // Deliberately keep the connection open without headers or a body. This + // is the observable shape of a CoreDevice route to a suspended app. + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const addr = server.address(); + const port = typeof addr === 'object' && addr ? addr.port : 0; + const started = Date.now(); + try { + const result = await proxyToDevice({ + inbound: { + method: 'GET', + url: '/screenshot', + headers: { 'content-type': 'application/json' }, + } as IncomingMessage, + body: Buffer.alloc(0), + tunnel: { + udid: 'CORE-1', + ipv6Addr: '127.0.0.1', + port, + bootTokenRotated: 'rotated-token', + }, + sessionId: null, + timeoutMs: 40, + }); + expect(result.status).toBe(504); + expect(JSON.parse(result.body.toString())).toEqual({ error: 'upstream_timeout' }); + expect(Date.now() - started).toBeLessThan(1_000); + } finally { + server.closeAllConnections?.(); + await new Promise((resolve) => server.close(() => resolve())); + } + }); + + test('coordinate actions carry the expected active bundle assertion', async () => { + let expectedBundleHeader: string | undefined; + const server = createServer((req, res) => { + expectedBundleHeader = req.headers['x-gstack-expected-bundle-id'] as string | undefined; + req.resume(); + req.on('end', () => { + res.writeHead(200, { 'content-type': 'application/json' }); + res.end(JSON.stringify({ ok: true })); + }); + }); + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const addr = server.address(); + const port = typeof addr === 'object' && addr ? addr.port : 0; + try { + const result = await proxyToDevice({ + inbound: { + method: 'POST', + url: '/tap', + headers: { 'content-type': 'application/json' }, + } as IncomingMessage, + body: Buffer.from('{"x":10,"y":20}'), + tunnel: { + udid: 'CORE-1', + bundleId: 'com.gstack.fixture', + ipv6Addr: '127.0.0.1', + port, + bootTokenRotated: 'rotated-token', + }, + sessionId: 'session-1', + }); + expect(result.status).toBe(200); + expect(expectedBundleHeader).toBe('com.gstack.fixture'); + } finally { + await new Promise((resolve) => server.close(() => resolve())); + } + }); +}); diff --git a/ios-qa/daemon/test/tunnel-bootstrap.test.ts b/ios-qa/daemon/test/tunnel-bootstrap.test.ts index 188659b78..3d609482c 100644 --- a/ios-qa/daemon/test/tunnel-bootstrap.test.ts +++ b/ios-qa/daemon/test/tunnel-bootstrap.test.ts @@ -81,6 +81,37 @@ describe('bootstrapTunnel', () => { if (!r.ok) expect(r.error).toBe('no_devices'); }); + test('does not misclassify a devicectl failure as no_devices', async () => { + const spawn = makeSpawn([ + { + argsMatch: /devicectl list devices/, + exitCode: 1, + stderr: 'xcrun: error: unable to find utility devicectl', + }, + ]); + const r = await bootstrapTunnel({ bundleId: 'com.test', spawnImpl: spawn }); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.error).toBe('device_discovery_failed'); + expect(r.detail).toContain('devicectl'); + } + }); + + test('does not turn malformed devicectl JSON into an empty successful list', async () => { + const spawn = makeSpawn([ + { + argsMatch: /devicectl list devices/, + jsonOutput: { result: { unexpected: [] } }, + }, + ]); + const r = await bootstrapTunnel({ bundleId: 'com.test', spawnImpl: spawn }); + expect(r.ok).toBe(false); + if (!r.ok) { + expect(r.error).toBe('device_discovery_bad_response'); + expect(r.detail).toContain('result.devices'); + } + }); + test('returns no_paired_device when device is connected but not paired', async () => { const spawn = makeSpawn([ { @@ -225,6 +256,7 @@ describe('bootstrapTunnel', () => { expect(r.tunnel.udid).toBe('TEST-UDID'); expect(r.tunnel.ipv6Addr).toBe('fd99::beef'); expect(r.tunnel.port).toBe(9999); + expect(r.tunnel.bundleId).toBe('com.test'); expect(r.tunnel.bootTokenRotated).toMatch(/^[A-Za-z0-9_-]+$/); expect(r.tunnel.bootTokenRotated).not.toBe('BOOT-TOKEN-XYZ-123'); expect(r.tunnel.bootTokenRotated.length).toBeGreaterThan(20); @@ -265,39 +297,39 @@ describe('bootstrapTunnel', () => { if (!r.ok) expect(r.error).toBe('resolve_failed'); }); - test('respects explicit udid when set', async () => { + test('accepts a hardware UDID but uses the matching CoreDevice UUID for commands', async () => { const spawn = makeSpawn([ { argsMatch: /devicectl list devices/, jsonOutput: { result: { devices: [ { identifier: 'A', connectionProperties: { tunnelState: 'connected', pairingState: 'paired' }, deviceProperties: { name: 'A' }, hardwareProperties: { productType: 'iPhone18,2' } }, - { identifier: 'B', connectionProperties: { tunnelState: 'connected', pairingState: 'paired' }, deviceProperties: { name: 'B' }, hardwareProperties: { productType: 'iPhone18,2' } }, + { identifier: 'COREDEVICE-B', connectionProperties: { tunnelState: 'connected', pairingState: 'paired' }, deviceProperties: { name: 'B' }, hardwareProperties: { productType: 'iPhone18,2', udid: 'HARDWARE-B' } }, ] }, }, }, { - argsMatch: /devicectl device info processes -d B/, + argsMatch: /devicectl device info processes -d COREDEVICE-B/, jsonOutput: { result: { runningProcesses: [{ executable: 'file:///var/containers/Bundle/Application/X/com.test.app/com.test' }] } }, }, { - argsMatch: /devicectl device info details --device B/, + argsMatch: /devicectl device info details --device COREDEVICE-B/, jsonOutput: { result: { connectionProperties: { tunnelIPAddress: 'fd00::b' } } }, }, { - argsMatch: /devicectl device copy from --device B/, + argsMatch: /devicectl device copy from --device COREDEVICE-B/, destOutput: 'TOKEN\n', }, ]); const r = await bootstrapTunnel({ - udid: 'B', + udid: 'HARDWARE-B', bundleId: 'com.test', spawnImpl: spawn, resolveImpl: async () => ['fd00::b'], fetchImpl: (async () => new Response('{"ok":true}', { status: 200 })) as typeof fetch, }); expect(r.ok).toBe(true); - if (r.ok) expect(r.tunnel.udid).toBe('B'); + if (r.ok) expect(r.tunnel.udid).toBe('COREDEVICE-B'); }); }); diff --git a/ios-qa/templates/StateServer.swift.template b/ios-qa/templates/StateServer.swift.template index 803bedf31..26cd3e26d 100644 --- a/ios-qa/templates/StateServer.swift.template +++ b/ios-qa/templates/StateServer.swift.template @@ -284,6 +284,7 @@ public final class StateServer { "version": "1.0.0", "build": appBuildId, "accessor_hash": accessorHash, + "bundle_id": Bundle.main.bundleIdentifier ?? "unknown", ]) return } @@ -511,12 +512,35 @@ public final class StateServer { private func handleMutation(connection: NWConnection, request: ParsedRequest, op: String) { guard requireSession(in: request, connection: connection) else { return } + let bundleBefore = Bundle.main.bundleIdentifier ?? "unknown" + if let expected = request.headers["x-gstack-expected-bundle-id"], expected != bundleBefore { + send(connection: connection, status: 409, body: [ + "error": "active_bundle_mismatch", + "expected_bundle": expected, + "active_bundle": bundleBefore, + ]) + return + } guard let payload = try? JSONSerialization.jsonObject(with: request.body) as? JSONDict else { send(connection: connection, status: 400, body: ["error": "invalid_json"]) return } let ok = MutationBridge.dispatch(op: op, payload: payload) - send(connection: connection, status: ok ? 200 : 400, body: ["op": op, "ok": ok]) + let bundleAfter = Bundle.main.bundleIdentifier ?? "unknown" + guard bundleAfter == bundleBefore else { + send(connection: connection, status: 409, body: [ + "error": "active_bundle_changed", + "before_bundle": bundleBefore, + "after_bundle": bundleAfter, + ]) + return + } + send(connection: connection, status: ok ? 200 : 400, body: [ + "op": op, + "ok": ok, + "active_bundle_before": bundleBefore, + "active_bundle_after": bundleAfter, + ]) } // MARK: Response diff --git a/test/fixtures/ios-qa/FixtureApp/Sources/DebugBridgeCore/StateServer.swift b/test/fixtures/ios-qa/FixtureApp/Sources/DebugBridgeCore/StateServer.swift index 803bedf31..26cd3e26d 100644 --- a/test/fixtures/ios-qa/FixtureApp/Sources/DebugBridgeCore/StateServer.swift +++ b/test/fixtures/ios-qa/FixtureApp/Sources/DebugBridgeCore/StateServer.swift @@ -284,6 +284,7 @@ public final class StateServer { "version": "1.0.0", "build": appBuildId, "accessor_hash": accessorHash, + "bundle_id": Bundle.main.bundleIdentifier ?? "unknown", ]) return } @@ -511,12 +512,35 @@ public final class StateServer { private func handleMutation(connection: NWConnection, request: ParsedRequest, op: String) { guard requireSession(in: request, connection: connection) else { return } + let bundleBefore = Bundle.main.bundleIdentifier ?? "unknown" + if let expected = request.headers["x-gstack-expected-bundle-id"], expected != bundleBefore { + send(connection: connection, status: 409, body: [ + "error": "active_bundle_mismatch", + "expected_bundle": expected, + "active_bundle": bundleBefore, + ]) + return + } guard let payload = try? JSONSerialization.jsonObject(with: request.body) as? JSONDict else { send(connection: connection, status: 400, body: ["error": "invalid_json"]) return } let ok = MutationBridge.dispatch(op: op, payload: payload) - send(connection: connection, status: ok ? 200 : 400, body: ["op": op, "ok": ok]) + let bundleAfter = Bundle.main.bundleIdentifier ?? "unknown" + guard bundleAfter == bundleBefore else { + send(connection: connection, status: 409, body: [ + "error": "active_bundle_changed", + "before_bundle": bundleBefore, + "after_bundle": bundleAfter, + ]) + return + } + send(connection: connection, status: ok ? 200 : 400, body: [ + "op": op, + "ok": ok, + "active_bundle_before": bundleBefore, + "active_bundle_after": bundleAfter, + ]) } // MARK: Response