diff --git a/GraphcodeKit/Sources/Sessions/SessionIDStore.swift b/GraphcodeKit/Sources/Sessions/SessionIDStore.swift index db087eff..74902eb2 100644 --- a/GraphcodeKit/Sources/Sessions/SessionIDStore.swift +++ b/GraphcodeKit/Sources/Sessions/SessionIDStore.swift @@ -67,6 +67,25 @@ public enum SessionIDStore { } } + /// The daemon-side twin of the `SessionStart` hook's write, for a backend that has no + /// hook to bank its own ID (Copilot): history line first, then the pointer, and nothing + /// at all when the pointer already names this ID — an ensure tick must not grow the + /// history. + public static func bank(_ sessionID: String, forNodeID nodeID: UUID, workingDirectory: String) { + guard load(forNodeID: nodeID) != sessionID else { return } + try? FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + let line = "\(Int(Date().timeIntervalSince1970)) \(sessionID) \(workingDirectory)\n" + let history = historyFile(forNodeID: nodeID) + if let handle = try? FileHandle(forWritingTo: history) { + _ = try? handle.seekToEnd() + try? handle.write(contentsOf: Data(line.utf8)) + try? handle.close() + } else { + try? line.write(to: history, atomically: true, encoding: .utf8) + } + save(sessionID, forNodeID: nodeID) + } + public static func load(forNodeID nodeID: UUID) -> String? { let url = file(forNodeID: nodeID) guard let text = try? String(contentsOf: url, encoding: .utf8) else { return nil } diff --git a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift index 115b82e7..33dc4844 100644 --- a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift +++ b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift @@ -1550,19 +1550,12 @@ public enum ZmxSessionLauncher { // no session a keystroke can reach, so the run branch relaunches it — this is // what lets an ensure, a send, or the sweep wake a loop that died unattended // (issue #215), which a `zmx get` check could never do. - let sessionID: String? = - SessionIDStore.load(forNodeID: node.id) - ?? { - switch node.backend { - case .copilotCLI: - let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName - return CopilotSessionLog.directory(forSessionNamed: name)?.lastPathComponent - case .claudeCode, .codex, .openCode: - return nil - } - }() + let sessionID = resumableSessionID(forNodeID: node.id, backend: node.backend) guard let runArgs = arguments(forNode: node, projectPath: projectPath) else { return } let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName + let copilotSessionBefore = + node.backend == .copilotCLI + ? CopilotSessionLog.directory(forSessionNamed: name)?.lastPathComponent : nil if let sessionID, let resumeArgs = resumeArguments( forNode: node, sessionID: sessionID, projectPath: projectPath) @@ -1581,7 +1574,14 @@ public enum ZmxSessionLauncher { // only if the session really failed to survive is the ID treated as dead: it is // dropped and the fresh launch runs. A resume that took is left alone, and its // `SessionStart` hook has already rebanked the same ID. - guard await sessionDiedImmediately(node: node) else { return } + guard await sessionDiedImmediately(node: node) else { + // A resume that took is the same conversation as before, so its ID is what a + // Copilot node banks — the only backend that cannot bank one itself. + if node.backend == .copilotCLI { + bankCopilotSessionID(sessionID, forNodeID: node.id, workingDirectory: wd ?? "") + } + return + } DialLog.record(session: name, dial: "ensure", event: "resume-dead") SessionIDStore.remove(forNodeID: node.id) } @@ -1598,18 +1598,73 @@ public enum ZmxSessionLauncher { case .codex, .openCode: break } } - // Noted *before* the launch: the first pass below waits for a Copilot session - // directory that was not already there, which is how it tells a session it just - // started from one this ensure found already running. - let copilotSessionBefore = - firstPassMessage(for: node) != nil - ? CopilotSessionLog.directory(forSessionNamed: name)?.lastPathComponent : nil + // `copilotSessionBefore` was noted *before* the launch: the first pass and the ID + // bank below both wait for a Copilot session directory that was not already there, + // which is how they tell a session this ensure just started from one it found + // already running. await atomicCheckOrRun( checkCommand: aliveCheck, runArguments: runArgs, zmxPath: zmxPath, workingDirectory: wd, logFragment: DialLog.fragment(session: name, dial: "ensure", event: "fresh")) await kickOffFirstPass( of: node, sessionNamed: name, projectPath: projectPath, after: copilotSessionBefore) + await bankCopilotSessionIDWhenItAppears( + forNode: node, sessionNamed: name, workingDirectory: wd, after: copilotSessionBefore) + } + + /// The ID a node's session would resume from: the banked pointer, or for Copilot — the + /// one backend with no hook to bank its own — the session-state directory carrying + /// the `--name` graphcode launched it with. Public because the app's open path + /// (`GhosttyTerminalView.localResumeOrFreshCommand`) must make the same choice: for + /// as long as it read only the pointer, a local Copilot loop whose session was gone + /// was relaunched from its goal under the same name, and nothing logged the duplicate. + public static func resumableSessionID(forNodeID nodeID: UUID, backend: CLISessionBackendKind) + -> String? + { + if let banked = SessionIDStore.load(forNodeID: nodeID) { return banked } + guard backend == .copilotCLI else { return nil } + let name = SurfaceRef(id: nodeID, launchesClaudeCode: true).zmxSessionName + return CopilotSessionLog.directory(forSessionNamed: name)?.lastPathComponent + } + + /// Banks a Copilot session's resume ID on this machine — the local twin of + /// `CopilotSessionLog.remoteIDBankFragment`, and the same dial-log line. Before this + /// the daemon rediscovered the directory on every ensure and the app never looked, so + /// the two launchers could disagree on whether there was anything to resume. + public static func bankCopilotSessionID( + _ sessionID: String, forNodeID nodeID: UUID, workingDirectory: String + ) { + guard SessionIDStore.load(forNodeID: nodeID) != sessionID else { return } + SessionIDStore.bank(sessionID, forNodeID: nodeID, workingDirectory: workingDirectory) + let name = SurfaceRef(id: nodeID, launchesClaudeCode: true).zmxSessionName + DialLog.record(session: name, dial: "bank", event: "copilot-id") + } + + /// After a fresh ensure, waits for the directory a just-launched Copilot creates — + /// one that was not there before — and banks it. A session the ensure found already + /// running makes no new directory; when the wait runs out, the one seen before the + /// launch is banked instead, since that is the session still running. + private static func bankCopilotSessionIDWhenItAppears( + forNode node: LoopNode, sessionNamed name: String, workingDirectory: String?, + after previous: String? + ) async { + guard node.backend == .copilotCLI else { return } + Task { + guard await NodeTickets.copilotBank.claim(node.id) else { return } + defer { Task { await NodeTickets.copilotBank.release(node.id) } } + let deadline = Date().addingTimeInterval(firstPassWaitSeconds) + var observed: String? + while Date() < deadline, observed == nil { + let current = CopilotSessionLog.directory(forSessionNamed: name)?.lastPathComponent + if let current, current != previous { + observed = current + } else { + try? await Task.sleep(for: .seconds(firstPassPollSeconds)) + } + } + guard let sessionID = observed ?? previous else { return } + bankCopilotSessionID(sessionID, forNodeID: node.id, workingDirectory: workingDirectory ?? "") + } } /// The message that gives a Copilot time-based loop the pass its schedule will not give @@ -1655,8 +1710,8 @@ public enum ZmxSessionLauncher { ) async { guard let message = firstPassMessage(for: node), let projectPath else { return } Task { - guard await FirstPassTickets.shared.claim(node.id) else { return } - defer { Task { await FirstPassTickets.shared.release(node.id) } } + guard await NodeTickets.firstPass.claim(node.id) else { return } + defer { Task { await NodeTickets.firstPass.release(node.id) } } let deadline = Date().addingTimeInterval(firstPassWaitSeconds) var observed: String? while Date() < deadline, observed == nil { @@ -1770,8 +1825,9 @@ public enum ZmxSessionLauncher { /// One first-pass message per node at a time. Two ensure ticks that both see a fresh /// session would otherwise type the task in twice. - private actor FirstPassTickets { - static let shared = FirstPassTickets() + private actor NodeTickets { + static let firstPass = NodeTickets() + static let copilotBank = NodeTickets() private var claimed: Set = [] func claim(_ id: UUID) -> Bool { claimed.insert(id).inserted } func release(_ id: UUID) { claimed.remove(id) } diff --git a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift index 9ed029ae..2b221e89 100644 --- a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift +++ b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift @@ -353,52 +353,79 @@ struct GhosttyTerminalView: NSViewRepresentable { /// at them. Every reboot afterwards faithfully resumed the near-empty replacements. /// /// So this path resumes too, and the two launchers now make the same choice from the - /// same banked ID. The check-then-launch is one `/bin/sh` script rather than an argv + /// same banked ID. The check-then-launch is one shell script rather than an argv /// because the decision has to be made *here*, on the machine, at the moment the pane /// opens: whether a session exists, and whether the resume survived, are both facts /// only the shell holding the terminal can see. /// + /// Every local agent launch takes this script — there is no silent branch. The one + /// there used to be, a bare `zmx attach ` for a node with + /// nothing banked, is how a Copilot loop (no hook to bank its own ID; the daemon found + /// its session directory by name, this view never looked) was relaunched from its goal + /// under the same `--name` after every reboot, a second Copilot session for one loop + /// that no dial log ever recorded. Now the ID is discovered and banked first + /// (`ZmxSessionLauncher.resumableSessionID`), and a fresh launch says so. + /// + /// The liveness check is the daemon's husk-aware one (`daemonReadyCheckCommand`), not + /// `zmx get`: a session whose agent died leaves its shell at a prompt, which answers + /// `zmx get` for as long as the machine stays up. Attaching to that showed the corpse + /// and nothing more. Such a husk is killed and the launch proceeds as if the session + /// were gone, which keeps the resume-or-fresh verdict below measurable. + /// /// Deliberately *not* consuming the ID up front, which is what the remote path does: /// there, one restorer owns the loop, and here the daemon's ensure may be running the /// same resume concurrently. Two consumers racing on one `rm` is how the loser falls /// through to a fresh launch — the very failure this exists to end. It is dropped only /// once a resume has been *seen* to fail, which is also the check the daemon makes. /// - /// `nil` for a backend that cannot resume, or a node with nothing banked: both take - /// the ordinary fresh launch. + /// `nil` only for a surface that is not a node's; the plain agent argv is what that + /// gets. func localResumeOrFreshCommand(agentLaunch: [String]) -> [String]? { + guard let nodeID = SurfaceRef.nodeID(fromZmxSessionName: sessionName) else { return nil } let settings = GraphcodeSettingsStore.load() - guard let nodeID = SurfaceRef.nodeID(fromZmxSessionName: sessionName), - SessionIDStore.load(forNodeID: nodeID) != nil, - let resumeLaunch = resumeCommand( - settings: settings, hooksFile: presenceHooksFile(), remoteSettingsPath: nil) - else { return nil } + if SessionIDStore.load(forNodeID: nodeID) == nil, + let discovered = ZmxSessionLauncher.resumableSessionID(forNodeID: nodeID, backend: backend) + { + ZmxSessionLauncher.bankCopilotSessionID( + discovered, forNodeID: nodeID, workingDirectory: workingDirectory ?? "") + } + let resumeLaunch = resumeCommand( + settings: settings, hooksFile: presenceHooksFile(), remoteSettingsPath: nil) let zmx = ZmxLocator.binaryURL.path let quoted = RemoteProjectLocation.shellQuoted let idFile = quoted(SessionIDStore.file(forNodeID: nodeID).path) let attach = ZmxSessionLauncher.quotedCommand([zmx, "attach", sessionName]) - let resume = ZmxSessionLauncher.quotedCommand( - [zmx, "attach", sessionName] + resumeLaunch) let fresh = ZmxSessionLauncher.quotedCommand([zmx, "attach", sessionName] + agentLaunch) - let exists = ZmxSessionLauncher.quotedCommand([zmx, "get", sessionName]) - // A live session is joined as it always was — the resume argv would be ignored by - // `zmx attach` anyway, and building it costs a settings read nobody needs. + let live = ZmxSessionLauncher.daemonReadyCheckCommand( + zmxPath: zmx, sessionName: sessionName, executable: nil) + let answers = ZmxSessionLauncher.quotedCommand([zmx, "get", sessionName]) + let kill = ZmxSessionLauncher.quotedCommand([zmx, "kill", sessionName]) let idVariable = ZmxSessionLauncher.remoteResumeIDVariable let settle = ZmxSessionLauncher.resumeSettleSeconds let log = { (event: String) in DialLog.fragment(session: self.sessionName, dial: "open", event: event) + "; " } - let joined = "\(exists) >/dev/null 2>&1 && { \(log("attach-live"))exec \(attach); }; " - let read = "\(idVariable)=$(cat \(idFile) 2>/dev/null); " - let attempt = - "if [ -n \"$\(idVariable)\" ]; then export \(idVariable); \(log("resume"))" - + "gc_t0=$(date +%s); " - + resume + "; gc_rc=$?; " - let verdict = - "[ $(($(date +%s) - gc_t0)) -ge \(settle) ] && exit \"$gc_rc\"; rm -f \(idFile); " - + log("resume-dead") - + #"printf '\033[1;33m── Resume did not take; starting fresh. ──\033[0m\r\n'; fi; "# - let script = joined + read + attempt + verdict + log("fresh") + "exec \(fresh)" - return ["/bin/sh", "-c", script] + // A live session is joined as it always was — the resume argv would be ignored by + // `zmx attach` anyway. + let joined = "\(live) >/dev/null 2>&1 && { \(log("attach-live"))exec \(attach); }; " + let revive = "\(answers) >/dev/null 2>&1 && { \(log("husk-killed"))\(kill) >/dev/null 2>&1; }; " + var script = joined + revive + if let resumeLaunch { + let resume = ZmxSessionLauncher.quotedCommand([zmx, "attach", sessionName] + resumeLaunch) + let read = "\(idVariable)=$(cat \(idFile) 2>/dev/null); " + let attempt = + "if [ -n \"$\(idVariable)\" ]; then export \(idVariable); \(log("resume"))" + + "gc_t0=$(date +%s); " + + resume + "; gc_rc=$?; " + let verdict = + "[ $(($(date +%s) - gc_t0)) -ge \(settle) ] && exit \"$gc_rc\"; rm -f \(idFile); " + + log("resume-dead") + + #"printf '\033[1;33m── Resume did not take; starting fresh. ──\033[0m\r\n'; fi; "# + script += read + attempt + verdict + } + script += log("fresh") + "exec \(fresh)" + // zsh, like the daemon's own check-or-run: the alive check's `$'\t'` needs a shell + // that reads ANSI-C quoting, which `/bin/sh` is only when it happens to be bash. + return ["/bin/zsh", "-c", script] } } diff --git a/graphcode/Tests/LocalSessionResumeTests.swift b/graphcode/Tests/LocalSessionResumeTests.swift index 573fc62e..26dfd04e 100644 --- a/graphcode/Tests/LocalSessionResumeTests.swift +++ b/graphcode/Tests/LocalSessionResumeTests.swift @@ -61,14 +61,120 @@ struct LocalSessionResumeTests { } @Test - func aNodeWithNothingBankedStillLaunchesFresh() { + func aNodeWithNothingBankedStillLaunchesFreshAndSaysSo() throws { + // The fresh launch used to be a bare argv with no log line — the one silent branch, + // and the one a user's `dials.log` could never explain. The decision is made by the + // script at run time from what is banked *then*, so the shape is the same either way. let nodeID = UUID() let view = surface(nodeID: nodeID) - let command = withBankedID(nil, forNode: nodeID) { - view.localResumeOrFreshCommand(agentLaunch: ["claude", "the prompt"]) + let script = try withBankedID(nil, forNode: nodeID) { + try #require(view.localResumeOrFreshCommand(agentLaunch: ["claude", "the prompt"])?.last) } - // Nothing to resume is the first launch, and the ordinary argv is what it gets. - #expect(command == nil) + #expect(script.contains("open fresh")) + #expect(script.contains("the prompt")) + #expect(script.contains(SessionIDStore.file(forNodeID: nodeID).path)) + } + + @Test + func aDeadShellLeftByTheAgentIsKilledBeforeTheRelaunch() throws { + // An agent that died inside its session leaves the shell at a prompt, which answers + // `zmx get` for as long as the machine stays up. The old check joined that corpse; + // the daemon's husk-aware check (`ended=`) falls through, and the husk is killed so + // the resume-or-fresh verdict below it stays measurable. + let nodeID = UUID() + let view = surface(nodeID: nodeID) + let script = try withBankedID("abc-123", forNode: nodeID) { + try #require(view.localResumeOrFreshCommand(agentLaunch: ["claude", "the prompt"])?.last) + } + #expect(script.contains("ended=")) + #expect(script.contains("open husk-killed")) + let kill = try #require(script.range(of: "'kill'")) + let resume = try #require(script.range(of: "open resume")) + #expect(kill.upperBound < resume.lowerBound) + // zsh, so the alive check's `$'\t'` is read as a tab wherever `/bin/sh` points. + #expect(view.localResumeOrFreshCommand(agentLaunch: ["claude"])?.first == "/bin/zsh") + } + + // MARK: - Copilot, which cannot bank its own ID + + @Test + func aCopilotLoopWithNothingBankedResumesFromItsSessionDirectory() throws { + // Copilot has no hook; on a remote host the daemon banks its directory name, locally + // nobody did. Opening such a loop with its session gone relaunched `copilot --name + // graphcode-` from the goal: a second Copilot session for one loop, every reboot, + // unlogged. The pane now discovers and banks the same ID the daemon would. + let nodeID = UUID() + let name = SurfaceRef(id: nodeID, launchesClaudeCode: true).zmxSessionName + let root = try temporaryRoot() + defer { try? FileManager.default.removeItem(at: root) } + let directory = try copilotSession(named: name, in: root) + CopilotSessionLog.stateDirectory = root + defer { + CopilotSessionLog.stateDirectory = FileManager.default.homeDirectoryForCurrentUser + .appendingPathComponent(".copilot/session-state", isDirectory: true) + } + let view = surface(nodeID: nodeID, backend: .copilotCLI) + let script = try withBankedID(nil, forNode: nodeID) { + let script = try #require( + view.localResumeOrFreshCommand(agentLaunch: ["copilot", "the prompt"])?.last) + #expect(SessionIDStore.load(forNodeID: nodeID) == directory.lastPathComponent) + #expect( + SessionIDStore.history(forNodeID: nodeID).last?.sessionID == directory.lastPathComponent) + return script + } + defer { try? FileManager.default.removeItem(at: SessionIDStore.historyFile(forNodeID: nodeID)) } + #expect(script.contains(#"--resume "$GRAPHCODE_RESUME_ID""#)) + } + + @Test + func theNewestDirectoryWithTheNameWinsAndTheBankIsIdempotent() throws { + // A loop that already has a duplicate must resume the one being written to now, and + // an ensure tick that finds the same ID banked must not grow the history. + let nodeID = UUID() + let name = SurfaceRef(id: nodeID, launchesClaudeCode: true).zmxSessionName + let root = try temporaryRoot() + defer { try? FileManager.default.removeItem(at: root) } + let older = try copilotSession(named: name, in: root) + try FileManager.default.setAttributes( + [.modificationDate: Date(timeIntervalSinceNow: -3600)], ofItemAtPath: older.path) + let newer = try copilotSession(named: name, in: root) + CopilotSessionLog.stateDirectory = root + defer { + CopilotSessionLog.stateDirectory = FileManager.default.homeDirectoryForCurrentUser + .appendingPathComponent(".copilot/session-state", isDirectory: true) + } + defer { + SessionIDStore.remove(forNodeID: nodeID) + try? FileManager.default.removeItem(at: SessionIDStore.historyFile(forNodeID: nodeID)) + } + let discovered = ZmxSessionLauncher.resumableSessionID(forNodeID: nodeID, backend: .copilotCLI) + #expect(discovered == newer.lastPathComponent) + ZmxSessionLauncher.bankCopilotSessionID( + newer.lastPathComponent, forNodeID: nodeID, workingDirectory: projectPath) + ZmxSessionLauncher.bankCopilotSessionID( + newer.lastPathComponent, forNodeID: nodeID, workingDirectory: projectPath) + #expect(SessionIDStore.history(forNodeID: nodeID).count == 1) + #expect(SessionIDStore.history(forNodeID: nodeID).first?.workingDirectory == projectPath) + // Once banked, the pointer is the answer — for a Claude node there is no directory + // to fall back to at all. + #expect( + ZmxSessionLauncher.resumableSessionID(forNodeID: nodeID, backend: .claudeCode) + == newer.lastPathComponent) + } + + private func temporaryRoot() throws -> URL { + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("copilot-resume-\(UUID().uuidString)", isDirectory: true) + try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true) + return root + } + + private func copilotSession(named name: String, in root: URL) throws -> URL { + let directory = root.appendingPathComponent(UUID().uuidString, isDirectory: true) + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + try "client_name: github/cli\nname: \(name)\n".write( + to: directory.appendingPathComponent("workspace.yaml"), atomically: true, encoding: .utf8) + return directory } @Test