Skip to content

Avoid beachball during update relaunch #4988

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lawrencecchen wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Avoid beachball during update relaunch #4988

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
43e0923
Add update relaunch snapshot policy tests
lawrencecchen May 29, 2026
ff8b446
Avoid full session snapshot during update relaunch
lawrencecchen May 29, 2026
0433c3c
Use bounded Ghostty tail reads for session snapshots
lawrencecchen May 29, 2026
3e71a96
Pin GhosttyKit checksum for bounded tail API
lawrencecchen May 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 40 additions & 8 deletions Sources/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1001,6 +1001,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
var shouldDeferInitialMainWindowBootstrapForExternalConfirmation = false
private var didBootstrapInitialMainWindow = false
private var isTerminatingApp = false
private var didPersistUpdateRelaunchSnapshot = false
private var closedWindowHistorySuppressedWindowIds: Set<UUID> = []
#if DEBUG
var closeMainWindowContainingTabIdObserverForTesting: ((UUID, Bool) -> Void)?
Expand Down Expand Up @@ -1763,7 +1764,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
StartupBreadcrumbLog.append("appDelegate.willTerminate.begin")
isTerminatingApp = true
closeAllWebInspectorsBeforeAppTeardown()
_ = saveSessionSnapshotIncludingProcessDetectedIndexes(includeScrollback: true, removeWhenEmpty: false)
saveSessionSnapshotForAppTermination()
stopSessionAutosaveTimer()
CloudVMActionLauncher.shared.terminateAll()
CmuxSSHURLProcessLauncher.shared.terminateAll()
Expand Down Expand Up @@ -1792,7 +1793,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent

func persistSessionForUpdateRelaunch() {
isTerminatingApp = true
_ = saveSessionSnapshotIncludingProcessDetectedIndexes(includeScrollback: true, removeWhenEmpty: false)
stopSessionAutosaveTimer()
didPersistUpdateRelaunchSnapshot = saveSessionSnapshotIncludingProcessDetectedIndexes(
includeScrollback: true,
removeWhenEmpty: false,
forceSynchronousWrite: true
)
Comment on lines +1797 to +1801
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Show the snapshot entry point and how includeScrollback flows into the write decision.
ast-grep --pattern 'func saveSessionSnapshotIncludingProcessDetectedIndexes($$$) {
  $$$
}'

ast-grep --pattern 'func saveSessionSnapshot($$$) {
  $$$
}'

ast-grep --pattern 'static func shouldWriteSessionSnapshotSynchronously($$$) -> Bool {
  $$$
}'

# Inspect what includeScrollback triggers (e.g. bounded tail read vs full scrollback export).
rg -nP --type=swift -C4 '\bincludeScrollback\b'

# Confirm the relaunch call site runs on the main thread synchronously.
rg -nP --type=swift -C4 'persistSessionForUpdateRelaunch\s*\('

Repository: manaflow-ai/cmux

Length of output: 50373

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Inspect the exact relaunch snapshot code region.
echo "=== AppDelegate.swift around persistSessionForUpdateRelaunch ==="
sed -n '1765,1825p' Sources/AppDelegate.swift

# 2) Locate updaterWillRelaunchApplication call path and where persistSessionForUpdateRelaunch is invoked.
echo "=== Find updaterWillRelaunchApplication ==="
rg -n "updaterWillRelaunchApplication" -S Sources

echo "=== Find persistSessionForUpdateRelaunch call sites ==="
rg -n "persistSessionForUpdateRelaunch\\s*\\(" -S Sources

# If UpdateDelegate.swift exists, inspect the likely area mentioned in the review comment.
if [ -f Sources/UpdateDelegate.swift ]; then
  echo "=== UpdateDelegate.swift: around 80-170 ==="
  sed -n '80,170p' Sources/UpdateDelegate.swift
fi

# 3) Inspect snapshot write-policy logic directly.
echo "=== shouldWriteSessionSnapshotSynchronously ==="
sed -n '4000,4060p' Sources/AppDelegate.swift

