Skip to content

Decouple debug logging from Bonsplit - #3128

Merged
lawrencecchen merged 6 commits into
mainfrom
task-decouple-debug-logging-from-bonsplit
Apr 23, 2026
Merged

lawrencecchen merged 6 commits into
mainfrom
task-decouple-debug-logging-from-bonsplit

Conversation

@lawrencecchen

@lawrencecchen lawrencecchen commented Apr 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Add Packages/CMUXDebugLog for cmux-owned debug event log buffering and file output.
  • Add a cmuxDebugLog app shim and move cmux debug call sites off Bonsplit's exported dlog.
  • Update agent notes to point debug logging at the cmux package.

Testing

  • ./scripts/reload.sh --tag dbglog passed.

Issues

  • Task: refactor debug logging so cmux is not coupled to Bonsplit for it.

Summary by cubic

Decouples cmux debug logging from Bonsplit by introducing a cmux-owned logger and switching all call sites to cmuxDebugLog(...). Behavior is unchanged (timestamped streaming with a 500-entry ring buffer, immediate file append, synchronous dump, DEBUG-only), and log persistence now uses centralized redaction for sensitive fields.

  • New Features
    • CMUXDebugLog package with a 500-entry ring buffer, immediate file append, synchronous dump, and centralized redaction (URLs reduced to origin, file paths and payloads redacted); public API CMUXDebugLog.logDebugEvent(...) with unit tests.
    • App shim Sources/App/DebugLogging.swift exposing cmuxDebugLog(_:) in #if DEBUG builds and migrating all cmux debug call sites off Bonsplit.

Written for commit 4739c02. Summary will update on new commits.

Summary by CodeRabbit

  • Chores

    • Added a local CMUXDebugLog package to the build and switched DEBUG logging to a new app-side helper.
    • Updated numerous DEBUG call sites to use the new helper.
  • New Features

    • DEBUG-only ring-buffered in-memory logger with asynchronous file append, dump-to-file, and exposed resolved log path.
    • Sensitive-field redaction and URL/path normalization in debug output.
  • Tests

    • Added unit tests exercising the debug-message redaction behavior.
  • Documentation

    • Docs updated to reference the new logging package, helper name, and dump semantics.

@vercel

vercel Bot commented Apr 23, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
cmux Ready Ready Preview, Comment Apr 23, 2026 11:42am

@coderabbitai

coderabbitai Bot commented Apr 23, 2026 •

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a local Swift package CMUXDebugLog implementing a DEBUG-only in-memory ring-buffer logger with disk persistence, an app shim cmuxDebugLog(...), wires the package into the Xcode project, and replaces dlog(...) call sites with cmuxDebugLog(...) across the app.

Changes

Cohort / File(s) Summary
Docs
CLAUDE.md
Updated docs to reference CMUXDebugLog, cmuxDebugLog free-function, and CMUXDebugLog.DebugEventLog.shared.dump() usage.
Xcode Project
GhosttyTabs.xcodeproj/project.pbxproj
Added local Swift package reference Packages/CMUXDebugLog, registered the CMUXDebugLog product dependency, and included Sources/App/DebugLogging.swift in app sources.
Local Swift Package
Packages/CMUXDebugLog/Package.swift, Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift
New SwiftPM package and DebugEventLog implementation: DEBUG-only singleton, 500-entry ring buffer, timestamped entries, async disk append, dump() and currentLogPath(), plus logDebugEvent(_:).
App Shim
Sources/App/DebugLogging.swift
Added #if DEBUG cmuxDebugLog(_:) autoclosure wrapper forwarding to CMUXDebugLog.logDebugEvent.
Call-site Replacements
Sources/...
Sources/AppDelegate.swift, Sources/BrowserWindowPortal.swift, Sources/ContentView.swift, Sources/GhosttyTerminalView.swift, Sources/Panels/..., Sources/Find/..., Sources/TabManager.swift, Sources/Workspace.swift, Sources/cmuxApp.swift, etc.
Replaced many #if DEBUG dlog(...) calls with cmuxDebugLog(...) across UI, lifecycle, terminal, navigation, drag/drop, and workspace debug sites; preserved message contents and control flow.
Selective Redaction Logic
Sources/Panels/CmuxWebView.swift
Added DEBUG-only tokenization/redaction for key= fields in context-menu download debug logs, normalizing/obfuscating URL-like and payload values before logging.
Tests
Packages/CMUXDebugLog/Tests/CMUXDebugLogTests/DebugLogRedactorTests.swift
Added debug-only unit tests for redaction logic validating URL/path/payload redaction behavior.

Sequence Diagram(s)

sequenceDiagram
    participant App as App (cmux)
    participant Shim as App Shim\ncmuxDebugLog
    participant Package as CMUXDebugLog\nDebugEventLog
    participant FS as File System

    App->>Shim: cmuxDebugLog("message") (autoclosure)
    Shim->>Package: logDebugEvent(evaluated message)
    Package->>Package: enqueue to ring buffer (max 500)
    Package->>Package: async append on serial queue
    Package->>FS: open/create log file and append
    Note right of Package: dump() rewrites file from buffer synchronously
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐰 I hopped through code with tiny feet,
Swapped dlog for cmux — tidy and neat.
Five hundred whispers loop in a chest,
I nibble secrets, redact the rest.
Hoppity-hop — the debug trail is sweet.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 6.55% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Decouple debug logging from Bonsplit' clearly and concisely describes the main objective of the changeset: moving the debug logging system away from Bonsplit's dlog and onto a cmux-owned implementation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The PR description covers the key changes: decoupling debug logging from Bonsplit, adding a CMUXDebugLog package, implementing an app shim, and testing confirmation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task-decouple-debug-logging-from-bonsplit

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a18457373

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift Outdated
@greptile-apps

greptile-apps Bot commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR extracts debug event logging from Bonsplit into a new first-party Packages/CMUXDebugLog local SPM package and introduces a thin #if DEBUG app shim (cmuxDebugLog). All 29 changed source files perform a mechanical dlog → cmuxDebugLog rename; every call site remains correctly guarded by #if DEBUG either at the enclosing class/enum level or via explicit conditional blocks.

Confidence Score: 5/5

Safe to merge; all changes are a clean mechanical rename and the new package is well-structured.

