diff --git a/.claude/rules/control-api.md b/.claude/rules/control-api.md index 894b0131..d489c27d 100644 --- a/.claude/rules/control-api.md +++ b/.claude/rules/control-api.md @@ -107,8 +107,14 @@ paths: may have quit. Its `stop()` returns early without unlinking, leaving the owner's socket intact. - One newline-delimited JSON request and response uses each connection, capped at 1 MiB. Unknown commands return structured errors. Mutations may return `result.id`; trees use `result.tree`. + A decode failure reports the `DecodingError`'s `debugDescription`, not `localizedDescription`, so the + error NAMES the rejected `cmd`. That is the only signal a caller gets that its agterm predates its + agtermctl, and the reason a new command needs no version handshake. - Human output shows IDs only for created session/workspace/window, retains them in JSON, uses `result.affected` for session counts, and reserves `result.count` for diagnostics/search. + A command that reuses `count` for something else must carry its own `result.text`, which the shared + formatter prefers over every count spelling; `restore.capture` does, or its pane total would print as + "N diagnostic(s)". - Targets accept active, case-insensitive UUID, or unique prefix. Batch targets resolve within the first target's store, deduplicate, and fail atomically. Preserve the first target in the legacy top-level field so old servers degrade to it rather than active. @@ -132,7 +138,8 @@ renumbering. Do not reintroduce a count anywhere. - `font.inc`, `font.dec`, `font.reset` - `window.new`, `.list`, `.select`, `.close`, `.rename`, `.delete`, `.resize`, `.move`, `.zoom`, `.fullscreen`, `.minimize` -- `keymap.reload`, `keymap.list`, `config.reload`, `theme.set`, `theme.list`, `restore.clear` +- `keymap.reload`, `keymap.list`, `config.reload`, `theme.set`, `theme.list`, `restore.capture`, + `restore.clear` `debug.appearance` is a private `Command` case, absent from the list above, used only by `AppearanceFlipUITests`. It accepts light/dark, sets `NSApp.appearance`, posts `.agtermSystemAppearanceChanged`, echoes the effective @@ -593,6 +600,12 @@ side, and reads `lastAppliedIsDark` when bare. Refuse it outside XCUITest; provi ## Restore commands +- `restore.capture` fills those same captured main/split slots on demand, from every open window's live + panes, saves immediately, and captures no hidden split. It is app-global. It reports the slots it + actually WROTE in `result.count` plus its own `result.text`; counting the slots afterwards instead would + read a stale split capture as a fresh one. It REFUSES while `restoreRunningCommand` is off, which is the + one place this API does not follow `session.restore`'s note-and-succeed: see [[settings]] for why the + two differ and for the exits the command exists for. - `restore.clear` clears captured main/split foreground commands across open windows and saves immediately. It never clears durable `initialCommand`; it is app-global. - `session.restore` pins per-session, per-pane next-launch behavior for discussion #264: diff --git a/.claude/rules/settings.md b/.claude/rules/settings.md index 03535469..4e2efddf 100644 --- a/.claude/rules/settings.md +++ b/.claude/rules/settings.md @@ -126,14 +126,23 @@ paths: libghostty diagnostics across all sources, clear all session zoom, post appearance change, and notify non-zero diagnostics. A config-directory change reloads both co-located files. Launch also reports cached diagnostics. -- **Restore running commands is opt-in, and both capture and replay are exit-scoped.** - `AppDelegate.captureForegroundCommands` runs at two points: `applicationWillTerminate` before - `saveAllOpen()`, and the LAST window's `willClose` before its surface teardown, which precedes +- **Restore running commands is opt-in. Replay is launch-scoped; capture runs at two exits and on demand.** + `AppDelegate.captureForegroundCommands` runs at three points: `applicationWillTerminate` before + `saveAllOpen()`, the LAST window's `willClose` before its surface teardown, which precedes `applicationWillTerminate` and is therefore the only point where a close-the-last-window exit's - commands are still readable. - Guarded by `openIDs() == [windowID]`, skipped under `isTerminating`. - A NON-last close captures nothing: a launch restore can't tell that window's file from one open at - exit, so its argv could replay via the never-windowless reopen fallback. + commands are still readable, and `restore.capture` on demand, which exists for the exit that reaches + neither: a force quit, a crash, a hard reset, a power loss. A system shutdown/restart/logout is NOT in + that set — since #447 it reaches `applicationWillTerminate` like any quit — so do not re-motivate the + command with an OS update. The on-demand arm changes nothing else: it fills the same + slots, persists through the same `saveAllOpen`, and replay stays launch-only and one-shot. + All three arms are gated on the setting, and only the on-demand one SAYS so: it refuses while the + setting is off rather than capturing what nothing would replay. Deliberately unlike a `session.restore` + pin, which succeeds with an explanatory note in the same state, because a pin outlives the toggle and + a capture only goes stale. + The `willClose` arm alone is guarded by `openIDs() == [windowID]` and skipped under `isTerminating`. + A NON-last close captures nothing AND clears both persisted slots plus the pending pair: a launch + restore can't tell that window's file from one open at exit, so its argv could replay via the + never-windowless reopen fallback, and on demand a capture can now have written argv there mid-run. Argv comes from `ghostty_surface_foreground_pid`, `sysctl(KERN_PROCARGS2)`, and host-free parsing. Capture no hidden split. Strip login `-` before shell recognition; a known shell with only flags is idle and omitted, while diff --git a/agterm/AppDelegate.swift b/agterm/AppDelegate.swift index d495dbbc..9a539e31 100644 --- a/agterm/AppDelegate.swift +++ b/agterm/AppDelegate.swift @@ -333,23 +333,36 @@ final class AppDelegate: NSObject, NSApplicationDelegate { /// Capture the given panes' foreground commands (main + split) into their `Session` fields for the /// snapshot save. `ForegroundProcess` returns nil for a pane at its shell prompt, so plain shells stay - /// plain. Two callers on different lifecycle edges: `applicationWillTerminate` passes every open - /// session, `WindowAccessor`'s `willClose` passes one closing window's — on a close-the-last-window - /// exit the quit-time capture runs after that teardown, too late to see any surface. Both sites and - /// the launch-only replay gate are stated in `.claude/rules/settings.md`. + /// plain. Three callers on different edges: `applicationWillTerminate` passes every open session, + /// `WindowAccessor`'s `willClose` passes one closing window's — on a close-the-last-window exit the + /// quit-time capture runs after that teardown, too late to see any surface — and `restore.capture` + /// passes every open session on demand. All three sites and the launch-only replay gate are stated in + /// `.claude/rules/settings.md`. + /// + /// Returns how many slots it actually WROTE a command into, which is what an on-demand caller reports. + /// Counting the slots afterwards instead would include a value this call never touched: the split slot + /// of a session whose split is hidden or gone still holds whatever an earlier capture put there. @MainActor - static func captureForegroundCommands(sessions: [Session]) { + @discardableResult + static func captureForegroundCommands(sessions: [Session]) -> Int { let shellBasename = ProcessInfo.processInfo.environment["SHELL"].map(CommandRestore.basename) + var captured = 0 for session in sessions { if let view = session.surface as? GhosttySurfaceView { session.foregroundCommand = ForegroundProcess.command(for: view, shellBasename: shellBasename) + if session.foregroundCommand != nil { captured += 1 } } // only a SHOWN split is recreated on restore, so gate on isSplit — a hidden split's captured - // command would sit stale until the next ⌘D fires it. + // command would sit stale until the next ⌘D fires it. Clearing it in the else keeps that stale + // value out of the snapshot now that a capture can run more than once per launch. if session.isSplit, let split = session.splitSurface as? GhosttySurfaceView { session.splitForegroundCommand = ForegroundProcess.command(for: split, shellBasename: shellBasename) + if session.splitForegroundCommand != nil { captured += 1 } + } else { + session.splitForegroundCommand = nil } } + return captured } func applicationShouldTerminateAfterLastWindowClosed(_: NSApplication) -> Bool { diff --git a/agterm/Control/ControlServer.swift b/agterm/Control/ControlServer.swift index 4ad235b1..7b8c502b 100644 --- a/agterm/Control/ControlServer.swift +++ b/agterm/Control/ControlServer.swift @@ -328,7 +328,12 @@ final class ControlServer { do { request = try JSONDecoder().decode(ControlRequest.self, from: line) } catch { - writeResponse(conn, ControlResponse(ok: false, error: "invalid request: \(error.localizedDescription)")) + // `debugDescription` over `localizedDescription`: a DecodingError's localized string is the generic + // "data couldn't be read", while the debug one names the offending value — for an unknown `cmd` that + // is the difference between a mystery and "Cannot initialize Command from invalid String value + // restore.capture", which tells a caller its agterm is older than its agtermctl. + let detail = (error as? DecodingError).map(String.init(describing:)) ?? error.localizedDescription + writeResponse(conn, ControlResponse(ok: false, error: "invalid request: \(detail)")) return } @@ -433,7 +438,7 @@ final class ControlServer { .windowNew, .windowList, .windowSelect, .windowClose, .windowRename, .windowDelete, .windowResize, .windowMove, .windowZoom, .windowFullscreen, .windowMinimize, - .restoreClear, .dashboard: + .restoreClear, .restoreCapture, .dashboard: return ControlResponse(ok: false, error: "control dispatcher did not handle \(request.cmd.rawValue)") case .debugAppearance: return setDebugAppearance(args: request.args) @@ -492,6 +497,36 @@ final class ControlServer { return ControlResponse(ok: true) } + /// Capture every open pane's live foreground command NOW, filling the same slots the quit-time capture + /// fills, then persist them. The point is the exit that never runs `applicationWillTerminate`: a crash, a + /// SIGKILL, a hard reset, or a restart that outruns the app's termination window. Run this from a + /// scheduled job or a keybind and such an exit restores like a ⌘Q. + /// + /// App-global like `clearRestoreCommands`, its inverse over the same slots: no `--window` selector, every + /// open window. Consumption stays one-shot and launch-only, so nothing here changes replay. + /// + /// Gated on the same setting as the two exit-time captures, and refuses rather than answering ok: with the + /// setting off a capture is write-only — the launch replay reads the setting too, so nothing would run — + /// and a scheduled caller needs a non-zero exit to notice. It also keeps "the setting is off" meaning + /// "argv never reaches the disk", which a silent capture would break for a user who opted out. + func captureRestoreCommands() -> ControlResponse { + guard settingsModel.settings.restoreRunningCommand == true else { + return ControlResponse(ok: false, + error: "\"Restore running commands on restart\" is off, nothing was captured") + } + let sessions = library.allOpenSessions() + let captured = AppDelegate.captureForegroundCommands(sessions: sessions) + // this command's whole claim is that the argv reached disk, so the ack waits on the write and not on + // the assignment: `saveAllOpen` swallows the result, `saveAllOpenChecked` reports it. + guard library.saveAllOpenChecked() else { + return ControlResponse(ok: false, error: "captured \(captured) pane\(captured == 1 ? "" : "s") " + + "but at least one window's save failed; the argv stays in memory and the next save writes it") + } + var result = ControlResult(count: captured) + result.text = "captured \(captured) pane\(captured == 1 ? "" : "s")" + return ControlResponse(ok: true, result: result) + } + /// Open or close the target window's dashboard overlay — the app side of the host-free `dashboard` /// command (the dispatcher validated the args and built `fontMode`, but does not cap the ids). Resolves /// `window ?? frontmost` to an OPEN window's store. `mru` takes up to `DashboardLayout.maxCells` of that diff --git a/agterm/Views/WindowAccessor.swift b/agterm/Views/WindowAccessor.swift index 3c96cc95..821abe19 100644 --- a/agterm/Views/WindowAccessor.swift +++ b/agterm/Views/WindowAccessor.swift @@ -153,6 +153,20 @@ struct WindowAccessor: NSViewRepresentable { if !library.isTerminating, library.openIDs() == [windowID], GhosttyApp.shared.restoreRunningCommand { AppDelegate.captureForegroundCommands(sessions: store.workspaces.flatMap(\.sessions)) + } else if !library.isTerminating { + // a NON-last close must leave no argv in this window's file: a launch restore cannot + // tell it from a file open at exit, so the never-windowless reopen fallback could + // replay it. Before `restore.capture` the live field was always nil mid-run and the + // invariant held by construction; now it has to be enforced here. + // Termination belongs to NEITHER arm: `applicationWillTerminate` captured over live + // surfaces and persisted already, then `closeWindow` no-ops under the flag so this + // runs with the store still loaded. Clearing here writes nulls over that capture and + // every ⌘Q comes back a plain shell, `restore.capture` callers included. + for session in store.workspaces.flatMap(\.sessions) { + session.foregroundCommand = nil + session.splitForegroundCommand = nil + session.clearPendingForegroundCommands() + } } store.save() } diff --git a/agtermCore/Sources/agtermCore/AppStore+Panes.swift b/agtermCore/Sources/agtermCore/AppStore+Panes.swift index fd6147b2..e03ca69e 100644 --- a/agtermCore/Sources/agtermCore/AppStore+Panes.swift +++ b/agtermCore/Sources/agtermCore/AppStore+Panes.swift @@ -87,10 +87,13 @@ extension AppStore { session.splitCwd = nil session.splitTitle = nil session.initialSplitCwd = nil - // the right pane is gone: drop both the persisted pin and any payload still armed for this launch, - // so a fresh split is a plain shell. + // the right pane is gone: drop the persisted pin, the captured command, and any payload still armed + // for this launch, so a fresh split is a plain shell. The capture slot matters since `restore.capture` + // can fill it mid-run: left behind, a re-split would arm the dead pane's command on the next launch. session.splitRestoreCommand = nil session.pendingSplitRestoreCommand = nil + session.splitForegroundCommand = nil + session.pendingSplitForegroundCommand = nil session.splitRatio = nil // tearing down the split clears its geometry too, so a fresh split opens even // the right pane is gone, so its overlay has nothing left to cover and nobody left to read its status. session.teardownPaneOverlay(.right) diff --git a/agtermCore/Sources/agtermCore/CommandRestore.swift b/agtermCore/Sources/agtermCore/CommandRestore.swift index bc1e747b..5e8e358a 100644 --- a/agtermCore/Sources/agtermCore/CommandRestore.swift +++ b/agtermCore/Sources/agtermCore/CommandRestore.swift @@ -116,9 +116,9 @@ public enum CommandRestore { /// - `wasRestored`: the session came from a restore (a FRESH command session always runs its command, a /// RESTORED one only when the opt-in is on). /// - `restoreEnabled`: the `restoreRunningCommand` opt-in. - /// - `hadForeground`: a foreground command was CAPTURED at the last quit. It PREEMPTS `initialCommand` - /// even when suppressed (denylisted/off → `foregroundInput` nil), yielding a plain shell rather than - /// the stale creation command — so gate on capture, not on the input surviving. + /// - `hadForeground`: a foreground command was CAPTURED, at the last quit or by `restore.capture`. It + /// PREEMPTS `initialCommand` even when suppressed (denylisted/off → `foregroundInput` nil), yielding a + /// plain shell rather than the stale creation command — so gate on capture, not on the input surviving. /// - `foregroundInput`: the rendered foreground command line to type, or nil (none / suppressed). /// - `initialCommand`: the session's persisted `--command`. /// - `restoreOverride`: the pane's pinned restore command (`session.restore`), tri-state — nil = no diff --git a/agtermCore/Sources/agtermCore/ControlDispatcher.swift b/agtermCore/Sources/agtermCore/ControlDispatcher.swift index 685d18ee..bd271d4f 100644 --- a/agtermCore/Sources/agtermCore/ControlDispatcher.swift +++ b/agtermCore/Sources/agtermCore/ControlDispatcher.swift @@ -119,6 +119,9 @@ public protocol ControlActions { /// Cancel a native picker. The host owns window resolution, registry lookup, and dismissal. func cancelPick(_ target: String, window: String?) -> ControlResponse func clearRestoreCommands() -> ControlResponse + /// Capture every open pane's foreground command now, the same read `applicationWillTerminate` does. The + /// host owns the `sysctl` read, the save, and the count it reports back. + func captureRestoreCommands() -> ControlResponse } public extension ControlActions { @@ -228,7 +231,7 @@ public struct ControlDispatcher { return dispatchWorkspaceCommand(request) case .quick, .fontInc, .fontDec, .fontReset, .keymapReload, .keymapList, .configReload, .notify, .themeSet, .themeList, .sidebar, .sidebarMode, .sidebarExpand, - .sidebarCollapse, .restoreClear: + .sidebarCollapse, .restoreClear, .restoreCapture: return dispatchAppCommand(request) case .quickType, .quickText: return await dispatchQuickCommand(request) @@ -737,6 +740,8 @@ public struct ControlDispatcher { return actions.collapseSidebar(window: request.args?.window) case .restoreClear: return actions.clearRestoreCommands() + case .restoreCapture: + return actions.captureRestoreCommands() default: preconditionFailure("unexpected app command: \(request.cmd.rawValue)") } diff --git a/agtermCore/Sources/agtermCore/ControlProtocol.swift b/agtermCore/Sources/agtermCore/ControlProtocol.swift index 4a5ecff4..d150a20d 100644 --- a/agtermCore/Sources/agtermCore/ControlProtocol.swift +++ b/agtermCore/Sources/agtermCore/ControlProtocol.swift @@ -80,6 +80,7 @@ public enum Command: String, Codable, Sendable { case pickResult = "pick.result" case pickCancel = "pick.cancel" case restoreClear = "restore.clear" + case restoreCapture = "restore.capture" /// UI-TEST-ONLY: forces the app-level appearance (`light`|`dark` via `args.name`) so an XCUITest can /// simulate a macOS light/dark flip; with NO name it READS the side the last config feed applied, so a /// test can assert the flip drove the reload. Refused outside an XCUITest launch, and EXEMPT from the diff --git a/agtermCore/Sources/agtermCore/Snapshot.swift b/agtermCore/Sources/agtermCore/Snapshot.swift index 020c65d1..761ec858 100644 --- a/agtermCore/Sources/agtermCore/Snapshot.swift +++ b/agtermCore/Sources/agtermCore/Snapshot.swift @@ -154,8 +154,9 @@ public struct SessionSnapshot: Codable, Equatable, Sendable { public var splitRatio: Double? /// Whether the session is in the flagged working-set; nil = not flagged. public var flagged: Bool? - /// The main pane's foreground command (full argv) at the last clean quit, re-run on restore when - /// `AppSettings.restoreRunningCommand` is on. nil at a shell prompt, or with the feature off. + /// The main pane's foreground command (full argv) as of the last clean quit or the last + /// `restore.capture`, re-run on restore when `AppSettings.restoreRunningCommand` is on. nil at a shell + /// prompt, or with the feature off, which gates every capture site. public var foregroundCommand: [String]? /// The split (right) pane's foreground command (full argv), the split analogue of `foregroundCommand`. public var splitForegroundCommand: [String]? diff --git a/agtermCore/Sources/agtermCore/WindowLibrary.swift b/agtermCore/Sources/agtermCore/WindowLibrary.swift index 0b17ce3f..406b2af5 100644 --- a/agtermCore/Sources/agtermCore/WindowLibrary.swift +++ b/agtermCore/Sources/agtermCore/WindowLibrary.swift @@ -531,7 +531,20 @@ public final class WindowLibrary { /// Flushes every open window's store — the quit-time flush persisting cwd changes made since the last /// structural mutation. public func saveAllOpen() { - for store in stores.values { store.save() } + saveAllOpenChecked() + } + + /// `saveAllOpen()` that REPORTS whether every window's write landed, for a caller whose acknowledgement + /// must not outrun the disk. `restore.capture` is the one today: an `ok` carrying a pane count claims the + /// argv is on disk, and a swallowed failure would promise a restore that cannot happen. Every store is + /// attempted before the verdict, so one unwritable window does not skip the others, and `saveAllOpen()` + /// is this with the result discarded so the two cannot drift — the same shape `AppStore.saveChecked` + /// keeps one layer down. + @discardableResult + public func saveAllOpenChecked() -> Bool { + var allLanded = true + for store in stores.values where !store.saveChecked() { allLanded = false } + return allLanded } /// Finalizes any grace-period session/workspace closes in open windows before a window/app teardown. diff --git a/agtermCore/Sources/agtermctlKit/MiscCommands.swift b/agtermCore/Sources/agtermctlKit/MiscCommands.swift index 508e6cb0..6e5417e1 100644 --- a/agtermCore/Sources/agtermctlKit/MiscCommands.swift +++ b/agtermCore/Sources/agtermctlKit/MiscCommands.swift @@ -57,9 +57,35 @@ struct Config: ParsableCommand { struct Restore: ParsableCommand { static let configuration = CommandConfiguration( abstract: "Restore-running-command commands.", - subcommands: [Clear.self] + subcommands: [Capture.self, Clear.self] ) + struct Capture: RequestCommand { + static let configuration = CommandConfiguration( + abstract: "Capture every pane's running command now, so a forced exit restores it.", + discussion: """ + agterm captures the running commands when it quits, so an exit that never gets there leaves \ + every pane restoring a plain shell: a force quit, a crash, a hard reset, a power loss. This \ + runs the same capture on demand, app-global across every open window, and prints how many panes \ + had a command to capture. + + Consumption is unchanged: the next launch arms each captured command once and clears it. Run it \ + from a scheduled job every so often, or bind it, and an exit nobody was there for restores \ + like a deliberate quit. + + A capture is only as fresh as its last run, so a pager or a build that has finished since still \ + re-runs after a crash. "restore clear" drops every captured command, and restore-denylist.conf \ + in the config directory keeps a named program from re-running at all. + + Needs "Restore running commands on restart", which is what replays the capture: with the \ + setting off this captures nothing and fails, saying so. + """) + // app-global, like `restore clear`: every open window, so no `--window` selector. + @OptionGroup var options: BasicOptions + + func makeRequest() throws -> ControlRequest { ControlRequest(cmd: .restoreCapture) } + } + struct Clear: RequestCommand { static let configuration = CommandConfiguration( abstract: "Clear every session's saved foreground command so the next restart restores plain shells.", diff --git a/agtermCore/Tests/agtermCoreTests/AppStorePaneTests.swift b/agtermCore/Tests/agtermCoreTests/AppStorePaneTests.swift index 72dea966..a688e045 100644 --- a/agtermCore/Tests/agtermCoreTests/AppStorePaneTests.swift +++ b/agtermCore/Tests/agtermCoreTests/AppStorePaneTests.swift @@ -103,6 +103,22 @@ struct AppStorePaneTests { #expect(session.splitAxis == .topBottom) } + /// `restore.capture` can fill the split capture slot mid-run, so closing the split has to drop it: left + /// behind, a later re-split makes `isSplit` true again and the next launch arms the dead pane's command. + @Test func closeSplitDropsTheCapturedSplitCommand() { + let store = makeStore() + let ws = store.addWorkspace(name: "work") + let session = store.addSession(toWorkspace: ws.id, cwd: "/a")! + store.toggleSplit(session.id) + session.splitForegroundCommand = ["htop"] + session.pendingSplitForegroundCommand = ["htop"] + + store.closeSplit(session.id) + + #expect(session.splitForegroundCommand == nil) + #expect(session.pendingSplitForegroundCommand == nil) + } + @Test func closeSplitHidesAndTearsDownSurface() { let store = makeStore() let ws = store.addWorkspace(name: "work") diff --git a/agtermCore/Tests/agtermCoreTests/ControlDispatcherTests.swift b/agtermCore/Tests/agtermCoreTests/ControlDispatcherTests.swift index 66b3b1e4..6c39c299 100644 --- a/agtermCore/Tests/agtermCoreTests/ControlDispatcherTests.swift +++ b/agtermCore/Tests/agtermCoreTests/ControlDispatcherTests.swift @@ -1480,6 +1480,17 @@ struct ControlDispatcherTests { #expect(actions.calls == [.restoreClear]) } + @Test func restoreCaptureRoutesThroughActions() async { + let actions = MockControlActions() + let dispatcher = ControlDispatcher(actions: actions) + actions.nextRestoreCaptureResponse = ControlResponse(ok: true, result: ControlResult(count: 3)) + + let response = await dispatcher.dispatch(ControlRequest(cmd: .restoreCapture)) + + #expect(response == ControlResponse(ok: true, result: ControlResult(count: 3))) + #expect(actions.calls == [.restoreCapture]) + } + @Test func quickRoutesRawModeAndKeepsActionResponse() async { let actions = MockControlActions() let dispatcher = ControlDispatcher(actions: actions) diff --git a/agtermCore/Tests/agtermCoreTests/ControlProtocolTests.swift b/agtermCore/Tests/agtermCoreTests/ControlProtocolTests.swift index 117eeb14..20131f0b 100644 --- a/agtermCore/Tests/agtermCoreTests/ControlProtocolTests.swift +++ b/agtermCore/Tests/agtermCoreTests/ControlProtocolTests.swift @@ -1302,6 +1302,11 @@ struct ControlProtocolTests { #expect(node?.background?.colorHex == "#112233") } + @Test func restoreCaptureRoundTrips() throws { + let request = ControlRequest(cmd: .restoreCapture) + #expect(try roundTrip(request) == request) + } + @Test func restoreClearRoundTrips() throws { let request = ControlRequest(cmd: .restoreClear) #expect(try roundTrip(request) == request) diff --git a/agtermCore/Tests/agtermCoreTests/MockControlActions.swift b/agtermCore/Tests/agtermCoreTests/MockControlActions.swift index e29095be..bd64f957 100644 --- a/agtermCore/Tests/agtermCoreTests/MockControlActions.swift +++ b/agtermCore/Tests/agtermCoreTests/MockControlActions.swift @@ -87,6 +87,7 @@ final class MockControlActions: ControlActions { case pickResult(target: String, window: String?) case pickCancel(target: String, window: String?) case restoreClear + case restoreCapture } var calls: [Call] = [] @@ -153,6 +154,7 @@ final class MockControlActions: ControlActions { var nextPickResultResponse = ControlResponse(ok: true) var nextPickCancelResponse = ControlResponse(ok: true) var nextRestoreClearResponse = ControlResponse(ok: true) + var nextRestoreCaptureResponse = ControlResponse(ok: true) var nextSessionRestoreResponse = ControlResponse(ok: true) func controlTree(window: String?) -> ControlResponse { @@ -567,4 +569,9 @@ final class MockControlActions: ControlActions { calls.append(.restoreClear) return nextRestoreClearResponse } + + func captureRestoreCommands() -> ControlResponse { + calls.append(.restoreCapture) + return nextRestoreCaptureResponse + } } diff --git a/agtermCore/Tests/agtermCoreTests/WindowLibraryTests.swift b/agtermCore/Tests/agtermCoreTests/WindowLibraryTests.swift index c33e3346..83e89682 100644 --- a/agtermCore/Tests/agtermCoreTests/WindowLibraryTests.swift +++ b/agtermCore/Tests/agtermCoreTests/WindowLibraryTests.swift @@ -1459,4 +1459,20 @@ final class WindowLibraryTests { #expect(reloadedSession.initialCwd == "/changed") _ = ws } + /// `restore.capture` answers `ok` with a pane count, which is a claim that the argv is on disk, so the + /// library has to REPORT a failed flush rather than swallow it. An unwritable windows directory is the + /// same lever the stale-file test above uses. + @Test func saveAllOpenCheckedReportsAFailedWrite() throws { + let id = UUID() + try writeWindowFile(id, Snapshot(workspaces: [WorkspaceSnapshot(id: UUID(), name: "work", sessions: [])])) + try writeIndex(WindowsIndex(frontmost: id, windows: [WindowEntry(id: id, name: "work", isOpen: true)])) + let library = WindowLibrary(directory: directory) + #expect(library.saveAllOpenChecked()) + + let windowsDir = directory.appendingPathComponent("windows") + try FileManager.default.setAttributes([.posixPermissions: 0o500], ofItemAtPath: windowsDir.path) + defer { try? FileManager.default.setAttributes([.posixPermissions: 0o700], ofItemAtPath: windowsDir.path) } + #expect(!library.saveAllOpenChecked()) + } + } diff --git a/agtermCore/Tests/agtermctlKitTests/CommandsTests.swift b/agtermCore/Tests/agtermctlKitTests/CommandsTests.swift index 7d581d37..17eb60f0 100644 --- a/agtermCore/Tests/agtermctlKitTests/CommandsTests.swift +++ b/agtermCore/Tests/agtermctlKitTests/CommandsTests.swift @@ -665,6 +665,10 @@ struct CommandsTests { #expect(validationMessage(["session", "status", "blocked", "--pane", "other"]) == "--pane must be left, right, or scratch") } + @Test func restoreCaptureIsAppGlobalAndCarriesNoArgs() throws { + #expect(try request(["restore", "capture"]) == ControlRequest(cmd: .restoreCapture)) + } + @Test func sessionRestorePinsCommand() throws { let expected = ControlRequest(cmd: .sessionRestore, target: "s1", args: ControlArgs(mode: "set", command: "claude --resume abc")) diff --git a/agtermCore/Tests/agtermctlKitTests/SocketClientTests.swift b/agtermCore/Tests/agtermctlKitTests/SocketClientTests.swift index b8d7f23f..55f12333 100644 --- a/agtermCore/Tests/agtermctlKitTests/SocketClientTests.swift +++ b/agtermCore/Tests/agtermctlKitTests/SocketClientTests.swift @@ -771,6 +771,15 @@ struct SocketClientTests { #expect(SocketClient.formatResponse(response, json: false) == "ok") } + /// `restore.capture` carries both `count` and its own `text`; the text must win, or the shared `count` + /// branch below would render a pane count as "3 diagnostic(s)". + @Test func formatResponsePrefersTextOverCount() { + var result = ControlResult(count: 3) + result.text = "captured 3 panes" + let response = ControlResponse(ok: true, result: result) + #expect(SocketClient.formatResponse(response, json: false) == "captured 3 panes") + } + @Test func formatResponseNonZeroCountPluralizes() { let response = ControlResponse(ok: true, result: ControlResult(count: 3)) #expect(SocketClient.formatResponse(response, json: false) == "3 diagnostic(s)") diff --git a/agtermTests/ControlServerRestoreCaptureTests.swift b/agtermTests/ControlServerRestoreCaptureTests.swift new file mode 100644 index 00000000..6593669b --- /dev/null +++ b/agtermTests/ControlServerRestoreCaptureTests.swift @@ -0,0 +1,83 @@ +import AppKit +import XCTest +@testable import agterm +import agtermCore + +/// Hosted coverage for `restore.capture`: the gate on the master setting and the response shape. The argv +/// read itself needs a live `GhosttySurfaceView`, so a hosted session captures nothing and reports zero — +/// which is exactly what makes the gate and the reported text testable without driving the UI. +@MainActor +final class ControlServerRestoreCaptureTests: XCTestCase { + private var stateDir: URL! + private var library: WindowLibrary! + private var settingsModel: SettingsModel! + private var server: ControlServer! + + override func setUp() async throws { + try await super.setUp() + await MainActor.run { + stateDir = FileManager.default.temporaryDirectory + .appendingPathComponent("agterm-restore-capture-tests-\(UUID().uuidString)", isDirectory: true) + library = WindowLibrary(directory: stateDir) + settingsModel = SettingsModel(library: library, settingsStore: SettingsStore(directory: stateDir)) + server = ControlServer( + library: library, + actions: AppActions(library: library), + settingsModel: settingsModel, + socketPath: stateDir.appendingPathComponent("control.sock").path + ) + } + } + + override func tearDown() async throws { + await MainActor.run { + server = nil + settingsModel = nil + library = nil + try? FileManager.default.removeItem(at: stateDir) + } + try await super.tearDown() + } + + func testRefusesWithTheSettingOff() { + settingsModel.setRestoreRunningCommand(nil) + + let response = server.captureRestoreCommands() + + XCTAssertFalse(response.ok, "a capture that can never replay must refuse, not answer ok") + XCTAssertEqual(response.error, + "\"Restore running commands on restart\" is off, nothing was captured") + XCTAssertNil(response.result) + } + + func testTheRefusalLeavesAnEarlierCaptureAlone() { + settingsModel.setRestoreRunningCommand(nil) + for session in library.allOpenSessions() { + session.foregroundCommand = ["sleep", "12345"] + session.splitForegroundCommand = ["sleep", "12345"] + } + + _ = server.captureRestoreCommands() + + // the SPLIT slot is what pins the gate. A hosted session has no `GhosttySurfaceView`, so the main + // slot is never assigned and survives with the guard deleted too; the split slot is nil'd + // unconditionally by the capture's `else` for a session with no shown split, so it goes red the + // moment the guard stops returning early. + XCTAssertTrue(library.allOpenSessions().allSatisfy { $0.splitForegroundCommand == ["sleep", "12345"] }, + "with the setting off the command must not clear the split slot") + XCTAssertTrue(library.allOpenSessions().allSatisfy { $0.foregroundCommand == ["sleep", "12345"] }, + "with the setting off the command must not touch the slots at all") + } + + func testReportsPaneCountInItsOwnText() { + settingsModel.setRestoreRunningCommand(true) + + let response = server.captureRestoreCommands() + + XCTAssertTrue(response.ok) + // no realized surfaces in a hosted store, so nothing is captured — the point here is that the command + // renders its own sentence instead of leaning on `count`, whose CLI branch prints "N diagnostic(s)". + XCTAssertEqual(response.result?.count, 0) + XCTAssertEqual(response.result?.text, "captured 0 panes") + } +} diff --git a/agtermUITests/RestoreCommandUITests.swift b/agtermUITests/RestoreCommandUITests.swift index 3da1bf47..b9f98cdd 100644 --- a/agtermUITests/RestoreCommandUITests.swift +++ b/agtermUITests/RestoreCommandUITests.swift @@ -360,6 +360,13 @@ final class RestoreCommandUITests: XCTestCase { runTeeMarker() let windowID = try firstWindowID() + // capture on demand FIRST, or this window's slots are nil going into the close and the clearing + // branch is only ever seen nil to nil — which passes just as happily with the branch deleted. + XCTAssertEqual(try sendCommand(#"{"cmd":"restore.capture"}"#)["ok"] as? Bool, true, + "restore.capture should fill this window's slots before the close") + XCTAssertFalse(capturedForegroundCommands().isEmpty, + "the on-demand capture must reach this window's file before the close is tested") + XCTAssertEqual(try sendCommand(#"{"cmd":"window.new"}"#)["ok"] as? Bool, true, "a second window keeps the app alive while the first is closed") XCTAssertEqual(try sendCommand(#"{"cmd":"window.close","target":"\#(windowID)"}"#)["ok"] as? Bool, true, @@ -448,12 +455,94 @@ final class RestoreCommandUITests: XCTestCase { "the override must be persisted eagerly enough to survive a force quit") } + // `restore.capture` fills the capture slot mid-run, so the SIGKILL exit that skips both the quit-time + // capture and the flush still restores. `terminate()` is that exit. + func testRestoreCaptureSurvivesForceQuit() throws { + seedRestoreFlag(true) + app.launchForUITest() + try startTeeMarkerOverSocket() + + let response = try sendCommand(#"{"cmd":"restore.capture"}"#) + XCTAssertEqual(response["ok"] as? Bool, true, "restore.capture should succeed: \(response)") + let result = response["result"] as? [String: Any] + XCTAssertEqual(result?["count"] as? Int, 1, "one pane was running `tee`: \(response)") + XCTAssertEqual(result?["text"] as? String, "captured 1 pane", "the command renders its own sentence") + XCTAssertFalse(capturedForegroundCommands().isEmpty, "the capture must be persisted immediately") + + try FileManager.default.removeItem(at: marker) + app.terminate() + _ = app.wait(for: .notRunning, timeout: 10) + app.launchForUITest() + + XCTAssertTrue(poll { FileManager.default.fileExists(atPath: self.marker.path) }, + "an on-demand capture must survive a force quit and re-run on the next launch") + } + + // The quit path has its own way to lose a capture: `applicationWillTerminate` captures and saves, then + // the last window's `willClose` runs with the store still loaded, and a clearing branch that fires under + // termination writes nulls over it. That breaks every clean quit, not only an on-demand capture, and + // SIGKILL cannot see it because `willClose` never runs. + func testRestoreCaptureSurvivesCleanQuit() throws { + seedRestoreFlag(true) + app.launchForUITest() + try startTeeMarkerOverSocket() + + XCTAssertEqual(try sendCommand(#"{"cmd":"restore.capture"}"#)["ok"] as? Bool, true, + "restore.capture should succeed before the quit") + try FileManager.default.removeItem(at: marker) + gracefulQuit() + + XCTAssertFalse(capturedForegroundCommands().isEmpty, + "a clean quit must leave the captured argv on disk, not clear it on the way out") + app.launchForUITest() + XCTAssertTrue(poll { FileManager.default.fileExists(atPath: self.marker.path) }, + "the captured command must re-run on the launch after a clean quit") + } + + // With the setting off a capture could never replay, so the command refuses instead of quietly writing + // argv to disk for a user who opted the feature out. + func testRestoreCaptureRefusesWhenTheSettingIsOff() throws { + seedRestoreFlag(false) + app.launchForUITest() + try startTeeMarkerOverSocket() + + let response = try sendCommand(#"{"cmd":"restore.capture"}"#) + XCTAssertEqual(response["ok"] as? Bool, false, "the command must refuse with the setting off: \(response)") + XCTAssertTrue(capturedForegroundCommands().isEmpty, "a refused capture must write no argv to disk") + } + // MARK: - Helpers /// The shell line a pinned override runs: `touch `, which EXITS. That leaves the pane at its /// prompt, so the next quit captures nothing and only the sticky override can recreate the file. private func touchLine(_ file: URL) -> String { "/usr/bin/touch \(file.path)" } + /// Start the blocking `tee` marker through `session.type` instead of `runTeeMarker`'s `XCUIElement` + /// typing. Same keystrokes into the same surface, minus the HID synthesis: a capture test only needs a + /// live foreground process, and event synthesis needs the machine running the tests to be allowed to + /// post events, which a headless or unattended run is not. `session.new --command` is no substitute — + /// such a pane's process group is led by unreadable setuid-root `login`, so the capture, which stays + /// leader-only, reads nothing. + private func startTeeMarkerOverSocket() throws { + XCTAssertTrue(app.staticTexts["session-row"].firstMatch.waitForExistence(timeout: 30), "control server up") + // retried like `runTeeMarker`: `session.type` lands in the surface as soon as it mounts, which can be + // before the login shell has printed its first prompt, and a line typed into that gap is swallowed. + // ^U opens each retry, so a partially-read line cannot concatenate with the next one. + for attempt in 0..<3 { + if attempt > 0 { _ = try? typeOverSocket("\u{15}") } + let response = try typeOverSocket("tee \(marker.path)\n") + XCTAssertEqual(response["ok"] as? Bool, true, "session.type should succeed: \(response)") + if poll({ FileManager.default.fileExists(atPath: self.marker.path) }, timeout: 6) { return } + } + XCTFail("the foreground `tee` should create its marker file on start") + } + + @discardableResult + private func typeOverSocket(_ text: String) throws -> [String: Any] { + let obj: [String: Any] = ["cmd": "session.type", "args": ["text": text]] + return try sendCommand(String(decoding: try JSONSerialization.data(withJSONObject: obj), as: UTF8.self)) + } + /// Pin/clear the active session's restore-command override over the control socket, asserting the /// request succeeded. `mode` is `set` | `none` | `clear`; `pane` defaults to the main pane. @discardableResult diff --git a/plugins/agterm/skills/agterm/SKILL.md b/plugins/agterm/skills/agterm/SKILL.md index 873a82d1..2e43957f 100644 --- a/plugins/agterm/skills/agterm/SKILL.md +++ b/plugins/agterm/skills/agterm/SKILL.md @@ -481,8 +481,11 @@ terminal theme app-wide, per slot: a NAME sets the light/single theme (a dark th appearance automatically; `theme set --dark none` stops tracking. The app default is the bundled **agterm** theme; omit the name for ghostty's built-in default ("default ghostty"); an unknown name errors. -**restore** — `restore clear` — clear every session's saved foreground command (the -restore-running-command capture) so the next restart restores plain shells. +**restore** — `restore capture` — capture every pane's running command now, into the slot the quit-time +capture fills, so an exit that never reaches a clean quit (a force quit, a crash, a hard reset) still +restores; prints how many panes were captured, and refuses while **Restore running commands on restart** +is off, since nothing would replay the capture · `restore clear` — clear every session's saved foreground +command (the restore-running-command capture) so the next restart restores plain shells. ## Displaying an image inline diff --git a/plugins/agterm/skills/agterm/examples.md b/plugins/agterm/skills/agterm/examples.md index c26b5df0..83ebe209 100644 --- a/plugins/agterm/skills/agterm/examples.md +++ b/plugins/agterm/skills/agterm/examples.md @@ -15,7 +15,7 @@ agtermctl window list --json # windows, with open/active flags agtermctl tree --json | jq -r '.result.tree.workspaces[].sessions[] | "\(.name): \(.foreground // "shell")"' ``` -## Reset the restore-on-restart commands +## Capture or reset the restore-on-restart commands The opt-in "Restore running commands on restart" setting saves each pane's foreground command at quit. Clear those saved commands so the next launch restores plain shells: @@ -24,6 +24,13 @@ Clear those saved commands so the next launch restores plain shells: agtermctl restore clear ``` +Or capture them now, so an exit that never reaches a clean quit (a force quit, a crash, a hard reset) +restores like one. It needs the setting on, and answers with the number of panes it captured: + +```bash +agtermctl restore capture +``` + ## Pin what a pane restores (per-session override) `session restore` pins a shell line that a pane re-runs on the NEXT launch, overriding the captured diff --git a/plugins/agterm/skills/agterm/reference.md b/plugins/agterm/skills/agterm/reference.md index fc9fc4fa..d079d0ae 100644 --- a/plugins/agterm/skills/agterm/reference.md +++ b/plugins/agterm/skills/agterm/reference.md @@ -1169,6 +1169,19 @@ kept); over the socket `theme set` is the commit, with no preview. ## restore +`agtermctl restore capture` — capture every open pane's live foreground command NOW, into the same slot the +quit-time capture fills, and persist it. For the exit that never reaches `applicationWillTerminate`: a force +quit, a crash, a hard reset, a power loss, all of which today leave every pane restoring a plain shell. (A +shutdown, restart or logout is not one of them: that path quits the app normally and captures by itself.) +Run it from a scheduled job or bind it, and an exit nobody was there for restores like a deliberate +quit. Consumption is unchanged — the next launch arms each captured command once and clears it. App-global +(no `--window`), prints `count`, the number of panes it captured a command for (main and shown split count +one each). With the **Restore running commands on restart** setting off it captures nothing and returns an +error saying so: nothing would replay the capture, and it would go stale where a `session.restore` pin +would keep waiting for the setting. A capture is also only as fresh as its last run: a pager or a build that +has finished since still re-runs after a crash, which `restore clear` drops wholesale and +`restore-denylist.conf` prevents per program. + `agtermctl restore clear` — clear every session's saved CAPTURED foreground command and persist, so the next restart restores plain shells for those panes (not whatever each pane was running). It does NOT clear a `session.new --command` session's own command (`initialCommand`, the durable creation identity), which diff --git a/site/commands.html b/site/commands.html index 6c88aa81..aeaf9160 100644 --- a/site/commands.html +++ b/site/commands.html @@ -2605,6 +2605,25 @@ restore +
+
+ agtermctl restore capture +
+
restore.capture
+