# 4) Confirm includeScrollback propagates into build snapshot for the relaunch snapshot helper.
echo "=== saveSessionSnapshotIncludingProcessDetectedIndexes / saveSessionSnapshot ==="
rg -n "saveSessionSnapshotIncludingProcessDetectedIndexes\\(" -S Sources/AppDelegate.swift
sed -n '3950,4020p' Sources/AppDelegate.swift

# 5) Find any existing comment about relaunch snapshot being minimal/no-scrollback or with scrollback.
echo "=== Search for 'relaunch' + 'scrollback' ==="
rg -n "relaunch.*scrollback|scrollback.*relaunch" -S Sources/AppDelegate.swift Sources

Repository: manaflow-ai/cmux

Length of output: 8910

🏁 Script executed:

#!/bin/bash
# (placeholder, only one shell block should be used)

Repository: owner/repo

Length of output: 2026

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== UpdateDelegate.swift: updaterWillRelaunchApplication ==="
sed -n '70,160p' Sources/Update/UpdateDelegate.swift

echo "=== saveSessionSnapshot: sync vs async write behavior ==="
sed -n '3685,3785p' Sources/AppDelegate.swift

echo "=== persistSessionSnapshot write helper (if present) ==="
rg -n "persistSessionSnapshot\\(" -S Sources/AppDelegate.swift
# show the first match region around the definition
rg -n "private func persistSessionSnapshot\\(" -S Sources/AppDelegate.swift && sed -n '3785,3920p' Sources/AppDelegate.swift

Repository: manaflow-ai/cmux

Length of output: 12336

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== persistSessionSnapshot implementation ==="
# show around first definition
rg -n "private func persistSessionSnapshot\\(" Sources/AppDelegate.swift
sed -n '4070,4180p' Sources/AppDelegate.swift

echo "=== Find where 'synchronously:' is used/checked ==="
rg -n "synchronously:\\s*|synchronously\\b" Sources/AppDelegate.swift -C2

echo "=== Search for bounded tail / scrollback export behavior tied to includeScrollback ==="
rg -n "bounded|tail|scrollback" Sources/AppDelegate.swift -S -g'*.swift' -C2

# Also search for bounded tail in the broader codebase for snapshot persistence logic.
rg -n "bounded.*tail|tail.*read|scrollback.*bounded|includeScrollback" Sources -S -g'*.swift' -C2 | head -n 80

Repository: manaflow-ai/cmux

Length of output: 25784

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== SessionPersistencePolicy scrollback limits ==="
rg -n "maxScrollbackLinesPerTerminal|SessionPersistencePolicy" Sources/SessionPersistencePolicy.swift Sources -S -g'*.swift' -C1

# If SessionPersistencePolicy is in another file, the rg above will include it; also directly search:
rg -n "maxScrollbackLinesPerTerminal" Sources -S -g'*.swift' -C3

echo "=== Workspace.swift: capturedScrollback gating (shouldPersistScrollback, hibernationState) ==="
sed -n '520,640p' Sources/Workspace.swift
rg -n "shouldPersistScrollback" Sources/Workspace.swift -S -C3

echo "=== Workspace.swift: TerminalController.shared.readTerminalTextForSnapshot arguments ==="
sed -n '540,610p' Sources/Workspace.swift

echo "=== Any comments describing update relaunch snapshot goal (minimal/no-scrollback) ==="
rg -n "update relaunch|relaunch.*session|persistSessionForUpdateRelaunch|minimal.*scrollback|no scrollback" Sources/AppDelegate.swift Sources -S -g'*.swift' -C2

Repository: manaflow-ai/cmux

Length of output: 11536

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Workspace.swift: scrollback capture gating around capturedScrollback ==="
sed -n '540,620p' Sources/Workspace.swift

echo "=== Workspace.swift: shouldPersistScrollback definition/usage in this file ==="
rg -n "shouldPersistScrollback" Sources/Workspace.swift -S
# print a bit of the surrounding definitions
rg -n "shouldPersistScrollback" Sources/Workspace.swift -S -C3

echo "=== SessionPersistence.swift: scrollback limits and truncation helpers ==="
sed -n '1,120p' Sources/SessionPersistence.swift