The only finding (missing #if DEBUG on the package file) is a P2 style/cleanup concern — debug dead code shipping in release builds does not affect correctness or user-facing behavior. No logic regressions or security issues were found.

Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift — consider adding a top-level #if DEBUG guard to match the old Bonsplit contract.

Important Files Changed

Filename Overview
Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift New standalone debug ring-buffer; logic is correct but lacks #if DEBUG guard, so debug infrastructure ships in release builds
Sources/App/DebugLogging.swift Thin #if DEBUG app shim mapping cmuxDebugLog → CMUXDebugLog.logDebugEvent; correct pattern
Packages/CMUXDebugLog/Package.swift Local SPM package definition; well-formed, targets macOS 13, no external dependencies
Sources/AppDelegate.swift Mechanical dlog → cmuxDebugLog rename; all call sites remain properly guarded by class-level or explicit #if DEBUG blocks
Sources/cmuxApp.swift Mechanical dlog → cmuxDebugLog rename throughout; all substitutions are inside existing #if DEBUG guards
GhosttyTabs.xcodeproj/project.pbxproj Adds XCLocalSwiftPackageReference for CMUXDebugLog and links it to the main target; wiring looks correct
CLAUDE.md Agent notes updated to point at new package path and shim; accurately reflects the new design

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Call site (cmuxDebugLog)"] -->|"#if DEBUG only"| B["Sources/App/DebugLogging.swift\ncmuxDebugLog shim"]
    B --> C["CMUXDebugLog.logDebugEvent()"]
    C --> D["DebugEventLog.shared.log()"]
    D --> E["Serial DispatchQueue\n(cmux.debug-event-log)"]
    E --> F["Ring buffer\n(500 entries)"]
    E --> G["Append to /tmp/*.log\nFileHandle open/write/close"]
    H["DebugEventLog.shared.dump()"] --> E
    E --> I["Atomic overwrite of log file\nwith full buffer"]
    J["resolveLogPath()"] -->|"CMUX_DEBUG_LOG env"| G
    J -->|"CMUX_TAG env"| G
    J -->|"CMUX_SOCKET_PATH env"| G
    J -->|"Bundle ID fallback"| G
Loading

Reviews (1): Last reviewed commit: "Decouple debug logging from Bonsplit" | Re-trigger Greptile

Comment thread Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (9)
Sources/SessionIndexStore.swift (1)

985-1032: ⚠️ Potential issue | 🟠 Major

Avoid persisting raw search text and cwd fragments in debug logs.

With cmuxDebugLog now backed by cmux-owned file output, Line 989 can persist user search text and Line 1031 can persist filesystem path fragments. Log presence/length instead.

🛡️ Proposed sanitization
 `#if` DEBUG
 let totalStart = ProcessInfo.processInfo.systemUptime
 defer {
     let totalMs = (ProcessInfo.processInfo.systemUptime - totalStart) * 1000
-    cmuxDebugLog("session.search.total ms=\(String(format: "%.0f", totalMs)) needle=\"\(trimmed.prefix(20))\" offset=\(offset) limit=\(limit) errors=\(bag.snapshot().count)")
+    cmuxDebugLog("session.search.total ms=\(String(format: "%.0f", totalMs)) hasNeedle=\(!trimmed.isEmpty ? 1 : 0) needleChars=\(trimmed.count) offset=\(offset) limit=\(limit) errors=\(bag.snapshot().count)")
 }
 `#endif`
@@
 let start = ProcessInfo.processInfo.systemUptime
 let result = await searchAgent(needle: needle, agent: agent, cwdFilter: cwdFilter, offset: offset, limit: limit, errorBag: errorBag)
 let ms = (ProcessInfo.processInfo.systemUptime - start) * 1000
-cmuxDebugLog("session.search.agent agent=\(agent.rawValue) ms=\(String(format: "%.0f", ms)) results=\(result.count) cwd=\(cwdFilter?.suffix(40) ?? "nil")")
+cmuxDebugLog("session.search.agent agent=\(agent.rawValue) ms=\(String(format: "%.0f", ms)) results=\(result.count) hasCwdFilter=\(cwdFilter == nil ? 0 : 1)")
 return result

Based on learnings, avoid logging sensitive paths/tokens or raw input-like content in DEBUG; log non-sensitive metadata instead.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/SessionIndexStore.swift` around lines 985 - 1032, The debug logs
currently embed raw user input and path fragments via cmuxDebugLog in the
total-search block (using trimmed.prefix(20)) and in timedAgent (using
cwdFilter?.suffix(40)), which can persist sensitive data; change both logs to
avoid raw content and instead emit non-sensitive metadata such as needle length
(e.g., needleLength=trimmed.count or needlePresent=!(trimmed.isEmpty)) and cwd
presence/length (e.g., cwdPresent=(cwdFilter != nil) and
cwdLength=cwdFilter?.count), keeping existing fields like
offset/limit/agent/results/ms/errors; update the two cmuxDebugLog invocations in
the SearchOutcome return path and in timedAgent to use these metadata values
instead of the raw needle or path fragments.
Sources/Panels/BrowserPopupWindowController.swift (1)

570-580: ⚠️ Potential issue | 🟠 Major

Avoid logging full URLs in debug traces

Line 570 and Line 579 log url.absoluteString, which may include query params/fragments with secrets. Please log sanitized components (e.g., scheme + host + path) instead.

🔧 Proposed fix (sanitize URL logging)
-            cmuxDebugLog("popup.nav.external url=\(url.absoluteString)")
+            let safeURL = "\(url.scheme ?? "unknown")://\(url.host ?? "unknown")\(url.path)"
+            cmuxDebugLog("popup.nav.external url=\(safeURL)")
@@
-            cmuxDebugLog("popup.nav.insecureHTTP url=\(url.absoluteString)")
+            let safeURL = "\(url.scheme ?? "unknown")://\(url.host ?? "unknown")\(url.path)"
+            cmuxDebugLog("popup.nav.insecureHTTP url=\(safeURL)")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Panels/BrowserPopupWindowController.swift` around lines 570 - 580,
The debug logs in BrowserPopupWindowController that call cmuxDebugLog with
url.absoluteString (the "popup.nav.external" and "popup.nav.insecureHTTP" log
sites) should stop printing the full URL; instead build and log a sanitized
string containing only safe components (scheme, host, and path) and omit
query/fragment/userinfo. Update the two cmuxDebugLog calls to use the sanitized
representation (e.g., using url.scheme, url.host, url.path) before logging; keep
the same log labels ("popup.nav.external" and "popup.nav.insecureHTTP") for
consistency.
Sources/Panels/ReactGrab.swift (1)

5-7: ⚠️ Potential issue | 🟠 Major

Remove now-unused import Bonsplit.

With every dlog(...) call site in this file replaced by cmuxDebugLog(...), the DEBUG-only import Bonsplit is no longer referenced. Leaving it behind defeats the stated goal of this PR (decoupling cmux debug logging from Bonsplit) for this translation unit and will likely trigger an unused-import warning under DEBUG.

✂️ Proposed fix
-#if DEBUG
-import Bonsplit
-#endif
-
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Panels/ReactGrab.swift` around lines 5 - 7, Remove the now-unused
DEBUG-only import by deleting the conditional import Bonsplit block; locate the
conditional at the top of ReactGrab.swift (the `#if` DEBUG / import Bonsplit /
`#endif`) and remove it so the file no longer references Bonsplit—confirm usages
were migrated to cmuxDebugLog (and that there are no remaining dlog(...) calls)
before committing.
Sources/TerminalWindowPortal.swift (1)

