**Actionable comments posted: 3** > [!CAUTION] > Some comments are outside the diff and can’t be posted inline due to platform limitations. > > > >

⚠️ Outside diff range comments (1)

> >
>
Sources/GhosttyTerminalView.swift (1)
> > `4305-4319`: _⚠️ Potential issue_ | _🟠 Major_ > > **Keep closed terminal surfaces from being recreated.** > > After `teardownSurface()` marks the lifecycle closed and clears `surface`, stale attach/background-start callbacks can still call `createSurface(for:)`. That can spawn a new Ghostty runtime for a panel that `canAcceptPortalBinding(...)` would reject as non-live. > > > >
>
Proposed lifecycle guard
> > ```diff > func attachToView(_ view: GhosttyNSView) { > `#if` DEBUG > dlog( > "surface.attach surface=\(id.uuidString.prefix(5)) view=\(Unmanaged.passUnretained(view).toOpaque()) " + > "attached=\(attachedView != nil ? 1 : 0) hasSurface=\(surface != nil ? 1 : 0) inWindow=\(view.window != nil ? 1 : 0)" > ) > `#endif` > + guard portalLifecycleState == .live else { > +#if DEBUG > + dlog( > + "surface.attach.skip surface=\(id.uuidString.prefix(5)) " + > + "reason=lifecycleState state=\(portalLifecycleState.rawValue)" > + ) > +#endif > + return > + } > > // If already attached to this view, nothing to do. > ``` > > ```diff > private func createSurface(for view: GhosttyNSView) { > + guard portalLifecycleState == .live else { > +#if DEBUG > + dlog( > + "surface.create.skip surface=\(id.uuidString.prefix(5)) " + > + "reason=lifecycleState state=\(portalLifecycleState.rawValue)" > + ) > +#endif > + return > + } > + > `#if` DEBUG > let resourcesDir = getenv("GHOSTTY_RESOURCES_DIR").flatMap { String(cString: $0) } ?? "(unset)" > ``` > > ```diff > - guard surface == nil, attachedView != nil else { return } > + guard portalLifecycleState == .live, surface == nil, attachedView != nil else { return } > guard !backgroundSurfaceStartQueued else { return } > backgroundSurfaceStartQueued = true > > DispatchQueue.main.async { [weak self] in > guard let self else { return } > self.backgroundSurfaceStartQueued = false > + guard self.portalLifecycleState == .live else { return } > guard self.surface == nil, let view = self.attachedView else { return } > ``` >
> > > Also applies to: 4334-4341, 4918-4938 > >
>
🤖 Prompt for AI Agents
> > ``` > Verify each finding against the current code and only fix it if needed. > > In `@Sources/GhosttyTerminalView.swift` around lines 4305 - 4319, The > attach/background-start paths call createSurface(for:) even after > teardownSurface() has set the lifecycle to closed and cleared surface; add a > lifecycle guard so these callbacks bail out when the panel is closed: before > calling createSurface(for:) (in the branches around surface == nil at the shown > sites and the other similar blocks), check the lifecycle/closed flag (or call > canAcceptPortalBinding(...) / a new isClosed/isLive property) and return if > closed, ensuring createSurface(for:) is never invoked for a closed panel and > preventing a new Ghostty runtime from being spawned for non-live panels. > ``` > >
> >
> >

🧹 Nitpick comments (2)

Sources/App/WorkspaceRuntimeSettings.swift (1)
`123-126`: **Centralize the long-press clamp bounds.** The 0.15/1.00 bounds are duplicated in `Sources/KeyboardShortcutSettingsFileStore.swift` (Line 445). Exposing them here avoids drift if the range is ever tuned.

♻️ Proposed change
```diff static let defaultLongPressDuration: TimeInterval = 0.30 + static let minLongPressDuration: TimeInterval = 0.15 + static let maxLongPressDuration: TimeInterval = 1.00 @@ static func longPressDuration(defaults: UserDefaults = .standard) -> TimeInterval { let raw = defaults.object(forKey: longPressDurationKey) as? Double ?? defaultLongPressDuration - return min(max(raw, 0.15), 1.00) + return min(max(raw, minLongPressDuration), maxLongPressDuration) } ``` And in `KeyboardShortcutSettingsFileStore.swift`: ```diff - let clamped = min(max(raw, 0.15), 1.00) + let clamped = min( + max(raw, TerminalRightClickSettings.minLongPressDuration), + TerminalRightClickSettings.maxLongPressDuration + ) ```

🤖 Prompt for AI Agents
``` Verify each finding against the current code and only fix it if needed. In `@Sources/App/WorkspaceRuntimeSettings.swift` around lines 123 - 126, Introduce centralized min/max constants in WorkspaceRuntimeSettings and use them instead of hard-coded literals: add e.g. static let minLongPressDuration: TimeInterval = 0.15 and static let maxLongPressDuration: TimeInterval = 1.0 alongside existing defaultLongPressDuration and longPressDurationKey, then update the longPressDuration(defaults:) implementation to clamp using those constants (min/max) rather than 0.15/1.00; finally update the duplicate clamp in KeyboardShortcutSettingsFileStore (referencing the KeyboardShortcutSettingsFileStore type) to use WorkspaceRuntimeSettings.minLongPressDuration and .maxLongPressDuration so both places share the same source of truth and run tests to confirm behavior. ```

Sources/GhosttyTerminalView.swift (1)
`8148-8173`: **Add DEBUG `dlog()` coverage for the new right-click gesture paths.** The new paste-mode and long-press branches are mouse event paths, but they do not emit DEBUG events, making right-click regressions hard to diagnose. As per coding guidelines, All debug events (keys, mouse, focus, splits, tabs) must be logged to the unified debug log using the `dlog()` function and wrapped in `#if DEBUG` / `#endif` preprocessor directives. Also applies to: 8309-8329

