feat: add network traffic capture for test runs#75
Open
Conversation
Adds a new CLI command that captures network traffic from Android emulators/devices and iOS simulators via an HTTPS-intercepting proxy. Streams requests live to the terminal and exports HAR 1.2 files. Key design decisions: - Android: system proxy via `adb shell settings put global http_proxy` - iOS: PAC file served over HTTP with DIRECT fallback (crash-safe — Mac internet survives process death, unlike direct networksetup proxy) - CA cert generated once at ~/.finalrun/ca/, reused across runs - Pre-capture connectivity test prevents broken internet if CA not trusted - Crash recovery via ~/.finalrun/proxy-state.json restores proxy on next run - TLS failures labeled by cause: pinning vs missing user CA trust Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wire NetworkCaptureManager into the test execution lifecycle following
the device logs pattern. Key additions:
- NetworkCaptureManager: session-scoped mockttp proxy with per-test HAR
slicing, full request/response body capture, TLS error tracking
- NetworkCaptureSetup: Android (adb proxy) + iOS (PAC server) config
- Test lifecycle: network capture start/stop per test in sessionRunner,
opt-in via `network.capture: true` in .finalrun/config.yaml
- HAR artifact writing with header redaction (Authorization, Cookie,
tokens) and ${secrets.*} value redaction
- Report UI: Network tab with request table, status filters, search,
slide-out detail panel (Headers/Request/Response tabs), video sync
- Types: networkLogFile/StartedAt/CompletedAt on TestResult
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Standalone capture.ts now captures request/response bodies (text up to 512KB, binary skipped) matching NetworkCaptureManager behavior - Added comment noting relationship between standalone and integrated - Verified end-to-end on both Android (Wikipedia + Google Services) and iOS (Wikipedia + Apple Services) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The per-test network capture was wired but the session-level setup was
missing: no proxy started, no device proxy configured, no CA
verification. This meant "Network capture started" printed successfully
even when nothing was actually capturing.
Now when network.capture is enabled:
- Starts mockttp proxy session before the test loop
- Configures device proxy (Android: adb settings, iOS: PAC server)
- Verifies CA is trusted via test HTTPS request
- If CA not trusted: prints clear warning with fix instructions
("run finalrun log-network to set up"), continues without capture
- Per-test capture only runs if session setup succeeded
- Proxy restored in finally block on both success and failure
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Suppress mockttp "Failed to handle request" error during CA verification by temporarily raising log level to ERROR - Demote internal proxy setup logs (port, proxy target) from INFO to DEBUG — user shouldn't see these - Replace noisy per-test "Network capture started" with debug-only marker log - Session-level success message now shows one clean line: "Network capture active (proxy on 127.0.0.1:8000, CA verified)" Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Three fixes for network capture during test runs: 1. Remove CA cert push from Android configureProxy() — pushing during a test causes CertPathValidatorException. Cert setup belongs in the standalone `finalrun log-network` command only. 2. Replace host-side HTTPS verification (which caused "Failed to handle request: unable to get local issuer certificate" errors) with traffic-based detection: wait 3s for device background traffic, then check if proxy saw successful entries vs TLS errors. This tests the DEVICE's trust, not the host's. 3. Downgrade per-test "Network capture started" to debug-only — the session-level message is the meaningful confirmation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- New docs/network-logging.md: user-facing guide covering setup, what you get, what gets captured/redacted, pinning, troubleshooting - docs/configuration.md: add network.capture field to config table - docs/cli-reference.md: add log-network command section - README.md: add Network Logging link to docs section - Downgrade all network capture Logger.i to Logger.d (no more noisy "proxy started on port" / "proxy set to" / "proceeding optimistically" messages in normal output) - Remove 3-second traffic-based verification wait — proxy setup is now silent, warnings come from post-test HAR analysis Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…sues Binary response bodies (compressed, protobuf, images) were being stored as UTF-8 text in the HAR, producing garbled output. Rather than adding binary detection heuristics, remove body capture entirely for now. Changes: - NetworkCaptureManager: stop reading req.body/response.body - capture.ts (standalone): same - reportWriter: remove body text redaction (header redaction stays) - reportServer: remove body fields from view model - reportTemplate: remove Request Body / Response Body tabs from detail panel, keep Headers tab only (General + Response Headers + Request Headers) - docs/network-logging.md: mark request/response body as "Not yet" What stays: method, URL, status, all headers, timing, size, query params, TLS failure labeling, video sync, search, status filters. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR implements comprehensive HTTP/HTTPS network capture for test runs and via a standalone CLI command. It adds CA certificate generation/management, platform-specific MITM proxy configuration (Android/iOS), HAR file export, test report integration with a Network tab, and crash-safe proxy state persistence. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (CLI)
participant Cmd as log-network<br/>Command
participant CA as CA<br/>Manager
participant Device as Device<br/>(adb/xcrun)
participant Proxy as Capture<br/>Proxy (mockttp)
participant Stream as Output<br/>(HAR/Terminal)
User->>Cmd: finalrun log-network --platform=android --device=<serial>
Cmd->>Cmd: Validate platform, recover stale proxy state
Cmd->>CA: loadOrGenerateCA()
CA-->>Cmd: {cert, key, files}
Cmd->>Device: Detect/select target device
Device-->>Cmd: Device serial
Cmd->>Device: Push CA cert to device
Device-->>Cmd: Success
Cmd->>Device: Guide trust setup (API-level dependent)
Cmd->>Proxy: start(cert, key, callbacks)
Proxy-->>Cmd: Running on port P
Cmd->>Device: Configure proxy (adb reverse or emulator loopback)
Device-->>Cmd: Proxy configured
Cmd->>Cmd: Persist proxy state for crash recovery
Cmd->>Proxy: Verify HTTPS via test request
Proxy->>Proxy: Intercept request, check TLS
Proxy-->>Cmd: Verify result (trusted/untrusted/unknown)
alt Verification Failed
Cmd->>Device: Clean up proxy
Cmd-->>User: Error: Device does not trust CA
else Verification Succeeded
Cmd-->>User: Capture started, streaming requests...
loop Until Ctrl+C
Proxy->>Proxy: Intercept request/response
Proxy->>Stream: printEntry(entry) live to terminal
Proxy->>Proxy: Record in entries array
end
User->>Cmd: Ctrl+C (SIGTERM)
Cmd->>Proxy: stop()
Proxy-->>Cmd: Stopped
Cmd->>Device: Clear proxy config
Device-->>Cmd: Cleared
Cmd->>Cmd: Clear persisted proxy state
Proxy->>Stream: toHar() → write to file
Cmd->>Stream: Write TLS error summary
Cmd-->>User: Complete, HAR saved
end
sequenceDiagram
participant TestRunner as Test Runner
participant SessionMgr as Session<br/>Manager
participant Device as Device<br/>(Network Capture)
participant Proxy as Capture<br/>Proxy
participant Writer as Report<br/>Writer
TestRunner->>SessionMgr: Initialize with networkCapture=true
SessionMgr->>Device: startNetworkSession(cert, key)
Device->>Proxy: Initialize proxy
Proxy-->>Device: proxyPort
Device-->>SessionMgr: Success
SessionMgr->>Device: setupNetworkProxy(platform, proxyPort)
Device-->>SessionMgr: Proxy configured
loop For Each Test
SessionMgr->>Device: startNetworkCapture(runId, testId)
Device->>Proxy: Record start index
Proxy-->>Device: Started
Device-->>SessionMgr: Capture began
SessionMgr->>SessionMgr: Execute test
SessionMgr->>Device: stopNetworkCapture(runId, testId)
Device->>Proxy: Extract captured entries/TLS errors
Proxy-->>Device: Entries slice
Device->>Device: Build HAR file
Device-->>SessionMgr: {filePath, startedAt, completedAt}
SessionMgr->>SessionMgr: Attach networkLog to test result
end
SessionMgr->>Device: stopNetworkSession()
Device->>Proxy: Stop proxy
Proxy-->>Device: Stopped
Device-->>SessionMgr: Cleaned up
SessionMgr->>Writer: Write test artifacts
Writer->>Writer: Copy HAR, redact sensitive data
Writer->>Writer: Record networkLogFile path
Writer-->>TestRunner: Report ready
sequenceDiagram
participant User as User (Browser)
participant UI as Report UI
participant Video as Video<br/>Player
participant Network as Network<br/>Tab
User->>UI: Open test report
UI->>UI: Load networkLogEntries from manifest
UI->>UI: Render Network tab with entries
User->>Network: Click request row
Network->>Network: Extract startedDateTime
Network->>Video: Seek to timestamp
Video-->>Network: Playing at time
User->>Video: Play test video
loop During playback
Video->>Video: timeupdate event
Video->>Network: Get current time
Network->>Network: Find nearest entry by timestamp
Network->>UI: Highlight row
end
User->>Network: Type in search filter
Network->>Network: Filter entries by URL/status
User->>Network: Select entry → view Headers
Network->>Network: Display request/response headers
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (13)
packages/cli/package.json (1)
42-60: KeepbundleDependenciesaligned with new runtime dependency.
mockttpwas added todependenciesbut not tobundleDependencies, which can create packaging drift in bundled/offline install flows.📦 Proposed update
"bundleDependencies": [ @@ "dotenv", + "mockttp", "ora",Also applies to: 87-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/package.json` around lines 42 - 60, bundleDependencies in package.json is missing the newly added runtime dependency "mockttp", causing packaging drift; update the bundleDependencies array (the "bundleDependencies" entry in package.json) to include "mockttp" alongside the other entries to keep bundled/offline installs consistent with the dependencies list.packages/cli/src/commands/logNetwork/colors.ts (1)
6-16: HonorNO_COLOR/non-TTY output to avoid escape codes in piped logs.Right now color codes are always emitted. Add a runtime gate so redirected output stays clean.
♻️ Proposed refactor
const ESC = '\x1b['; const RESET = `${ESC}0m`; +const ENABLE_COLORS = Boolean(process.stdout.isTTY) && !process.env.NO_COLOR; +const wrap = (code: number, s: string) => (ENABLE_COLORS ? `${ESC}${code}m${s}${RESET}` : s); export const colors = { - green: (s: string) => `${ESC}32m${s}${RESET}`, - red: (s: string) => `${ESC}31m${s}${RESET}`, - yellow: (s: string) => `${ESC}33m${s}${RESET}`, - cyan: (s: string) => `${ESC}36m${s}${RESET}`, - magenta: (s: string) => `${ESC}35m${s}${RESET}`, - blue: (s: string) => `${ESC}34m${s}${RESET}`, - dim: (s: string) => `${ESC}2m${s}${RESET}`, - bold: (s: string) => `${ESC}1m${s}${RESET}`, + green: (s: string) => wrap(32, s), + red: (s: string) => wrap(31, s), + yellow: (s: string) => wrap(33, s), + cyan: (s: string) => wrap(36, s), + magenta: (s: string) => wrap(35, s), + blue: (s: string) => wrap(34, s), + dim: (s: string) => wrap(2, s), + bold: (s: string) => wrap(1, s), white: (s: string) => s, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/logNetwork/colors.ts` around lines 6 - 16, The colors object currently always emits ANSI escapes; change it to respect NO_COLOR and non-TTY output by computing a single boolean (e.g., allowColors = !process.env.NO_COLOR && process.stdout?.isTTY && process.env.TERM !== 'dumb') at module init and, if false, replace all functions on the exported colors object (green, red, yellow, cyan, magenta, blue, dim, bold) to be identity functions that return the input string (and keep white as-is); keep ESC and RESET defined but only used when allowColors is true so redirected/piped logs contain no escape codes.packages/cli/src/reportServer.ts (1)
306-327: Consider extracting HAR entry mapping for readability.The nested type casts and property access chains are verbose. While the defensive approach is appropriate for parsing external JSON, extracting the entry mapping could improve readability.
♻️ Optional: Extract helper for cleaner entry mapping
function mapHarEntry(e: Record<string, unknown>) { const request = e['request'] as Record<string, unknown> | undefined; const response = e['response'] as Record<string, unknown> | undefined; const content = response?.['content'] as Record<string, unknown> | undefined; return { startedDateTime: e['startedDateTime'], method: request?.['method'], url: request?.['url'], status: response?.['status'], statusText: response?.['statusText'], time: e['time'], responseSize: content?.['size'] ?? response?.['bodySize'] ?? 0, requestHeaders: request?.['headers'], responseHeaders: response?.['headers'], }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/reportServer.ts` around lines 306 - 327, The readNetworkHar function has verbose nested casts and property chains; extract the HAR entry mapping into a small helper (e.g., mapHarEntry) to improve readability and reduce repetition: inside readNetworkHar call entries = (har?.log?.entries ?? []).map(mapHarEntry) and implement mapHarEntry to safely pull request/response/content as Record<string, unknown> | undefined and return the object fields (startedDateTime, method, url, status, statusText, time, responseSize using content?.size ?? response?.bodySize ?? 0, requestHeaders, responseHeaders). Keep the existing defensive checks and error handling behavior unchanged.packages/cli/src/workspace.ts (1)
431-437: Consider throwing on malformednetworkconfig for consistency.The function silently returns
undefinedwhennetworkis present but malformed (e.g., an array or primitive). Other config readers in this file likereadOptionalTrimmedStringthrow errors for invalid types, which helps users identify misconfigurations.If a user writes
network: "true"instead ofnetwork: { capture: true }, they won't receive any feedback that their config is being ignored.♻️ Optional: Add validation for malformed network config
function readNetworkConfig(raw: unknown): { capture?: boolean } | undefined { if (raw === undefined || raw === null) return undefined; - if (typeof raw !== 'object' || Array.isArray(raw)) return undefined; + if (typeof raw !== 'object' || Array.isArray(raw)) { + throw new Error('network config must be a YAML mapping.'); + } const obj = raw as Record<string, unknown>; return { capture: typeof obj['capture'] === 'boolean' ? obj['capture'] : undefined, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/workspace.ts` around lines 431 - 437, readNetworkConfig currently silently returns undefined for malformed but present `network` values (e.g., strings/arrays); change it to validate and throw when `raw` is not undefined/null but also not a plain object (or is an array), and also throw if `obj['capture']` is present but not a boolean. Update the function readNetworkConfig to keep the existing behavior for undefined/null, but if raw is present and (typeof raw !== 'object' || Array.isArray(raw')) throw a TypeError with a clear message like "Invalid 'network' config: expected an object with optional boolean 'capture'"; likewise throw if capture exists and typeof capture !== 'boolean'. This preserves the return shape { capture?: boolean } on success and provides consistent error feedback to users.packages/cli/src/testRunner.ts (1)
258-266: Complex one-liner and potential partial setup state.Two observations:
- Line 259 is a dense one-liner with dynamic import, instantiation, and method chaining. Consider extracting for readability:
const { CliFilePathUtil } = await import('./filePathUtil.js'); const filePathUtil = new CliFilePathUtil(undefined, undefined, { downloadAssets: false }); const adbPath = (await filePathUtil.getADBPath()) ?? 'adb';
- If
configureProxy(Line 265) throws, thecatchblock on Line 270 logs a warning butnetworkCaptureAvailableremainsfalse. However, the network session was already started successfully (Line 252). The cleanup block (Lines 432-438) only stops the session ifnetworkCaptureAvailableis true, potentially leaving the session running.♻️ Ensure session cleanup even on proxy config failure
if (sessionResponse.success) { const proxyPort = goalSession.device.getNetworkProxyPort(); const deviceInfo = goalSession.device.getDeviceInfo(); const deviceId = deviceInfo.id ?? deviceInfo.deviceUUID; const platform = goalSession.platform; if (platform === 'android') { - const adbPath = (await new (await import('./filePathUtil.js')).CliFilePathUtil(undefined, undefined, { downloadAssets: false }).getADBPath()) ?? 'adb'; + const { CliFilePathUtil } = await import('./filePathUtil.js'); + const filePathUtil = new CliFilePathUtil(undefined, undefined, { downloadAssets: false }); + const adbPath = (await filePathUtil.getADBPath()) ?? 'adb'; networkProxySetup = new AndroidNetworkProxySetup(adbPath, deviceId); } else { networkProxySetup = new IOSNetworkProxySetup(deviceId); } - await networkProxySetup.configureProxy(proxyPort); - networkCaptureAvailable = true; + try { + await networkProxySetup.configureProxy(proxyPort); + networkCaptureAvailable = true; + } catch (proxyError) { + Logger.w('Network proxy configuration failed:', proxyError); + // Stop the session since proxy setup failed + await goalSession.device.stopNetworkSession().catch(() => {}); + networkProxySetup = undefined; + } } else { Logger.w(`Failed to start network capture session: ${sessionResponse.message}`); } } catch (error) { Logger.w('Network capture setup failed:', error); }packages/device-node/src/device/NetworkCaptureSetup.ts (2)
90-96: Shell command constructed with string interpolation.The shell commands use string interpolation with
valueparameter. While the values in this context come from controlled sources (proxy port numbers or previously read device settings), consider validating the proxy format to prevent unexpected behavior if the input is malformed.Optional: Add basic format validation
private async _setGlobalProxy(value: string): Promise<void> { + // Basic validation: proxy should be host:port format + if (!/^[\w.-]+:\d+$/.test(value) && value !== ':0') { + throw new Error(`Invalid proxy format: ${value}`); + } await this._shell(`settings put global http_proxy ${value}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/device-node/src/device/NetworkCaptureSetup.ts` around lines 90 - 96, The shell commands in _setGlobalProxy and _clearGlobalProxy are built via string interpolation with the unvalidated value; add a validation step in _setGlobalProxy to ensure value matches the expected proxy format (e.g., host:port or host:port;scheme) and explicitly allow the special clear token ":0" used by _clearGlobalProxy, rejecting or throwing for malformed inputs; then call this._shell only after validation (or sanitize/escape the value) and keep _clearGlobalProxy using the constant ':0' to avoid interpolation issues.
225-244: Fallback returns first service even without active IP.If no service has an active IP address, the function falls back to
services[0](line 243). This may configure proxy on an inactive interface. Consider returningnullinstead to let the caller handle the no-active-service case explicitly.Optional: Return null when no active service found
for (const svc of services) { try { const info = await run('networksetup', ['-getinfo', svc]); if (info.includes('IP address') && !info.includes('IP address: none')) { return svc; } } catch { continue; } } - return services[0] ?? null; + return null; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/device-node/src/device/NetworkCaptureSetup.ts` around lines 225 - 244, The function findActiveNetworkService currently falls back to returning services[0] when no service has an active IP, which can cause proxy configuration on an inactive interface; change the fallback to return null instead and update any callers of findActiveNetworkService to handle a null return (e.g., skip configuration, surface an explicit error, or choose a safe default). Locate the logic in findActiveNetworkService (the final return statement) and replace the services[0] fallback with null, and audit usages of findActiveNetworkService to add null-checks and appropriate error handling or user-facing messages.packages/cli/src/reportTemplate.ts (1)
1595-1598: Consider escaping single quotes for consistency.The
escapeHtmlJSfunction escapes&,<,>, and", but not single quotes ('). While this is generally safe for innerHTML contexts, the server-sideescapeHtmlfunction (line 2617-2624) does escape single quotes. Consider aligning these for consistency.Optional: align escapeHtmlJS with escapeHtml
function escapeHtmlJS(s) { if (!s) return ''; - return s.replace(/&/g,'&').replace(/</g,'<').replace(/>/g,'>').replace(/"/g,'"'); + return s.replace(/&/g,'&').replace(/</g,'<').replace(/>/g,'>').replace(/"/g,'"').replace(/'/g,'''); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/reportTemplate.ts` around lines 1595 - 1598, The client-side escapeHtmlJS function currently escapes &, <, > and " but omits single quotes, causing inconsistency with the server-side escapeHtml; update escapeHtmlJS to also replace single quotes (') with an HTML entity (e.g., ') so its output matches escapeHtml and avoids single-quote injection issues, keeping the replacement sequence consistent with the existing chain in escapeHtmlJS.packages/device-node/src/device/NetworkCaptureManager.ts (2)
294-342: HTTP version is hardcoded to HTTP/1.1.The HAR builder hardcodes
httpVersion: 'HTTP/1.1'(lines 305, 314) regardless of the actual protocol used. While mockttp may not expose this information easily, this could be misleading for HTTP/2 traffic.Consider adding a comment noting this limitation, or storing the actual HTTP version if mockttp provides it in the request/response objects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/device-node/src/device/NetworkCaptureManager.ts` around lines 294 - 342, The HAR builder in function buildHar hardcodes httpVersion: 'HTTP/1.1' for both request and response entries which can misrepresent HTTP/2 traffic; update the request and response httpVersion fields to use any real protocol/version properties available on the NetworkCapturedEntry (e.g., e.httpVersion, e.protocol, or e.requestProtocol if such fields exist), falling back to 'HTTP/1.1' when absent, and add a short code comment explaining the fallback and limitation if the capture layer (mockttp) doesn't expose the protocol.
238-240: Confusing fallback logic for startedAt calculation.The ternary chain for
startedAtis hard to follow. The second conditionthis._testStartIndexMap.get(mapKey) !== undefinedis always true at this point sincestartIndexwas already retrieved from it.Simplify the startedAt calculation
- const startedAt = testEntries.length > 0 - ? testEntries[0]!.startedAt - : (this._testStartIndexMap.get(mapKey) !== undefined ? new Date() : completedAt); + const startedAt = testEntries.length > 0 ? testEntries[0]!.startedAt : new Date();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/device-node/src/device/NetworkCaptureManager.ts` around lines 238 - 240, The startedAt ternary is confusing and redundantly calls this._testStartIndexMap.get(mapKey) even though startIndex was already retrieved; replace the chain to use the local startIndex variable and testEntries only: set startedAt = testEntries.length > 0 ? testEntries[0]!.startedAt : (startIndex !== undefined ? new Date() : completedAt), ensuring you reference testEntries, startIndex and completedAt (and remove the extra _testStartIndexMap.get call) so the logic is clear and correct.packages/cli/src/commands/logNetwork/ios.ts (1)
65-96: Code duplication with NetworkCaptureSetup.ts.This
startPacServerfunction and the autoproxy management functions (lines 105-147) are nearly identical to those inpackages/device-node/src/device/NetworkCaptureSetup.ts. Consider extracting these shared utilities into a common module to reduce maintenance burden.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/logNetwork/ios.ts` around lines 65 - 96, The startPacServer function and the autoproxy management logic duplicated from NetworkCaptureSetup.ts should be extracted into a shared utility module and imported where needed; specifically, move startPacServer and the corresponding autoproxy functions into a new common module (e.g., a shared/network utility) with the same exported function names (startPacServer and any autoproxy enable/disable helpers) and appropriate return types, update packages/cli/src/commands/logNetwork/ios.ts to import those functions instead of defining them, and update packages/device-node/src/device/NetworkCaptureSetup.ts to import the same shared exports; ensure the new module exports identical signatures and that any internal types or dependencies are adjusted so both callers compile without duplicated logic.packages/cli/src/commands/logNetwork/adb.ts (2)
40-48: Unnecessary try-catch block.The try-catch on lines 41-47 doesn't serve a purpose since
return 'adb'cannot throw. The catch block is unreachable.Simplify by removing try-catch
- // Fallback: hope it's on PATH. - try { - // We rely on execFile('adb', ...) working if PATH resolves it. - // A richer resolve path can be added later. - return 'adb'; - } catch { - return null; - } + // Fallback: hope it's on PATH. We rely on execFile('adb', ...) working + // if PATH resolves it. A richer resolve path can be added later. + return 'adb';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/logNetwork/adb.ts` around lines 40 - 48, The try-catch around the fallback return is unnecessary because "return 'adb'" cannot throw; remove the try { return 'adb'; } catch { return null; } block and replace it with a direct return 'adb' (leaving the surrounding function—the fallback branch that currently returns 'adb'—intact), ensuring any earlier code paths that might return null remain unchanged.
141-156: Shell commands use string interpolation with input values.The
setGlobalProxy(line 146) andgetProp(line 155) functions pass parameters directly into shell commands via string interpolation. While the current callers use controlled inputs, this pattern is fragile if the functions are reused with user-provided values.Optional: Consider input validation for shell commands
For
setGlobalProxy, validate the host:port format. ForgetProp, consider using a whitelist of allowed property names or escaping the input.export async function getProp(adbPath: string, serial: string, key: string): Promise<string> { + // Validate key contains only safe characters + if (!/^[\w.]+$/.test(key)) { + throw new AdbError(`Invalid property key: ${key}`); + } return await shell(adbPath, serial, `getprop ${key}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/logNetwork/adb.ts` around lines 141 - 156, The shell commands in setGlobalProxy and getProp interpolate untrusted values; validate and/or escape inputs before calling shell. For setGlobalProxy (function setGlobalProxy) validate hostPort with a strict regex (e.g. host or IP + ":" + port range) and reject or throw on invalid formats, or escape it by wrapping in single quotes and replacing any embedded single quotes; for getProp (function getProp) either restrict key to an allowlist or validate against a tight pattern like /^[A-Za-z0-9._-]+$/ and reject invalid characters, or escape the key similarly; alternatively refactor shell invocation to accept argv-style args to avoid building a single interpolated command. Ensure clear error handling when validation fails.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/network-logging.md`:
- Around line 77-85: The Markdown contains unlabeled fenced code blocks; add
language identifiers to each unlabeled triple-backtick fence to satisfy MD040:
use "text" for plain directory listing and HTTP log samples (e.g., the block
starting with "artifacts/{runId}/tests/{testId}/" and the block containing "GET
https://en.wikipedia.org/..."), and use "bash" for command examples (e.g., the
block with "finalrun log-network --platform=<android|ios> [options]"). Update
the three affected ranges (around the directory tree, the HTTP/har sample, and
the CLI invocation) to include these labels so markdownlint passes and rendering
stays consistent.
- Line 3: The intro sentence currently claiming "request and response bodies"
are captured contradicts the later note that bodies are not yet captured; update
the intro (the paragraph that mentions "method, URL, status code, headers,
request and response bodies, and timing" and the HAR 1.2 sentence) to accurately
state that request and response bodies are not captured yet (e.g., list captured
fields as method, URL, status code, headers, and timing, and note bodies are
planned/future), and make the phrasing consistent with the later line that says
bodies are not captured yet so both places (the intro and the existing note
about bodies) match.
In `@packages/cli/bin/finalrun.ts`:
- Around line 242-250: The CLI shows inconsistent platform support for the
log-network command: update the help text for program.command('log-network') and
its .requiredOption('--platform <platform>') to match actual supported platforms
used by runLogNetworkCommand; if iOS is supported, change the option help from
"(android)" to "(android|ios)" and adjust the command description to mention
both platforms, otherwise keep the description generic or explicitly state
"android only" and update docs/cli-reference.md to match; ensure any mention of
supported platforms in runLogNetworkCommand or related validation logic is
consistent with the help text.
In `@packages/cli/src/commands/logNetwork/capture.ts`:
- Around line 49-53: The start method retains previous in-memory capture state
(entries/TLS errors/pending requests) when reused, causing cross-session
contamination; modify start(cert, key, callbacks) to reset/clear the internal
state before creating or starting the mockttp server — e.g., reinitialize
this._entries, this._tlsErrors, this._pendingRequests (and any other per-run
buffers) to empty arrays/objects and remove any leftover listeners on
this._server, or recreate this._server anew so prior data is not reused; apply
the same reset logic where the server can be restarted (the corresponding
stop/restart flow referenced around the other start/stop call sites) so each run
begins with a clean in-memory state.
In `@packages/cli/src/commands/logNetwork/index.ts`:
- Around line 453-454: Add an actual xcrun availability check before marking the
iOS host-tools step as passed: import execFileAsync and call
execFileAsync('xcrun', ['--version']) (or equivalent) where printStep('Checking
host tools (xcrun)') is currently invoked; if the call succeeds, call
printStep(..., 'pass'), otherwise call printStep(..., 'fail') and surface an
error (exit or throw) so listBootedSimulators doesn't get a cryptic failure
later. Reference symbols: printStep, listBootedSimulators, and execFileAsync
(ensure execFileAsync is imported at top of the file).
- Around line 191-236: recoverStaleProxy currently only checks
isProxyResponding(state.proxyPort) and may allow multiple processes to
concurrently restore proxy settings; modify recoverStaleProxy to first validate
the saved PID/state by using isProcessAlive(state.pid) or
isAnyProcessAlive(state) after loadProxyState() (and before acting) and only
proceed with restoration if the original process is confirmed dead, otherwise
abort; additionally implement a simple lock/atomic marker around recovery (e.g.,
create a recovery lock file or use an existing lock helper) so that only one
process can perform restoration, and ensure clearProxyState() is only called by
the process that acquired the lock.
- Around line 148-185: testProxyConnectivity is currently creating an HTTPS
request to example.com directly and passing non-standard host/port on
https.Agent (which are ignored), so the proxy is bypassed; change the
implementation to actually target the proxy or use a proper proxy agent: either
(A) make the request connect to 127.0.0.1:proxyPort (set request hostname/port
to the proxy and set the request path to the full destination URL or use a
CONNECT method) and remove the invalid agent.host/port usage, or (B) install and
use a proxy agent (e.g., http-proxy-agent / https-proxy-agent) and pass that
agent to https.request so the request is tunneled through the mockttp proxy;
update testProxyConnectivity, the https.request options and agent construction
accordingly so the request is routed through the proxy and captured by
NetworkCapture.
In `@packages/cli/src/commands/logNetwork/livePrinter.ts`:
- Around line 20-23: The code currently reads terminal width from process.stdout
(cols) causing incorrect URL truncation when an alternate output stream is
passed to livePrinter; change the width source to use the passed-in stream by
replacing references to (process.stdout as { columns?: number }).columns with
(stream as { columns?: number }).columns (or similar) so cols is derived from
the function's stream parameter, then recompute fixedWidth/urlWidth and call
truncate(entry.url, urlWidth) as before; update any variable names if needed
inside livePrinter to ensure the stream argument is used consistently for column
sizing.
In `@packages/cli/src/commands/logNetwork/proxyState.ts`:
- Around line 29-32: saveProxyState currently writes STATE_FILE directly and can
leave a partially written proxy-state.json on crashes; change saveProxyState to
write atomically by: ensure STATE_DIR exists, create a temp file in STATE_DIR
(e.g. STATE_FILE + '.tmp' or use a unique suffix), open the temp file with
fsp.open, write JSON into it, call filehandle.datasync()/fsync to flush to disk,
close the handle, then rename (fs.rename) the temp file to STATE_FILE to perform
an atomic replace; reference saveProxyState, STATE_DIR and STATE_FILE when
locating where to implement this change.
In `@packages/cli/src/reportWriter.ts`:
- Around line 781-785: The HAR redaction loop currently only calls
redactHarHeaders and redactHarQueryParams, leaving request.url and non-keyword
secret values unredacted and not applying provided bindings; update the loop
over har.log.entries to also call the HAR value redaction routine on
entry.request.url and entry.response?.url (e.g., call
redactHarValues(entry.request?.url, bindings) and
redactHarValues(entry.response?.url, bindings)), and ensure redactHarHeaders and
redactHarQueryParams accept and use the bindings parameter so they redact
non-keyword secret values (and update any internal helper that performs value
redaction to apply bindings-based replacements); apply the same changes to the
other HAR-processing block (the code around lines 1010-1036) so both places
sanitize request.url and use bindings consistently.
- Around line 789-791: The current behavior leaves an unredacted HAR file on
disk when redaction fails (the call that logs via Logger.w and uses
this._formatError); update this failure path to securely remove the unredacted
HAR instead of keeping it: after catching the redaction error, attempt to delete
the on-disk HAR within a safe try/catch (log any deletion error via Logger.e),
and only retain any redacted output if redaction actually succeeded; ensure you
reference the same symbols (Logger.w, this._formatError) and perform the
filesystem unlink/remove call in the same error-handling block so the raw HAR is
not persisted on disk on redaction failure.
In `@packages/cli/src/sessionRunner.ts`:
- Around line 535-540: The code clears activeNetworkCapture when
stopNetworkCapture returns success: false, preventing the finally cleanup from
calling abort; update the failure branch in the stop logic (the block that logs
with Logger.w and handles stopNetResponse) to call activeNetworkCapture.abort()
(or activeNetworkCapture?.abort()) before setting activeNetworkCapture =
undefined, guarding with a null check and swallowing/logging any abort errors so
cleanup proceeds safely; reference the stopNetworkCapture call, the
activeNetworkCapture variable, and the Logger.w log path when making this
change.
In `@packages/device-node/src/device/NetworkCaptureManager.ts`:
- Around line 113-144: Pending request entries in _pendingRequests can be left
orphaned if no response arrives; add a cleanup mechanism to prevent memory leaks
by: when starting the capture (where _server.on('request') and
_server.on('response') are registered) also start a periodic timer that scans
_pendingRequests and removes entries older than a threshold (e.g., 60s) and
optionally converts them into completed NetworkCapturedEntry records in _entries
with an appropriate status/duration; also subscribe to request-level
abort/error/TLS events from _server to delete the specific pending entry
immediately; ensure the timer is cleared and any remaining pending entries are
flushed/cleared in stopSession so no timer or map entries survive session end.
---
Nitpick comments:
In `@packages/cli/package.json`:
- Around line 42-60: bundleDependencies in package.json is missing the newly
added runtime dependency "mockttp", causing packaging drift; update the
bundleDependencies array (the "bundleDependencies" entry in package.json) to
include "mockttp" alongside the other entries to keep bundled/offline installs
consistent with the dependencies list.
In `@packages/cli/src/commands/logNetwork/adb.ts`:
- Around line 40-48: The try-catch around the fallback return is unnecessary
because "return 'adb'" cannot throw; remove the try { return 'adb'; } catch {
return null; } block and replace it with a direct return 'adb' (leaving the
surrounding function—the fallback branch that currently returns 'adb'—intact),
ensuring any earlier code paths that might return null remain unchanged.
- Around line 141-156: The shell commands in setGlobalProxy and getProp
interpolate untrusted values; validate and/or escape inputs before calling
shell. For setGlobalProxy (function setGlobalProxy) validate hostPort with a
strict regex (e.g. host or IP + ":" + port range) and reject or throw on invalid
formats, or escape it by wrapping in single quotes and replacing any embedded
single quotes; for getProp (function getProp) either restrict key to an
allowlist or validate against a tight pattern like /^[A-Za-z0-9._-]+$/ and
reject invalid characters, or escape the key similarly; alternatively refactor
shell invocation to accept argv-style args to avoid building a single
interpolated command. Ensure clear error handling when validation fails.
In `@packages/cli/src/commands/logNetwork/colors.ts`:
- Around line 6-16: The colors object currently always emits ANSI escapes;
change it to respect NO_COLOR and non-TTY output by computing a single boolean
(e.g., allowColors = !process.env.NO_COLOR && process.stdout?.isTTY &&
process.env.TERM !== 'dumb') at module init and, if false, replace all functions
on the exported colors object (green, red, yellow, cyan, magenta, blue, dim,
bold) to be identity functions that return the input string (and keep white
as-is); keep ESC and RESET defined but only used when allowColors is true so
redirected/piped logs contain no escape codes.
In `@packages/cli/src/commands/logNetwork/ios.ts`:
- Around line 65-96: The startPacServer function and the autoproxy management
logic duplicated from NetworkCaptureSetup.ts should be extracted into a shared
utility module and imported where needed; specifically, move startPacServer and
the corresponding autoproxy functions into a new common module (e.g., a
shared/network utility) with the same exported function names (startPacServer
and any autoproxy enable/disable helpers) and appropriate return types, update
packages/cli/src/commands/logNetwork/ios.ts to import those functions instead of
defining them, and update packages/device-node/src/device/NetworkCaptureSetup.ts
to import the same shared exports; ensure the new module exports identical
signatures and that any internal types or dependencies are adjusted so both
callers compile without duplicated logic.
In `@packages/cli/src/reportServer.ts`:
- Around line 306-327: The readNetworkHar function has verbose nested casts and
property chains; extract the HAR entry mapping into a small helper (e.g.,
mapHarEntry) to improve readability and reduce repetition: inside readNetworkHar
call entries = (har?.log?.entries ?? []).map(mapHarEntry) and implement
mapHarEntry to safely pull request/response/content as Record<string, unknown> |
undefined and return the object fields (startedDateTime, method, url, status,
statusText, time, responseSize using content?.size ?? response?.bodySize ?? 0,
requestHeaders, responseHeaders). Keep the existing defensive checks and error
handling behavior unchanged.
In `@packages/cli/src/reportTemplate.ts`:
- Around line 1595-1598: The client-side escapeHtmlJS function currently escapes
&, <, > and " but omits single quotes, causing inconsistency with the
server-side escapeHtml; update escapeHtmlJS to also replace single quotes (')
with an HTML entity (e.g., ') so its output matches escapeHtml and avoids
single-quote injection issues, keeping the replacement sequence consistent with
the existing chain in escapeHtmlJS.
In `@packages/cli/src/workspace.ts`:
- Around line 431-437: readNetworkConfig currently silently returns undefined
for malformed but present `network` values (e.g., strings/arrays); change it to
validate and throw when `raw` is not undefined/null but also not a plain object
(or is an array), and also throw if `obj['capture']` is present but not a
boolean. Update the function readNetworkConfig to keep the existing behavior for
undefined/null, but if raw is present and (typeof raw !== 'object' ||
Array.isArray(raw')) throw a TypeError with a clear message like "Invalid
'network' config: expected an object with optional boolean 'capture'"; likewise
throw if capture exists and typeof capture !== 'boolean'. This preserves the
return shape { capture?: boolean } on success and provides consistent error
feedback to users.
In `@packages/device-node/src/device/NetworkCaptureManager.ts`:
- Around line 294-342: The HAR builder in function buildHar hardcodes
httpVersion: 'HTTP/1.1' for both request and response entries which can
misrepresent HTTP/2 traffic; update the request and response httpVersion fields
to use any real protocol/version properties available on the
NetworkCapturedEntry (e.g., e.httpVersion, e.protocol, or e.requestProtocol if
such fields exist), falling back to 'HTTP/1.1' when absent, and add a short code
comment explaining the fallback and limitation if the capture layer (mockttp)
doesn't expose the protocol.
- Around line 238-240: The startedAt ternary is confusing and redundantly calls
this._testStartIndexMap.get(mapKey) even though startIndex was already
retrieved; replace the chain to use the local startIndex variable and
testEntries only: set startedAt = testEntries.length > 0 ?
testEntries[0]!.startedAt : (startIndex !== undefined ? new Date() :
completedAt), ensuring you reference testEntries, startIndex and completedAt
(and remove the extra _testStartIndexMap.get call) so the logic is clear and
correct.
In `@packages/device-node/src/device/NetworkCaptureSetup.ts`:
- Around line 90-96: The shell commands in _setGlobalProxy and _clearGlobalProxy
are built via string interpolation with the unvalidated value; add a validation
step in _setGlobalProxy to ensure value matches the expected proxy format (e.g.,
host:port or host:port;scheme) and explicitly allow the special clear token ":0"
used by _clearGlobalProxy, rejecting or throwing for malformed inputs; then call
this._shell only after validation (or sanitize/escape the value) and keep
_clearGlobalProxy using the constant ':0' to avoid interpolation issues.
- Around line 225-244: The function findActiveNetworkService currently falls
back to returning services[0] when no service has an active IP, which can cause
proxy configuration on an inactive interface; change the fallback to return null
instead and update any callers of findActiveNetworkService to handle a null
return (e.g., skip configuration, surface an explicit error, or choose a safe
default). Locate the logic in findActiveNetworkService (the final return
statement) and replace the services[0] fallback with null, and audit usages of
findActiveNetworkService to add null-checks and appropriate error handling or
user-facing messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 25ce6a74-69c0-427b-80f1-119e65d3a4db
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (30)
README.mddocs/cli-reference.mddocs/configuration.mddocs/network-capture.mddocs/network-logging.mdpackages/cli/bin/finalrun.tspackages/cli/package.jsonpackages/cli/src/commands/logNetwork/adb.tspackages/cli/src/commands/logNetwork/ca.tspackages/cli/src/commands/logNetwork/capture.tspackages/cli/src/commands/logNetwork/colors.tspackages/cli/src/commands/logNetwork/index.tspackages/cli/src/commands/logNetwork/ios.tspackages/cli/src/commands/logNetwork/livePrinter.tspackages/cli/src/commands/logNetwork/proxyState.tspackages/cli/src/reportServer.tspackages/cli/src/reportTemplate.tspackages/cli/src/reportWriter.tspackages/cli/src/sessionRunner.tspackages/cli/src/testRunner.tspackages/cli/src/workspace.tspackages/common/src/index.tspackages/common/src/interfaces/DeviceAgent.tspackages/common/src/models/NetworkLog.tspackages/common/src/models/TestResult.tspackages/device-node/src/device/Device.tspackages/device-node/src/device/NetworkCaptureManager.tspackages/device-node/src/device/NetworkCaptureSetup.tspackages/device-node/src/index.tspackages/goal-executor/src/TestExecutor.ts
| @@ -0,0 +1,222 @@ | |||
| # Network Logging | |||
|
|
|||
| FinalRun can capture every HTTP/HTTPS request your app makes during a test run — method, URL, status code, headers, request and response bodies, and timing. Traffic is recorded as [HAR 1.2](https://w3c.github.io/web-performance/specs/HAR/Overview.html) (the same format Chrome DevTools uses) and displayed in your test report with a searchable Network tab. | |||
There was a problem hiding this comment.
Intro text contradicts current body-capture behavior.
Line [3] says request/response bodies are captured, but Lines [139]-[140] say they are not captured yet. Please align the intro with current behavior.
Also applies to: 139-140
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/network-logging.md` at line 3, The intro sentence currently claiming
"request and response bodies" are captured contradicts the later note that
bodies are not yet captured; update the intro (the paragraph that mentions
"method, URL, status code, headers, request and response bodies, and timing" and
the HAR 1.2 sentence) to accurately state that request and response bodies are
not captured yet (e.g., list captured fields as method, URL, status code,
headers, and timing, and note bodies are planned/future), and make the phrasing
consistent with the later line that says bodies are not captured yet so both
places (the intro and the existing note about bodies) match.
Comment on lines
+77
to
+85
| ``` | ||
| artifacts/{runId}/tests/{testId}/ | ||
| ├── result.json | ||
| ├── recording.mp4 | ||
| ├── device.log | ||
| ├── network.har ← new | ||
| ├── actions/ | ||
| └── screenshots/ | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to unlabeled fenced blocks (MD040).
Use text/bash on these fences to satisfy markdownlint and keep rendering consistent.
🔧 Proposed fix
-```
+```text
artifacts/{runId}/tests/{testId}/
├── result.json
├── recording.mp4
@@
-```
+```text
GET https://en.wikipedia.org/api/rest_v1/feed/featured/2026/04/07 200 111ms 31.8 KB
POST https://intake-analytics.wikimedia.org/v1/events 201 194ms 0 B
@@
-```
+```bash
finalrun log-network --platform=<android|ios> [options]Also applies to: 121-126, 211-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/network-logging.md` around lines 77 - 85, The Markdown contains
unlabeled fenced code blocks; add language identifiers to each unlabeled
triple-backtick fence to satisfy MD040: use "text" for plain directory listing
and HTTP log samples (e.g., the block starting with
"artifacts/{runId}/tests/{testId}/" and the block containing "GET
https://en.wikipedia.org/..."), and use "bash" for command examples (e.g., the
block with "finalrun log-network --platform=<android|ios> [options]"). Update
the three affected ranges (around the directory tree, the HTTP/har sample, and
the CLI invocation) to include these labels so markdownlint passes and rendering
stays consistent.
Comment on lines
+242
to
+250
| program | ||
| .command('log-network') | ||
| .description('Capture and display HTTP(S) network traffic from a connected device') | ||
| .requiredOption('--platform <platform>', 'Target platform (android)') | ||
| .option('--device <serial>', 'Device serial (auto-detected if only one)') | ||
| .option('--out <path>', 'Output HAR file path (default: auto-generated)') | ||
| .action(async (options: { platform: string; device?: string; out?: string }) => { | ||
| await runLogNetworkCommand(options); | ||
| }); |
There was a problem hiding this comment.
Inconsistent platform description between command and help text.
The command description on Line 244 says "HTTP(S) network traffic from a connected device" (implying both platforms), while the --platform option help text on Line 245 says only (android). However, the CLI reference documentation (docs/cli-reference.md) shows --platform <android|ios>.
If iOS is supported, update the help text:
- .requiredOption('--platform <platform>', 'Target platform (android)')
+ .requiredOption('--platform <platform>', 'Target platform (android or ios)')If only Android is currently supported, update the documentation to match.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| program | |
| .command('log-network') | |
| .description('Capture and display HTTP(S) network traffic from a connected device') | |
| .requiredOption('--platform <platform>', 'Target platform (android)') | |
| .option('--device <serial>', 'Device serial (auto-detected if only one)') | |
| .option('--out <path>', 'Output HAR file path (default: auto-generated)') | |
| .action(async (options: { platform: string; device?: string; out?: string }) => { | |
| await runLogNetworkCommand(options); | |
| }); | |
| program | |
| .command('log-network') | |
| .description('Capture and display HTTP(S) network traffic from a connected device') | |
| .requiredOption('--platform <platform>', 'Target platform (android or ios)') | |
| .option('--device <serial>', 'Device serial (auto-detected if only one)') | |
| .option('--out <path>', 'Output HAR file path (default: auto-generated)') | |
| .action(async (options: { platform: string; device?: string; out?: string }) => { | |
| await runLogNetworkCommand(options); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/bin/finalrun.ts` around lines 242 - 250, The CLI shows
inconsistent platform support for the log-network command: update the help text
for program.command('log-network') and its .requiredOption('--platform
<platform>') to match actual supported platforms used by runLogNetworkCommand;
if iOS is supported, change the option help from "(android)" to "(android|ios)"
and adjust the command description to mention both platforms, otherwise keep the
description generic or explicitly state "android only" and update
docs/cli-reference.md to match; ensure any mention of supported platforms in
runLogNetworkCommand or related validation logic is consistent with the help
text.
Comment on lines
+49
to
+53
| async start(cert: string, key: string, callbacks: NetworkCaptureCallbacks): Promise<number> { | ||
| this._server = mockttp.getLocal({ https: { cert, key } }); | ||
| await this._server.forAnyRequest().thenPassThrough(); | ||
| await this._server.start(); | ||
| this._port = this._server.port; |
There was a problem hiding this comment.
Reset in-memory capture state between runs to prevent cross-session contamination.
Current lifecycle keeps prior entries/TLS errors/pending requests if the same instance is started again, producing incorrect HAR data.
🧹 Proposed fix
async start(cert: string, key: string, callbacks: NetworkCaptureCallbacks): Promise<number> {
+ if (this._server) throw new Error('NetworkCapture is already running');
+ this._entries = [];
+ this._tlsErrors = [];
+ this._pendingRequests.clear();
+ this._port = 0;
+
this._server = mockttp.getLocal({ https: { cert, key } });
await this._server.forAnyRequest().thenPassThrough();
await this._server.start();
this._port = this._server.port;
@@
async stop(): Promise<void> {
if (this._server) {
await this._server.stop();
this._server = null;
}
+ this._pendingRequests.clear();
+ this._port = 0;
}Also applies to: 108-113
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/logNetwork/capture.ts` around lines 49 - 53, The
start method retains previous in-memory capture state (entries/TLS
errors/pending requests) when reused, causing cross-session contamination;
modify start(cert, key, callbacks) to reset/clear the internal state before
creating or starting the mockttp server — e.g., reinitialize this._entries,
this._tlsErrors, this._pendingRequests (and any other per-run buffers) to empty
arrays/objects and remove any leftover listeners on this._server, or recreate
this._server anew so prior data is not reused; apply the same reset logic where
the server can be restarted (the corresponding stop/restart flow referenced
around the other start/stop call sites) so each run begins with a clean
in-memory state.
Comment on lines
+148
to
+185
| async function testProxyConnectivity( | ||
| proxyPort: number, | ||
| ca: { cert: string }, | ||
| capture: NetworkCapture, | ||
| ): Promise<boolean> { | ||
| const countBefore = capture.entries.length; | ||
| const result = await new Promise<boolean>((resolve) => { | ||
| const req = https.request( | ||
| { | ||
| hostname: 'example.com', | ||
| port: 443, | ||
| path: '/', | ||
| method: 'HEAD', | ||
| agent: new https.Agent({ | ||
| host: '127.0.0.1', | ||
| port: proxyPort, | ||
| ca: ca.cert, | ||
| } as https.AgentOptions), | ||
| timeout: 5000, | ||
| }, | ||
| (res) => { | ||
| res.resume(); | ||
| resolve(res.statusCode !== undefined); | ||
| }, | ||
| ); | ||
| req.on('error', () => resolve(false)); | ||
| req.on('timeout', () => { | ||
| req.destroy(); | ||
| resolve(false); | ||
| }); | ||
| req.end(); | ||
| }); | ||
| // Remove the test request from captured entries so it doesn't pollute the HAR. | ||
| if (capture.entries.length > countBefore) { | ||
| (capture.entries as CapturedEntry[]).splice(countBefore); | ||
| } | ||
| return result; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for how the capture proxy handles HTTPS - does it handle CONNECT tunneling?
rg -n "CONNECT|tunnel" --type=ts packages/cli/src/commands/logNetwork/Repository: final-run/finalrun-agent
Length of output: 187
🏁 Script executed:
# Check the full file context around the test function
wc -l packages/cli/src/commands/logNetwork/index.tsRepository: final-run/finalrun-agent
Length of output: 117
🏁 Script executed:
# Read the full file to understand the context
head -200 packages/cli/src/commands/logNetwork/index.tsRepository: final-run/finalrun-agent
Length of output: 6262
🏁 Script executed:
# Search for NetworkCapture and proxy implementations
rg -n "class NetworkCapture|interface NetworkCapture|function.*Proxy" --type=ts packages/cli/src/commands/logNetwork/Repository: final-run/finalrun-agent
Length of output: 1379
🏁 Script executed:
# Check if there's a custom proxy server implementation
find packages/cli/src -name "*proxy*" -type fRepository: final-run/finalrun-agent
Length of output: 118
🏁 Script executed:
# Check imports and dependencies in this file
head -50 packages/cli/src/commands/logNetwork/index.tsRepository: final-run/finalrun-agent
Length of output: 1274
🏁 Script executed:
# Search for where the proxy server is created (createServer, listen, etc.)
rg -n "createServer|listen.*proxy|new.*Proxy" --type=ts packages/cli/src/commands/logNetwork/Repository: final-run/finalrun-agent
Length of output: 535
🏁 Script executed:
# Check the capture.js file where NetworkCapture is defined
cat packages/cli/src/commands/logNetwork/capture.ts | head -100Repository: final-run/finalrun-agent
Length of output: 3464
🏁 Script executed:
# Search for how the proxy is started/created in the main file
grep -n "startProxy\|createProxy\|new.*Server\|listen" packages/cli/src/commands/logNetwork/index.tsRepository: final-run/finalrun-agent
Length of output: 247
🏁 Script executed:
# Look for CONNECT method handling or tunnel logic
rg -n "CONNECT|\.connect\(" --type=ts packages/cli/src/commands/logNetwork/Repository: final-run/finalrun-agent
Length of output: 50
🏁 Script executed:
# Check package.json for http-proxy-agent or https-proxy-agent
rg -n "proxy-agent|mockttp" packages/cli/package.jsonRepository: final-run/finalrun-agent
Length of output: 95
🏁 Script executed:
# Check if there are any other uses of https.Agent in the codebase
rg -n "https.Agent|http.Agent|new Agent" --type=ts packages/cli/src/Repository: final-run/finalrun-agent
Length of output: 234
🏁 Script executed:
# Look at how mockttp is configured and used
grep -A 20 "mockttp.getLocal" packages/cli/src/commands/logNetwork/capture.tsRepository: final-run/finalrun-agent
Length of output: 813
🏁 Script executed:
# Check if maybe the test proxy uses environment variables or global proxy settings
rg -n "http_proxy|https_proxy|HTTP_PROXY|HTTPS_PROXY" --type=ts packages/cli/src/commands/logNetwork/Repository: final-run/finalrun-agent
Length of output: 436
🏁 Script executed:
# Search for how the test/proxy is actually being used elsewhere in the code
rg -n "testProxyConnectivity|isProxyResponding" --type=ts packages/cli/src/commands/logNetwork/ -B 2 -A 2Repository: final-run/finalrun-agent
Length of output: 2382
🏁 Script executed:
# Check if mockttp provides any special client methods
rg -n "mockttp\." --type=ts packages/cli/src/commands/logNetwork/capture.ts | head -20Repository: final-run/finalrun-agent
Length of output: 186
🏁 Script executed:
# Check the exact context around where testProxyConnectivity is called
sed -n '405,420p' packages/cli/src/commands/logNetwork/index.tsRepository: final-run/finalrun-agent
Length of output: 742
🏁 Script executed:
# Look for any documentation or comments about how this proxy test works
sed -n '140,180p' packages/cli/src/commands/logNetwork/index.tsRepository: final-run/finalrun-agent
Length of output: 1127
🏁 Script executed:
# Check if Node.js has documentation on how agent options work
web_search
Node.js https.Agent host port properties proxyRepository: final-run/finalrun-agent
Length of output: 164
🏁 Script executed:
# Search for proper proxy agent usage patterns in Node.js ecosystem
rg -n "https-proxy-agent|http-proxy-agent|HttpProxyAgent|HttpsProxyAgent" --type=ts packages/Repository: final-run/finalrun-agent
Length of output: 50
🏁 Script executed:
# Check Node.js documentation or comments about agent configuration
rg -n "Agent.*proxy|proxy.*Agent" --type=ts packages/cli/src/Repository: final-run/finalrun-agent
Length of output: 50
🌐 Web query:
Node.js https.Agent host port properties socket connection
💡 Result:
In Node.js, an https.Agent (like http.Agent) pools/reuses sockets by a “name” derived from the request’s connection options, not from some agent.host / agent.port property.
How the Agent decides which socket to use (host/port)
- The agent computes a key via
agent.getName(options). - For
http.Agent, that name is essentially:host:port:localAddress(and sometimes:family). - For
https.Agent, the name includeshost:port:localAddressplus TLS options (CA/cert/ciphers/etc.), because those affect whether a socket can be reused. [1]
So, host/port are per request options, and the agent’s pools (e.g., agent.sockets, agent.freeSockets) are organized by that computed key.
Where to read the actual connected host/port (on the socket)
Once a request has a socket, the socket exposes the real endpoints:
socket.remoteAddress,socket.remotePort= the peer you connected tosocket.localAddress,socket.localPort= your side of the TCP connection [2]
You can access it from the request:
const req = https.request({ hostname: 'example.com', port: 443, agent }, (res) => {});
req.on('socket', (socket) => {
console.log(socket.remoteAddress, socket.remotePort);
console.log(socket.localAddress, socket.localPort);
});
req.end();Which options control host/port
http.request/https.requesttakehost/hostnameandportin the options;hostis “domain/IP of the server to issue the request to” (andfamilyaffects resolution). [3]- For
https.request, the defaultportis 443 if you don’t specify it. [4]
References:
[1] Node.js HTTP docs (agent.getName(options) and HTTPS agent name includes TLS options) (nodejs.org)
[2] Node.js net.Socket docs (remotePort, localAddress, etc.) (nodejs.org)
[3] Node.js HTTP request option host / hostname (and related options) (nodejs.org)
[4] Node.js HTTPS docs (port default 443) (nodejs.org)
Citations:
- 1: https://nodejs.org/download/release/v4.2.2/docs/api/http.html?utm_source=openai
- 2: https://nodejs.org/download/release/v22.1.0/docs/api/net.html?utm_source=openai
- 3: https://nodejs.org/download/nightly/v22.0.0-nightly202311113cce03a03f/docs/api/http.html?utm_source=openai
- 4: https://nodejs.org/download/release/v14.1.0/docs/api/https.html?utm_source=openai
The HTTPS agent configuration does not route requests through the proxy.
The https.request() call uses hostname: 'example.com' and port: 443 to determine the connection destination. The non-standard host and port properties on the https.Agent are ignored at runtime—Node.js only uses the request's own hostname/port options to determine where to connect. This means the request connects directly to example.com:443, bypassing the mockttp proxy at 127.0.0.1:proxyPort entirely.
For proper HTTPS proxy testing, the request must use either the CONNECT tunneling method (which requires an http-proxy-agent or https-proxy-agent library) or make the request to the proxy itself (not to the final destination). As written, this test only validates that the CA certificate is accepted by example.com, not that the proxy is working or intercepting traffic as intended.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/logNetwork/index.ts` around lines 148 - 185,
testProxyConnectivity is currently creating an HTTPS request to example.com
directly and passing non-standard host/port on https.Agent (which are ignored),
so the proxy is bypassed; change the implementation to actually target the proxy
or use a proper proxy agent: either (A) make the request connect to
127.0.0.1:proxyPort (set request hostname/port to the proxy and set the request
path to the full destination URL or use a CONNECT method) and remove the invalid
agent.host/port usage, or (B) install and use a proxy agent (e.g.,
http-proxy-agent / https-proxy-agent) and pass that agent to https.request so
the request is tunneled through the mockttp proxy; update testProxyConnectivity,
the https.request options and agent construction accordingly so the request is
routed through the proxy and captured by NetworkCapture.
Comment on lines
+29
to
+32
| export async function saveProxyState(state: SavedProxyState): Promise<void> { | ||
| await fsp.mkdir(STATE_DIR, { recursive: true }); | ||
| await fsp.writeFile(STATE_FILE, JSON.stringify(state, null, 2), 'utf8'); | ||
| } |
There was a problem hiding this comment.
Make state-file writes atomic to preserve crash recovery guarantees.
Line [31] can leave a partially written proxy-state.json on abrupt termination, which breaks stale-session restoration on next run.
🛠️ Proposed fix
export async function saveProxyState(state: SavedProxyState): Promise<void> {
await fsp.mkdir(STATE_DIR, { recursive: true });
- await fsp.writeFile(STATE_FILE, JSON.stringify(state, null, 2), 'utf8');
+ const tmpFile = `${STATE_FILE}.tmp-${process.pid}`;
+ await fsp.writeFile(tmpFile, JSON.stringify(state, null, 2), { encoding: 'utf8', mode: 0o600 });
+ await fsp.rename(tmpFile, STATE_FILE);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/commands/logNetwork/proxyState.ts` around lines 29 - 32,
saveProxyState currently writes STATE_FILE directly and can leave a partially
written proxy-state.json on crashes; change saveProxyState to write atomically
by: ensure STATE_DIR exists, create a temp file in STATE_DIR (e.g. STATE_FILE +
'.tmp' or use a unique suffix), open the temp file with fsp.open, write JSON
into it, call filehandle.datasync()/fsync to flush to disk, close the handle,
then rename (fs.rename) the temp file to STATE_FILE to perform an atomic
replace; reference saveProxyState, STATE_DIR and STATE_FILE when locating where
to implement this change.
Comment on lines
+781
to
+785
| for (const entry of har.log.entries) { | ||
| redactHarHeaders(entry.request?.headers); | ||
| redactHarHeaders(entry.response?.headers); | ||
| redactHarQueryParams(entry.request?.queryString); | ||
| } |
There was a problem hiding this comment.
Redaction is incomplete: sensitive data can still leak via request.url and non-keyword secret values.
Current logic redacts header/query arrays, but request.url (used in report rendering) can still retain raw query secrets. Also, bindings is passed but not applied during HAR value redaction.
🔧 Proposed fix
- redactHarHeaders(entry.request?.headers);
- redactHarHeaders(entry.response?.headers);
- redactHarQueryParams(entry.request?.queryString);
+ redactHarHeaders(entry.request?.headers, bindings);
+ redactHarHeaders(entry.response?.headers, bindings);
+ redactHarQueryParams(entry.request?.queryString, bindings);
+ redactHarRequestUrl(entry.request, bindings);-function redactHarHeaders(headers: Array<{ name: string; value: string }> | undefined): void {
+function redactHarHeaders(
+ headers: Array<{ name: string; value: string }> | undefined,
+ bindings: RuntimeBindings,
+): void {
if (!headers) return;
for (const header of headers) {
- if (shouldRedactHeader(header.name)) {
- header.value = '[REDACTED]';
- }
+ const value = redactResolvedValue(header.value, bindings) ?? header.value;
+ header.value = shouldRedactHeader(header.name) ? '[REDACTED]' : value;
}
}-function redactHarQueryParams(queryString: Array<{ name: string; value: string }> | undefined): void {
+function redactHarQueryParams(
+ queryString: Array<{ name: string; value: string }> | undefined,
+ bindings: RuntimeBindings,
+): void {
if (!queryString) return;
for (const param of queryString) {
- if (REDACTED_QUERY_PARAM_NAMES.has(param.name.toLowerCase())) {
- param.value = '[REDACTED]';
- }
+ const value = redactResolvedValue(param.value, bindings) ?? param.value;
+ param.value = REDACTED_QUERY_PARAM_NAMES.has(param.name.toLowerCase()) ? '[REDACTED]' : value;
}
}
+
+function redactHarRequestUrl(
+ request: { url?: string } | undefined,
+ bindings: RuntimeBindings,
+): void {
+ if (!request?.url) return;
+ try {
+ const u = new URL(request.url);
+ for (const [key, value] of u.searchParams.entries()) {
+ if (REDACTED_QUERY_PARAM_NAMES.has(key.toLowerCase())) {
+ u.searchParams.set(key, '[REDACTED]');
+ } else {
+ u.searchParams.set(key, redactResolvedValue(value, bindings) ?? value);
+ }
+ }
+ request.url = u.toString();
+ } catch {
+ request.url = redactResolvedValue(request.url, bindings) ?? request.url;
+ }
+}Also applies to: 1010-1036
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/reportWriter.ts` around lines 781 - 785, The HAR redaction
loop currently only calls redactHarHeaders and redactHarQueryParams, leaving
request.url and non-keyword secret values unredacted and not applying provided
bindings; update the loop over har.log.entries to also call the HAR value
redaction routine on entry.request.url and entry.response?.url (e.g., call
redactHarValues(entry.request?.url, bindings) and
redactHarValues(entry.response?.url, bindings)), and ensure redactHarHeaders and
redactHarQueryParams accept and use the bindings parameter so they redact
non-keyword secret values (and update any internal helper that performs value
redaction to apply bindings-based replacements); apply the same changes to the
other HAR-processing block (the code around lines 1010-1036) so both places
sanitize request.url and use bindings consistently.
Comment on lines
+789
to
+791
| Logger.w(`Failed to redact network HAR file: ${this._formatError(error)}`); | ||
| // Keep the unredacted copy rather than deleting — better to have data. | ||
| } |
There was a problem hiding this comment.
Redaction failure currently leaves an unredacted HAR on disk.
Keeping raw HAR data after parse/redaction errors is a fail-open behavior and can expose secrets/PII in artifacts.
🔧 Proposed fix
} catch (error) {
Logger.w(`Failed to redact network HAR file: ${this._formatError(error)}`);
- // Keep the unredacted copy rather than deleting — better to have data.
+ await fsp.unlink(targetPath).catch(() => {});
+ return undefined;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Logger.w(`Failed to redact network HAR file: ${this._formatError(error)}`); | |
| // Keep the unredacted copy rather than deleting — better to have data. | |
| } | |
| Logger.w(`Failed to redact network HAR file: ${this._formatError(error)}`); | |
| await fsp.unlink(targetPath).catch(() => {}); | |
| return undefined; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/reportWriter.ts` around lines 789 - 791, The current
behavior leaves an unredacted HAR file on disk when redaction fails (the call
that logs via Logger.w and uses this._formatError); update this failure path to
securely remove the unredacted HAR instead of keeping it: after catching the
redaction error, attempt to delete the on-disk HAR within a safe try/catch (log
any deletion error via Logger.e), and only retain any redacted output if
redaction actually succeeded; ensure you reference the same symbols (Logger.w,
this._formatError) and perform the filesystem unlink/remove call in the same
error-handling block so the raw HAR is not persisted on disk on redaction
failure.
Comment on lines
+535
to
+540
| Logger.w( | ||
| `Unable to stop network capture for test ${activeNetworkCapture.testId}: ` + | ||
| `${stopNetResponse.message ?? 'unknown error'}`, | ||
| ); | ||
| activeNetworkCapture = undefined; | ||
| } |
There was a problem hiding this comment.
Call abort on failed stopNetworkCapture before clearing state.
When stopNetworkCapture returns success: false, the code clears activeNetworkCapture immediately, so the finally cleanup won’t run abort for that run.
🔧 Proposed fix
} else {
Logger.w(
`Unable to stop network capture for test ${activeNetworkCapture.testId}: ` +
`${stopNetResponse.message ?? 'unknown error'}`,
);
+ try {
+ await session.device.abortNetworkCapture(activeNetworkCapture.runId);
+ } catch (error) {
+ Logger.w('Failed to finalize network capture after stop failure:', error);
+ }
activeNetworkCapture = undefined;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Logger.w( | |
| `Unable to stop network capture for test ${activeNetworkCapture.testId}: ` + | |
| `${stopNetResponse.message ?? 'unknown error'}`, | |
| ); | |
| activeNetworkCapture = undefined; | |
| } | |
| Logger.w( | |
| `Unable to stop network capture for test ${activeNetworkCapture.testId}: ` + | |
| `${stopNetResponse.message ?? 'unknown error'}`, | |
| ); | |
| try { | |
| await session.device.abortNetworkCapture(activeNetworkCapture.runId); | |
| } catch (error) { | |
| Logger.w('Failed to finalize network capture after stop failure:', error); | |
| } | |
| activeNetworkCapture = undefined; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/src/sessionRunner.ts` around lines 535 - 540, The code clears
activeNetworkCapture when stopNetworkCapture returns success: false, preventing
the finally cleanup from calling abort; update the failure branch in the stop
logic (the block that logs with Logger.w and handles stopNetResponse) to call
activeNetworkCapture.abort() (or activeNetworkCapture?.abort()) before setting
activeNetworkCapture = undefined, guarding with a null check and
swallowing/logging any abort errors so cleanup proceeds safely; reference the
stopNetworkCapture call, the activeNetworkCapture variable, and the Logger.w log
path when making this change.
Comment on lines
+113
to
+144
| // Subscribe to request events — store request metadata (no body). | ||
| await this._server.on('request', (req) => { | ||
| this._pendingRequests.set(req.id, { | ||
| startedAt: new Date(), | ||
| method: req.method, | ||
| url: req.url, | ||
| headers: flattenHeaders(req.headers), | ||
| bodySize: req.body?.buffer?.byteLength ?? 0, | ||
| }); | ||
| }); | ||
|
|
||
| // Subscribe to response events — correlate with request, store entry. | ||
| await this._server.on('response', (response) => { | ||
| const completedAt = new Date(); | ||
| const pending = this._pendingRequests.get(response.id); | ||
| this._pendingRequests.delete(response.id); | ||
|
|
||
| const entry: NetworkCapturedEntry = { | ||
| startedAt: pending?.startedAt ?? completedAt, | ||
| completedAt, | ||
| method: pending?.method ?? '???', | ||
| url: pending?.url ?? '', | ||
| statusCode: response.statusCode, | ||
| statusMessage: response.statusMessage ?? '', | ||
| requestHeaders: pending?.headers ?? {}, | ||
| responseHeaders: flattenHeaders(response.headers), | ||
| requestBodySize: pending?.bodySize ?? 0, | ||
| responseBodySize: response.body?.buffer?.byteLength ?? 0, | ||
| durationMs: completedAt.getTime() - (pending?.startedAt ?? completedAt).getTime(), | ||
| }; | ||
| this._entries.push(entry); | ||
| }); |
There was a problem hiding this comment.
Potential memory leak from orphaned pending requests.
If a request is initiated but never receives a response (e.g., due to timeout, connection reset, or TLS error), its entry remains in _pendingRequests indefinitely. Over a long session with many failed requests, this could accumulate significant memory.
Consider adding a cleanup mechanism, such as:
- Periodic pruning of old entries (e.g., older than 60 seconds)
- Clearing on
abortor TLS error events for the specific request
Suggested improvement: Add orphan cleanup on TLS errors
// Subscribe to TLS errors.
await this._server.on('tls-client-error', (failure) => {
const f = failure as unknown as {
failureCause?: string;
tlsMetadata?: { sniHostname?: string };
+ remoteIpAddress?: string;
};
this._tlsErrors.push({
hostname: f.tlsMetadata?.sniHostname ?? 'unknown',
failureCause: f.failureCause ?? 'unknown',
timestamp: new Date(),
});
+ // Note: mockttp doesn't provide request ID for TLS errors,
+ // so we can't directly clean up _pendingRequests here.
+ // Consider adding periodic cleanup in stopSession or a timer.
});A more complete solution would add a periodic cleanup timer or clear old pending requests when stopping the session (which already happens in stopSession).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/device-node/src/device/NetworkCaptureManager.ts` around lines 113 -
144, Pending request entries in _pendingRequests can be left orphaned if no
response arrives; add a cleanup mechanism to prevent memory leaks by: when
starting the capture (where _server.on('request') and _server.on('response') are
registered) also start a periodic timer that scans _pendingRequests and removes
entries older than a threshold (e.g., 60s) and optionally converts them into
completed NetworkCapturedEntry records in _entries with an appropriate
status/duration; also subscribe to request-level abort/error/TLS events from
_server to delete the specific pending entry immediately; ensure the timer is
cleared and any remaining pending entries are flushed/cleared in stopSession so
no timer or map entries survive session end.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
finalrun log-networkstandalone command to capture HTTP/HTTPS traffic from Android/iOS devices, streaming requests live and writing HAR files on exitTest plan
finalrun log-network --platform androidand verify live traffic output + HAR file generation🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
network.capture: trueconfiguration to automatically record HTTP/HTTPS traffic for each test with automatic HAR exportfinalrun log-networkcommand for standalone interactive network capture on Android and iOS devices with live terminal outputNew Configuration
network.captureworkspace configuration option to enable/disable network capture globallyDocumentation