feat: professional trace viewer, Bedrock decoder, real-IP interception#10
Open
feat: professional trace viewer, Bedrock decoder, real-IP interception#10
Conversation
Add macOS notification after capture starts advising to restart IDE/CLI. Replace broken confirm() with HTML dialog for purge confirmation. Recreate tray on icon change to preserve template flag.
Bedrock EventStream decoder: - Parse AWS binary event stream frames (application/vnd.amazon.eventstream) - Extract base64-encoded JSON payloads into SSEChunk[] for token counting - Fallback: preserve raw response body when chunks not parsed Real-IP PF interception (fixes c-ares DNS bypass): - Resolve target domains to real IPs via DoH - Create PF rdr rules on physical interface for resolved IPs - Periodic 60s re-resolution for DNS round-robin - Complements existing /etc/hosts sentinel approach Health monitor fix: - Fire degraded notification once per episode, not every cooldown cycle Trace viewer professional overhaul: - Resizable panes with drag handles (sidebar, detail panel) - Column sorting (click headers) with direction indicators - Column resizing with drag handles - JSON syntax highlighting with collapsible tree view - Keyboard navigation (arrows, j/k, Enter, Escape, Cmd+F) - Status code color coding (2xx green, 4xx orange, 5xx red) - Duration performance indicators with mini timing bars - Auto-scroll toggle for live traces - Streaming progress indicator - Professional polish (alternating rows, sticky headers, trace count, custom scrollbars, responsive layout, ARIA roles)
📝 WalkthroughWalkthroughExtends PF anchor validation and cleanup to support real-IP DNS interception; adds Bedrock EventStream decoding; introduces user notifications for capture activation; implements real-IP DNS resolution and PF rules management; deduplicates health monitor alerts; and significantly enhances the UI with trace sorting, resizable panes, auto-scroll, and interactive JSON tree view. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 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)
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: 2
🧹 Nitpick comments (5)
src/bun/system/redirect-manager.ts (1)
18-22: Remove unusedstopRealIpRefreshimport.
clearRealIpRulescallsstopRealIpRefreshinternally (pf-manager.ts:357), so the explicit import is redundant.♻️ Remove unused import
import { initPfManager, ensureHelperInstalled, applyRules, clearRules, cleanupStaleRules, verifyDnsInterception, verifyRulesActive, applyRealIpRules, clearRealIpRules, startRealIpRefresh, - stopRealIpRefresh, } from "./pf-manager";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun/system/redirect-manager.ts` around lines 18 - 22, Remove the redundant import of stopRealIpRefresh from redirect-manager.ts: the function clearRealIpRules already calls stopRealIpRefresh internally, so delete stopRealIpRefresh from the import list (leaving applyRealIpRules, clearRealIpRules, startRealIpRefresh). Ensure no other references to stopRealIpRefresh remain in this module; if there are, either keep a single needed import or adjust those calls to use clearRealIpRules/startRealIpRefresh as intended.src/views/mainview/index.html (1)
75-75: Consider addingrole="option"to trace rows for ARIA compliance.The listbox has
role="listbox", but child trace rows should haverole="option"andaria-selectedfor proper assistive technology support. This would need to be set dynamically inindex.tswhen rendering rows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/mainview/index.html` at line 75, The trace list container with id "trace-rows" is marked role="listbox" but each rendered trace row must be given role="option" and a corresponding aria-selected attribute; update the row-rendering logic in index.ts (where trace rows are created/updated) to set role="option" on each row element and toggle aria-selected ("true"/"false") based on the currently focused/selected item, and ensure keyboard/mouse selection handlers update aria-selected accordingly.src/bun/sse/bedrock-decoder.ts (2)
63-75: Redundant JSON.parse calls.The decoded payload is parsed twice: once at line 64 (discarded) and again at line 70. Parse once and reuse.
♻️ Proposed fix
const decoded = Buffer.from(payload.bytes, "base64").toString("utf-8"); - // Try parsing to validate JSON; store the raw string as data - JSON.parse(decoded); - - // Map :event-type to SSE event name — Bedrock uses "chunk" for data frames - // The decoded JSON itself contains the Anthropic event type - let sseEvent: string | undefined; - try { - const decodedJson = JSON.parse(decoded); - sseEvent = decodedJson.type; - } catch { - // Use the EventStream event type as fallback - sseEvent = eventType; - } + // Parse once to validate and extract event type + const decodedJson = JSON.parse(decoded); + const sseEvent = decodedJson.type ?? eventType; chunks.push({ timestamp: Date.now(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun/sse/bedrock-decoder.ts` around lines 63 - 75, The code is parsing the same JSON twice (first JSON.parse(decoded) then again when assigning decodedJson); update the Bedrock decoder to parse decoded only once and reuse the result: remove the initial discarded JSON.parse(decoded) and instead call JSON.parse(decoded) once into a variable (e.g., decodedJson) then derive sseEvent = decodedJson.type with a try/catch or check for valid object, falling back to eventType if parsing fails or type is missing; adjust references to decodedJson and sseEvent in the surrounding logic so no duplicate parsing occurs.
156-160: Minor: Missing bounds check for Bytes type.For type 6 (Bytes),
pos += 2 + valueLenadvances without verifyingpos + 2 + valueLen <= headersEnd. IfvalueLenis malformed/large, this could skip past valid data. The loop will exit safely on the next iteration's bounds check, but an explicit guard would be cleaner.♻️ Proposed fix
} else if (valueType === 6) { // Bytes if (pos + 2 > headersEnd) break; const valueLen = buffer.readUInt16BE(pos); - pos += 2 + valueLen; + if (pos + 2 + valueLen > headersEnd) break; + pos += 2 + valueLen;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/bun/sse/bedrock-decoder.ts` around lines 156 - 160, In the valueType === 6 (Bytes) branch, add an explicit bounds check before advancing pos: after reading valueLen via buffer.readUInt16BE(pos) ensure that pos + 2 + valueLen <= headersEnd (using the existing pos, headersEnd, and valueLen variables); if the check fails, break (or return/throw consistent with surrounding logic) instead of blindly doing pos += 2 + valueLen to avoid skipping past valid data when valueLen is malformed.src/views/mainview/index.ts (1)
666-678: Silent clipboard failure could confuse users.The empty catch block means copy failures are invisible. Consider a brief visual indicator on failure.
Proposed improvement
navigator.clipboard.writeText(val).then(() => { target.textContent = "copied"; target.classList.add("copied"); setTimeout(() => { target.textContent = "copy"; target.classList.remove("copied"); }, 1200); - }).catch(() => {}); + }).catch(() => { + target.textContent = "failed"; + setTimeout(() => { + target.textContent = "copy"; + }, 1200); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/views/mainview/index.ts` around lines 666 - 678, The silent clipboard failure occurs in the click handler branch checking target.classList.contains("json-tree-copy") where navigator.clipboard.writeText(...).catch(() => {}) swallows errors; update the catch to both log the error (e.g., console.error) and provide a brief visual failure state on the target element (set a failure message like "copy failed", add a distinct class such as "copy-error", and remove/reset the text and class after the same timeout), so users get immediate feedback when navigator.clipboard.writeText 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 `@src/bun/system/pf-manager.ts`:
- Around line 306-319: The current ipPattern (/^\d{1,3}(\.\d{1,3}){3}$/) in
pf-manager.ts accepts invalid octets (e.g. 999); replace this loose check by
implementing a strict IPv4 validator (e.g. isValidIPv4 that splits on '.' and
ensures there are 4 numeric octets each 0–255 with no leading plus signs) and
use it instead of ipPattern.test inside the loop that populates allIps (refer to
ipPattern, domainIPs, allIps); keep the existing ifacePattern check untouched
and only swap the octet validation to call isValidIPv4(ip) before adding to
allIps.
In `@src/views/mainview/index.ts`:
- Around line 910-920: The call to applyColWidth inside cols.forEach is missing
the third argument, causing a runtime error when the function accesses
current[cls]; update the call applyColWidth(cls, savedWidths[cls]) to pass the
appropriate third parameter (the current map/object referenced as current inside
applyColWidth) so it becomes applyColWidth(cls, savedWidths[cls], current) (or
the actual variable name used in scope), ensuring applyColWidth can safely
read/update current[cls]; verify the variable passed matches the one used by
applyColWidth.
---
Nitpick comments:
In `@src/bun/sse/bedrock-decoder.ts`:
- Around line 63-75: The code is parsing the same JSON twice (first
JSON.parse(decoded) then again when assigning decodedJson); update the Bedrock
decoder to parse decoded only once and reuse the result: remove the initial
discarded JSON.parse(decoded) and instead call JSON.parse(decoded) once into a
variable (e.g., decodedJson) then derive sseEvent = decodedJson.type with a
try/catch or check for valid object, falling back to eventType if parsing fails
or type is missing; adjust references to decodedJson and sseEvent in the
surrounding logic so no duplicate parsing occurs.
- Around line 156-160: In the valueType === 6 (Bytes) branch, add an explicit
bounds check before advancing pos: after reading valueLen via
buffer.readUInt16BE(pos) ensure that pos + 2 + valueLen <= headersEnd (using the
existing pos, headersEnd, and valueLen variables); if the check fails, break (or
return/throw consistent with surrounding logic) instead of blindly doing pos +=
2 + valueLen to avoid skipping past valid data when valueLen is malformed.
In `@src/bun/system/redirect-manager.ts`:
- Around line 18-22: Remove the redundant import of stopRealIpRefresh from
redirect-manager.ts: the function clearRealIpRules already calls
stopRealIpRefresh internally, so delete stopRealIpRefresh from the import list
(leaving applyRealIpRules, clearRealIpRules, startRealIpRefresh). Ensure no
other references to stopRealIpRefresh remain in this module; if there are,
either keep a single needed import or adjust those calls to use
clearRealIpRules/startRealIpRefresh as intended.
In `@src/views/mainview/index.html`:
- Line 75: The trace list container with id "trace-rows" is marked
role="listbox" but each rendered trace row must be given role="option" and a
corresponding aria-selected attribute; update the row-rendering logic in
index.ts (where trace rows are created/updated) to set role="option" on each row
element and toggle aria-selected ("true"/"false") based on the currently
focused/selected item, and ensure keyboard/mouse selection handlers update
aria-selected accordingly.
In `@src/views/mainview/index.ts`:
- Around line 666-678: The silent clipboard failure occurs in the click handler
branch checking target.classList.contains("json-tree-copy") where
navigator.clipboard.writeText(...).catch(() => {}) swallows errors; update the
catch to both log the error (e.g., console.error) and provide a brief visual
failure state on the target element (set a failure message like "copy failed",
add a distinct class such as "copy-error", and remove/reset the text and class
after the same timeout), so users get immediate feedback when
navigator.clipboard.writeText fails.
🪄 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: dcc79afb-f641-4f13-903e-ee90b1672061
📒 Files selected for processing (13)
native/Helper/HelperTool.swiftsrc/bun/index.tssrc/bun/proxy/proxy-server.tssrc/bun/sse/bedrock-decoder.test.tssrc/bun/sse/bedrock-decoder.tssrc/bun/storage/trace-writer.tssrc/bun/system/health-monitor.tssrc/bun/system/log-manager.tssrc/bun/system/pf-manager.tssrc/bun/system/redirect-manager.tssrc/views/mainview/index.csssrc/views/mainview/index.htmlsrc/views/mainview/index.ts
Comment on lines
+306
to
+319
| // Collect all unique IPs, validating each | ||
| const ipPattern = /^\d{1,3}(\.\d{1,3}){3}$/; | ||
| const ifacePattern = /^[a-z]+\d+$/; | ||
| if (!ifacePattern.test(ifaceName)) { | ||
| console.error(`[pf] Invalid interface name: ${ifaceName}`); | ||
| return false; | ||
| } | ||
|
|
||
| const allIps = new Set<string>(); | ||
| for (const ips of domainIPs.values()) { | ||
| for (const ip of ips) { | ||
| if (ipPattern.test(ip)) allIps.add(ip); | ||
| } | ||
| } |
There was a problem hiding this comment.
IP validation regex allows invalid octets.
The pattern /^\d{1,3}(\.\d{1,3}){3}$/ accepts values like "999.999.999.999". Since IPs come from DoH resolution, this is low risk, but defense-in-depth would validate each octet is ≤255.
🛡️ Proposed stricter validation
- const ipPattern = /^\d{1,3}(\.\d{1,3}){3}$/;
+ const isValidIPv4 = (ip: string): boolean => {
+ const parts = ip.split(".");
+ if (parts.length !== 4) return false;
+ return parts.every((p) => {
+ const n = parseInt(p, 10);
+ return !isNaN(n) && n >= 0 && n <= 255 && String(n) === p;
+ });
+ };Then use if (isValidIPv4(ip)) allIps.add(ip); at line 317.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bun/system/pf-manager.ts` around lines 306 - 319, The current ipPattern
(/^\d{1,3}(\.\d{1,3}){3}$/) in pf-manager.ts accepts invalid octets (e.g. 999);
replace this loose check by implementing a strict IPv4 validator (e.g.
isValidIPv4 that splits on '.' and ensures there are 4 numeric octets each 0–255
with no leading plus signs) and use it instead of ipPattern.test inside the loop
that populates allIps (refer to ipPattern, domainIPs, allIps); keep the existing
ifacePattern check untouched and only swap the octet validation to call
isValidIPv4(ip) before adding to allIps.
Comment on lines
+910
to
+920
| // Apply saved widths | ||
| cols.forEach((col) => { | ||
| const el = col as HTMLElement; | ||
| const cls = Array.from(el.classList).find((c) => c.startsWith("col-") && c !== "col"); | ||
| if (cls && savedWidths[cls]) { | ||
| el.style.width = `${savedWidths[cls]}px`; | ||
| el.style.flexShrink = "0"; | ||
| // Apply to trace rows too | ||
| applyColWidth(cls, savedWidths[cls]); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Missing third argument causes runtime error.
applyColWidth requires 3 parameters but only 2 are passed. This will throw when accessing current[cls] on undefined.
Proposed fix
if (cls && savedWidths[cls]) {
el.style.width = `${savedWidths[cls]}px`;
el.style.flexShrink = "0";
- applyColWidth(cls, savedWidths[cls]);
+ applyColWidth(cls, savedWidths[cls], savedWidths);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/views/mainview/index.ts` around lines 910 - 920, The call to
applyColWidth inside cols.forEach is missing the third argument, causing a
runtime error when the function accesses current[cls]; update the call
applyColWidth(cls, savedWidths[cls]) to pass the appropriate third parameter
(the current map/object referenced as current inside applyColWidth) so it
becomes applyColWidth(cls, savedWidths[cls], current) (or the actual variable
name used in scope), ensuring applyColWidth can safely read/update current[cls];
verify the variable passed matches the one used by applyColWidth.
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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
application/vnd.amazon.eventstreambinary frames are now parsed, extracting base64 JSON payloads into SSEChunk[] for token counting and response display. Previously ALL Bedrock streaming responses had NULL response bodies.dns.resolve4()(c-ares, used by Claude Code's AWS SDK) bypass/etc/hostssentinel IPs entirely. Periodic 60s re-resolution handles DNS round-robin.confirm()(silently returns false in WKWebView) with HTML<dialog>.setImage()bug workaround).Test plan
npm run build)bun test)Summary by CodeRabbit
New Features
Bug Fixes
Tests