diff --git a/TODOS.md b/TODOS.md index 8fa16cc39..ee3216ea1 100644 --- a/TODOS.md +++ b/TODOS.md @@ -5,6 +5,27 @@ > **Recently done** at the bottom — including the traps found while doing it, which are the parts worth > reading before touching the same code. Full write-ups for finished items live in git history. +## P0 — before the next release + +### `CredentialRelayTests.testIdleStrangersDoNotBlockAKnownWorkroom` fails under full-suite load (macapp) — #309 follow-up, issue #322 + +**What:** The test failed once in a full `make app-test` run with `failed - connect 54` +(`ECONNRESET`): one of its `maxConcurrent + 4` idle "stranger" connects to the relay's loopback +listener, or the final real request's connect, was reset rather than accepted. Seen on +`feat/254-remote-pane-parity` (2026-10-04, xcresult `Test-WorkroomApp-2026.10.04_08-52-46`). The +branch doesn't touch `CredentialRelay`. + +**Why:** It passed 5 times out of 5 run alone, and in two other full runs the same day, so it looks +load-dependent. That is either a flaky test or a real limit: a listen backlog that the parallel +test workers' load overflows would also turn real git credential requests away while a box is +busy. + +**How to start:** Find which connect fails (the strangers' or the real one) and the listener's +`listen` backlog in `CredentialRelay`. Then reproduce under load, for example with the suite's +parallel workers, or with many more strangers than `maxConcurrent`. + +**Priority:** P0 (chosen at ship time on #254) + ## P1 — before GA ## P2 — perf, correctness, and the next VCS phase diff --git a/docs/designs/remote-workrooms.md b/docs/designs/remote-workrooms.md index 1641cb10f..8deb1a955 100644 --- a/docs/designs/remote-workrooms.md +++ b/docs/designs/remote-workrooms.md @@ -1707,11 +1707,67 @@ these are the subsystems that actually gate "a remote workroom is a real workroo hostname. From a remote agent that is a *remote* path labelled local, and `TerminalLinkOpener.absolutePath(for:cwd:)` (`TerminalLinkOpener.swift:27-31,202`) will then resolve a cmd-clicked file against it **on the Mac**. Week-one bug; needs a host-tagged cwd and a - link opener that routes through the file service. + link opener that routes through the file service. Done: see "As built (#254)" below. - Per-workroom credential: enrol on create, revoke on destroy, and revoke on the failure paths too. It is a broker grant, not a deploy key (OQ20). - Idle state per remote workroom in the sidebar. **Cost display is deliberately deferred** — - it needs per-provider billing APIs and there is no spending policy yet (open question 7). + it needs per-provider billing APIs and there is no spending policy yet (open question 7). Idle + state done: see "As built (#254)" below. +- **As built (#254, remote pane parity).** Pane close was done first, in #306 (#283). The rest: + - **Cmd-click (C7).** A remote pane's links never reach `TerminalLinkOpener`'s local resolver. + `TerminalSessions.wireRemoteLinks` resolves a clicked path against the shell's directory on the + host, as its agent reports it (`hostCwd`, #245), or the workroom root before the first report. + `TerminalLinkOpener.remoteCandidates` keeps the local parsing (`:line[:col]`, trailing dots) + and drops what the host's file service can't read: anything outside the workroom root, and + `~`, which is this Mac's home. Each candidate is probed with a one-byte read, which the agent + answers from the file's size without sending it (`file.rs`, `read_file`). The first that exists + opens in the in-app viewer as the workroom's preview tab, since no editor on the Mac can open + a file on another machine. Web URLs still go to the Mac's browser. + - **The ⌘-click gate.** `GhosttySurfaceView.mouseDown` passes a ⌘-click on only when the + synchronous `resolveCmdHoverFile` says the word is a file, and only the host knows, a round + trip away. So the gate decides from the word alone (`TerminalLinkOpener.looksLikeRemotePath`): + a candidate in the workroom that has a `/`, a `name.ext` (dotfiles included) or a `:line` + passes, and the click asks the host, connecting it if need be (`RepositoryRouter`). A hover + never asks the host. + The costs: a path-shaped word with no file behind it shows the hand and its click opens nothing + (logged), and a bare name with none of the three (`Gemfile`) isn't clickable in a remote pane. + A URL or a path outside the workroom never passes, so it still reaches libghostty. This was + first built as a cache of the host's answers behind the gate, and each of the three review + passes found a new failure in it: a click made straight after pressing ⌘ lost, every click + taken during an outage, and probes against a dead link piling up while the gate held "yes". + The owner chose the gate that needs no host (decided 2026-10-04). + - **Ports.** `PortForwardingModel` is one model per host (`model(for:)`), kept for the launch, + and the inspector's Ports section shows the selected workroom's host. A remote workroom the + app can't reach shows no Ports section, rather than the Mac's list. A forward never connects a + host; selecting the workroom does. This closed #208's last criterion. + - **Idle state.** A remote workroom's sidebar row carries the wakefulness badge, backed by one + `WakefulnessModel` per remote host (`model(forHost:)`). It polls only while the row is shown + and never connects the host. Its "Keep awake" click does connect it first + (`RemoteHosts.ensureConnected`), as the Mac's own spawns an agent. Quitting drains every host's + in-flight keep, not only the Mac's (`WakefulnessModel.drainAllKeeps`), waiting up to 15 s for a + remote one, which may have to reconnect over ssh first; a keep slower than that can miss the + quit. The Linux agent runs the classifier unconditionally (`serve.rs`), so neither the agent nor + host setup changed. A remote badge's tooltip leaves out the settings-mismatch sentence: that + agent was never started with the Mac's settings. + - **Known limits.** The viewer opens a cmd-clicked file at the top: `FileDescriptor` has no line. + A path outside the workroom (`/etc/hosts`, a sibling checkout) opens nothing. The ceiling prompt + is still raised only for the Mac's agent: a remote host's "Keep awake" works from its badge, but + no prompt card is shown for it, and a remote keep that fails shows nothing either (#324). + `hostCwd` is asked for again each time a command finishes, one round trip later, so a click + inside that round trip after a `cd` resolves against the previous directory. `hostCwd` is the + host's resolved path while the workroom root is the recorded one, so where the root goes + through a symlink (`/home` → `/var/home`) relative cmd-clicks open nothing (#325; the + first-party container image is not affected). A remote forward is reachable by every account on + the Mac, as any `127.0.0.1` forward is; accepted for now, with peer-uid scoping tracked in #323. + - **Path handling, from the final review.** A remote path is split into components, never run + through `URL`: Foundation decomposes Unicode filenames (`é`, C3 A9, becomes `e` + U+0301, + 65 CC 81), which a Linux host reads as another name. `..` is resolved here, lexically, because + the agent refuses any path with a `.` or `..` component before it opens anything + (`vcs::relative`); sending `..` through, as one review suggested, made every `../file` click + open nothing (caught on the PR). The cost: `link/../file.rb` names the file beside the link, + not beside its target. A newer click replaces the one before it, and an answer whose pane has + closed opens nothing. An error reading one path moves on to the next candidate; only a + transport failure stops the probe. ## Phase 0 Results diff --git a/macapp/WorkroomApp/Core/AgentPortForwarding.swift b/macapp/WorkroomApp/Core/AgentPortForwarding.swift index 6c4cf2131..eb53df26c 100644 --- a/macapp/WorkroomApp/Core/AgentPortForwarding.swift +++ b/macapp/WorkroomApp/Core/AgentPortForwarding.swift @@ -894,33 +894,57 @@ private final class ForwardedConnection: @unchecked Sendable { } } -/// The forwards the user has asked for on the local box, and the listeners serving them. +/// The forwards the user has asked for on one host, and the listeners serving them. /// -/// One list, not one per host: only the local host has an agent today, and `wakefulness()` is shared -/// the same way. Forwards are per HOST rather than per workroom, so two workrooms on this box see -/// the same list. +/// One model per HOST (`model(for:)`), not per workroom: a forward runs on the host's agent +/// connection, so two workrooms on one host see the same list. A remote workroom is its own host +/// (#254), so in practice each remote workroom has its own list and every local one shares this +/// Mac's. @MainActor final class PortForwardingModel: ObservableObject { - static let shared = PortForwardingModel() + /// Each host's model, made on first use and kept for the launch: its listeners must outlive the + /// Ports panel showing another workroom and coming back. A host that goes away takes its + /// forwards with its connection (`watch()`), leaving an empty model behind. + private static var models: [HostID: PortForwardingModel] = [:] + + static func model(for host: HostID) -> PortForwardingModel { + if let model = models[host] { return model } + let model = PortForwardingModel(host: host, transport: .live(host)) + models[host] = model + return model + } /// How the model reaches the agent, so a test can hand it a scripted agent and a connection /// stream it controls — the shape `WakefulnessModel.Transport` set. struct Transport { - /// The service and the lease of the connection it runs on. Never spawns an agent; the reasoning - /// is on `LocalAgentVCS.forwarding()`, the decision point. + /// The service and the lease of the connection it runs on. Never spawns an agent or connects a + /// host; the reasoning is on `LocalAgentVCS.forwarding()`, the decision point. var forwarding: @Sendable () async throws -> (HostConnectionManager.Lease, AgentForwardService) var updates: @Sendable () async -> AsyncStream /// The connected lease right now, or nil. What `add()` reconciles against: the watch's last /// snapshot can lag a reconnect that `forwarding` has already seen. var current: @Sendable () async -> HostConnectionManager.Lease? - static let live = Transport( - forwarding: { try await LocalAgentVCS.shared.forwarding() }, - updates: { await HostConnectionManager.shared.updates(for: .local) }, - current: { - let snapshot = await HostConnectionManager.shared.snapshot(for: .local) - return snapshot.status == .connected ? snapshot.lease : nil - }) + static func live(_ host: HostID) -> Transport { + var transport = on(host, manager: .shared) + if host == .local { transport.forwarding = { try await LocalAgentVCS.shared.forwarding() } } + return transport + } + + /// `host`'s connection in `manager`. A remote host's connection is made by selecting its + /// workroom (`RemoteHosts.ensureConnected`, through the inspector's reads), so a forward asks + /// for the one there is and never makes one: the local reasoning in + /// `LocalAgentVCS.forwarding()` holds for a host too, and connecting one could start a + /// container the user stopped. + static func on(_ host: HostID, manager: HostConnectionManager) -> Transport { + Transport( + forwarding: { try await manager.forwarding(host: host) }, + updates: { await manager.updates(for: host) }, + current: { + let snapshot = await manager.snapshot(for: host) + return snapshot.status == .connected ? snapshot.lease : nil + }) + } } struct Entry: Identifiable, Equatable { @@ -943,11 +967,12 @@ final class PortForwardingModel: ObservableObject { @Published private(set) var forwards: [Entry] = [] /// What went wrong adding one, or why the last forwards went away. Cleared by the next attempt. @Published private(set) var message: String? - /// Whether the local agent connection is up, for the row's caption: the controls are useless + /// Whether the host's agent connection is up, for the row's caption: the controls are useless /// without one, and a caption says so before the user finds out from `+`. @Published private(set) var connected = false @Published var draft = "" + let host: HostID private let transport: Transport private var listeners: [UUID: PortForward] = [:] private var watching = false @@ -957,7 +982,8 @@ final class PortForwardingModel: ObservableObject { /// How many snapshots the watch has consumed, for the tests. private(set) var snapshotsSeen = 0 - init(transport: Transport = .live) { + init(host: HostID = .local, transport: Transport) { + self.host = host self.transport = transport } @@ -1003,6 +1029,8 @@ final class PortForwardingModel: ObservableObject { /// `VCSError` is not `LocalizedError`, so interpolating it prints its Swift case. static func describe(_ error: Error) -> String { switch error { + case RepositoryRoutingError.unavailable(.remote): + return "This workroom's host is not connected." case RepositoryRoutingError.unavailable: return "No agent is running on this Mac. Open a workroom to start one." case VCSError.backendVersion(let detail): diff --git a/macapp/WorkroomApp/Core/AgentWakefulness.swift b/macapp/WorkroomApp/Core/AgentWakefulness.swift index 2b580b0a9..49f40586f 100644 --- a/macapp/WorkroomApp/Core/AgentWakefulness.swift +++ b/macapp/WorkroomApp/Core/AgentWakefulness.swift @@ -397,8 +397,9 @@ struct AwakeCeilingPromptState: Equatable { } } -/// What the UI reads: the local box's wakefulness, polled while it is on screen, plus the ceiling -/// prompt. One instance, because the verdict is per BOX — every workroom on this machine shares it. +/// What the UI reads: a box's wakefulness, polled while it is on screen, plus the ceiling prompt. +/// One instance per BOX, because the verdict is per box: `shared` is this Mac's, which every local +/// workroom shares, and each remote host has its own (`model(forHost:)`, #254). @MainActor final class WakefulnessModel: ObservableObject { static let shared = WakefulnessModel() @@ -418,6 +419,23 @@ final class WakefulnessModel: ObservableObject { status: { try await LocalAgentVCS.shared.wakefulness(connecting: .never).status() }, keep: { try await LocalAgentVCS.shared.wakefulness(connecting: .spawn).keep() }, prompts: { try await LocalAgentVCS.shared.wakefulness(connecting: .reconnect).prompts }) + + /// A remote host's service in `manager` (#254). A poll and the prompt watch never connect + /// the host: a badge is not a reason to reach it. A "keep" runs `connect` first, as the local + /// transport spawns, because a click is the one caller for which a dropped connection is not an + /// answer (`keep()`). + static func on( + _ host: HostID, manager: HostConnectionManager, + connect: @escaping @Sendable (HostID) async throws -> Void = { _ in } + ) -> Transport { + Transport( + status: { try await manager.wakefulness(host: host).status() }, + keep: { + try await connect(host) + try await manager.wakefulness(host: host).keep() + }, + prompts: { try await manager.wakefulness(host: host).prompts }) + } } private let transport: Transport @@ -426,6 +444,40 @@ final class WakefulnessModel: ObservableObject { self.transport = transport } + /// Each remote host's model, made on first use and kept for the launch, as + /// `PortForwardingModel`'s are. This Mac's is `shared`, whose transport may start an agent. + private static var hosts: [UUID: WakefulnessModel] = [:] + + static func model(forHost id: UUID) -> WakefulnessModel { + if let model = hosts[id] { return model } + let model = WakefulnessModel( + transport: .on(.remote(id), manager: .shared) { + try await RemoteHosts.shared.ensureConnected($0) + }) + hosts[id] = model + return model + } + + /// Every model's in-flight "Keep awake", this Mac's and each remote host's, for quit. At once, so + /// the quit waits for the slowest, not their sum. A remote host's badge is its only keep control. + /// + /// A remote keep may have to reconnect first, over ssh (`ConnectTimeout 10`), so it gets + /// `remoteKeepDrain`, longer than this Mac's. Longer still is possible (a relayed workroom's + /// relay install runs on that connect), and such a keep can miss the quit; the bound is what a + /// quit is worth. + static func drainAllKeeps() async { + await withTaskGroup(of: Void.self) { group in + group.addTask { await shared.drainKeep() } + for model in hosts.values { + group.addTask { await model.drainKeep(timeout: remoteKeepDrain) } + } + } + } + + /// How long quitting waits for a remote host's in-flight keep: ssh's 10 s connect timeout, plus + /// the agent's greeting and the request. + static let remoteKeepDrain: Duration = .seconds(15) + /// Nil until a poll succeeds, and again once one fails: no agent, an agent that predates the /// service, or a macOS agent (which answers `running: false`) all show nothing rather than a /// guess. diff --git a/macapp/WorkroomApp/Core/TerminalLinkOpener.swift b/macapp/WorkroomApp/Core/TerminalLinkOpener.swift index 02c441680..0940caa89 100644 --- a/macapp/WorkroomApp/Core/TerminalLinkOpener.swift +++ b/macapp/WorkroomApp/Core/TerminalLinkOpener.swift @@ -279,6 +279,68 @@ enum TerminalLinkOpener { return true } + // MARK: Remote panes (#254, C7) + + /// The workroom-relative files a remote pane's ⌘-clicked `word` could name, in the order to probe + /// them on the host. `cwd` and `root` are paths on the HOST: the pane's working directory as its + /// agent reports it, and the workroom's root. Nothing here touches this Mac's filesystem, which + /// is the point (C7): on this Mac the same path is another file, or none. + /// + /// A candidate outside `root` is dropped, because the host's file service reads only under the + /// workroom's root. So is a `~` path: `~` is this Mac's home, not the host's. A link that names a + /// URL rather than a path gives none, through the same `filePath(from:)` a local pane uses. + static func remoteCandidates(for word: String, cwd: String?, root: String) -> [PathCandidate] { + guard let path = filePath(from: word), root.hasPrefix("/") else { return [] } + // Split into components, never through `URL`: Foundation decomposes Unicode filenames + // (`é` → `e` + U+0301), which a Linux host reads as another name. Empty and `.` components + // go, which also absorbs a trailing or doubled slash. + func parts(_ path: String) -> [Substring] { + path.split(separator: "/").filter { $0 != "." } + } + let rootParts = parts(root) + let baseParts = cwd.flatMap { $0.hasPrefix("/") ? parts($0) : nil } ?? rootParts + var seen = Set() + return pathCandidates(from: path).compactMap { candidate in + guard !candidate.path.hasPrefix("~") else { return nil } + let joined = + candidate.path.hasPrefix("/") ? parts(candidate.path) : baseParts + parts(candidate.path) + // `..` is resolved here, lexically: the agent refuses any path with a `.` or `..` + // component before it opens anything (`vcs::relative`). The cost is a symlink before the + // `..`: `link/../file.rb` names the file beside the link, not beside its target. + var resolved: [Substring] = [] + for part in joined { + if part == ".." { _ = resolved.popLast() } else { resolved.append(part) } + } + guard resolved.count > rootParts.count, resolved.starts(with: rootParts) else { return nil } + let relative = resolved.dropFirst(rootParts.count).joined(separator: "/") + guard !relative.isEmpty, seen.insert(relative).inserted else { return nil } + return PathCandidate(path: relative, line: candidate.line, column: candidate.column) + } + } + + /// Whether a remote pane's ⌘-hover cursor and ⌘-click gate pass `word` (#254), decided without + /// the host: it names a candidate in the workroom (`remoteCandidates`) and looks like a path, + /// with a `/`, a `name.ext` (a dotfile such as `.env` counts) or a `:line`. The click then asks + /// the host. The costs: a path-shaped word with no file behind it shows the hand and its click + /// opens nothing, and a bare name with none of the three (`Gemfile`) isn't clickable in a remote + /// pane. + static func looksLikeRemotePath(_ word: String, cwd: String?, root: String) -> Bool { + guard let path = filePath(from: word), + !remoteCandidates(for: word, cwd: cwd, root: root).isEmpty + else { return false } + return path.contains("/") + || path.range(of: "(^|[A-Za-z0-9_-])\\.[A-Za-z0-9]", options: .regularExpression) != nil + || path.range(of: ":[0-9]", options: .regularExpression) != nil + } + + /// The text of an open-URL link to resolve as a path in a remote pane: a `file:` URL's path, else + /// the link as written. Only after `isSystemHandledURL` has turned it down, as locally. + static func remoteLink(from url: URL) -> String { + if url.isFileURL { return url.path } + let link = url.absoluteString + return link.removingPercentEncoding ?? link + } + /// Does an installed app actually claim `url`'s scheme (`https:`, `mailto:`, `vscode:`, …)? /// /// Only then may it reach `NSWorkspace.open`; everything else is a silent no-op. libghostty's link diff --git a/macapp/WorkroomApp/Core/TerminalSessions.swift b/macapp/WorkroomApp/Core/TerminalSessions.swift index c83ad9a0c..03e3276d4 100644 --- a/macapp/WorkroomApp/Core/TerminalSessions.swift +++ b/macapp/WorkroomApp/Core/TerminalSessions.swift @@ -2162,6 +2162,10 @@ final class TerminalSessions: ObservableObject { self.onSurfaceFocused?(targetID) } + if target.remoteHost != nil { + wireRemoteLinks(view, tab: tabID, target: target) + return tab + } let projectPath = target.path view.onCmdClickFile = { [weak view] word in TerminalLinkOpener.handleCmdClickFile(word, cwd: view?.lastKnownCwd ?? projectPath) @@ -2175,6 +2179,104 @@ final class TerminalSessions: ObservableObject { return tab } + // MARK: Remote pane links (#254, C7) + + /// How a remote pane reaches its host's files: through the router, which connects the host, since + /// only a click asks and a click is a deliberate act. Swapped by tests. + var remoteFiles: (RepositoryLocation) async throws -> FileProviding = { + try await RepositoryRouter.shared.files(for: $0) + } + + /// ⌘-click in a remote pane resolves on the HOST (C7): against the shell's directory there, as + /// its agent reports it (`hostCwd`), and through the host's file service. The file opens in the + /// in-app viewer, since no editor on this Mac can open a file on another machine. Web URLs still + /// go to this Mac's browser. + /// + /// The ⌘-hover cursor is also the ⌘-click's gate (`GhosttySurfaceView.mouseDown`), and it is + /// synchronous, so it can't ask the host. It decides from the word alone + /// (`TerminalLinkOpener.looksLikeRemotePath`), and the click asks the host. Gating on a cache + /// of the host's answers regressed on every review of #254: it lost the click made straight + /// after pressing ⌘, took every click during an outage, and let hung probes pile up. + private func wireRemoteLinks( + _ view: GhosttySurfaceView, tab tabID: TerminalTab.ID, target: TerminalTarget + ) { + view.onCmdClickFile = { [weak self] word in + self?.openRemoteFile(word, tab: tabID, target: target) + } + view.resolveCmdHoverFile = { [weak self] word in + TerminalLinkOpener.looksLikeRemotePath( + word, cwd: self?.hostCwd(of: tabID, in: target.id), root: target.path) + } + view.onOpenURL = { [weak self] url in + if TerminalLinkOpener.isSystemHandledURL(url) { + NSWorkspace.shared.open(url) + } else { + self?.openRemoteFile(TerminalLinkOpener.remoteLink(from: url), tab: tabID, target: target) + } + return true + } + } + + private func hostCwd(of tabID: TerminalTab.ID, in target: TerminalTarget.ID) -> String? { + guard case .terminal(let state) = tabsByTarget[target]?[tabID]?.content else { return nil } + return state.hostCwd + } + + /// Each workroom's click in flight. The host can take seconds to answer (a reconnect, then a + /// probe per candidate), so a newer click replaces the one before it: a slow answer must not + /// retarget the preview after a faster one, or open a tab in a workroom that has gone. + private var remoteOpens: [TerminalTarget.ID: Task] = [:] + + func openRemoteFile(_ word: String, tab tabID: TerminalTab.ID, target: TerminalTarget) { + let cwd = hostCwd(of: tabID, in: target.id) + remoteOpens[target.id]?.cancel() + remoteOpens[target.id] = Task { [weak self] in + guard let self else { return } + let file = await self.remoteFile(word, cwd: cwd, target: target) + guard !Task.isCancelled else { return } + self.remoteOpens[target.id] = nil + guard let file else { + // As `TerminalLinkOpener.handleOpenURL` does locally: a dropped click is otherwise + // indistinguishable from a broken app. + NSLog("Workroom: no file on the host for terminal link %@", word) + return + } + // The pane that asked may have closed while the host answered. Tab IDs are never reused, + // so a reopened workroom doesn't revive it. + guard self.tabsByTarget[target.id]?[tabID] != nil else { return } + self.openFilePreview(FileDescriptor(path: file.path, isPreview: true), for: target) + } + } + + /// The first of `word`'s candidates that is a file on the host, or nil. A read capped at one byte + /// is the probe: it answers `.tooLarge` for any longer file without sending it, and fails for + /// anything missing, refused or outside the workroom. + func remoteFile(_ word: String, cwd: String?, target: TerminalTarget) async + -> TerminalLinkOpener.PathCandidate? + { + guard let location = target.remoteLocation else { return nil } + let candidates = TerminalLinkOpener.remoteCandidates(for: word, cwd: cwd, root: target.path) + guard !candidates.isEmpty, let files = try? await remoteFiles(location) else { return nil } + for candidate in candidates { + do { + _ = try await files.read(path: candidate.path, symlinks: .followWithinRoot, maxBytes: 1) + return candidate + } catch FileServiceError.tooLarge { + return candidate + } catch is FileServiceError { + // About this path: missing, refused, or an error reading it (too long, not a directory + // on the way). The next candidate, `file.rb` from `file.rb:12`, may still be there. An + // agent turning reads away as busy lands here too, and costs only the remaining probes. + continue + } catch { + // The host or its connection, not the candidate: the next one would fail the same way. + NSLog("Workroom: probing the host for %@: %@", candidate.path, "\(error)") + return nil + } + } + return nil + } + /// Reconstruct a minimal `TerminalTarget` from its id for the `onFocused` callback (which only /// carries ids). `focus` keys off the id alone, so a minimal target is sufficient. private func target(forID id: TerminalTarget.ID) -> TerminalTarget? { diff --git a/macapp/WorkroomApp/Views/ChangesPanel.swift b/macapp/WorkroomApp/Views/ChangesPanel.swift index 214356357..09368172e 100644 --- a/macapp/WorkroomApp/Views/ChangesPanel.swift +++ b/macapp/WorkroomApp/Views/ChangesPanel.swift @@ -385,11 +385,11 @@ private struct ChangedFileCountBadge: View { } } -/// Whether this box is busy, as the agent's wakefulness service reports it (issue #208). +/// Whether a box is busy, as its agent's wakefulness service reports it (issues #208, #254). /// -/// In the Changes header rather than in the sidebar row, matching the PR badge it sits beside: the -/// verdict is per BOX, not per workroom, so every sidebar row would carry the same glyph — the -/// inspector shows it once, for whatever is selected. +/// This Mac's is in the Changes header rather than in the sidebar rows, matching the PR badge it +/// sits beside: the verdict is per BOX, not per workroom, so every local row would carry the same +/// glyph. A remote workroom is its own box, so its row carries its own (`ProjectSidebar`). /// /// It owns its polling through `.task`, which is what makes "no polling while hidden" true rather /// than merely intended: closing the inspector or the window unmounts this view and cancels the poll. @@ -397,6 +397,9 @@ private struct ChangedFileCountBadge: View { /// classifier does not run), so this is invisible on a local-only setup. struct WakefulnessBadge: View { @ObservedObject var model: WakefulnessModel = .shared + /// False for a remote host's badge, whose agent was not started with this Mac's settings. + var isLocal = true + var identifier = "changes.wakefulness" private let theme = ThemeService.shared var body: some View { @@ -425,9 +428,9 @@ struct WakefulnessBadge: View { image } } - .help(Self.help(for: status, settings: .current)) + .help(Self.help(for: status, settings: isLocal ? .current : nil)) .accessibilityLabel(glyph.label) - .accessibilityIdentifier("changes.wakefulness") + .accessibilityIdentifier(identifier) } } .task { await model.poll() } @@ -453,7 +456,9 @@ struct WakefulnessBadge: View { } } - static func help(for status: AgentWakefulness, settings: AgentWakefulnessSettings) -> String { + /// `settings` is this Mac's, to name a running agent's that differ; nil for a remote host's + /// agent, which this Mac's settings never started. + static func help(for status: AgentWakefulness, settings: AgentWakefulnessSettings?) -> String { let awake = wakefulnessDuration(status.awakeSeconds).formatted( .units(allowed: [.hours, .minutes], width: .narrow)) var text: String @@ -472,21 +477,26 @@ struct WakefulnessBadge: View { : "This machine is busy, but the agent could not write its verdict, so nothing is keeping " + "it awake. Click to keep it awake." } - if let mismatch = status.settingsMismatch(against: settings) { text += " " + mismatch } + if let settings, let mismatch = status.settingsMismatch(against: settings) { + text += " " + mismatch + } return text } } -/// Ports forwarded from the agent's box to this Mac (issue #208). +/// Ports forwarded from a host's agent to this Mac (issues #208, #254). /// -/// Per HOST, not per workroom — two workrooms on the same box share this list, which is why it sits -/// above the change list rather than inside the workroom-dependent part of it. Nothing is persisted -/// across launches: the agent closes every forwarded socket when the client detaches, so a list -/// restored at launch would name addresses that no longer answer. +/// Per HOST, not per workroom — two local workrooms share this Mac's list, which is why it sits +/// above the change list rather than inside the workroom-dependent part of it; a remote workroom is +/// its own host and shows its own. Nothing is persisted across launches: the agent closes every +/// forwarded socket when the client detaches, so a list restored at launch would name addresses +/// that no longer answer. private struct PortsSection: View { - @ObservedObject private var model = PortForwardingModel.shared + @ObservedObject var model: PortForwardingModel private let theme = ThemeService.shared + private var machine: String { model.host == .local ? "this Mac" : "this workroom's host" } + var body: some View { VStack(alignment: .leading, spacing: 6) { HStack(spacing: 6) { @@ -500,8 +510,7 @@ private struct PortsSection: View { .frame(width: 48) .onSubmit { Task { await model.add() } } .accessibilityIdentifier("ports.field") - InspectorHeaderButton(systemImage: "plus", help: "Forward a port from this machine's agent") - { + InspectorHeaderButton(systemImage: "plus", help: "Forward a port from \(machine)") { Task { await model.add() } } .accessibilityIdentifier("ports.add") @@ -519,7 +528,9 @@ private struct PortsSection: View { } .padding(.horizontal, 12) .padding(.vertical, 8) - .task { model.watch() } + // Keyed by host: the section is the same view across workrooms, and each host's model is + // watched from the first time it is shown. + .task(id: model.host) { model.watch() } } @ViewBuilder @@ -884,9 +895,11 @@ struct ChangesPanel: View { var body: some View { VStack(alignment: .leading, spacing: 0) { - // Per box, so it must not vanish when no workroom is selected; see `PortsSection`. - PortsSection() - Divider() + // Per host, so it must not vanish when no workroom is selected; see `PortsSection`. + if let host = portsHost { + PortsSection(model: .model(for: host)) + Divider() + } Group { if let sid = store.inspectorTargetID, sid.isStatusable { content(for: sid) @@ -898,6 +911,14 @@ struct ChangesPanel: View { .frame(maxWidth: .infinity, alignment: .leading) } + /// The host whose ports the section forwards: a remote workroom's own host (#254), else this + /// Mac's. Nil for a remote workroom this app can't reach, whose ports this Mac's list would only + /// misname. + private var portsHost: HostID? { + guard let workroom = store.selectedWorkroom, workroom.isRemote else { return .local } + return workroom.reachableHost.map(HostID.remote) + } + @ViewBuilder private func content(for sid: SidebarID) -> some View { let target = store.target(for: sid) diff --git a/macapp/WorkroomApp/Views/ProjectSidebar.swift b/macapp/WorkroomApp/Views/ProjectSidebar.swift index 80994736c..1678dae52 100644 --- a/macapp/WorkroomApp/Views/ProjectSidebar.swift +++ b/macapp/WorkroomApp/Views/ProjectSidebar.swift @@ -431,6 +431,13 @@ struct ProjectSidebar: View { .foregroundStyle(.yellow) .help(warning.message) } + // A remote workroom is its own box, so its busy or idle verdict is its own (#254). Nothing + // shows while its host isn't connected: the poll never connects one. + if let host = workroom.reachableHost { + WakefulnessBadge( + model: .model(forHost: host), isLocal: false, + identifier: "sidebar.workroom.\(workroom.name).wakefulness") + } // Spinner/delete slot: a progress spinner while the workroom's setup runs (issue #116) or a // command runs (issue #28), swapped for the delete button on hover — so a workroom stays // deletable even mid-run. The delete button is always laid out (it reveals via opacity), so it diff --git a/macapp/WorkroomApp/WorkroomApp.swift b/macapp/WorkroomApp/WorkroomApp.swift index 2295ff158..774a7cf49 100644 --- a/macapp/WorkroomApp/WorkroomApp.swift +++ b/macapp/WorkroomApp/WorkroomApp.swift @@ -754,12 +754,13 @@ final class AppDelegate: NSObject, NSApplicationDelegate, UNUserNotificationCent // Stop every window's run commands, not just the focused one's. registry.gracefullyStopAllWindows(timeout: 5) { group.leave() } } - // A "Keep awake" clicked moments before quitting (issue #208): the card cleared on the click, - // the request may still be reconnecting. Bounded; a box the user asked to keep awake is worth - // two seconds of a quit. + // A "Keep awake" clicked moments before quitting (issues #208, #254): the card cleared on the + // click, the request may still be reconnecting. Bounded, this Mac's and each remote host's at + // once (`WakefulnessModel.drainAllKeeps`): a box the user asked to keep awake is worth a few + // seconds of a quit. group.enter() Task { - await WakefulnessModel.shared.drainKeep() + await WakefulnessModel.drainAllKeeps() group.leave() } group.notify(queue: .main) { sender.reply(toApplicationShouldTerminate: true) } diff --git a/macapp/WorkroomAppTests/AgentPortForwardingTests.swift b/macapp/WorkroomAppTests/AgentPortForwardingTests.swift index b4f3b2f15..e1e18c807 100644 --- a/macapp/WorkroomAppTests/AgentPortForwardingTests.swift +++ b/macapp/WorkroomAppTests/AgentPortForwardingTests.swift @@ -944,6 +944,47 @@ final class PortForwardingModelTests: XCTestCase { old.draft = "5173" await old.add() XCTAssertEqual(old.message, "Agent does not support port forwarding.") + + // A remote workroom's host is not "this Mac" (#254). + let remote = try await model( + lease: lease(), forwarding: { throw RepositoryRoutingError.unavailable(.remote(UUID())) }) + remote.draft = "5173" + await remote.add() + XCTAssertEqual(remote.message, "This workroom's host is not connected.") + } + + /// One model per host, kept: the Ports section going to another workroom and back finds its + /// forwards where it left them, and two hosts never share a list (#254). + func testEachHostHasItsOwnModelForTheLaunch() { + let host = HostID.remote(UUID()) + XCTAssertTrue(PortForwardingModel.model(for: host) === PortForwardingModel.model(for: host)) + XCTAssertTrue(PortForwardingModel.model(for: .local) === PortForwardingModel.model(for: .local)) + XCTAssertFalse(PortForwardingModel.model(for: host) === PortForwardingModel.model(for: .local)) + XCTAssertEqual(PortForwardingModel.model(for: host).host, host) + } + + /// A remote host's transport reads that host's connection in the manager and never another's. + /// With only this Mac connected, a forward on the host is refused as the host's, not served by + /// the local agent. + func testARemoteHostsTransportAsksForThatHostsConnection() async throws { + let agent = try FakeAgent(version: 5, forward: true) + fakes.append(agent) + let manager = HostConnectionManager() + _ = try await manager.connect(host: .local) { + try await AgentVCSConnection.connect(host: .local, socketPath: agent.socketPath) + } + let host = HostID.remote(UUID()) + let transport = PortForwardingModel.Transport.on(host, manager: manager) + do { + _ = try await transport.forwarding() + XCTFail("a remote host with no connection was served") + } catch RepositoryRoutingError.unavailable(let refused) { + XCTAssertEqual(refused, host) + } + let current = await transport.current() + XCTAssertNil(current) + let local = await PortForwardingModel.Transport.on(.local, manager: manager).current() + XCTAssertNotNil(local) } /// A refusal on a row is a fact about one connection — the dev server was not up yet — and the diff --git a/macapp/WorkroomAppTests/AgentWakefulnessTests.swift b/macapp/WorkroomAppTests/AgentWakefulnessTests.swift index a4676748e..6728e24e6 100644 --- a/macapp/WorkroomAppTests/AgentWakefulnessTests.swift +++ b/macapp/WorkroomAppTests/AgentWakefulnessTests.swift @@ -166,6 +166,76 @@ final class AgentWakefulnessTests: XCTestCase { XCTAssertNil(older.settingsMismatch(against: AgentWakefulnessSettings(ceiling: 1, ask: false))) } + /// A remote host's agent was never started with this Mac's settings, so its badge names no + /// mismatch with them (#254); this Mac's badge still does. + func testOnlyThisMacsBadgeComparesThisMacsSettings() throws { + let status = try status() + let settings = AgentWakefulnessSettings(ceiling: 7200, ask: false) + XCTAssertTrue( + WakefulnessBadge.help(for: status, settings: settings).contains("when it next starts")) + XCTAssertFalse( + WakefulnessBadge.help(for: status, settings: nil).contains("when it next starts")) + } + + /// One model per remote host, kept for the launch, and never this Mac's (#254). + @MainActor + func testEachRemoteHostHasItsOwnWakefulnessModel() { + let id = UUID() + XCTAssertTrue(WakefulnessModel.model(forHost: id) === WakefulnessModel.model(forHost: id)) + XCTAssertFalse(WakefulnessModel.model(forHost: id) === WakefulnessModel.model(forHost: UUID())) + XCTAssertFalse(WakefulnessModel.model(forHost: id) === WakefulnessModel.shared) + } + + /// A remote host's poll reads that host's connection and never connects one: with only this Mac + /// connected, it is refused as the host's, so the row shows nothing rather than this Mac's + /// verdict. + @MainActor + func testARemoteHostsPollAsksForThatHostsConnection() async throws { + let manager = HostConnectionManager() + let host = HostID.remote(UUID()) + do { + _ = try await WakefulnessModel.Transport.on(host, manager: manager).status() + XCTFail("a remote host with no connection answered") + } catch RepositoryRoutingError.unavailable(let refused) { + XCTAssertEqual(refused, host) + } + let model = WakefulnessModel(transport: .on(host, manager: manager)) + await model.refresh() + XCTAssertNil(model.status) + } + + /// A remote host's "Keep awake" connects the host first, as this Mac's spawns an agent: a click + /// is the one caller for which a dropped connection is not an answer. The prompt watch never + /// connects, as the poll never does. + @MainActor + func testOnlyARemoteKeepConnectsTheHost() async throws { + let manager = HostConnectionManager() + let host = HostID.remote(UUID()) + let connects = Connects() + let transport = WakefulnessModel.Transport.on(host, manager: manager) { connects.add($0) } + do { + _ = try await transport.prompts() + XCTFail("a remote host with no connection gave a prompt stream") + } catch RepositoryRoutingError.unavailable(let refused) { + XCTAssertEqual(refused, host) + } + XCTAssertEqual(connects.hosts, [], "the prompt watch connected the host") + do { + try await transport.keep() + XCTFail("a keep with no connection after connecting reported success") + } catch RepositoryRoutingError.unavailable(let refused) { + XCTAssertEqual(refused, host) + } + XCTAssertEqual(connects.hosts, [host], "a keep did not connect the host first") + } + + private final class Connects: @unchecked Sendable { + private let lock = NSLock() + private var asked: [HostID] = [] + func add(_ host: HostID) { lock.withLock { asked.append(host) } } + var hosts: [HostID] { lock.withLock { asked } } + } + func testAnErrorReplyIsAServiceFailure() { let json = #"{"version":1,"error":{"unsupported":"status requests are ..."}}"# XCTAssertThrowsError(try decode(json)) { error in diff --git a/macapp/WorkroomAppTests/RemoteHostIntegrationTests.swift b/macapp/WorkroomAppTests/RemoteHostIntegrationTests.swift index c2f03719d..02a019e8b 100644 --- a/macapp/WorkroomAppTests/RemoteHostIntegrationTests.swift +++ b/macapp/WorkroomAppTests/RemoteHostIntegrationTests.swift @@ -156,6 +156,67 @@ final class RemoteHostIntegrationTests: XCTestCase { XCTAssertEqual(banner, "SSH-2.0-") } + /// The Ports panel's model on a remote workroom's host (#254, the last of #208): a port added + /// there is the HOST's port, and a client on this Mac reaches the process listening on it. The + /// fixture's sshd is that process; it speaks first, so the banner proves the bytes came from the + /// host and not from anything on this Mac. + @MainActor + func testThePortsModelForwardsAPortFromARemoteHost() async throws { + let fixture = try fixture() + let (connection, id) = try await connect(fixture.host) + let host = HostID.remote(id) + let manager = HostConnectionManager() + _ = try await manager.connect(host: host) { connection } + let model = PortForwardingModel(host: host, transport: .on(host, manager: manager)) + model.draft = "22" + await model.add() + XCTAssertNil(model.message) + let entry = try XCTUnwrap(model.forwards.first) + defer { model.remove(entry.id) } + let client = try TCPClient(port: entry.localPort) + defer { client.close() } + let banner = String(decoding: try client.read(8, timeout: 10), as: UTF8.self) + XCTAssertEqual(banner, "SSH-2.0-") + } + + /// A remote workroom's sidebar badge (#254): the host's agent runs its classifier (Linux always + /// does), and the model reads its verdict through that host's connection. + @MainActor + func testTheWakefulnessModelReadsARemoteHostsVerdict() async throws { + let fixture = try fixture() + let (connection, id) = try await connect(fixture.host) + let host = HostID.remote(id) + let manager = HostConnectionManager() + _ = try await manager.connect(host: host) { connection } + let model = WakefulnessModel(transport: .on(host, manager: manager)) + await model.refresh() + let status = try XCTUnwrap(model.status, "no verdict from the host") + XCTAssertTrue(status.running, "the host's classifier is not running") + } + + /// ⌘-click in a remote pane (#254, C7) finds a file on the host through its file service, + /// relative to the shell's directory there, with its line. The path does not exist on this Mac. + @MainActor + func testACmdClickedPathResolvesOnTheHost() async throws { + let fixture = try fixture() + let path = try repository(fixture) + try onHost(fixture, "mkdir -p \(path)/lib && echo hi > \(path)/lib/only-here.rb") + let (connection, id) = try await connect(fixture.host) + let target = TerminalTarget( + id: "remote", title: "remote", path: path, unavailability: .remote, remoteHost: id) + let sessions = TerminalSessions() + sessions.remoteFiles = { location in + try connection.files(context: FileContext(location: location, sharedLocation: nil)) + } + let found = await sessions.remoteFile("only-here.rb:1", cwd: "\(path)/lib", target: target) + XCTAssertEqual(found, .init(path: "lib/only-here.rb", line: 1, column: nil)) + let missing = await sessions.remoteFile("nowhere.rb", cwd: "\(path)/lib", target: target) + XCTAssertNil(missing) + // A parent traversal reaches the real agent as a path it accepts: it refuses any `..`. + let parent = await sessions.remoteFile("../file", cwd: "\(path)/lib", target: target) + XCTAssertEqual(parent, .init(path: "file", line: nil, column: nil)) + } + /// A failed remote write is classified from the HOST's disk (#229): a leftover `index.lock` there /// is named by its path on the host. Read from this Mac's disk, that path is not there, and the /// failure would name no lock at all. diff --git a/macapp/WorkroomAppTests/TerminalLinkOpenerTests.swift b/macapp/WorkroomAppTests/TerminalLinkOpenerTests.swift index 68b7fc099..56ee26400 100644 --- a/macapp/WorkroomAppTests/TerminalLinkOpenerTests.swift +++ b/macapp/WorkroomAppTests/TerminalLinkOpenerTests.swift @@ -530,4 +530,371 @@ final class TerminalLinkOpenerTests: XCTestCase { XCTAssertEqual(resolved?.line, line, "\(link) should carry its line") } } + + // MARK: Remote panes (#254, C7) + + /// A remote pane's link resolves against the HOST's paths, to a workroom-relative file, in the + /// local order: the literal first, then the decoration-stripped forms carrying their line. + func testARemoteLinkResolvesAgainstTheHostsDirectory() { + let root = "/home/workroom/repo" + let candidates = TerminalLinkOpener.remoteCandidates( + for: "app/user.rb:5:2", cwd: "\(root)/lib", root: root) + XCTAssertEqual( + candidates, + [ + .init(path: "lib/app/user.rb:5:2", line: nil, column: nil), + .init(path: "lib/app/user.rb", line: 5, column: 2), + ]) + // No cwd yet: the workroom's root. + XCTAssertEqual( + TerminalLinkOpener.remoteCandidates(for: "README.md", cwd: nil, root: root).map(\.path), + ["README.md"]) + // An absolute path inside the workroom, and `..` back into it. + XCTAssertEqual( + TerminalLinkOpener.remoteCandidates(for: "\(root)/a/b.txt", cwd: "/", root: root).map( + \.path), ["a/b.txt"]) + XCTAssertEqual( + TerminalLinkOpener.remoteCandidates(for: "../x.txt", cwd: "\(root)/lib", root: root).map( + \.path), ["x.txt"]) + } + + /// A remote path reaches the host with the bytes it was written in. Foundation's URL + /// normalisation decomposes `é` (C3 A9) into `e` + U+0301 (65 CC 81), which a Linux host reads as + /// another filename; Swift's `==` calls the two equal, so the bytes are compared. + func testARemoteLinkKeepsItsFilenameBytes() throws { + let name = "caf\u{e9}.txt" + let found = try XCTUnwrap( + TerminalLinkOpener.remoteCandidates(for: name, cwd: nil, root: "/home/workroom/repo").first) + XCTAssertEqual(Array(found.path.utf8), Array(name.utf8)) + } + + /// What the host is sent never has a `.` or `..` component: the agent refuses such a path + /// before it opens anything (`vcs::relative`), so a parent traversal is resolved here. Sent as + /// written, every `../file.rb` click opened nothing (found in review). + func testARemoteLinkNeverSendsTheHostAParentComponent() { + let root = "/home/workroom/repo" + for (link, cwd) in [ + ("../x.txt", "\(root)/lib"), ("link/../file.rb", root), ("./a/./b.rb", root), + ("../lib/../y.rb:3", "\(root)/lib/deep"), + ] { + let candidates = TerminalLinkOpener.remoteCandidates(for: link, cwd: cwd, root: root) + XCTAssertFalse(candidates.isEmpty, link) + for candidate in candidates { + let parts = candidate.path.split(separator: "/", omittingEmptySubsequences: false) + XCTAssertFalse(parts.contains { $0 == ".." || $0 == "." || $0.isEmpty }, candidate.path) + } + } + XCTAssertEqual( + TerminalLinkOpener.remoteCandidates(for: "link/../../../out.rb", cwd: root, root: root), []) + } + + /// Only what the host's file service can read: nothing outside the workroom's root, no `~` (this + /// Mac's home), and no URL. + func testARemoteLinkOutsideTheWorkroomOrNotAPathGivesNothing() { + let root = "/home/workroom/repo" + for link in [ + "/etc/passwd", "../../other/file.txt", "~/notes.txt", "https://example.com/a.rb", + "javascript:payload.command", "/home/workroom/repository/file", + ] { + XCTAssertEqual( + TerminalLinkOpener.remoteCandidates(for: link, cwd: root, root: root), [], link) + } + // Value: protects=a remote link never resolves against this Mac's cwd; + // fails_when=the absolute-cwd or absolute-root guard is dropped; + // why_new=rows above only use absolute roots and cwds; seam=none + // A cwd that is not absolute is ignored for the root, and a root that is not absolute gives + // nothing: `URL` would otherwise resolve either one against THIS Mac's working directory. + XCTAssertEqual( + TerminalLinkOpener.remoteCandidates(for: "a.rb", cwd: "lib", root: root).map(\.path), ["a.rb"] + ) + XCTAssertEqual( + TerminalLinkOpener.remoteCandidates(for: "a.rb", cwd: nil, root: "relative/repo"), []) + } + + /// The acceptance test for C7: the workroom's path exists on THIS Mac too, holding a different + /// file. ⌘-click resolves only what the host has, and a file only this Mac has resolves to + /// nothing. + @MainActor + func testARemotePaneOpensTheHostsFileNotTheMacs() async throws { + let fm = FileManager.default + let root = fm.temporaryDirectory.appendingPathComponent(UUID().uuidString) + try fm.createDirectory(at: root, withIntermediateDirectories: true) + defer { try? fm.removeItem(at: root) } + fm.createFile(atPath: root.appendingPathComponent("only-on-mac.txt").path, contents: Data()) + + let host = UUID() + let target = TerminalTarget( + id: "remote", title: "remote", path: root.path, unavailability: .remote, remoteHost: host) + let location = try XCTUnwrap(target.remoteLocation) + let sessions = TerminalSessions() + sessions.remoteFiles = { location in + HostFiles( + context: FileContext(location: location, sharedLocation: nil), + files: ["only-on-host.rb": Data("puts 1\n".utf8)]) + } + + let found = await sessions.remoteFile("only-on-host.rb:3", cwd: nil, target: target) + XCTAssertEqual(found, .init(path: "only-on-host.rb", line: 3, column: nil)) + let onMac = await sessions.remoteFile("only-on-mac.txt", cwd: nil, target: target) + XCTAssertNil(onMac, "a file that is only on this Mac was taken for the host's") + XCTAssertEqual(location.host, .remote(host)) + } + + /// The wiring: a remote target's pane routes ⌘-click to the host and opens the file it finds as + /// the workroom's preview tab, in the in-app viewer. + @MainActor + func testCmdClickInARemotePaneOpensTheHostsFileInTheViewer() async throws { + let target = TerminalTarget( + id: "remote", title: "remote", path: "/home/workroom/repo", unavailability: .remote, + remoteHost: UUID()) + let sessions = TerminalSessions() + sessions.makeView = { _, cwd, _ in GhosttySurfaceView(workingDirectory: cwd) } + sessions.recordUnrecognizedTool = { _ in } + sessions.remoteFiles = { location in + HostFiles( + context: FileContext(location: location, sharedLocation: nil), + files: ["lib/user.rb": Data("class User; end\n".utf8)]) + } + let tab = sessions.addTab(for: target) + let view = try XCTUnwrap(sessions.view(forTab: tab.id, inTarget: target.id)) + view.onCmdClickFile?("lib/user.rb:1") + func opened() -> FileDescriptor? { + for tab in sessions.tabs(for: target) { + if case .file(let file) = tab.content { return file } + } + return nil + } + for _ in 0..<200 where opened() == nil { try await Task.sleep(for: .milliseconds(10)) } + XCTAssertEqual(opened()?.path, "lib/user.rb") + } + + /// What the probe makes of the host's answers, one candidate at a time. A refused candidate (a + /// link out of the root, a directory) is skipped for the next, as a missing one is. Any other + /// failure is the HOST's (it dropped, the agent errored), so the next candidate would fail the + /// same way and the probe gives up with nothing, rather than reporting a file it never read. + // Value: protects=the host probe skips refused candidates but stops on a host failure; + // fails_when=refused stops the probe, or a host failure continues to a later candidate; + // why_new=existing tests only reach found, tooLarge and notFound; seam=none + @MainActor + func testTheRemoteProbeSkipsRefusedCandidatesAndStopsOnAHostFailure() async throws { + let target = TerminalTarget( + id: "remote", title: "remote", path: "/home/workroom/repo", unavailability: .remote, + remoteHost: UUID()) + let sessions = TerminalSessions() + let files: [String: Data] = ["a.rb": Data("x\n".utf8)] + func provider(failing: [String: Error]) -> (RepositoryLocation) async throws + -> FileProviding + { + { location in + HostFiles( + context: FileContext(location: location, sharedLocation: nil), files: files, + failing: failing) + } + } + + // `a.rb:3` probes the literal first, then `a.rb` carrying line 3. + sessions.remoteFiles = provider(failing: [ + "a.rb:3": FileServiceError.refused("not a regular file") + ]) + let skipped = await sessions.remoteFile("a.rb:3", cwd: nil, target: target) + XCTAssertEqual(skipped, .init(path: "a.rb", line: 3, column: nil)) + + // An error reading one path (too long, unreadable) moves on to the next candidate. + sessions.remoteFiles = provider(failing: [ + "a.rb:3": FileServiceError.failed("File name too long (os error 63)") + ]) + let pathError = await sessions.remoteFile("a.rb:3", cwd: nil, target: target) + XCTAssertEqual(pathError, .init(path: "a.rb", line: 3, column: nil)) + + sessions.remoteFiles = provider(failing: ["a.rb:3": HostConnectionError.connectionLost]) + let stopped = await sessions.remoteFile("a.rb:3", cwd: nil, target: target) + XCTAssertNil(stopped, "a host failure went on to probe a later candidate") + + // The host's service itself unreachable: nothing to probe, nothing found. + sessions.remoteFiles = { _ in throw RepositoryRoutingError.unavailable(.remote(UUID())) } + let unreachable = await sessions.remoteFile("a.rb", cwd: nil, target: target) + XCTAssertNil(unreachable) + } + + /// A link libghostty hands over as a URL (OSC 8, or a printed `file:` URL) reaches the host's + /// file in a remote pane, whether it is a `file:` URL or a bare path: through `remoteLink`, which + /// decodes percent-escapes and takes a `file:` URL's path. + // Value: protects=a remote pane's URL links open the host's file, decoded; + // fails_when=onOpenURL stops routing to the host or remoteLink stops decoding; + // why_new=only the word-based click path is tested; seam=none + @MainActor + func testAnOpenURLLinkInARemotePaneOpensTheHostsFile() async throws { + let target = TerminalTarget( + id: "remote", title: "remote", path: "/home/workroom/repo", unavailability: .remote, + remoteHost: UUID()) + let sessions = TerminalSessions() + sessions.makeView = { _, cwd, _ in GhosttySurfaceView(workingDirectory: cwd) } + sessions.recordUnrecognizedTool = { _ in } + sessions.remoteFiles = { location in + HostFiles( + context: FileContext(location: location, sharedLocation: nil), + files: ["lib/user.rb": Data("class User; end\n".utf8), "my file.rb": Data("x\n".utf8)]) + } + let tab = sessions.addTab(for: target) + let view = try XCTUnwrap(sessions.view(forTab: tab.id, inTarget: target.id)) + func opened() -> String? { + for tab in sessions.tabs(for: target) { + if case .file(let file) = tab.content { return file.path } + } + return nil + } + func click(_ link: String, opens path: String) async throws { + let url = try XCTUnwrap(URL(string: link), link) + XCTAssertEqual(view.onOpenURL?(url), true, "the click was not consumed: \(link)") + for _ in 0..<200 where opened() != path { try await Task.sleep(for: .milliseconds(10)) } + XCTAssertEqual(opened(), path, link) + } + try await click("file:///home/workroom/repo/lib/user.rb", opens: "lib/user.rb") + try await click("my%20file.rb", opens: "my file.rb") + } + + /// The remote ⌘-click gate decides from the word alone (#254): a path-shaped candidate passes + /// (a `/`, a `name.ext` or a `:line`), and anything else falls through to the terminal: a plain + /// word, a dotless bare name, a URL, a path outside the workroom. + func testTheRemoteClickGateTakesPathShapedWordsOnly() { + let root = "/home/workroom/repo" + for word in [ + "lib/user.rb", "user.rb", "user.rb:5", "Makefile:12", "./bin/dev", "a.b.c", ".gitignore", + ".env", + ] { + XCTAssertTrue(TerminalLinkOpener.looksLikeRemotePath(word, cwd: root, root: root), word) + } + for word in [ + "hello", "Gemfile", "done.", "https://example.com/a.rb", "/etc/passwd", "~/notes.txt", + "../../elsewhere/a.rb", + ] { + XCTAssertFalse(TerminalLinkOpener.looksLikeRemotePath(word, cwd: root, root: root), word) + } + } + + /// The gate never asks the host: a hover in a remote pane, ⌘ held, makes no request at all, + /// so a host that is slow, down or hung can't lose, take or pile up clicks. Only the click asks. + @MainActor + func testARemotePanesGateNeverAsksTheHost() async throws { + let target = TerminalTarget( + id: "remote", title: "remote", path: "/home/workroom/repo", unavailability: .remote, + remoteHost: UUID()) + let asked = Counter() + let sessions = TerminalSessions() + sessions.makeView = { _, cwd, _ in GhosttySurfaceView(workingDirectory: cwd) } + sessions.recordUnrecognizedTool = { _ in } + sessions.remoteFiles = { location in + asked.bump() + return HostFiles( + context: FileContext(location: location, sharedLocation: nil), + files: ["a.rb": Data("x\n".utf8)]) + } + let tab = sessions.addTab(for: target) + let view = try XCTUnwrap(sessions.view(forTab: tab.id, inTarget: target.id)) + XCTAssertEqual(view.resolveCmdHoverFile?("a.rb"), true) + XCTAssertEqual(view.resolveCmdHoverFile?("hello"), false) + try await Task.sleep(for: .milliseconds(100)) + XCTAssertEqual(asked.count, 0, "a hover asked the host") + view.onCmdClickFile?("a.rb") + for _ in 0..<200 where asked.count == 0 { try await Task.sleep(for: .milliseconds(5)) } + XCTAssertEqual(asked.count, 1, "the click did not ask the host") + } + + private final class Counter: @unchecked Sendable { + private let lock = NSLock() + private var value = 0 + func bump() { lock.withLock { value += 1 } } + var count: Int { lock.withLock { value } } + } + + /// A newer remote ⌘-click replaces the one before it (#254): the host can take seconds, and a + /// slow answer must not retarget the preview after a faster one. Nor may an answer open anything + /// once the pane that asked has closed, even with other panes still open in the workroom. + @MainActor + func testARemoteClickIsReplacedByTheNextAndDroppedForAClosedPane() async throws { + let target = TerminalTarget( + id: "remote", title: "remote", path: "/home/workroom/repo", unavailability: .remote, + remoteHost: UUID()) + let sessions = TerminalSessions() + sessions.makeView = { _, cwd, _ in GhosttySurfaceView(workingDirectory: cwd) } + sessions.recordUnrecognizedTool = { _ in } + sessions.remoteFiles = { location in + SlowHostFiles( + inner: HostFiles( + context: FileContext(location: location, sharedLocation: nil), + files: ["slow.rb": Data("x\n".utf8), "fast.rb": Data("x\n".utf8)])) + } + let tab = sessions.addTab(for: target) + let view = try XCTUnwrap(sessions.view(forTab: tab.id, inTarget: target.id)) + func opened() -> String? { + for tab in sessions.tabs(for: target) { + if case .file(let file) = tab.content { return file.path } + } + return nil + } + view.onCmdClickFile?("slow.rb") + view.onCmdClickFile?("fast.rb") + try await Task.sleep(for: .milliseconds(800)) + XCTAssertEqual(opened(), "fast.rb", "a slower, older click retargeted the preview") + + // The pane that asked closes while the host is still answering, with another pane still + // open in the workroom: nothing opens there. + let other = sessions.addTab(for: target) + view.onCmdClickFile?("slow.rb") + sessions.closeTab(tab.id, for: target) + try await Task.sleep(for: .milliseconds(800)) + XCTAssertNotEqual(opened(), "slow.rb", "a closed pane's click opened a preview") + XCTAssertNotNil(sessions.tab(other.id, for: target)) + } + + /// `HostFiles` whose `slow.rb` takes half a second to answer. + private struct SlowHostFiles: FileProviding { + let inner: HostFiles + var context: FileContext { inner.context } + func list() async throws -> CommandResult { try await inner.list() } + func read(path: String, symlinks: FileSymlinkPolicy, maxBytes: Int) async throws -> Data { + if path == "slow.rb" { try await Task.sleep(for: .milliseconds(500)) } + return try await inner.read(path: path, symlinks: symlinks, maxBytes: maxBytes) + } + func watch(root: String, onEvent: @escaping @Sendable (FileWatchEvent) -> Void) async throws + -> FileWatchHandle? + { nil } + } + + /// A root recorded with a trailing slash, or a link with a doubled one, still resolves inside + /// the workroom: `standardized` keeps `//`, and the host reads `/src/a.rb` as an absolute path. + func testARemoteLinkSurvivesDoubledAndTrailingSlashes() { + XCTAssertEqual( + TerminalLinkOpener.remoteCandidates(for: "src/a.rb", cwd: nil, root: "/repo/").map(\.path), + ["src/a.rb"]) + XCTAssertEqual( + TerminalLinkOpener.remoteCandidates(for: "/repo//src/a.rb", cwd: nil, root: "/repo").map( + \.path), ["src/a.rb"]) + XCTAssertEqual( + TerminalLinkOpener.remoteCandidates(for: "a.rb", cwd: "/repo//lib/", root: "/repo").map( + \.path), ["lib/a.rb"]) + } + +} + +/// A remote host's file service for the remote-pane tests (#254): `files` by workroom-relative +/// path, reads capped as the agent caps them. Shared with `RemotePaneFooterTests`. +struct HostFiles: FileProviding { + let context: FileContext + let files: [String: Data] + /// Paths whose read fails with this error, before `files` is consulted. + var failing: [String: Error] = [:] + + func list() async throws -> CommandResult { + CommandResult(stdout: "", stderr: "", exitCode: 0, timedOut: false) + } + func read(path: String, symlinks: FileSymlinkPolicy, maxBytes: Int) async throws -> Data { + if let error = failing[path] { throw error } + guard let data = files[path] else { throw FileServiceError.notFound(path) } + guard data.count <= maxBytes else { throw FileServiceError.tooLarge } + return data + } + func watch(root: String, onEvent: @escaping @Sendable (FileWatchEvent) -> Void) async throws + -> FileWatchHandle? + { nil } } diff --git a/macapp/WorkroomAppTests/TerminalSessionsTests.swift b/macapp/WorkroomAppTests/TerminalSessionsTests.swift index f5e1bd039..91508c3d3 100644 --- a/macapp/WorkroomAppTests/TerminalSessionsTests.swift +++ b/macapp/WorkroomAppTests/TerminalSessionsTests.swift @@ -1877,6 +1877,47 @@ final class RemotePaneFooterTests: XCTestCase { XCTAssertEqual(connection.asked, [session], "asked once, when the host connected") } + /// ⌘-click in a remote pane resolves a relative path against the directory the host's agent + /// reports for the shell (`hostCwd`), not the workroom's root and not this Mac's directory for + /// the pane (#254, C7). + // Value: protects=a remote ⌘-click resolves against the shell's directory on the host; + // fails_when=the pane's hostCwd stops reaching the click, so it resolves from the root; + // why_new=other C7 tests pass the cwd to remoteFile directly, never via the pane; seam=none + func testACmdClickInARemotePaneResolvesAgainstTheHostsDirectory() async throws { + let hostID = UUID() + let session = UUID() + registerRemote(session, on: hostID) + let connection = HostCwdConnection(directory: "/home/w/lib") + let s = try await makeSessions(connection, on: .remote(hostID)) + s.remoteFiles = { location in + HostFiles( + context: FileContext(location: location, sharedLocation: nil), + files: ["lib/user.rb": Data("class User; end\n".utf8)]) + } + let remote = TerminalTarget( + id: target.id, title: "r", path: "/home/w", unavailability: .remote, remoteHost: hostID) + s.addTab(for: remote, sessionID: session) + let view = try XCTUnwrap(s.tabs(for: remote).first?.surface) + view.persistentSessionID = session + view.onTitleChange?("~") + func hostCwd() -> String? { + guard case .terminal(let state)? = s.tabs(for: remote).first?.content else { return nil } + return state.hostCwd + } + try await waitFor { hostCwd() == "/home/w/lib" } + XCTAssertEqual(hostCwd(), "/home/w/lib") + + view.onCmdClickFile?("user.rb") + func opened() -> String? { + for tab in s.tabs(for: remote) { + if case .file(let file) = tab.content { return file.path } + } + return nil + } + try await waitFor { opened() != nil } + XCTAssertEqual(opened(), "lib/user.rb") + } + func testALocalPaneAndAHostWithNoConnectionAskNothing() async throws { let connection = HostCwdConnection(directory: "/home/w") let s = try await makeSessions(connection, on: .remote(UUID()))