3-5: ⚠️ Potential issue | 🟠 Major

Remove now-unused import Bonsplit.

All dlog(...) call sites in this file have been migrated to cmuxDebugLog(...), so the DEBUG-only import Bonsplit is dead. Dropping it completes the Bonsplit-decoupling for this file and avoids an unused-import warning in DEBUG builds.

✂️ Proposed fix
-#if DEBUG
-import Bonsplit
-#endif
-
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/TerminalWindowPortal.swift` around lines 3 - 5, Remove the now-unused
DEBUG-only import by deleting the conditional import block referencing Bonsplit
in TerminalWindowPortal.swift; since all dlog(...) calls were migrated to
cmuxDebugLog(...), remove the lines "#if DEBUG", "import Bonsplit", and "#endif"
so the file no longer references Bonsplit and no unused-import warning is
produced.
Sources/ContentView.swift (1)

4980-4984: ⚠️ Potential issue | 🟠 Major

Avoid logging user-entered text content in debug logs.

Line 4980, Line 8825, Line 8839, Line 12301, Line 13402, and Line 13415 currently log preview text derived from workspace descriptions/titles. Even in DEBUG, this can leak sensitive notes/tokens/paths from user content. Log only metadata (presence, char/byte count, newline count, workspace/surface IDs), not text payloads.

Suggested patch pattern
- cmuxDebugLog(
-     "palette.wsDescription.apply.begin workspace=\(target.workspaceId.uuidString.prefix(8)) " +
-     "proposedLen=\((proposedDescription as NSString).length) " +
-     "newlines=\(newlineCount) " +
-     "text=\"\(debugCommandPaletteTextPreview(proposedDescription))\""
- )
+ cmuxDebugLog(
+     "palette.wsDescription.apply.begin workspace=\(target.workspaceId.uuidString.prefix(8)) " +
+     "proposedLen=\((proposedDescription as NSString).length) " +
+     "newlines=\(newlineCount)"
+ )
- "desc=\"\(debugCommandPaletteTextPreview(description))\""
+ "hasDesc=\(((description as NSString).length > 0) ? 1 : 0)"

Based on learnings: “avoid logging raw startup commands or initialInput even in DEBUG… log only non-sensitive metadata such as presence flags, byte counts, and relevant id.”

Also applies to: 8825-8830, 8839-8844, 12301-12306, 13402-13407, 13415-13420

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/ContentView.swift` around lines 4980 - 4984, The debug log currently
logs raw user text via cmuxDebugLog with
debugCommandPaletteTextPreview(currentText) in the submitText handling; change
it to avoid emitting the text payload and instead log only safe metadata (e.g.,
presence flag, character/byte count, newline count, and relevant IDs like
workspace/surface) by replacing the debugCommandPaletteTextPreview(currentText)
usage with a constructed metadata string (e.g., "present=true len=\((currentText
as NSString).length) bytes=\(Data(currentText.utf8).count)
newlines=\(currentText.filter{ $0 == "\n" }.count) workspaceId=...") wherever
you see cmuxDebugLog calls that include debugCommandPaletteTextPreview or raw
currentText (including the listed other locations: the blocks around lines
referencing submitText and debugCommandPaletteTextPreview); ensure no raw user
content is logged.
Sources/Panels/BrowserPanel.swift (2)

2210-2238: ⚠️ Potential issue | 🟠 Major

Move find diagnostics to DEBUG cmuxDebugLog and avoid logging the raw needle.

These NSLog calls run in release builds, and Line 2228 logs the user’s find text verbatim. That can leak page content, secrets, or search terms into system logs.

Suggested change
     `@Published` var searchState: BrowserSearchState? = nil {
         didSet {
             if let searchState {
                 preferredFocusIntent = .findField
-                NSLog("Find: browser search state created panel=%@", id.uuidString)
+#if DEBUG
+                cmuxDebugLog("browser.find.state.created panel=\(id.uuidString.prefix(5))")
+#endif
                 searchNeedleCancellable = searchState.$needle
                     .removeDuplicates()
                     .map { needle -> AnyPublisher<String, Never> in
@@
                     .sink { [weak self] needle in
                         guard let self else { return }
-                        NSLog("Find: browser needle updated panel=%@ needle=%@", self.id.uuidString, needle)
+#if DEBUG
+                        cmuxDebugLog(
+                            "browser.find.needle.updated panel=\(self.id.uuidString.prefix(5)) " +
+                            "chars=\(needle.count)"
+                        )
+#endif
                         self.executeFindSearch(needle)
                     }
             } else if oldValue != nil {
@@
                 }
                 invalidateSearchFocusRequests(reason: "searchStateCleared")
-                NSLog("Find: browser search state cleared panel=%@", id.uuidString)
+#if DEBUG
+                cmuxDebugLog("browser.find.state.cleared panel=\(id.uuidString.prefix(5))")
+#endif
                 executeFindClear()
             }
         }
     }

