From ed71da120ae35865f756c03a6f631d17825d4230 Mon Sep 17 00:00:00 2001 From: scgopi Date: Tue, 1 Sep 2026 21:59:27 -0700 Subject: [PATCH 1/2] Start duplicate-session investigation Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Tqv8Rx8SGsLCjJQYCUjiFr From 4a4bb841cf63d729f00da0203601ad02604fa193 Mon Sep 17 00:00:00 2001 From: scgopi Date: Wed, 2 Sep 2026 06:49:45 -0700 Subject: [PATCH 2/2] Resume a local Copilot loop instead of relaunching it under the same name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user's dials.log showed loops resumed by the daemon after every restart and nothing else — no duplicate visible, because the launch that makes one never wrote a line. Copilot has no hook to bank its resume ID, and only the remote path banked it from outside; locally the daemon rediscovered the session directory on every ensure while the app's open path read only the pointer. Any local Copilot loop whose zmx session was gone (a reboot, or a turn-based loop, which the daemon never ensures) was relaunched by the pane as `copilot --name graphcode-` from its goal: a second Copilot session for one loop, and the daemon's next resume picked the newest directory, orphaning the original. - The daemon banks a Copilot session's directory name locally, the twin of the remote bank fragment: after a resume that took, and after a fresh launch once the new directory appears. - The pane makes the same choice from the same lookup (`ZmxSessionLauncher.resumableSessionID`), banking a discovered ID before its script reads it. - The pane's liveness check is the daemon's husk-aware one. An agent that died leaves its shell at a prompt, which answered `zmx get`; the pane joined the corpse. The husk is killed and the launch proceeds as if the session were gone, so the resume-or-fresh verdict stays measurable. - Every local agent launch goes through the one script and logs its branch, so `open fresh` appears in the dial log where it used to be silent. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Tqv8Rx8SGsLCjJQYCUjiFr --- .../Sources/Sessions/SessionIDStore.swift | 19 +++ .../Sources/Sessions/ZmxSessionLauncher.swift | 100 +++++++++++---- .../Ghostty/GhosttyTerminalView.swift | 77 ++++++++---- graphcode/Tests/LocalSessionResumeTests.swift | 116 +++++++++++++++++- 4 files changed, 260 insertions(+), 52 deletions(-) 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