🤖 Prompt for AI Agents
``` Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 8148 - 8173, Add DEBUG dlog() calls wrapped in `#if` DEBUG / `#endif` to cover the new right-click gesture paths in rightMouseDown(with:): log entering the pasteFromClipboard branch (before requestPointerFocusRecovery/armLongRightClickContextMenuIfNeeded), log when cancelLongRightClickContextMenu() is invoked, and log the mouse position/button send (before calling ghostty_surface_mouse_pos and ghostty_surface_mouse_button); similarly add matching dlog() coverage in the corresponding long-press/context-menu handling code paths (e.g., armLongRightClickContextMenuIfNeeded and the code around ghostty_surface_mouse_button) so all right-click branches emit unified debug events. ```

🤖 Prompt for all review comments with AI agents

``` Verify each finding against the current code and only fix it if needed. Inline comments: In `@Resources/Localizable.xcstrings`: - Around line 88589-88604: The Korean localization for terminalContextMenu.resetTerminal is stale (value '플래시 실행' from old key) — update the "ko" stringUnit for terminalContextMenu.resetTerminal to the correct Korean translation for "Reset Terminal" (e.g., "터미널 재설정") or change its stringUnit state to "needs review" so the English fallback is used; modify the "ko" block for terminalContextMenu.resetTerminal accordingly without altering the English or Japanese entries. In `@Sources/cmuxApp.swift`: - Around line 4481-4568: The picker for terminal long-right-click delay only offers fixed tags (0.20, 0.30, 0.45) so arbitrary clamped values (e.g. 0.25) don't show a coherent selection; update the SettingsPickerRow (selection: terminalLongRightClickDurationSelection) to append a dynamic "Custom" option when terminalRightClickLongPressDuration is not equal to any of the preset tags — render a localized label like "Custom (0.25s)" (using the current terminalRightClickLongPressDuration) and tag it with the actual terminalRightClickLongPressDuration so the picker reflects and preserves arbitrary clamped values. Ensure the existing binding terminalLongRightClickDurationSelection and clamping logic remain unchanged. In `@Sources/GhosttyTerminalView.swift`: - Around line 8175-8188: The rightMouseDragged handler currently bails out when `surface` is nil, leaving the delayed long-right-click work item and gesture flags alive; move or duplicate the cleanup so that `cancelLongRightClickContextMenu()` is invoked and gesture state (`didRightMouseDrag`, `rightMouseDownLocation` checks) is handled regardless of `surface` presence before any early return, then only call `ghostty_surface_mouse_pos(...)` if `surface` exists; apply the same change to the corresponding `rightMouseUp` handler (the block around lines 8190-8205) so pending menu work items and flags are always cleared even if the `surface` has been torn down or detached. --- Outside diff comments: In `@Sources/GhosttyTerminalView.swift`: - Around line 4305-4319: The attach/background-start paths call createSurface(for:) even after teardownSurface() has set the lifecycle to closed and cleared surface; add a lifecycle guard so these callbacks bail out when the panel is closed: before calling createSurface(for:) (in the branches around surface == nil at the shown sites and the other similar blocks), check the lifecycle/closed flag (or call canAcceptPortalBinding(...) / a new isClosed/isLive property) and return if closed, ensuring createSurface(for:) is never invoked for a closed panel and preventing a new Ghostty runtime from being spawned for non-live panels. --- Nitpick comments: In `@Sources/App/WorkspaceRuntimeSettings.swift`: - Around line 123-126: Introduce centralized min/max constants in WorkspaceRuntimeSettings and use them instead of hard-coded literals: add e.g. static let minLongPressDuration: TimeInterval = 0.15 and static let maxLongPressDuration: TimeInterval = 1.0 alongside existing defaultLongPressDuration and longPressDurationKey, then update the longPressDuration(defaults:) implementation to clamp using those constants (min/max) rather than 0.15/1.00; finally update the duplicate clamp in KeyboardShortcutSettingsFileStore (referencing the KeyboardShortcutSettingsFileStore type) to use WorkspaceRuntimeSettings.minLongPressDuration and .maxLongPressDuration so both places share the same source of truth and run tests to confirm behavior. In `@Sources/GhosttyTerminalView.swift`: - Around line 8148-8173: Add DEBUG dlog() calls wrapped in `#if` DEBUG / `#endif` to cover the new right-click gesture paths in rightMouseDown(with:): log entering the pasteFromClipboard branch (before requestPointerFocusRecovery/armLongRightClickContextMenuIfNeeded), log when cancelLongRightClickContextMenu() is invoked, and log the mouse position/button send (before calling ghostty_surface_mouse_pos and ghostty_surface_mouse_button); similarly add matching dlog() coverage in the corresponding long-press/context-menu handling code paths (e.g., armLongRightClickContextMenuIfNeeded and the code around ghostty_surface_mouse_button) so all right-click branches emit unified debug events. ```

🪄 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**: `be8fa21f-62fb-4295-a046-caa4035ea2cf`

📥 Commits

Reviewing files that changed from the base of the PR and between 84a5903b11cb905cbb5fa2782cdbe02bcd507e76 and c0ddfb2a66703814a6e1c303c439e35bb7d7d00d.

📒 Files selected for processing (5)

* `Resources/Localizable.xcstrings` * `Sources/App/WorkspaceRuntimeSettings.swift` * `Sources/GhosttyTerminalView.swift` * `Sources/KeyboardShortcutSettingsFileStore.swift` * `Sources/cmuxApp.swift`