As per coding guidelines, **/*.swift: Always wrap debug log calls and debug-only code with #if DEBUG / #endif preprocessor directives. Use the free function cmuxDebugLog("message") to log debug events with timestamp.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Panels/BrowserPanel.swift` around lines 2210 - 2238, Replace the
release NSLog diagnostics in the BrowserPanel searchState observer with
debug-only cmuxDebugLog calls wrapped in `#if` DEBUG/#endif: for symbols
searchState, searchNeedleCancellable, preferredFocusIntent, executeFindSearch
and executeFindClear, remove any logging of the raw needle string and instead
log non-sensitive context (e.g. panel id via id.uuidString and either a redacted
marker or needle length) using cmuxDebugLog; ensure the search-start,
needle-update and search-clear messages are only compiled in DEBUG and that
searchNeedleCancellable’s sink never writes the plain needle to system logs.

3255-3262: ⚠️ Potential issue | 🟠 Major

Scrub URL-bearing debug messages before writing them to the cmux debug log.

Several migrated cmuxDebugLog calls still write full absoluteString URLs. Since this new logger is file-backed, query strings/fragments can persist OAuth codes, signed URLs, tokens, and account identifiers. Prefer the existing browserNavigationDebugURL(_:) helper, or a stricter redactor, for all URL fields.

Suggested direction
-            "renderable=\(wasRenderable ? 1 : 0) restoreURL=\(restoreURLString ?? "nil") " +
+            "renderable=\(wasRenderable ? 1 : 0) restoreURL=\(browserNavigationDebugURL(restoreURL)) " +

-                "page=\(pageURL.absoluteString)"
+                "page=\(browserNavigationDebugURL(pageURL))"

-                "discovered=\(discoveredURL?.absoluteString ?? "<nil>") " +
-                "fallback=\(fallbackURL?.absoluteString ?? "<nil>") " +
-                "chosen=\(iconURL.absoluteString)"
+                "discovered=\(browserNavigationDebugURL(discoveredURL)) " +
+                "fallback=\(browserNavigationDebugURL(fallbackURL)) " +
+                "chosen=\(browserNavigationDebugURL(iconURL))"

-                        "url=\(effectiveRequest.url?.absoluteString ?? "<nil>")"
+                        "url=\(browserNavigationDebugURL(effectiveRequest.url))"

-            "from=\(url.absoluteString) " +
-            "to=\(rewrittenURL.absoluteString)"
+            "from=\(browserNavigationDebugURL(url)) " +
+            "to=\(browserNavigationDebugURL(rewrittenURL))"

-        let requestURL = navigationAction.request.url?.absoluteString ?? "nil"
+        let requestURL = browserNavigationDebugURL(navigationAction.request.url)

-                "url=\(url.absoluteString)"
+                "url=\(browserNavigationDebugURL(url))"

-            cmuxDebugLog("browser.navigation.external source=navDelegate opened=\(opened ? 1 : 0) url=\(url.absoluteString)")
+            cmuxDebugLog("browser.navigation.external source=navDelegate opened=\(opened ? 1 : 0) url=\(browserNavigationDebugURL(url))")

-                "browser.nav.decidePolicy.action kind=openInNewTab url=\(requestURL.absoluteString)"
+                "browser.nav.decidePolicy.action kind=openInNewTab url=\(browserNavigationDebugURL(requestURL))"

-        let targetURL = navigationAction.request.url?.absoluteString ?? "nil"
-        cmuxDebugLog("browser.nav.decidePolicy.action kind=allow url=\(targetURL)")
+        cmuxDebugLog("browser.nav.decidePolicy.action kind=allow url=\(browserNavigationDebugURL(navigationAction.request.url))")

-                cmuxDebugLog("browser.nav.createWebView.action kind=openInNewTab url=\(url.absoluteString)")
+                cmuxDebugLog("browser.nav.createWebView.action kind=openInNewTab url=\(browserNavigationDebugURL(url))")

Also applies to: 3312-3318, 3457-3462, 3528-3535, 3564-3589, 3866-3872, 4080-4085, 6373-6377, 6393-6395, 6403-6406, 6421-6424, 6431-6433, 6544-6567, 6576-6578, 6654-6656

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Panels/BrowserPanel.swift` around lines 3255 - 3262, The cmuxDebugLog
call in the browser replacement/navigation logging is writing raw URL strings
(e.g. restoreURLString and entries from
history.backHistoryURLStrings/history.forwardHistoryURLStrings); update these
log sites to scrub/redact URL-bearing data before logging by passing URLs
through the existing browserNavigationDebugURL(_:) helper (or a stricter
redactor) instead of using absoluteString, and apply the same change for other
cmuxDebugLog occurrences listed (e.g. the replace.begin call and the other
locations noted) so no query/fragments/tokens are written to the file-backed
logger.
Sources/GhosttyTerminalView.swift (1)

971-1028: ⚠️ Potential issue | 🟠 Major

Avoid logging raw URLs/paths in debug events

These messages log full URL/path payloads (input, url, resolved path). With the new centralized/file-backed debug log, this increases leakage risk (query tokens, local paths, identifiers). Prefer sanitized fields (scheme, host, path basename, byte counts) and strip query/fragment before logging.

🔧 Suggested hardening pattern
- cmuxDebugLog("link.openURL raw=\(urlString)")
+ cmuxDebugLog("link.openURL rawMeta bytes=\(urlString.utf8.count)")

- cmuxDebugLog("link.openURL ... url=\(url)")
+ cmuxDebugLog("link.openURL ... scheme=\(url.scheme ?? "nil") host=\(url.host ?? "nil") hasQuery=\(url.query != nil ? 1 : 0) hasFragment=\(url.fragment != nil ? 1 : 0)")

- cmuxDebugLog("link.wordFallback resolved=\(resolution.path) source=\(resolution.source.rawValue)")
+ cmuxDebugLog("link.wordFallback source=\(resolution.source.rawValue) basename=\(URL(fileURLWithPath: resolution.path).lastPathComponent)")

Also applies to: 3348-3479, 7596-7600, 7999-8001

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/GhosttyTerminalView.swift` around lines 971 - 1028, The debug logs in
this block (cmuxDebugLog calls inside the link.resolve logic) must stop printing
raw URLs/paths (variables like trimmed, parsed, webURL, fallback); instead
sanitize before logging by stripping query and fragment, replacing full paths
with their basename or a redacted placeholder, and logging only safe fields such
as scheme, normalized host (if present), path basename, and byte/length counts.
Update every cmuxDebugLog here (and the same pattern in
resolveBrowserNavigableURL-related uses) to build a sanitized summary from
trimmed/parsed/webURL/fallback and use that summary in the log messages rather
than the raw URL/path values.
Sources/TabManager.swift (1)

1919-1941: ⚠️ Potential issue | 🟡 Minor

Move this debug log into the reachable search path.

Line 1927 returns from the first selectedTerminalPanel branch, so Lines 1929-1941 are unreachable and the new cmuxDebugLog on Lines 1934-1939 never emits. The reachable path still uses bare NSLog on Line 1924 instead of the unified DEBUG log.

Suggested cleanup
     func startSearch() {
         if let panel = selectedTerminalPanel {
+            let hadExistingSearch = panel.searchState != nil
             if panel.searchState == nil {
                 panel.searchState = TerminalSurface.SearchState()
             }
-            NSLog("Find: startSearch workspace=%@ panel=%@", panel.workspaceId.uuidString, panel.id.uuidString)
             NotificationCenter.default.post(name: .ghosttySearchFocus, object: panel.surface)
-            _ = panel.performBindingAction("start_search")
-            return
-        }
-        if let panel = selectedTerminalPanel {
-            let hadExistingSearch = panel.searchState != nil
-            let handled = startOrFocusTerminalSearch(panel.surface)
-            NSLog("Find: startSearch workspace=%@ panel=%@", panel.workspaceId.uuidString, panel.id.uuidString)
+            let handled = panel.performBindingAction("start_search")
 `#if` DEBUG
             cmuxDebugLog(
                 "find.startSearch workspace=\(panel.workspaceId.uuidString.prefix(5)) " +
                 "panel=\(panel.id.uuidString.prefix(5)) existing=\(hadExistingSearch ? "yes" : "no") " +
                 "handled=\(handled ? 1 : 0) " +

As per coding guidelines, **/*.swift: Always wrap debug log calls and debug-only code with #if DEBUG / #endif preprocessor directives. Use the free function cmuxDebugLog("message") to log debug events with timestamp.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/TabManager.swift` around lines 1919 - 1941, The debug cmuxDebugLog
call is placed in an unreachable branch of startSearch because the first
selectedTerminalPanel if-block returns early; replace or augment the reachable
branch so DEBUG logging is emitted: inside the first if let panel =
selectedTerminalPanel branch (the one that calls NotificationCenter.default.post
and panel.performBindingAction("start_search")), wrap a cmuxDebugLog(...) call
in `#if` DEBUG / `#endif` (or replace the existing NSLog there with cmuxDebugLog
inside DEBUG) and remove the duplicate unreachable block (the second if let
panel = selectedTerminalPanel that calls startOrFocusTerminalSearch) or
consolidate its logic so debug logging is emitted from the actual execution
path; reference startSearch, selectedTerminalPanel, cmuxDebugLog, NSLog,
NotificationCenter.default.post, performBindingAction, and
startOrFocusTerminalSearch when making the change.
🧹 Nitpick comments (1)
Sources/Workspace.swift (1)

