Add hidden CLI command for live terminal debugging - #1599
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a CLI command "debug-terminals" that calls a new v2 method to enumerate TerminalSurface instances; introduces a thread-safe TerminalSurfaceRegistry, records surface lifecycle timestamps/teardown reasons, exposes debug accessors, and formats JSON or human-readable terminal-debug payloads (with DEBUG-gated test hooks). Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI User
participant CMux as CLI/cmux.swift
participant TermCtrl as TerminalController
participant Registry as TerminalSurfaceRegistry
participant Surface as TerminalSurface
CLI->>CMux: run "debug-terminals"
CMux->>TermCtrl: v2.call("debug.terminals", params)
TermCtrl->>Registry: allSurfaces()
Registry-->>TermCtrl: [TerminalSurface...]
loop per surface
TermCtrl->>Surface: call debug accessors (timestamps, state, pointers)
Surface-->>TermCtrl: metadata
end
TermCtrl-->>CMux: V2CallResult payload (terminals)
CMux->>CMux: format payload (JSON or human-readable)
CMux-->>CLI: output formatted debug info
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Poem
🚥 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 unit tests (beta)
📝 Coding Plan
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmuxTests/CLIProcessRunnerTests.swift`:
- Around line 3-6: The conditional import block using `#if` canImport(cmux_DEV) /
`#elseif` canImport(cmux) must be closed immediately after the import statements
so the test class CLIProcessRunnerTests is always compiled; move the `#endif` that
currently closes the conditional at the end of the file to directly follow the
import directives (i.e., right before the start of the CLIProcessRunnerTests
class) so the imports remain conditional but the test class is outside that
conditional.
In `@Sources/TerminalController.swift`:
- Around line 4969-4980: The current fallback only parses identifiers in
windowIdFromIdentifier(_:), which ignores windows that have plain identifiers or
that already exist in the enumerated windows list; update the lookup to first
try to find a match in the existing windows array returned by
app.scriptableMainWindows() (by object identity or by matching
window.windowNumber/windowId) before attempting to parse the "cmux.main.<uuid>"
prefix. Concretely, when resolving a NSWindow to a UUID (used where
windowIdFromIdentifier(_:), windows, and windowIndexById are referenced), check
the enumerated windows for the same NSWindow instance or same windowNumber and
return that window's windowId/window index if present; only if no match is found
fall back to parsing raw.identifier rawValue for the "cmux.main.<uuid>" pattern.
- Around line 4950-4956: The rectPayload helper currently builds a [String:
Double] from a CGRect whose components are CGFloat; update rectPayload(_ rect:
CGRect) to explicitly convert each component to Double (e.g.
Double(rect.origin.x), Double(rect.origin.y), Double(rect.size.width),
Double(rect.size.height)) so the dictionary values match the declared Double
type and the code type-checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8d19e25f-e210-4b00-83ab-c311e304bee7
📒 Files selected for processing (5)
CLI/cmux.swiftGhosttyTabs.xcodeproj/project.pbxprojSources/GhosttyTerminalView.swiftSources/TerminalController.swiftcmuxTests/CLIProcessRunnerTests.swift
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
CLI/cmux.swift (1)
3016-3033: Use the existing coercion helpers for rect/ports formatting too.
formatDebugRectandformatDebugPortscan still drop valid numeric payloads in mixed-type dictionaries; using the coercion helper keeps output stable.Suggested refactor
private func formatDebugRect(_ value: Any?) -> String? { guard let rect = value as? [String: Any], - let x = rect["x"] as? Double, - let y = rect["y"] as? Double, - let width = rect["width"] as? Double, - let height = rect["height"] as? Double else { + let x = doubleFromAny(rect["x"]), + let y = doubleFromAny(rect["y"]), + let width = doubleFromAny(rect["width"]), + let height = doubleFromAny(rect["height"]) else { return nil } return String(format: "{%.1f,%.1f %.1fx%.1f}", x, y, width, height) } private func formatDebugPorts(_ value: Any?) -> String { guard let array = value as? [Any], !array.isEmpty else { return "[]" } - return array + let ports = array .compactMap { intFromAny($0) } .map(String.init) - .joined(separator: ",") + return ports.isEmpty ? "[]" : ports.joined(separator: ",") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLI/cmux.swift` around lines 3016 - 3033, The current formatDebugRect and formatDebugPorts drop valid numeric values when types vary; update them to use the existing coercion helpers (e.g., doubleFromAny for rect components and intFromAny for ports) instead of direct casting. In formatDebugRect, extract x,y,width,height via doubleFromAny(rect["x"]), etc., guard that those return non-nil and then format with String(format:...), and in formatDebugPorts map array elements through intFromAny, filter out nils, return "[]" if the resulting list is empty, otherwise join with ","; keep the same function names (formatDebugRect, formatDebugPorts) and reuse intFromAny/doubleFromAny helper symbols to locate the helpers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLI/cmux.swift`:
- Around line 1852-1859: The "debug-terminals" subcommand currently ignores any
trailing tokens; update the case "debug-terminals" branch to reject unexpected
arguments by checking that there are no extra command tokens/arguments before
calling client.sendV2. If extra args are present, print a concise usage/error
message and exit with a non-zero status (matching existing CLI error handling),
otherwise proceed to call client.sendV2(...) and print either jsonString(...) or
formatDebugTerminalsPayload(...). Ensure you reference the same variables used
in this block (jsonOutput, client.sendV2, formatDebugTerminalsPayload, idFormat)
and reuse the project's standard error/exit helpers for consistency.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 2565-2568: The debug metadata fields (createdAt,
runtimeSurfaceCreatedAt, teardownRequestedAt, teardownRequestReason) are
accessed from background socket handlers via v2DebugTerminals/v2MainSync while
mutations occur off-main (e.g.
recordTeardownRequest()/beginPortalCloseLifecycle()), causing unsynchronized
cross-thread access; fix by making access to these fields main-isolated and
ensuring mutations happen on main: mark the properties (createdAt,
runtimeSurfaceCreatedAt, teardownRequestedAt, teardownRequestReason) as
`@MainActor` or move them into a `@MainActor` extension/actor, and annotate/move
mutating methods like recordTeardownRequest() and runtimeSurfaceCreatedAt =
Date() (or call sites such as beginPortalCloseLifecycle()) to run on the
MainActor (or dispatch them to main) so v2MainSync reads and all writes are
serialized and race-free; keep TerminalSurfaceRegistry/v2DebugTerminals
unchanged except for relying on the main isolation.
---
Nitpick comments:
In `@CLI/cmux.swift`:
- Around line 3016-3033: The current formatDebugRect and formatDebugPorts drop
valid numeric values when types vary; update them to use the existing coercion
helpers (e.g., doubleFromAny for rect components and intFromAny for ports)
instead of direct casting. In formatDebugRect, extract x,y,width,height via
doubleFromAny(rect["x"]), etc., guard that those return non-nil and then format
with String(format:...), and in formatDebugPorts map array elements through
intFromAny, filter out nils, return "[]" if the resulting list is empty,
otherwise join with ","; keep the same function names (formatDebugRect,
formatDebugPorts) and reuse intFromAny/doubleFromAny helper symbols to locate
the helpers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c47f6d30-35e6-436b-8be3-816759705534
📒 Files selected for processing (4)
CLI/cmux.swiftSources/GhosttyTerminalView.swiftSources/TerminalController.swiftcmuxTests/CLIProcessRunnerTests.swift
This was referenced Mar 20, 2026
Closed
3 tasks done
bn-l
pushed a commit
to bn-l/cmux
that referenced
this pull request
Apr 3, 2026
* Add hidden terminal debug CLI command * Expand orphan terminal debug metadata * Remove stray CLIProcessRunner test target wiring * Tighten debug terminal diagnostics handling --------- Co-authored-by: Lawrence Chen <lawrencecchen@users.noreply.github.com>
This was referenced Apr 6, 2026
This was referenced Apr 25, 2026
This was referenced May 5, 2026
This was referenced May 14, 2026
This branch was successfully deployed
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
cmux debug-terminalscommand backed by adebug.terminalssocket methodVerification
./scripts/reload.sh --tag task-cli-ghost-terminal-debugcmux-dev --socket /tmp/cmux-debug-task-cli-ghost-terminal-debug.sock debug-terminalsmapped=0 tree=0in the tagged appSummary by cubic
Add a hidden
cmux debug-terminalscommand to inspect live Ghostty terminal surfaces across all windows/workspaces, including orphans, with readable or--jsonoutput. Tightens diagnostics with richer metadata, safer formatting, and better window/workspace/pane mapping.New Features
debug.terminalssocket method and hiddencmux debug-terminalscommand with human-readable output,--json, usage help, and argument validation.TerminalSurfaceRegistryand per-surface debug metadata: created/runtime ages; teardown reason/age; initial command; portal host lease (id/inWindow/area).Refactors
CLIProcessRunnerTeststarget and test files.Written for commit f5774a8. Summary will update on new commits.
Summary by CodeRabbit
New Features
Improvements
Tests