Skip to content

refactor: drop the hidden-window snapshot compat path (merge on/after 10-30) #457

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

Draft
arzafran wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

refactor: drop the hidden-window snapshot compat path (merge on/after 10-30) #457

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 1 addition & 54 deletions Sources/AppDelegate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, @preconcurrency UNUser
private var startupSessionSnapshot: AppSessionSnapshot?
private var didPrepareStartupSessionSnapshot = false
private(set) var didAttemptStartupSessionRestore = false
/// Session ids whose shells `endShellsOfHiddenWindows` ended (or tried to) this launch.
/// `reconcileOrphanedEscrowedSessions` skips them: their WAL directory removal is
/// asynchronous, and a session the holder refused to hand over must not be revived either.
private var startupEndedHiddenSessionIds = Set<String>()
var isApplyingStartupSessionRestore = false
let sessionPersistenceQueue = DispatchQueue(
label: "com.darkroom.programa.app.sessionPersistence",
Expand Down Expand Up @@ -774,13 +770,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, @preconcurrency UNUser
// sessions silently. Say what happened.
notifyUncleanShutdownRecovery()
}
// Windows flagged closed in the snapshot are not shown again: their escrowed shells
// are ended here, before the orphan reconciler below could revive them into a
// recovery window.
let windowsToRestore = startupSnapshot.map { SessionPersistenceStore.windowsToRestore(from: $0) } ?? []
if let startupSnapshot {
endShellsOfHiddenWindows(SessionPersistenceStore.hiddenWindows(from: startupSnapshot))
}
let primaryWindowSnapshot = windowsToRestore.first
if let primaryWindowSnapshot {
isApplyingStartupSessionRestore = true
Expand Down Expand Up @@ -862,49 +852,6 @@ final class AppDelegate: NSObject, NSApplicationDelegate, @preconcurrency UNUser
reconcileOrphanedEscrowedSessions()
}

/// Ends the shells of windows the user had closed before the previous run ended.
/// Snapshots written by older builds flag closed windows `isHidden` and their PTYs were
/// escrowed at quit. A closed window must not come back on relaunch, and without this the
/// orphan reconciler would revive those shells into a recovery window instead. Each session is retrieved from the
/// holder exactly like a reattach, then hung up (SIGHUP to the child, master fd closed)
/// and its WAL directory removed. Runs synchronously on the main actor at launch, before
/// any window restore, with the same per-session retrieve timeout a reattach pays.
private func endShellsOfHiddenWindows(_ hiddenWindows: [SessionWindowSnapshot]) {
guard !hiddenWindows.isEmpty, !SessionMachineryGate.isUnitTesting else { return }
var ended = 0
var sessionIds: [String] = []
for window in hiddenWindows {
for workspace in window.tabManager.workspaces {
for panel in workspace.panels where panel.type == .terminal {
sessionIds.append(panel.id.uuidString)
}
}
}
for sessionId in sessionIds {
startupEndedHiddenSessionIds.insert(sessionId)
if let meta = SessionWALStore.shared.readMeta(sessionId: sessionId),
meta.escrowed == true,
let socketPath = meta.escrowSocketPath,
let tokenHex = meta.escrowToken,
let masterFD = SessionEscrowClient.retrieve(
sessionId: sessionId,
tokenHex: tokenHex,
socketPath: socketPath
) {
if let childPID = meta.childPID, childPID > 0 {
kill(childPID, SIGHUP)
}
close(masterFD)
ended += 1
}
SessionWALStore.shared.discardOrphanedSession(sessionId: sessionId, force: true)
}
dilog(
"session.restore",
"hiddenWindows=\(hiddenWindows.count) sessions=\(sessionIds.count) ended=\(ended)"
)
}

/// Issue #307 orphan-reconciliation fix: the coarse-snapshot restore
/// that just completed above is keyed entirely by the panel UUIDs
/// already present in `session-<bundleId>.json` -- if that snapshot was
Expand Down Expand Up @@ -938,7 +885,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate, @preconcurrency UNUser
dilog("escrow.reconcile", "skipped=automated_tests")
return
}
var known = startupEndedHiddenSessionIds
var known = Set<String>()
for context in mainWindowContexts.values {
for workspace in context.tabManager.tabs {
for panelId in workspace.panels.keys {
Expand Down
15 changes: 1 addition & 14 deletions Sources/SessionPersistence.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -433,12 +433,6 @@ struct SessionWindowSnapshot: Codable, Sendable {
var display: SessionDisplaySnapshot?
var tabManager: SessionTabManagerSnapshot
var sidebar: SessionSidebarSnapshot
/// `true` in snapshots written by older builds for a window the user had closed. Restore
/// never shows such a window: it ends the window's escrowed shells instead. Never written
/// by this build; `nil` means visible.
var isHidden: Bool?

var isHiddenWindow: Bool { isHidden == true }
}

struct AppSessionSnapshot: Codable, Sendable {
Expand Down Expand Up @@ -750,14 +744,7 @@ enum SessionPersistenceStore {
from snapshot: AppSessionSnapshot,
limit: Int = SessionPersistencePolicy.maxWindowsPerSnapshot
) -> [SessionWindowSnapshot] {
// Windows flagged closed by older builds are never shown again; their shells are
// ended instead (`hiddenWindows(from:)`).
Array(snapshot.windows.filter { !$0.isHiddenWindow }.prefix(max(0, limit)))
}

/// Windows an older build flagged as closed. Restore skips them and ends their shells.
static func hiddenWindows(from snapshot: AppSessionSnapshot) -> [SessionWindowSnapshot] {
snapshot.windows.filter(\.isHiddenWindow)
Array(snapshot.windows.prefix(max(0, limit)))
}

/// Archives the current snapshot file into `session-history/` before anything else can
Expand Down
28 changes: 0 additions & 28 deletions programaTests/SessionPersistenceTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,34 +465,6 @@ final class SessionPersistenceTests: XCTestCase {
)
}

func testWindowsToRestoreSkipsHiddenWindowsAndOlderSnapshotsStayVisible() throws {
let base = makeSnapshot(version: SessionSnapshotSchema.currentVersion)
var hidden = try XCTUnwrap(base.windows.first)
hidden.isHidden = true
let mixed = AppSessionSnapshot(
version: base.version,
createdAt: base.createdAt,
windows: base.windows + [hidden, hidden],
cleanShutdown: true
)

XCTAssertEqual(
SessionPersistenceStore.windowsToRestore(from: mixed).count, 1,
"A window the user closed before quitting must not be restored"
)
XCTAssertEqual(SessionPersistenceStore.hiddenWindows(from: mixed).count, 2)

// Snapshots written before `isHidden` existed carry no key at all.
let encoded = try JSONEncoder().encode(base)
XCTAssertFalse(String(decoding: encoded, as: UTF8.self).contains("isHidden"))
let decoded = try XCTUnwrap(SessionPersistenceStore.decodeSnapshot(from: encoded))
XCTAssertEqual(SessionPersistenceStore.windowsToRestore(from: decoded).count, 1)
XCTAssertTrue(SessionPersistenceStore.hiddenWindows(from: decoded).isEmpty)

let roundTripped = try XCTUnwrap(SessionPersistenceStore.decodeSnapshot(from: JSONEncoder().encode(mixed)))
XCTAssertEqual(SessionPersistenceStore.hiddenWindows(from: roundTripped).count, 2)
}

func testDecodeSnapshotDetectsVersionMismatchWithoutRequiringAppKit() throws {
let mismatched = makeSnapshot(version: SessionSnapshotSchema.currentVersion + 1)
let data = try JSONEncoder().encode(mismatched)
Expand Down
Loading