echo "=== SessionPersistence.swift: truncatedScrollback implementation ==="
rg -n "truncatedScrollback" Sources/SessionPersistence.swift -S
rg -n "truncatedScrollback\\(" Sources/SessionPersistence.swift -S -C2 | head -n 40

# show the specific implementation region (best-effort by locating a few line ranges)
# (Using approximate range; if too small/large, re-run with adjusted bounds.)
sed -n '150,260p' Sources/SessionPersistence.swift

echo "=== Any comments/logic in Workspace about update relaunch minimal/no scrollback ==="
rg -n "update relaunch|update.*relaunch|relaunch.*scrollback|minimal.*scrollback|no scrollback" Sources/Workspace.swift Sources/AppDelegate.swift Sources/Update/UpdateDelegate.swift -S -g'*.swift'

Repository: manaflow-ai/cmux

Length of output: 15512

Fix update-relaunch session snapshot: includeScrollback: true persists scrollback in a main-thread synchronous save

persistSessionForUpdateRelaunch() sets isTerminatingApp = true and calls saveSessionSnapshotIncludingProcessDetectedIndexes(includeScrollback: true, ..., forceSynchronousWrite: true). updaterWillRelaunchApplication(_:) invokes this on the main thread (directly if already on main, otherwise via DispatchQueue.main.sync). With forceSynchronousWrite: true, saveSessionSnapshot(...) performs an immediate write (persistSessionSnapshot(..., synchronously: true) runs writeBlock() inline).

Because includeScrollback is true, snapshot building captures scrollback (bounded) via TerminalController.shared.readTerminalTextForSnapshot(..., includeScrollback: true, lineLimit: SessionPersistencePolicy.maxScrollbackLinesPerTerminal) when shouldPersistScrollback && shouldReplaySessionScrollback && hibernationState == nil. This means the relaunch snapshot is not “no scrollback” (even though it’s truncated to policy limits).

If the update-relaunch path is intended to be “minimal, no scrollback”, switch includeScrollback to false (or ensure the policy conditions match that goal).

🐢 Proposed change if scrollback should be excluded on the relaunch path 
         didPersistUpdateRelaunchSnapshot = saveSessionSnapshotIncludingProcessDetectedIndexes(
-            includeScrollback: true,
+            includeScrollback: false,
             removeWhenEmpty: false,
             forceSynchronousWrite: true
         )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
