From a4228400d482ccdd6f14e90579c2f44680983207 Mon Sep 17 00:00:00 2001 From: scgopi Date: Wed, 2 Sep 2026 07:46:20 -0700 Subject: [PATCH] Kill a husk only on positive evidence of death, and never bank a stale Copilot directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of #248 found the pane's husk branch keyed off the liveness check failing, which also happens on a listing row zmx marks `err=` — a busy daemon missing its one-second probe, which zmx itself refuses to clean up on. Opening a loop under load could `zmx kill` a live agent mid-turn. The kill now requires the session's own row to carry `ended=`; the same rule keeps it from racing the daemon, whose relaunch into a husk clears that mark the instant the command is typed in. The daemon's Copilot bank task no longer falls back to the pre-launch directory when no new one appears within the wait: a cold `copilot` still at its trust dialog would have had the old, dead conversation banked over it, recreating the orphaning #248 set out to end. Nothing banked costs nothing — discovery by newest directory already covers it. History lines are appended with O_APPEND rather than seek-then-write, since the daemon's bank task and the app's bank-on-open can now land on the same file at once. The resume tests are serialized: they swap the process-global Copilot state directory. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01Tqv8Rx8SGsLCjJQYCUjiFr --- .../Sources/Sessions/SessionIDStore.swift | 15 ++++++------ .../Sources/Sessions/ZmxSessionLauncher.swift | 23 +++++++++++++++---- .../Ghostty/GhosttyTerminalView.swift | 13 +++++++++-- graphcode/Tests/LocalSessionResumeTests.swift | 13 ++++++++--- 4 files changed, 48 insertions(+), 16 deletions(-) diff --git a/GraphcodeKit/Sources/Sessions/SessionIDStore.swift b/GraphcodeKit/Sources/Sessions/SessionIDStore.swift index 74902eb2..edcbe66a 100644 --- a/GraphcodeKit/Sources/Sessions/SessionIDStore.swift +++ b/GraphcodeKit/Sources/Sessions/SessionIDStore.swift @@ -75,13 +75,14 @@ public enum SessionIDStore { 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) + // `O_APPEND`, not seek-then-write: the daemon's bank task and the app's bank-on-open + // can land on the same file in the same instant, and only an append-mode write + // keeps both lines. + let descriptor = open( + historyFile(forNodeID: nodeID).path, O_WRONLY | O_APPEND | O_CREAT, 0o644) + if descriptor >= 0 { + _ = Array(line.utf8).withUnsafeBytes { write(descriptor, $0.baseAddress, $0.count) } + close(descriptor) } save(sessionID, forNodeID: nodeID) } diff --git a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift index 33dc4844..74cd46f9 100644 --- a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift +++ b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift @@ -508,6 +508,17 @@ public enum ZmxSessionLauncher { return command } + /// Positive evidence that a session's task has ended: its listing row is there and + /// carries `ended=`. The complement of `daemonReadyCheckCommand` is *not* this — that + /// check also fails on a row marked `err=` (a busy daemon missing zmx's one-second + /// probe) and on an empty listing, neither of which says the agent is gone. Anything + /// destructive keys off this one. + public static func sessionEndedCheckCommand(zmxPath: String, sessionName: String) -> String { + let name = RemoteProjectLocation.shellQuoted("name=\(sessionName)\t") + return RemoteProjectLocation.shellQuoted(zmxPath) + + " ls 2>/dev/null | grep " + name + " | grep -q $'\\tended='" + } + public static func waitingAttachCommand( zmxPath: String, sessionName: String, executable: String? ) -> [String] { @@ -1641,9 +1652,13 @@ public enum ZmxSessionLauncher { } /// 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. + /// one that was not there before — and banks it. Nothing is banked when the wait runs + /// out: this cannot tell a session the ensure found already running (no new directory, + /// ever) from a cold `copilot` still at its trust dialog (a new directory, later), and + /// banking the old directory in the second case would point the next resume at the + /// dead conversation — the orphaning this whole change exists to end. An unbanked + /// Copilot node loses nothing: `resumableSessionID` falls back to the newest directory + /// with its name, and the next resume that takes banks it. private static func bankCopilotSessionIDWhenItAppears( forNode node: LoopNode, sessionNamed name: String, workingDirectory: String?, after previous: String? @@ -1662,7 +1677,7 @@ public enum ZmxSessionLauncher { try? await Task.sleep(for: .seconds(firstPassPollSeconds)) } } - guard let sessionID = observed ?? previous else { return } + guard let sessionID = observed else { return } bankCopilotSessionID(sessionID, forNodeID: node.id, workingDirectory: workingDirectory ?? "") } } diff --git a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift index 2b221e89..ebf7e010 100644 --- a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift +++ b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift @@ -372,6 +372,14 @@ struct GhosttyTerminalView: NSViewRepresentable { /// 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. /// + /// The kill needs *positive* evidence of death — a listing row carrying `ended=` + /// (`sessionEndedCheckCommand`) — not the liveness check's failure. That check also + /// fails on a row zmx marks `err=`, which is a busy daemon missing its one-second probe + /// and nothing more; zmx itself refuses to clean up on that. Killing on it would take a + /// live agent down mid-turn at exactly the moment a human opens loops after a reboot, + /// and the same evidence rule keeps this from racing the daemon: its relaunch into a + /// husk clears the `ended=` mark the instant the command is typed in. + /// /// 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 @@ -398,7 +406,8 @@ struct GhosttyTerminalView: NSViewRepresentable { let fresh = ZmxSessionLauncher.quotedCommand([zmx, "attach", sessionName] + agentLaunch) let live = ZmxSessionLauncher.daemonReadyCheckCommand( zmxPath: zmx, sessionName: sessionName, executable: nil) - let answers = ZmxSessionLauncher.quotedCommand([zmx, "get", sessionName]) + let ended = ZmxSessionLauncher.sessionEndedCheckCommand( + zmxPath: zmx, sessionName: sessionName) let kill = ZmxSessionLauncher.quotedCommand([zmx, "kill", sessionName]) let idVariable = ZmxSessionLauncher.remoteResumeIDVariable let settle = ZmxSessionLauncher.resumeSettleSeconds @@ -408,7 +417,7 @@ struct GhosttyTerminalView: NSViewRepresentable { // 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; }; " + let revive = "\(ended) >/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) diff --git a/graphcode/Tests/LocalSessionResumeTests.swift b/graphcode/Tests/LocalSessionResumeTests.swift index 26dfd04e..9b9027df 100644 --- a/graphcode/Tests/LocalSessionResumeTests.swift +++ b/graphcode/Tests/LocalSessionResumeTests.swift @@ -16,7 +16,7 @@ import Testing /// days of work, leaving those transcripts on disk with nothing naming them — and every /// reboot afterwards resumed the near-empty replacements. Observed 2026-08-11 on three /// loops at once. -@Suite +@Suite(.serialized) struct LocalSessionResumeTests { private let projectPath = "/tmp/widget" @@ -55,8 +55,9 @@ struct LocalSessionResumeTests { } #expect(script.contains(#"--resume "$GRAPHCODE_RESUME_ID""#)) #expect(script.contains(SessionIDStore.file(forNodeID: nodeID).path)) - // A live session is still just joined — the same reattach the pane always did. - #expect(script.contains("'get'")) + // A live session is still just joined — the same reattach the pane always did, now + // behind the daemon's husk-aware listing check rather than a bare `zmx get`. + #expect(script.contains("ls 2>/dev/null")) #expect(script.contains("'attach'")) } @@ -91,6 +92,12 @@ struct LocalSessionResumeTests { let kill = try #require(script.range(of: "'kill'")) let resume = try #require(script.range(of: "open resume")) #expect(kill.upperBound < resume.lowerBound) + // The kill keys off positive evidence — a listing row carrying `ended=` — not off + // the liveness check failing: that also fails on an `err=` row, which is a busy + // daemon missing its probe, and killing on it would take a live agent down. + let ended = try #require(script.range(of: #"grep -q $'\tended='"#)) + #expect(ended.upperBound < kill.lowerBound) + #expect(!script.contains("'get'")) // zsh, so the alive check's `$'\t'` is read as a tab wherever `/bin/sh` points. #expect(view.localResumeOrFreshCommand(agentLaunch: ["claude"])?.first == "/bin/zsh") }