+ Capture every pane's running command now, into the same slot the quit-time capture fills, and persist it. + App-global; prints + count, the number of panes captured. +

+

+ For the exit that never reaches a clean quit: a force quit, a crash, a hard reset, a power loss. (A shutdown, + restart or logout quits the app normally and captures by itself.) Run it from a scheduled job or bind it, and an + exit nobody was there for restores like a deliberate quit. Consumption is unchanged, the next launch arms each + captured command once and clears it. With Restore running commands on restart off it captures nothing and + returns an error saying so, since nothing would replay the capture. +

+
+
agtermctl restore clear diff --git a/site/docs.html b/site/docs.html index 88ceb830..825e48c4 100644 --- a/site/docs.html +++ b/site/docs.html @@ -2745,7 +2745,9 @@ Only a single-process command restores faithfully; a command whose name or arguments carry a control character starts a plain shell instead, since the restored line is typed and the line editor would read that byte as an editing key rather than text, and so does one carrying bytes that were not valid UTF-8, which are captured - lossily and would replay a different argument; a force-quit or crash captures nothing, and a capture replays + lossily and would replay a different argument; a force-quit or crash captures nothing unless + agtermctl restore capture ran + beforehand, and a capture replays exactly once, since the launch that arms it clears it from the state file; and the multiplexers in restore-denylist.conf (seeded with tmux/screen/zellij) start fresh. That file is yours to edit: agterm writes a commented starter at