didPersistUpdateRelaunchSnapshot = saveSessionSnapshotIncludingProcessDetectedIndexes(
includeScrollback: true,
removeWhenEmpty: false,
forceSynchronousWrite: true
)
didPersistUpdateRelaunchSnapshot = saveSessionSnapshotIncludingProcessDetectedIndexes(
includeScrollback: false,
removeWhenEmpty: false,
forceSynchronousWrite: true
)
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/AppDelegate.swift` around lines 1797 - 1801,
persistSessionForUpdateRelaunch() currently calls
saveSessionSnapshotIncludingProcessDetectedIndexes(includeScrollback: true,
forceSynchronousWrite: true) which causes
TerminalController.shared.readTerminalTextForSnapshot(..., includeScrollback:
true, ...) to capture scrollback synchronously during
updaterWillRelaunchApplication(_:). To make the relaunch snapshot minimal (no
scrollback), change the includeScrollback argument to false in the
saveSessionSnapshotIncludingProcessDetectedIndexes call inside
persistSessionForUpdateRelaunch(), or alternatively adjust the session
persistence policy checks (shouldPersistScrollback /
shouldReplaySessionScrollback / hibernationState) so that
readTerminalTextForSnapshot is not invoked with includeScrollback=true on the
update-relaunch path.

}

func configure(tabManager: TabManager, notificationStore: TerminalNotificationStore, sidebarState: SidebarState) {
Expand Down Expand Up @@ -3687,7 +3693,8 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
includeScrollback: Bool,
removeWhenEmpty: Bool = false,
restorableAgentIndex: RestorableAgentSessionIndex? = nil,
surfaceResumeBindingIndex: SurfaceResumeBindingIndex? = nil
surfaceResumeBindingIndex: SurfaceResumeBindingIndex? = nil,
forceSynchronousWrite: Bool = false
) -> Bool {
if Self.shouldSkipSessionSaveDuringRestore(
isApplyingSessionRestore: isApplyingSessionRestore,
Expand All @@ -3700,7 +3707,8 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
}
let writeSynchronously = Self.shouldWriteSessionSnapshotSynchronously(
isTerminatingApp: isTerminatingApp,
includeScrollback: includeScrollback
includeScrollback: includeScrollback,
forceSynchronousWrite: forceSynchronousWrite
)
if writeSynchronously {
TextBoxInputTextView.flushPendingSessionDraftAttachmentCopies()
Expand Down Expand Up @@ -3958,14 +3966,30 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent
@discardableResult
private func saveSessionSnapshotIncludingProcessDetectedIndexes(
includeScrollback: Bool,
removeWhenEmpty: Bool = false
removeWhenEmpty: Bool = false,
forceSynchronousWrite: Bool = false
) -> Bool {
let resumeIndexes = ProcessDetectedResumeIndexes.loadSynchronously()
return saveSessionSnapshot(
includeScrollback: includeScrollback,
removeWhenEmpty: removeWhenEmpty,
restorableAgentIndex: resumeIndexes.restorableAgentIndex,
surfaceResumeBindingIndex: resumeIndexes.surfaceResumeBindingIndex
surfaceResumeBindingIndex: resumeIndexes.surfaceResumeBindingIndex,
forceSynchronousWrite: forceSynchronousWrite
)
}

private func saveSessionSnapshotForAppTermination() {
guard Self.shouldSaveSessionSnapshotOnApplicationTerminate(
didPersistUpdateRelaunchSnapshot: didPersistUpdateRelaunchSnapshot
) else {
StartupBreadcrumbLog.append("appDelegate.willTerminate.snapshotSkipped.updateRelaunch")
return
}

_ = saveSessionSnapshotIncludingProcessDetectedIndexes(
includeScrollback: true,
removeWhenEmpty: false
)
}

Expand Down Expand Up @@ -4004,9 +4028,17 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent

nonisolated static func shouldWriteSessionSnapshotSynchronously(
isTerminatingApp: Bool,
includeScrollback: Bool
includeScrollback: Bool,
forceSynchronousWrite: Bool = false
) -> Bool {
if forceSynchronousWrite { return true }
return isTerminatingApp && includeScrollback
}

nonisolated static func shouldSaveSessionSnapshotOnApplicationTerminate(
didPersistUpdateRelaunchSnapshot: Bool
) -> Bool {
isTerminatingApp && includeScrollback
!didPersistUpdateRelaunchSnapshot
}

nonisolated static func shouldSkipSessionAutosaveForUnchangedFingerprint(
Expand Down
210 changes: 50 additions & 160 deletions Sources/TerminalController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,36 +752,6 @@ class TerminalController {
return trimmed
}

nonisolated static func normalizedExportedScreenPath(_ raw: String?) -> String? {
guard let raw else { return nil }
let trimmed = raw.trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmed.isEmpty else { return nil }
if let url = URL(string: trimmed),
url.isFileURL,
!url.path.isEmpty {
return url.path
}
return trimmed.hasPrefix("/") ? trimmed : nil
}

nonisolated static func shouldRemoveExportedScreenFile(
fileURL: URL,
temporaryDirectory: URL = FileManager.default.temporaryDirectory
) -> Bool {
let standardizedFile = fileURL.standardizedFileURL
let temporary = temporaryDirectory.standardizedFileURL
return standardizedFile.path.hasPrefix(temporary.path + "/")
}

nonisolated static func shouldRemoveExportedScreenDirectory(
fileURL: URL,
temporaryDirectory: URL = FileManager.default.temporaryDirectory
) -> Bool {
let directory = fileURL.deletingLastPathComponent().standardizedFileURL
let temporary = temporaryDirectory.standardizedFileURL
return directory.path.hasPrefix(temporary.path + "/")
}

nonisolated static func parseReportedShellActivityState(
_ rawState: String
) -> Workspace.PanelShellActivityState? {
Expand Down Expand Up @@ -10006,6 +9976,35 @@ class TerminalController {
return String(decoding: rawData, as: UTF8.self)
}

private func readTerminalTextTail(
terminalPanel: TerminalPanel,
scope: ghostty_text_scope_e,
format: ghostty_text_format_e,
lineLimit: Int?,
byteLimit: Int? = nil
) -> String? {
guard let surface = terminalPanel.surface.surface else { return nil }
let options = ghostty_text_tail_options_s(
scope: scope,
format: format,
max_lines: UInt(max(lineLimit ?? 0, 0)),
max_bytes: UInt(max(byteLimit ?? 0, 0))
)
var text = ghostty_text_s()
guard ghostty_surface_read_text_tail(surface, options, &text) else {
return nil
}
defer {
ghostty_surface_free_text(surface, &text)
}

guard let ptr = text.text, text.text_len > 0 else {
return ""
}
let rawData = Data(bytes: ptr, count: Int(text.text_len))
return String(decoding: rawData, as: UTF8.self)
}

private func readTerminalTextBase64(terminalPanel: TerminalPanel, includeScrollback: Bool = false, lineLimit: Int? = nil) -> String {
guard terminalPanel.surface.surface != nil else { return "ERROR: Terminal surface not found" }
func readSelectionText(pointTag: ghostty_point_tag_e) -> String? {
Expand All @@ -10014,44 +10013,15 @@ class TerminalController {

var output: String
if includeScrollback {
func candidateScore(_ text: String) -> (lines: Int, bytes: Int) {
let lines = text.isEmpty ? 0 : text.split(separator: "\n", omittingEmptySubsequences: false).count
return (lines, text.utf8.count)
}

// Read all available regions and pick the most complete candidate.
// Different point tags can lose different rows around resize/reflow boundaries.
let screen = readSelectionText(pointTag: GHOSTTY_POINT_SCREEN)
let history = readSelectionText(pointTag: GHOSTTY_POINT_SURFACE)
let active = readSelectionText(pointTag: GHOSTTY_POINT_ACTIVE)

var candidates: [String] = []
if let screen {
candidates.append(screen)
}
if history != nil || active != nil {
var merged = history ?? ""
if let active {
if !merged.isEmpty, !merged.hasSuffix("\n"), !active.isEmpty {
merged.append("\n")
}
merged.append(active)
}
candidates.append(merged)
}

if let best = candidates.max(by: { lhs, rhs in
let left = candidateScore(lhs)
let right = candidateScore(rhs)
if left.lines != right.lines {
return left.lines < right.lines
}
return left.bytes < right.bytes
}) {
output = best
} else {
guard let tail = readTerminalTextTail(
terminalPanel: terminalPanel,
scope: GHOSTTY_TEXT_SCOPE_SCREEN,
format: GHOSTTY_TEXT_FORMAT_PLAIN,
lineLimit: lineLimit
) else {
return "ERROR: Failed to read terminal text"
}
output = tail
} else {
guard let viewport = readSelectionText(pointTag: GHOSTTY_POINT_VIEWPORT) else {
return "ERROR: Failed to read terminal text"
Expand All @@ -10067,106 +10037,26 @@ class TerminalController {
return "OK \(base64)"
}

private struct PasteboardItemSnapshot {
let representations: [(type: NSPasteboard.PasteboardType, data: Data)]
}

private func snapshotPasteboardItems(_ pasteboard: NSPasteboard) -> [PasteboardItemSnapshot] {
guard let items = pasteboard.pasteboardItems else { return [] }
return items.map { item in
let representations = item.types.compactMap { type -> (type: NSPasteboard.PasteboardType, data: Data)? in
guard let data = item.data(forType: type) else { return nil }
return (type: type, data: data)
}
return PasteboardItemSnapshot(representations: representations)
}
}

private func restorePasteboardItems(
_ snapshots: [PasteboardItemSnapshot],
to pasteboard: NSPasteboard
) {
_ = pasteboard.clearContents()
guard !snapshots.isEmpty else { return }

let restoredItems = snapshots.compactMap { snapshot -> NSPasteboardItem? in
guard !snapshot.representations.isEmpty else { return nil }
let item = NSPasteboardItem()
for representation in snapshot.representations {
item.setData(representation.data, forType: representation.type)
}
return item
}
guard !restoredItems.isEmpty else { return }
_ = pasteboard.writeObjects(restoredItems)
}

private func readGeneralPasteboardString(_ pasteboard: NSPasteboard) -> String? {
if let urls = pasteboard.readObjects(forClasses: [NSURL.self]) as? [URL],
let firstURL = urls.first,
firstURL.isFileURL {
return firstURL.path
}
if let value = pasteboard.string(forType: .string) {
return value
}
return pasteboard.string(forType: NSPasteboard.PasteboardType("public.utf8-plain-text"))
}

private func readTerminalTextFromVTExportForSnapshot(
terminalPanel: TerminalPanel,
lineLimit: Int?
) -> String? {
let pasteboard = NSPasteboard.general
let snapshot = snapshotPasteboardItems(pasteboard)
defer {
restorePasteboardItems(snapshot, to: pasteboard)
}

let initialChangeCount = pasteboard.changeCount
guard terminalPanel.performBindingAction("write_screen_file:copy,vt") else {
return nil
}
guard pasteboard.changeCount != initialChangeCount else {
return nil
}
guard let exportedPath = Self.normalizedExportedScreenPath(readGeneralPasteboardString(pasteboard)) else {
return nil
}

let fileURL = URL(fileURLWithPath: exportedPath)
defer {
if Self.shouldRemoveExportedScreenFile(fileURL: fileURL) {
try? FileManager.default.removeItem(at: fileURL)
if Self.shouldRemoveExportedScreenDirectory(fileURL: fileURL) {
try? FileManager.default.removeItem(at: fileURL.deletingLastPathComponent())
}
}
}

guard let data = try? Data(contentsOf: fileURL),
var output = String(data: data, encoding: .utf8) else {
return nil
}
if let lineLimit {
output = tailTerminalLines(output, maxLines: lineLimit)
}
return output
}

func readTerminalTextForSnapshot(
terminalPanel: TerminalPanel,
includeScrollback: Bool = false,
lineLimit: Int? = nil,
allowVTExport: Bool = true
) -> String? {
if includeScrollback,
allowVTExport,
let vtOutput = readTerminalTextFromVTExportForSnapshot(
terminalPanel: terminalPanel,
lineLimit: lineLimit
) {
return vtOutput
if includeScrollback {
return readTerminalTextTail(
terminalPanel: terminalPanel,
scope: GHOSTTY_TEXT_SCOPE_SCREEN,
format: allowVTExport ? GHOSTTY_TEXT_FORMAT_VT : GHOSTTY_TEXT_FORMAT_PLAIN,
lineLimit: lineLimit,
byteLimit: SessionPersistencePolicy.maxScrollbackCharactersPerTerminal
Copy link
Copy Markdown

@cursor cursor Bot May 29, 2026

Choose a reason for hiding this comment

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

Character-count constant misused as byte limit

Low Severity

SessionPersistencePolicy.maxScrollbackCharactersPerTerminal (400,000) is a character-count constant, used with text.count in truncatedScrollback. Here it's passed as byteLimit, which maps to max_bytes in the ghostty C API. For multi-byte UTF-8 content (e.g., CJK text), bytes-per-character can be 3–4×, so the effective limit drops to ~100K–133K characters instead of the intended 400K, silently truncating more scrollback than expected for non-ASCII terminal sessions.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0433c3c. Configure here.

Comment on lines +10051 to +10052
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check SessionPersistencePolicy definition for maxScrollbackCharactersPerTerminal

# Find the policy definition
ast-grep --pattern 'maxScrollbackCharactersPerTerminal'

# Check for any documentation or comments explaining the unit
rg -B3 -A3 'maxScrollbackCharactersPerTerminal'

Repository: manaflow-ai/cmux

Length of output: 6297

Fix scrollback truncation unit mismatch (characters vs bytes)

SessionPersistencePolicy.maxScrollbackCharactersPerTerminal is defined and enforced as a character-count limit (text.count) in Sources/SessionPersistence.swift, but Sources/TerminalController.swift passes it to readTerminalTextTail as byteLimit. This mixes units (characters vs bytes) and will truncate fewer characters than intended for UTF-8; align the policy/unit with the parameter (maxScrollbackBytesPerTerminal vs character-bounded limit) so persistence/snapshot truncation match.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@Sources/TerminalController.swift` around lines 10051 - 10052, The code passes
SessionPersistencePolicy.maxScrollbackCharactersPerTerminal (a character-count
limit) into readTerminalTextTail as byteLimit, causing character/byte unit
mismatch; change the argument to the matching byte-based policy
(SessionPersistencePolicy.maxScrollbackBytesPerTerminal) or convert the
character limit to bytes before calling readTerminalTextTail so the units align;
update the call site of readTerminalTextTail in TerminalController (the
byteLimit parameter) to use the byte-based constant (or a utf8.count conversion
from the character limit) so persistence/snapshot truncation behavior matches
SessionPersistencePolicy.

) ?? readTerminalTextTail(
terminalPanel: terminalPanel,
scope: GHOSTTY_TEXT_SCOPE_VIEWPORT,
format: GHOSTTY_TEXT_FORMAT_PLAIN,
lineLimit: lineLimit,
byteLimit: SessionPersistencePolicy.maxScrollbackCharactersPerTerminal
)
Copy link
Copy Markdown