7809-7817: Sanitize debug payloads to avoid raw user text/path leakage.

These logs still emit raw custom description content and full directory paths. Please keep only presence/length/count metadata in DEBUG logs.

🔧 Suggested hardening diff
 `#if` DEBUG
         let inputNewlines = description?.reduce(into: 0) { count, character in
             if character == "\n" { count += 1 }
         } ?? 0
         let normalizedNewlines = normalizedDescription?.reduce(into: 0) { count, character in
             if character == "\n" { count += 1 }
         } ?? 0
         cmuxDebugLog(
             "workspace.customDescription.update workspace=\(id.uuidString.prefix(8)) " +
+            "hasInput=\(description == nil ? 0 : 1) hasNormalized=\(normalizedDescription == nil ? 0 : 1) " +
             "inputLen=\((description as NSString?)?.length ?? 0) " +
             "inputNewlines=\(inputNewlines) " +
             "normalizedLen=\((normalizedDescription as NSString?)?.length ?? 0) " +
-            "normalizedNewlines=\(normalizedNewlines) " +
-            "input=\"\(debugWorkspaceDescriptionPreview(description))\" " +
-            "normalized=\"\(debugWorkspaceDescriptionPreview(normalizedDescription))\""
+            "normalizedNewlines=\(normalizedNewlines)"
         )
 `#endif`
@@
 `#if` DEBUG
         cmuxDebugLog(
-            "split.cwd panelId=\(panelId.uuidString.prefix(5)) panelDir=\(panelDirectories[panelId] ?? "nil") requestedDir=\(terminalPanel(for: panelId)?.requestedWorkingDirectory ?? "nil") currentDir=\(currentDirectory) resolved=\(splitWorkingDirectory ?? "nil")"
+            "split.cwd panelId=\(panelId.uuidString.prefix(5)) " +
+            "hasPanelDir=\(panelDirectories[panelId] == nil ? 0 : 1) " +
+            "hasRequestedDir=\(terminalPanel(for: panelId)?.requestedWorkingDirectory == nil ? 0 : 1) " +
+            "hasCurrentDir=\(currentDirectory.isEmpty ? 0 : 1) " +
+            "hasResolved=\(splitWorkingDirectory == nil ? 0 : 1)"
         )
 `#endif`

Based on learnings In Swift source files like Sources/GhosttyTerminalView.swift, avoid logging raw startup commands or initialInput even in DEBUG (to prevent leaking sensitive paths/tokens and multiline content). If you need to diagnose startup/input, log only non-sensitive metadata such as (1) presence flags, (2) byte counts, and (3) the relevant surface id.

Also applies to: 9135-9137

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Sources/Workspace.swift` around lines 7809 - 7817, The cmuxDebugLog call is
emitting raw user text and paths (using debugWorkspaceDescriptionPreview and the
full description/normalizedDescription); change it to log only non-sensitive
metadata: keep the workspace id prefix (id.uuidString.prefix(8)), presence flags
(e.g., whether description/normalizedDescription are non-empty), byte/length
counts ((description as NSString?)?.length and (normalizedDescription as
NSString?)?.length), and newline counts (inputNewlines, normalizedNewlines), and
remove any inclusion of debugWorkspaceDescriptionPreview or the raw description
strings; also apply the same sanitization to the other similar logging site in
this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift`:
- Around line 3-97: Wrap the DebugEventLog implementation and the free function
logDebugEvent in `#if` DEBUG / `#else` / `#endif` so the full file is only compiled
into DEBUG builds; under the `#else` provide lightweight no-op stubs that preserve
the public symbols (public final class DebugEventLog with a shared instance,
log(_:), dump(), currentLogPath(), and the public logDebugEvent(_:) function)
but perform no file IO or allocations and return sensible defaults (e.g. empty
string for currentLogPath) so Release builds cannot write debug diagnostics yet
callers still compile.