@cursor cursor Bot May 29, 2026

Choose a reason for hiding this comment

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

Fallback loses scrollback by switching to viewport scope

Medium Severity

When includeScrollback is true, the first readTerminalTextTail call uses GHOSTTY_TEXT_SCOPE_SCREEN with VT format. If that returns nil, the fallback switches to GHOSTTY_TEXT_SCOPE_VIEWPORT (plain format), which only captures the visible portion and discards all scrollback history. If the failure was caused by the VT format rather than the screen scope, falling back to screen/plain would preserve scrollback. The current fallback undermines the purpose of includeScrollback: true for the session snapshot persistence path.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 3e71a96. Configure here.

}

let response = readTerminalTextBase64(
Expand Down
20 changes: 14 additions & 6 deletions Sources/Update/UpdateDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,22 @@ extension UpdateDriver: SPUUpdaterDelegate {
}

func updaterWillRelaunchApplication(_ updater: SPUUpdater) {
Task { @MainActor in
AppDelegate.shared?.persistSessionForUpdateRelaunch()
TerminalController.shared.stop()
NSApp.invalidateRestorableState()
for window in NSApp.windows {
window.invalidateRestorableState()
let prepareForRelaunch = {
MainActor.assumeIsolated {
AppDelegate.shared?.persistSessionForUpdateRelaunch()
TerminalController.shared.stop()
NSApp.invalidateRestorableState()
for window in NSApp.windows {
window.invalidateRestorableState()
}
}
}

if Thread.isMainThread {
prepareForRelaunch()
} else {
DispatchQueue.main.sync(execute: prepareForRelaunch)
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 29, 2026

Choose a reason for hiding this comment

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

P1 DispatchQueue.main.sync is a blocking primitive

DispatchQueue.main.sync is explicitly flagged by the cmux blocking-runtime rule. When Sparkle calls updaterWillRelaunchApplication from a background thread, this blocks that thread until all of the main-thread work — including persistSessionForUpdateRelaunch's synchronous file write — completes. If Sparkle holds any lock that the main thread is waiting to acquire (e.g., via another main.sync or an internal Sparkle XPC barrier), this produces a deadlock. The fix replaced a race with a hard block, but DispatchQueue.main.sync is still a timing-based synchronization primitive.

A conforming alternative is to check Sparkle's documented thread-delivery guarantee for updaterWillRelaunchApplication. If Sparkle guarantees main-thread delivery (or if the SPUStandardUpdaterController setup forces it), the Thread.isMainThread fast path already handles the call and the DispatchQueue.main.sync branch is dead code that can be removed entirely. If it is not guaranteed, a RunLoop.main.run(until:) spin or a dedicated Sparkle-owned notification fired on the main thread would be a safer coordination point.

Rule Used: Flag new blocking or timing-based synchronization ... (source)

}
}
}

Expand Down
Loading
Loading