In `@Sources/Panels/BrowserPanelView.swift`:
- Around line 4271-4276: The debug log in the Button action currently writes the
raw suggestion text via cmuxDebugLog("browser.suggestionClick index=\(idx)
text=\"\(item.listText)\""), which may persist sensitive URLs or queries; change
the log to avoid item.listText and instead record non-sensitive metadata such as
idx, item.id, item.type/category, and a redacted or hashed representation if you
need uniqueness (update the log call in the ForEach/Button block inside
BrowserPanelView.swift where cmuxDebugLog is called). Ensure the replacement
keeps the same context (e.g., "browser.suggestionClick index=... id=...
type=...") and remove or hash any plain-text suggestion content before calling
cmuxDebugLog so no raw omnibar text is written to disk.

In `@Sources/Panels/BrowserWebAuthnSupport.swift`:
- Around line 1185-1187: The DEBUG cmuxDebugLog calls (e.g., the call using
cmuxDebugLog with request.publicKey.rp, request.publicKey.user.name,
request.publicKey.authenticatorSelection?.attachment,
request.publicKey.requestedAlgorithms) are emitting raw identifiers/URLs/titles;
change them to emit only metadata (presence flags, counts, and sanitized host or
domain) instead of raw strings: log whether rp exists and its host (not full
URL), whether user.name is present (true/false) and not the name itself, the
authenticator attachment type as-is if safe or otherwise a presence flag, and
the number/count of requested algorithms; apply the same sanitization pattern to
the other cmuxDebugLog occurrences referenced (around lines 1258-1260,
1287-1289, 1317-1319, 1402-1406) so no raw user names, full URLs, or window
titles are ever logged.

In `@Sources/Panels/CmuxWebView.swift`:
- Around line 842-845: The debugContextDownload(_:) helper currently logs raw
sensitive values; create a redaction helper (e.g.,
redactedContextDownloadDebugMessage(_:) ) that strips or replaces sensitive
fragments from fields like url=, referer=, path=, imageURL=, linkURL=, and
payload= (keep only scheme/host, status, and byte-count metadata or replace
values with "<redacted>"), then update debugContextDownload(_:) to call the
sanitizer before passing the string to cmuxDebugLog; ensure all callers of
debugContextDownload(_:) continue working without changes because the redaction
happens centrally in the new helper.

---

Outside diff comments:
In `@Sources/ContentView.swift`:
- Around line 4980-4984: The debug log currently logs raw user text via
cmuxDebugLog with debugCommandPaletteTextPreview(currentText) in the submitText
handling; change it to avoid emitting the text payload and instead log only safe
metadata (e.g., presence flag, character/byte count, newline count, and relevant
IDs like workspace/surface) by replacing the
debugCommandPaletteTextPreview(currentText) usage with a constructed metadata
string (e.g., "present=true len=\((currentText as NSString).length)
bytes=\(Data(currentText.utf8).count) newlines=\(currentText.filter{ $0 == "\n"
}.count) workspaceId=...") wherever you see cmuxDebugLog calls that include
debugCommandPaletteTextPreview or raw currentText (including the listed other
locations: the blocks around lines referencing submitText and
debugCommandPaletteTextPreview); ensure no raw user content is logged.

In `@Sources/GhosttyTerminalView.swift`:
- Around line 971-1028: The debug logs in this block (cmuxDebugLog calls inside
the link.resolve logic) must stop printing raw URLs/paths (variables like
trimmed, parsed, webURL, fallback); instead sanitize before logging by stripping
query and fragment, replacing full paths with their basename or a redacted
placeholder, and logging only safe fields such as scheme, normalized host (if
present), path basename, and byte/length counts. Update every cmuxDebugLog here
(and the same pattern in resolveBrowserNavigableURL-related uses) to build a
sanitized summary from trimmed/parsed/webURL/fallback and use that summary in
the log messages rather than the raw URL/path values.

In `@Sources/Panels/BrowserPanel.swift`:
- Around line 2210-2238: Replace the release NSLog diagnostics in the
BrowserPanel searchState observer with debug-only cmuxDebugLog calls wrapped in
`#if` DEBUG/#endif: for symbols searchState, searchNeedleCancellable,
preferredFocusIntent, executeFindSearch and executeFindClear, remove any logging
of the raw needle string and instead log non-sensitive context (e.g. panel id
via id.uuidString and either a redacted marker or needle length) using
cmuxDebugLog; ensure the search-start, needle-update and search-clear messages
are only compiled in DEBUG and that searchNeedleCancellable’s sink never writes
the plain needle to system logs.
- Around line 3255-3262: The cmuxDebugLog call in the browser
replacement/navigation logging is writing raw URL strings (e.g. restoreURLString
and entries from
history.backHistoryURLStrings/history.forwardHistoryURLStrings); update these
log sites to scrub/redact URL-bearing data before logging by passing URLs
through the existing browserNavigationDebugURL(_:) helper (or a stricter
redactor) instead of using absoluteString, and apply the same change for other
cmuxDebugLog occurrences listed (e.g. the replace.begin call and the other
locations noted) so no query/fragments/tokens are written to the file-backed
logger.

In `@Sources/Panels/BrowserPopupWindowController.swift`:
- Around line 570-580: The debug logs in BrowserPopupWindowController that call
cmuxDebugLog with url.absoluteString (the "popup.nav.external" and
"popup.nav.insecureHTTP" log sites) should stop printing the full URL; instead
build and log a sanitized string containing only safe components (scheme, host,
and path) and omit query/fragment/userinfo. Update the two cmuxDebugLog calls to
use the sanitized representation (e.g., using url.scheme, url.host, url.path)
before logging; keep the same log labels ("popup.nav.external" and
"popup.nav.insecureHTTP") for consistency.

In `@Sources/Panels/ReactGrab.swift`:
- Around line 5-7: Remove the now-unused DEBUG-only import by deleting the
conditional import Bonsplit block; locate the conditional at the top of
ReactGrab.swift (the `#if` DEBUG / import Bonsplit / `#endif`) and remove it so the
file no longer references Bonsplit—confirm usages were migrated to cmuxDebugLog
(and that there are no remaining dlog(...) calls) before committing.

In `@Sources/SessionIndexStore.swift`:
- Around line 985-1032: The debug logs currently embed raw user input and path
fragments via cmuxDebugLog in the total-search block (using trimmed.prefix(20))
and in timedAgent (using cwdFilter?.suffix(40)), which can persist sensitive
data; change both logs to avoid raw content and instead emit non-sensitive
metadata such as needle length (e.g., needleLength=trimmed.count or
needlePresent=!(trimmed.isEmpty)) and cwd presence/length (e.g.,
cwdPresent=(cwdFilter != nil) and cwdLength=cwdFilter?.count), keeping existing
fields like offset/limit/agent/results/ms/errors; update the two cmuxDebugLog
invocations in the SearchOutcome return path and in timedAgent to use these
metadata values instead of the raw needle or path fragments.

In `@Sources/TabManager.swift`:
- Around line 1919-1941: The debug cmuxDebugLog call is placed in an unreachable
branch of startSearch because the first selectedTerminalPanel if-block returns
early; replace or augment the reachable branch so DEBUG logging is emitted:
inside the first if let panel = selectedTerminalPanel branch (the one that calls
NotificationCenter.default.post and panel.performBindingAction("start_search")),
wrap a cmuxDebugLog(...) call in `#if` DEBUG / `#endif` (or replace the existing
NSLog there with cmuxDebugLog inside DEBUG) and remove the duplicate unreachable
block (the second if let panel = selectedTerminalPanel that calls
startOrFocusTerminalSearch) or consolidate its logic so debug logging is emitted
from the actual execution path; reference startSearch, selectedTerminalPanel,
cmuxDebugLog, NSLog, NotificationCenter.default.post, performBindingAction, and
startOrFocusTerminalSearch when making the change.

In `@Sources/TerminalWindowPortal.swift`:
- Around line 3-5: Remove the now-unused DEBUG-only import by deleting the
conditional import block referencing Bonsplit in TerminalWindowPortal.swift;
since all dlog(...) calls were migrated to cmuxDebugLog(...), remove the lines
"#if DEBUG", "import Bonsplit", and "#endif" so the file no longer references
Bonsplit and no unused-import warning is produced.

---

Nitpick comments:
In `@Sources/Workspace.swift`:
- Around line 7809-7817: The cmuxDebugLog call is emitting raw user text and
paths (using debugWorkspaceDescriptionPreview and the full
description/normalizedDescription); change it to log only non-sensitive
metadata: keep the workspace id prefix (id.uuidString.prefix(8)), presence flags
(e.g., whether description/normalizedDescription are non-empty), byte/length
counts ((description as NSString?)?.length and (normalizedDescription as
NSString?)?.length), and newline counts (inputNewlines, normalizedNewlines), and
remove any inclusion of debugWorkspaceDescriptionPreview or the raw description
strings; also apply the same sanitization to the other similar logging site in
this file.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8b0ea08c-cd34-486a-ae7a-525693dfa4c3

📥 Commits

Reviewing files that changed from the base of the PR and between 444197f and 8a18457.

📒 Files selected for processing (29)
  • CLAUDE.md
  • GhosttyTabs.xcodeproj/project.pbxproj
  • Packages/CMUXDebugLog/Package.swift
  • Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift
  • Sources/App/DebugLogging.swift
  • Sources/AppDelegate.swift
  • Sources/BrowserWindowPortal.swift
  • Sources/ContentView.swift
  • Sources/Find/BrowserSearchOverlay.swift
  • Sources/Find/SurfaceSearchOverlay.swift
  • Sources/GhosttyTerminalView.swift
  • Sources/KeyboardShortcutSettings.swift
  • Sources/Panels/BrowserPanel.swift
  • Sources/Panels/BrowserPanelView.swift
  • Sources/Panels/BrowserPopupWindowController.swift
  • Sources/Panels/BrowserWebAuthnSupport.swift
  • Sources/Panels/CmuxWebView.swift
  • Sources/Panels/ReactGrab.swift
  • Sources/Panels/TerminalPanel.swift
  • Sources/SessionIndexStore.swift
  • Sources/TabManager.swift
  • Sources/TerminalController.swift
  • Sources/TerminalNotificationStore.swift
  • Sources/TerminalWindowPortal.swift
  • Sources/Update/UpdateTitlebarAccessory.swift
  • Sources/WindowDragHandleView.swift
  • Sources/Workspace.swift
  • Sources/WorkspaceContentView.swift
  • Sources/cmuxApp.swift

Comment thread Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift
Comment thread Sources/Panels/BrowserPanelView.swift
Comment thread Sources/Panels/BrowserWebAuthnSupport.swift
Comment thread Sources/Panels/CmuxWebView.swift

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift`:
- Around line 49-54: The dump() method currently enqueues the file write on the
private queue and returns immediately; change it to perform the write
synchronously so callers see the updated file before return: inside dump() use
queue.sync { let content = self.entries.joined(separator: "\n") + "\n"; try?
content.write(toFile: Self.logPath, atomically: true, encoding: .utf8) } so the
write on the private queue completes before dump() returns (alternatively, add a
completion handler parameter to dump(entries:completion:) and call it after the
async write if you prefer nonblocking semantics); reference symbols: dump(),
queue, entries, Self.logPath.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: af6f6eaa-1cb5-45bd-b8b8-a86d1e15c2b5

📥 Commits

Reviewing files that changed from the base of the PR and between 8a18457 and 94ce210.

📒 Files selected for processing (2)
  • CLAUDE.md
  • Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift
✅ Files skipped from review due to trivial changes (1)
  • CLAUDE.md

Comment thread Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 29 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift">

<violation number="1" location="Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift:7">
P2: Wrap this package logger implementation in `#if DEBUG` so release builds do not include a file-backed debug logging sink.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8643b9a444

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Sources/Panels/CmuxWebView.swift Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift`:
- Around line 24-44: The DebugEventLog.log(_:) sink is persisting raw messages
to disk; add centralized redaction inside that function by running the incoming
message through a redaction helper before anything else (e.g., call a new or
existing sanitizer like Self.redactedContextDownloadDebugMessage(_:) or
Self.redact(_:) to strip/obfuscate URLs, file paths, tokens), then use the
redacted string for timestamping, entries array, and file writes (references:
DebugEventLog.log(_:), Self.logPath, Self.formatter); alternatively, if you
prefer per-call sanitization, enforce and document that all callers (e.g.,
cmuxDebugLog callers) must pass sanitized text and add an assertion or debug
build-time check in log(_:) that flags non-redacted inputs to prevent accidental
raw persistence.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 46ed6bde-1b31-4c2f-9f00-e0d39ee8b485

📥 Commits

Reviewing files that changed from the base of the PR and between 94ce210 and 8643b9a.

📒 Files selected for processing (4)
  • Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift
  • Sources/Panels/BrowserPanelView.swift
  • Sources/Panels/BrowserWebAuthnSupport.swift
  • Sources/Panels/CmuxWebView.swift
🚧 Files skipped from review as they are similar to previous changes (2)
  • Sources/Panels/CmuxWebView.swift
  • Sources/Panels/BrowserWebAuthnSupport.swift

Comment thread Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 4 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="Sources/Panels/CmuxWebView.swift">

<violation number="1" location="Sources/Panels/CmuxWebView.swift:850">
P2: Space-based tokenization leaks parts of sensitive values when redacted fields (like `path=`) contain spaces.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread Sources/Panels/CmuxWebView.swift Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift (1)

254-318: Optional: dedupe redaction predicate rules.

Many of the explicit normalizedKey == "…" comparisons in shouldRedactDebugField are already covered by the contains(...)/hasSuffix(...) rules below them:

  • "path" ⊂ hasSuffix("path")
  • "url" ⊂ hasSuffix("url")
  • "command" ⊂ hasSuffix("command")
  • "file" / "filename" ⊂ hasSuffix("file") / contains("filename")
  • "header" ⊂ hasSuffix("header")
  • "input" / "initialinput" ⊂ hasSuffix("input")
  • "text" ⊂ hasSuffix("text")
  • "token" / "cookie" / "authorization" ⊂ matching contains(...)
  • "directory" ⊂ hasSuffix("dir") (since directory ends with... no — actually directory doesn't end in dir; keep that one).

Collapsing these into a small exactSensitiveKeys: Set<String> + the existing suffix/contains list would make the policy easier to audit and keep in sync with shouldConsumeRestOfDebugMessage. Also consider pulling knownDebugFieldNames and the two predicates into a single source of truth so additions like "exec", "local", "remote", "remotetemp" (currently in the predicate but missing from knownDebugFieldNames) don't drift.

No behavior change expected — purely maintainability.

As per coding guidelines (**/*.swift: "All #if DEBUG debug logging must wrap the cmuxDebugLog() function... Use the unified debug event log in Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift."), this file is the canonical redaction authority, so tightening it here pays off across all call sites.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift` around lines
254 - 318, The redaction predicates in shouldRedactDebugField(_:) and
shouldConsumeRestOfDebugMessage(_:) duplicate checks (many exact == comparisons
are subsumed by contains/hasSuffix) and can drift from knownDebugFieldNames;
refactor by introducing a single exactSensitiveKeys: Set<String> used by both
shouldRedactDebugField and shouldConsumeRestOfDebugMessage, keep the existing
contains(...) and hasSuffix(...) checks intact, and consolidate the predicates
(or pull both into a shared helper) so knownDebugFieldNames,
shouldRedactDebugField, and shouldConsumeRestOfDebugMessage reference the same
source of truth (ensure no behavioral change and include the unique function
names above to locate edits).
Packages/CMUXDebugLog/Tests/CMUXDebugLogTests/DebugLogRedactorTests.swift (1)

5-44: Consider broadening redaction test coverage.

The current suite covers the happy-path patterns nicely, but a few behaviors in redactedDebugMessage are not exercised and are easy to regress:

  • Consume-rest fields (body, payload, query, text, exec, *args, *command) swallowing any later fields — e.g. "x body=foo bar=baz" → "x body=<redacted:11b>".
  • Non-http URL schemes collapsed to data:<redacted> / file:<redacted> / <scheme>:<redacted>.
  • Unparseable URL values falling back to <redacted:Nb>.
  • Case-insensitive key handling (e.g. URL=..., PATH=...), given normalizedKey = key.lowercased().
  • A consume-rest field where the value itself contains an = (ensuring later key= tokens inside a payload are never treated as fresh fields).

These are cheap regression anchors for the new central redaction layer.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@Packages/CMUXDebugLog/Tests/CMUXDebugLogTests/DebugLogRedactorTests.swift`
around lines 5 - 44, Add unit tests to DebugLogRedactorTests that exercise edge
cases of DebugEventLog.redactedDebugMessage: (1) verify consume-rest fields
(body, payload, query, text, exec, *args, *command) swallow the remainder (e.g.
"x body=foo bar=baz" -> "x body=<redacted:...>"); (2) assert non-http schemes
collapse to "data:<redacted>", "file:<redacted>" and a generic
"<scheme>:<redacted>" for others; (3) add a case with an unparseable URL value
to ensure it falls back to "<redacted:Nb>"; (4) test case-insensitive keys like
"URL=..." and "PATH=..." to confirm normalization is applied; and (5) include a
consume-rest field whose value contains an '=' to ensure subsequent "key="
patterns inside the value are not treated as new fields. Use
DebugEventLog.redactedDebugMessage in each assertion and keep expected outputs
consistent with the existing redaction format.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift`:
- Around line 254-318: The redaction predicates in shouldRedactDebugField(_:)
and shouldConsumeRestOfDebugMessage(_:) duplicate checks (many exact ==
comparisons are subsumed by contains/hasSuffix) and can drift from
knownDebugFieldNames; refactor by introducing a single exactSensitiveKeys:
Set<String> used by both shouldRedactDebugField and
shouldConsumeRestOfDebugMessage, keep the existing contains(...) and
hasSuffix(...) checks intact, and consolidate the predicates (or pull both into
a shared helper) so knownDebugFieldNames, shouldRedactDebugField, and
shouldConsumeRestOfDebugMessage reference the same source of truth (ensure no
behavioral change and include the unique function names above to locate edits).

In `@Packages/CMUXDebugLog/Tests/CMUXDebugLogTests/DebugLogRedactorTests.swift`:
- Around line 5-44: Add unit tests to DebugLogRedactorTests that exercise edge
cases of DebugEventLog.redactedDebugMessage: (1) verify consume-rest fields
(body, payload, query, text, exec, *args, *command) swallow the remainder (e.g.
"x body=foo bar=baz" -> "x body=<redacted:...>"); (2) assert non-http schemes
collapse to "data:<redacted>", "file:<redacted>" and a generic
"<scheme>:<redacted>" for others; (3) add a case with an unparseable URL value
to ensure it falls back to "<redacted:Nb>"; (4) test case-insensitive keys like
"URL=..." and "PATH=..." to confirm normalization is applied; and (5) include a
consume-rest field whose value contains an '=' to ensure subsequent "key="
patterns inside the value are not treated as new fields. Use
DebugEventLog.redactedDebugMessage in each assertion and keep expected outputs
consistent with the existing redaction format.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5273b98e-14ba-4986-8470-bdc6e113b903

📥 Commits

Reviewing files that changed from the base of the PR and between ea4a357 and 4739c02.

📒 Files selected for processing (3)
  • Packages/CMUXDebugLog/Package.swift
  • Packages/CMUXDebugLog/Sources/CMUXDebugLog/DebugEventLog.swift
  • Packages/CMUXDebugLog/Tests/CMUXDebugLogTests/DebugLogRedactorTests.swift
✅ Files skipped from review due to trivial changes (1)
  • Packages/CMUXDebugLog/Package.swift

@lawrencecchen
lawrencecchen merged commit 7ebf223 into main Apr 23, 2026
23 checks passed
@lawrencecchen
lawrencecchen deleted the task-decouple-debug-logging-from-bonsplit branch April 23, 2026 11:55

This branch was successfully deployed

1 active deployment
Preview — 4739c02f Deployed Apr 23, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant