From 29034b5febda955dcd6b5609b5b33b64ede3bd8e Mon Sep 17 00:00:00 2001 From: scgopi Date: Thu, 3 Sep 2026 21:09:02 -0700 Subject: [PATCH 1/6] Start #272: Codex readiness label Branch marker so the hygiene sweep leaves this worktree alone. Co-Authored-By: Claude Opus 5 (1M context) From e6c951d2c0a82f05497a1362bcf129362728b12e Mon Sep 17 00:00:00 2001 From: scgopi Date: Thu, 3 Sep 2026 21:21:54 -0700 Subject: [PATCH 2/6] Wait on a label graphcode writes, not a command zmx never records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every Codex pane in the product failed to attach. It polled `zmx ls` at 10 Hz for sixty seconds, printed "never became ready to attach", exited, and left the human at a bare login shell — while the codex agent underneath ran fine and answered its goal headless with clients=0. The gate #228 added waits for proof that the daemon's session really is the Codex one, by grepping the session's `cmd=` field for the agent's name. That proof does not exist for a loop: zmx records a command only for `attach` (main.zig:217) and hardcodes `.command = null` for `run` (:259), and `zmx ls` prints the field only when it is non-nil (util.zig:911). Graphcode launches every loop with `zmx run -d`. Measured on a live machine: of 56 sessions, 7 carried a `cmd=` at all — the seven created by `attach` — and 6 of those were truncated to `/bin/zsh -i -l -c...` at zmx's 256-byte cap, which is a second reason the same grep could never match. So the gate was unsatisfiable, and it is `aliveCheckCommand` too: for a Codex node it was permanently false at the pane's attach, at the local atomic ensure (which therefore always took the `zmx run` branch — the very race the ensure exists to prevent), at the remote ensure, and at the remote send gate, where a message edge to a Codex loop silently did nothing. The gate now waits on `agent=`, a label graphcode writes itself on the channel `presence=` and `usage=` already travel on. The daemon writes it in the run branch of the ensure, after the launch it describes and only if that launch was made. Not from inside the launch script: what `zmx run` carries is typed into the session's tty, a canonical-mode tty drops everything past MAX_CANON (issue #57), and a stamp in there tipped theLaunchStillFitsInATypedCommandLine red on the first attempt. Out here it costs the typed line nothing. The regression test runs a real `zmx run -d` session and greps its real `zmx ls` output, through a PTY because zmx wants a terminal to create a session against. That shape is deliberate: the tests that existed asserted only that the pattern was *built*, and a synthetic attach-shaped `ls` line would have passed while the product stayed broken. One of its assertions is the old pattern failing against a real session — the bug itself, pinned. Fixes #272. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KMDUe3kmSwvx4Bth3xM6qF --- .../Sources/Sessions/ZmxSessionLauncher.swift | 74 +++++++++-- .../Ghostty/GhosttyTerminalView.swift | 2 +- .../Tests/AttachedSessionBriefingTests.swift | 2 +- graphcode/Tests/CodexReadinessGateTests.swift | 117 ++++++++++++++++++ graphcode/Tests/ZmxSessionLauncherTests.swift | 9 +- 5 files changed, 187 insertions(+), 17 deletions(-) create mode 100644 graphcode/Tests/CodexReadinessGateTests.swift diff --git a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift index 93b9786e..d9a42ffe 100644 --- a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift +++ b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift @@ -89,6 +89,34 @@ public enum ZmxSessionLauncher { /// than ride through `"$@"`: positional arguments pass untouched, so a `$HOME` in one /// stays a literal dollar sign forever. The remote hooks flag needs the expansion — /// the path is on a machine whose home directory only that shell knows. + /// `zmx set agent=` — the stamp that says graphcode has launched this + /// backend into this session, and the signal `daemonReadyCheckCommand` waits on. + /// + /// It replaces a gate that could never pass. That one grepped `zmx ls` for the agent's + /// name in the session's `cmd=` field, and a session created by `zmx run` has no `cmd=` + /// field at all: zmx records a command only for `attach` (`main.zig:217`, against + /// `.command = null` for `run` at `:259`) and prints the field only when it is non-nil + /// (`util.zig:911`). Graphcode launches every loop with `zmx run -d`, so the gate was + /// unsatisfiable — every Codex pane spun out its sixty seconds and dropped the human + /// into a bare login shell while the agent underneath it ran fine (issue #272). + /// + /// Written by the daemon in the same breath as the launch rather than from inside the + /// launch script, for one measured reason: what `zmx run` carries is *typed* into the + /// session's tty, and a canonical-mode tty drops everything past `MAX_CANON` + /// (issue #57). A stamp inside that argument spends a budget the Codex launch has + /// already nearly exhausted — it tipped `theLaunchStillFitsInATypedCommandLine` red the + /// first time it was tried. Out here it costs the typed line nothing. + static func agentLabelCommand(zmxPath: String, forNode node: LoopNode) -> String { + let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName + return RemoteProjectLocation.shellQuoted(zmxPath) + " set " + + RemoteProjectLocation.shellQuoted(name) + " " + + RemoteProjectLocation.shellQuoted("\(Self.agentLabelKey)=\(node.backend.rawValue)") + + " >/dev/null 2>&1" + } + + /// The label key `agentLabelCommand` writes and `daemonReadyCheckCommand` reads. + static let agentLabelKey = "agent" + static func loginShellInvocation( of command: String, arguments: [String], environment: [String: String] = [:], scriptSuffix: String = "" @@ -489,30 +517,38 @@ public enum ZmxSessionLauncher { let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName return daemonReadyCheckCommand( zmxPath: zmxPath, sessionName: name, - executable: node.backend == .codex ? node.backend.executableName : nil) + agent: node.backend == .codex ? node.backend.rawValue : nil) } + /// `agent` is a `CLISessionBackendKind.rawValue`, the same value the launch script + /// stamps (`agentLabelStatement`) — one source for the write and the read, so a backend + /// whose binary is named differently from its case (`claudeCode` → `claude`) cannot + /// make the two halves disagree. public static func daemonReadyCheckCommand( - zmxPath: String, sessionName: String, executable: String? + zmxPath: String, sessionName: String, agent: String? ) -> String { let name = RemoteProjectLocation.shellQuoted("name=\(sessionName)\t") var command = RemoteProjectLocation.shellQuoted(zmxPath) + " ls 2>/dev/null | grep -v -e $'\\tended=' -e $'\\terr=' | grep -q " + name - if let executable { + if let agent { + // The label graphcode writes (`agentLabelStatement`), not the process's own command + // line: `zmx ls` never shows a `cmd=` for a `zmx run` session, so the grep this + // replaced could not match however long it waited (issue #272). command += " && " + RemoteProjectLocation.shellQuoted(zmxPath) + " ls 2>/dev/null | grep -v -e $'\\tended=' -e $'\\terr=' | grep -q " - + RemoteProjectLocation.shellQuoted("name=\(sessionName)\t.*cmd=.*\(executable)") + + RemoteProjectLocation.shellQuoted( + "name=\(sessionName)\t.*\t\(Self.agentLabelKey)=\(agent)") } return command } public static func waitingAttachCommand( - zmxPath: String, sessionName: String, executable: String? + zmxPath: String, sessionName: String, agent: String? ) -> [String] { let check = daemonReadyCheckCommand( - zmxPath: zmxPath, sessionName: sessionName, executable: executable) + zmxPath: zmxPath, sessionName: sessionName, agent: agent) let attach = RemoteProjectLocation.shellQuoted(zmxPath) + " attach " + RemoteProjectLocation.shellQuoted(sessionName) @@ -1132,11 +1168,17 @@ public enum ZmxSessionLauncher { return "" } }() + // The readiness stamp closes the create branch, after the launch it describes + // (`agentLabelCommand`, issue #272). Joined with `;` rather than `&&` because the + // create script is an if/else whose exit status belongs to whichever branch ran — + // and it is safe to run unconditionally here, since `zmx set` against a session that + // was never created fails and leaves no label for the gate to find. + let stamp = agentLabelCommand(zmxPath: "zmx", forNode: node) let script = "cd \(RemoteProjectLocation.shellQuoted(location.remotePath)) && { " + deliveryFragment(delivery, ifSessionMissing: check) + "\(check) >/dev/null 2>&1\(bank) || { " + trustSeed + hooksWrite - + "\(create); }; }" + + "\(create); \(stamp); }; }" return location.sshInvocation(remoteCommand: location.remoteLoginShellCommand(script)) } @@ -1573,7 +1615,8 @@ public enum ZmxSessionLauncher { await atomicCheckOrRun( checkCommand: aliveCheck, runArguments: resumeArgs, zmxPath: zmxPath, workingDirectory: wd, - logFragment: DialLog.fragment(session: name, dial: "ensure", event: "resume")) + logFragment: DialLog.fragment(session: name, dial: "ensure", event: "resume"), + stampCommand: agentLabelCommand(zmxPath: zmxPath, forNode: node)) // `zmx run -d` reports that the *session* exists, not that what it launched // survived: `claude --resume` against a transcript its retention expired dies // within a second — and the wrapper shell it was typed with stays behind, a @@ -1610,7 +1653,8 @@ public enum ZmxSessionLauncher { await atomicCheckOrRun( checkCommand: aliveCheck, runArguments: runArgs, zmxPath: zmxPath, workingDirectory: wd, - logFragment: DialLog.fragment(session: name, dial: "ensure", event: "fresh")) + logFragment: DialLog.fragment(session: name, dial: "ensure", event: "fresh"), + stampCommand: agentLabelCommand(zmxPath: zmxPath, forNode: node)) await kickOffFirstPass( of: node, sessionNamed: name, projectPath: projectPath, after: copilotSessionBefore) } @@ -1805,12 +1849,18 @@ public enum ZmxSessionLauncher { /// woken — the husk answered every `zmx get` (#215). private static func atomicCheckOrRun( checkCommand: String, runArguments: [String], - zmxPath: String, workingDirectory: String?, logFragment: String? = nil + zmxPath: String, workingDirectory: String?, logFragment: String? = nil, + stampCommand: String? = nil ) async { let run = quotedCommand([zmxPath] + runArguments) + // The stamp rides in the run branch, after the launch it describes and only if that + // launch was made: it is what the readiness gate reads, so writing it anywhere a + // session might not exist would say the session is ready when it is not + // (`agentLabelCommand`, issue #272). + let launch = stampCommand.map { "\(run) && \($0)" } ?? run let script = - logFragment.map { "\(checkCommand) >/dev/null 2>&1 || { \($0); \(run); }" } - ?? "\(checkCommand) >/dev/null 2>&1 || \(run)" + logFragment.map { "\(checkCommand) >/dev/null 2>&1 || { \($0); \(launch); }" } + ?? "\(checkCommand) >/dev/null 2>&1 || \(launch)" guard let session = try? PTYProcessSession( executable: "/bin/zsh", arguments: ["-c", script], diff --git a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift index f6e86f0d..6519a4ac 100644 --- a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift +++ b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift @@ -327,7 +327,7 @@ struct GhosttyTerminalView: NSViewRepresentable { if defersCodexLaunchToDaemon { return ZmxSessionLauncher.waitingAttachCommand( zmxPath: ZmxLocator.binaryURL.path, sessionName: sessionName, - executable: backend.executableName) + agent: backend.rawValue) } if let resuming = localResumeOrFreshCommand(agentLaunch: agentCommand) { return resuming diff --git a/graphcode/Tests/AttachedSessionBriefingTests.swift b/graphcode/Tests/AttachedSessionBriefingTests.swift index fa6c417b..ce880da9 100644 --- a/graphcode/Tests/AttachedSessionBriefingTests.swift +++ b/graphcode/Tests/AttachedSessionBriefingTests.swift @@ -76,7 +76,7 @@ struct AttachedSessionBriefingTests { let view = surface(.codex, loopType: .goalBased) let command = view.command(briefingPath: briefing) #expect(command.first == "/bin/zsh") - #expect(command.contains { $0.contains("until") && $0.contains("cmd=.*codex") }) + #expect(command.contains { $0.contains("until") && $0.contains("agent=codex") }) #expect(command.last?.contains("exec") == true) #expect(command.last?.contains("attach 's'") == true) } diff --git a/graphcode/Tests/CodexReadinessGateTests.swift b/graphcode/Tests/CodexReadinessGateTests.swift new file mode 100644 index 00000000..02b1840b --- /dev/null +++ b/graphcode/Tests/CodexReadinessGateTests.swift @@ -0,0 +1,117 @@ +import Foundation +import Testing + +@testable import GraphcodeKit + +/// The gate that decides a Codex session is ready to attach, exercised against a **real** +/// `zmx` session created the way graphcode creates every loop's: `zmx run -d`. +/// +/// Issue #272 is why this suite runs a real session instead of asserting a pattern. The +/// old gate looked for the agent's name in the session's `cmd=` field, and every test of +/// it checked only that the string was *built* — so nobody noticed that a `zmx run` +/// session has no `cmd=` field at all (zmx records a command for `attach` and hardcodes +/// `.command = null` for `run`, and `zmx ls` prints the field only when it is non-nil). +/// A synthetic, attach-shaped `ls` line would pass happily while every Codex pane in the +/// product spun out its sixty seconds and gave up. +/// +/// Commands go through `PTYProcessSession` for the same reason the daemon's own ensure +/// does: `zmx` wants a terminal to create a session against. +@Suite(.serialized) +struct CodexReadinessGateTests { + private static let zmx = ZmxLocator.binaryURL.path + + private static func quoted(_ value: String) -> String { + RemoteProjectLocation.shellQuoted(value) + } + + @discardableResult + private func run(_ command: String) async -> (succeeded: Bool, output: String) { + guard + let session = try? PTYProcessSession( + executable: "/bin/zsh", arguments: ["-c", command], workingDirectory: nil) + else { return (false, "") } + return await session.waitCollectingOutput() + } + + /// A live session of the exact shape graphcode launches, named after a real node so the + /// production stamp — which derives the session name from the node — addresses it. + private func start(_ node: LoopNode) async -> String? { + let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName + await run( + "\(Self.quoted(Self.zmx)) run \(Self.quoted(name)) -d /bin/zsh -c 'sleep 30'") + for _ in 0..<40 { + if await run(listing(name)).succeeded { return name } + try? await Task.sleep(for: .milliseconds(100)) + } + return nil + } + + private func listing(_ name: String) -> String { + "\(Self.quoted(Self.zmx)) ls 2>/dev/null | grep -q $'name=\(name)\\t'" + } + + private func kill(_ name: String) async { + await run("\(Self.quoted(Self.zmx)) kill \(Self.quoted(name)) >/dev/null 2>&1") + } + + private func codexNode() -> LoopNode { + LoopNode(title: "Codex", loopType: .goalBased, goal: GoalSpec(summary: "work"), backend: .codex) + } + + @Test + func aRunCreatedSessionNeverCarriesTheCommandTheOldGateLookedFor() async throws { + guard ZmxLocator.isInstalled else { return } + let name = try #require(await start(codexNode())) + defer { Task { await kill(name) } } + + let row = await run( + "\(Self.quoted(Self.zmx)) ls 2>/dev/null | grep -F \(Self.quoted("name=\(name)\t"))") + #expect(row.succeeded) + // The whole of #272 in one assertion: the field the old gate matched on is absent + // from a session created the way graphcode creates every one of them. + #expect(!row.output.contains("cmd=")) + + // So the old gate could not pass however long the pane waited for it. + let legacy = + "\(Self.quoted(Self.zmx)) ls 2>/dev/null | grep -q $'name=\(name)\\t.*cmd=.*codex'" + #expect(await !run(legacy).succeeded) + } + + @Test + func theGateFailsUntilTheDaemonStampsTheAgentLabel() async throws { + guard ZmxLocator.isInstalled else { return } + let node = codexNode() + let name = try #require(await start(node)) + defer { Task { await kill(name) } } + + // The production gate, run against a real session. + let gate = ZmxSessionLauncher.daemonReadyCheckCommand( + zmxPath: Self.zmx, sessionName: name, agent: CLISessionBackendKind.codex.rawValue) + #expect(await !run(gate).succeeded) + + // The production stamp, run the way the daemon's ensure runs it. + await run(ZmxSessionLauncher.agentLabelCommand(zmxPath: Self.zmx, forNode: node)) + #expect(await run(gate).succeeded) + + // A session running some *other* agent must still not satisfy a Codex gate — that is + // the whole point of the clause #228 added, and this keeps it. + let copilotGate = ZmxSessionLauncher.daemonReadyCheckCommand( + zmxPath: Self.zmx, sessionName: name, agent: CLISessionBackendKind.copilotCLI.rawValue) + #expect(await !run(copilotGate).succeeded) + } + + @Test + func theLabelTheDaemonWritesIsTheOneTheGateReads() { + // One source for the write and the read. The gate used to take `executableName` + // while nothing wrote anything, so a backend whose binary is named differently from + // its case (`claudeCode` → `claude`) could have made the two halves disagree. + let node = codexNode() + let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName + let stamp = ZmxSessionLauncher.agentLabelCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) + let gate = ZmxSessionLauncher.daemonReadyCheckCommand( + zmxPath: "/usr/local/bin/zmx", sessionName: name, + agent: CLISessionBackendKind.codex.rawValue) + #expect(stamp.contains("agent=codex")) + #expect(gate.contains("agent=codex")) + } +} diff --git a/graphcode/Tests/ZmxSessionLauncherTests.swift b/graphcode/Tests/ZmxSessionLauncherTests.swift index cd5bf439..caf221b5 100644 --- a/graphcode/Tests/ZmxSessionLauncherTests.swift +++ b/graphcode/Tests/ZmxSessionLauncherTests.swift @@ -221,16 +221,19 @@ struct ZmxSessionLauncherTests { title: "Codex", loopType: .goalBased, goal: GoalSpec(summary: "work"), backend: .codex) let command = ZmxSessionLauncher.aliveCheckCommand( zmxPath: "/usr/local/bin/zmx", forNode: node) - #expect(command.contains("cmd=.*codex")) + // The label graphcode writes, never the process command line: `zmx run` records no + // command, so `cmd=` is a field a loop session simply does not have (issue #272). + #expect(command.contains("agent=codex")) + #expect(!command.contains("cmd=")) } @Test func appWaitsForTheDaemonBeforeAttachingCodex() { let command = ZmxSessionLauncher.waitingAttachCommand( - zmxPath: "/usr/local/bin/zmx", sessionName: "graphcode-a", executable: "codex") + zmxPath: "/usr/local/bin/zmx", sessionName: "graphcode-a", agent: "codex") #expect(command.first == "/bin/zsh") #expect(command.last?.contains("until") == true) - #expect(command.last?.contains("cmd=.*codex") == true) + #expect(command.last?.contains("agent=codex") == true) #expect(command.last?.contains("exec '/usr/local/bin/zmx' attach 'graphcode-a'") == true) // A session the daemon never creates must not be polled at 10 Hz forever: the wait // gives up after a minute with a message instead of spinning. From b5680c4ff94a75b94f385b70ab594e7d0b6bca45 Mon Sep 17 00:00:00 2001 From: scgopi Date: Thu, 3 Sep 2026 21:23:56 -0700 Subject: [PATCH 3/6] Name the decoy in the gate's own comment `zmx ls` does truncate a long `cmd=` to a literal `...`, and the first reading of #272 led with that. It is not the mechanism, and someone raising the cap or dropping the ellipsis would change nothing while believing they had fixed this: a `zmx run` session has no `cmd=` to truncate at all. Said where the next person will be standing when the thought occurs to them. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KMDUe3kmSwvx4Bth3xM6qF --- GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift index d9a42ffe..52408652 100644 --- a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift +++ b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift @@ -100,6 +100,12 @@ public enum ZmxSessionLauncher { /// unsatisfiable — every Codex pane spun out its sixty seconds and dropped the human /// into a bare login shell while the agent underneath it ran fine (issue #272). /// + /// The decoy, for whoever reads this next: `zmx ls` *also* truncates a long `cmd=` to a + /// literal `...` at a 256-byte cap (`ipc.zig:58`, `main.zig:1322`), which looks like the + /// bug and is not. Raising that cap or dropping the ellipsis would change nothing here — + /// a loop session has no `cmd=` to truncate in the first place. Only sessions created by + /// `attach` carry one at all. + /// /// Written by the daemon in the same breath as the launch rather than from inside the /// launch script, for one measured reason: what `zmx run` carries is *typed* into the /// session's tty, and a canonical-mode tty drops everything past `MAX_CANON` From 188ae4bb28a702728b19500fc59061c4d3e2391c Mon Sep 17 00:00:00 2001 From: scgopi Date: Thu, 3 Sep 2026 21:45:27 -0700 Subject: [PATCH 4/6] Scope the readiness label to Codex, and stop a lost stamp costing a relaunch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three defects in the first cut, all in the same seam. The stamp decided the ensure's exit status. A `zmx set` that failed — ssh dropping mid-PTY-spawn on a loaded codespace is the case that found it — made the ensure exit 1, which is a retry, and the retry's check still failed because the label still was not there, so create ran again and `zmx run` typed the whole launch command into a session that was by then live. That is precisely the failure atomicCheckOrRun exists to prevent. The stamp is now `|| true`. Not, though, `{ create; stamp || true; }` as prescribed: with `;` the group's status becomes the stamp's, which swallows a genuinely failed create and disarms the retry chain that is supposed to fire for one. `create && { stamp || true; }` keeps a failed create failing and neutralises only the stamp. A lost stamp was also permanent. Nothing re-launches a healthy local loop — the liveness sweep filters to remote projects, the local ensure runs at graph load, node creation and promotion, and a local send checks sessionExists — so one transient failure left the loop unattachable exactly as in #272, but intermittently, which is worse than the bug it came from. The ensure now adopts a session that is alive under the node's name and carries no agent label at all, stamping it instead of relaunching it. Never over a label that disagrees: that is a session running the wrong agent, and relaunching it is the right answer. And the match was unanchored, so `agent=codexFoo` would satisfy a Codex gate. No backend's rawValue prefixes another today, which makes it a trap for whoever adds the one that does. Scoped to Codex throughout, per the human: it is the only backend whose session the daemon creates while the pane waits on it, and the other three had no part in #272. `readinessAgent` is the single place that decides, read by the gate, the stamp and the repair alike so they cannot drift. Five more tests, each against a real `zmx run -d` session: the repair adopts an unlabelled one, refuses one labelled for another agent, the gate rejects a prefix and accepts a trailing label, a stamp against a missing session really does fail (so `|| true` is load-bearing), and nothing is stamped for a backend the gate never judges. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KMDUe3kmSwvx4Bth3xM6qF --- .../Sources/Sessions/ZmxSessionLauncher.swift | 85 +++++++++++++--- graphcode/Tests/CodexReadinessGateTests.swift | 96 ++++++++++++++++++- .../Tests/RemoteSessionResumeTests.swift | 6 +- 3 files changed, 169 insertions(+), 18 deletions(-) diff --git a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift index 52408652..9a1eed55 100644 --- a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift +++ b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift @@ -112,7 +112,20 @@ public enum ZmxSessionLauncher { /// (issue #57). A stamp inside that argument spends a budget the Codex launch has /// already nearly exhausted — it tipped `theLaunchStillFitsInATypedCommandLine` red the /// first time it was tried. Out here it costs the typed line nothing. - static func agentLabelCommand(zmxPath: String, forNode node: LoopNode) -> String { + /// The agent a node's readiness is proved with, or `nil` for a backend whose sessions + /// are judged by name alone. + /// + /// Codex only, because Codex is the only backend whose session the daemon creates while + /// the pane waits on it (`defersCodexLaunchToDaemon`) — the race #228 found. Read from + /// one place by the gate, the stamp and the repair together, so they cannot drift into + /// judging different sets of loops; and kept narrow so a bug in any of the three cannot + /// reach a backend that was never part of this. + static func readinessAgent(forNode node: LoopNode) -> String? { + node.backend == .codex ? node.backend.rawValue : nil + } + + static func agentLabelCommand(zmxPath: String, forNode node: LoopNode) -> String? { + guard readinessAgent(forNode: node) != nil else { return nil } let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName return RemoteProjectLocation.shellQuoted(zmxPath) + " set " + RemoteProjectLocation.shellQuoted(name) + " " @@ -120,6 +133,34 @@ public enum ZmxSessionLauncher { + " >/dev/null 2>&1" } + /// Stamps a session that is alive under this node's name and carries **no** agent + /// label — the daemon adopting a session it must have launched itself. + /// + /// Without it, one failed `zmx set` is permanent. A stamp written only in the launch + /// branch is only ever retried by another launch, and nothing re-launches a healthy + /// local loop: the liveness sweep filters to remote projects, the local ensure runs at + /// graph load, node creation and promotion, and a local send checks `sessionExists` + /// rather than this gate. The loop would sit unattachable exactly as in #272, but + /// intermittently — which is worse than the deterministic bug it came from. + /// + /// Only when there is no label at all, never over one that disagrees: a session already + /// stamped with a different agent is a session running the wrong thing, and relaunching + /// it is the right answer, not relabelling it. And only from the daemon — the pane must + /// keep waiting for positive proof, since a bare attach shell is exactly what #228's + /// clause exists to reject. + static func adoptUnlabelledCommand(zmxPath: String, forNode node: LoopNode) -> String? { + guard let stamp = agentLabelCommand(zmxPath: zmxPath, forNode: node) else { return nil } + let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName + let listed = daemonReadyCheckCommand(zmxPath: zmxPath, sessionName: name, agent: nil) + let unlabelled = + "! " + RemoteProjectLocation.shellQuoted(zmxPath) + + " ls 2>/dev/null | grep -q " + + RemoteProjectLocation.shellQuoted("name=\(name)\t.*\t\(Self.agentLabelKey)=") + // The stamp cannot decide this branch's answer: having skipped the relaunch is the + // outcome that matters, and a stamp that failed is retried by the next ensure. + return "\(listed) >/dev/null 2>&1 && \(unlabelled) && { \(stamp) || true; }" + } + /// The label key `agentLabelCommand` writes and `daemonReadyCheckCommand` reads. static let agentLabelKey = "agent" @@ -523,7 +564,7 @@ public enum ZmxSessionLauncher { let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName return daemonReadyCheckCommand( zmxPath: zmxPath, sessionName: name, - agent: node.backend == .codex ? node.backend.rawValue : nil) + agent: readinessAgent(forNode: node)) } /// `agent` is a `CLISessionBackendKind.rawValue`, the same value the launch script @@ -541,11 +582,15 @@ public enum ZmxSessionLauncher { // The label graphcode writes (`agentLabelStatement`), not the process's own command // line: `zmx ls` never shows a `cmd=` for a `zmx run` session, so the grep this // replaced could not match however long it waited (issue #272). + // Anchored on the label's own end — a tab, or the end of the line when it is the + // last label zmx printed. Unanchored, `agent=codexFoo` would satisfy a Codex gate; + // no backend's rawValue is a prefix of another today, which makes this a trap for + // whoever adds the one that is, not a live bug. command += " && " + RemoteProjectLocation.shellQuoted(zmxPath) - + " ls 2>/dev/null | grep -v -e $'\\tended=' -e $'\\terr=' | grep -q " + + " ls 2>/dev/null | grep -v -e $'\\tended=' -e $'\\terr=' | grep -qE " + RemoteProjectLocation.shellQuoted( - "name=\(sessionName)\t.*\t\(Self.agentLabelKey)=\(agent)") + "name=\(sessionName)\t.*\t\(Self.agentLabelKey)=\(agent)(\t|$)") } return command } @@ -1179,12 +1224,15 @@ public enum ZmxSessionLauncher { // create script is an if/else whose exit status belongs to whichever branch ran — // and it is safe to run unconditionally here, since `zmx set` against a session that // was never created fails and leaves no label for the gate to find. - let stamp = agentLabelCommand(zmxPath: "zmx", forNode: node) + let repair = adoptUnlabelledCommand(zmxPath: "zmx", forNode: node).map { "\($0) || " } ?? "" + let launch = + agentLabelCommand(zmxPath: "zmx", forNode: node) + .map { "\(create) && { \($0) || true; }" } ?? create let script = "cd \(RemoteProjectLocation.shellQuoted(location.remotePath)) && { " + deliveryFragment(delivery, ifSessionMissing: check) - + "\(check) >/dev/null 2>&1\(bank) || { " + trustSeed + hooksWrite - + "\(create); \(stamp); }; }" + + "\(check) >/dev/null 2>&1\(bank) || \(repair){ " + trustSeed + hooksWrite + + "\(launch); }; }" return location.sshInvocation(remoteCommand: location.remoteLoginShellCommand(script)) } @@ -1622,7 +1670,8 @@ public enum ZmxSessionLauncher { checkCommand: aliveCheck, runArguments: resumeArgs, zmxPath: zmxPath, workingDirectory: wd, logFragment: DialLog.fragment(session: name, dial: "ensure", event: "resume"), - stampCommand: agentLabelCommand(zmxPath: zmxPath, forNode: node)) + stampCommand: agentLabelCommand(zmxPath: zmxPath, forNode: node), + repairCommand: adoptUnlabelledCommand(zmxPath: zmxPath, forNode: node)) // `zmx run -d` reports that the *session* exists, not that what it launched // survived: `claude --resume` against a transcript its retention expired dies // within a second — and the wrapper shell it was typed with stays behind, a @@ -1660,7 +1709,8 @@ public enum ZmxSessionLauncher { checkCommand: aliveCheck, runArguments: runArgs, zmxPath: zmxPath, workingDirectory: wd, logFragment: DialLog.fragment(session: name, dial: "ensure", event: "fresh"), - stampCommand: agentLabelCommand(zmxPath: zmxPath, forNode: node)) + stampCommand: agentLabelCommand(zmxPath: zmxPath, forNode: node), + repairCommand: adoptUnlabelledCommand(zmxPath: zmxPath, forNode: node)) await kickOffFirstPass( of: node, sessionNamed: name, projectPath: projectPath, after: copilotSessionBefore) } @@ -1856,17 +1906,24 @@ public enum ZmxSessionLauncher { private static func atomicCheckOrRun( checkCommand: String, runArguments: [String], zmxPath: String, workingDirectory: String?, logFragment: String? = nil, - stampCommand: String? = nil + stampCommand: String? = nil, repairCommand: String? = nil ) async { let run = quotedCommand([zmxPath] + runArguments) // The stamp rides in the run branch, after the launch it describes and only if that // launch was made: it is what the readiness gate reads, so writing it anywhere a // session might not exist would say the session is ready when it is not - // (`agentLabelCommand`, issue #272). - let launch = stampCommand.map { "\(run) && \($0)" } ?? run + // (`agentLabelCommand`, issue #272). `|| true` because a failed stamp must not read + // as a failed ensure — that is a retry, and a retry runs `zmx run` against a session + // that is now live, which types the whole launch command into the agent's input. + let launch = stampCommand.map { "\(run) && { \($0) || true; }" } ?? run + // Repair before relaunch: an alive, unlabelled session is one whose stamp was lost, + // and re-running it would be that same typing disaster (`adoptUnlabelledCommand`). + let repair = repairCommand.map { "\($0) || " } ?? "" let script = - logFragment.map { "\(checkCommand) >/dev/null 2>&1 || { \($0); \(launch); }" } - ?? "\(checkCommand) >/dev/null 2>&1 || \(launch)" + logFragment.map { + "\(checkCommand) >/dev/null 2>&1 || \(repair){ \($0); \(launch); }" + } + ?? "\(checkCommand) >/dev/null 2>&1 || \(repair)\(launch)" guard let session = try? PTYProcessSession( executable: "/bin/zsh", arguments: ["-c", script], diff --git a/graphcode/Tests/CodexReadinessGateTests.swift b/graphcode/Tests/CodexReadinessGateTests.swift index 02b1840b..32552af7 100644 --- a/graphcode/Tests/CodexReadinessGateTests.swift +++ b/graphcode/Tests/CodexReadinessGateTests.swift @@ -90,7 +90,7 @@ struct CodexReadinessGateTests { #expect(await !run(gate).succeeded) // The production stamp, run the way the daemon's ensure runs it. - await run(ZmxSessionLauncher.agentLabelCommand(zmxPath: Self.zmx, forNode: node)) + await run(try #require(ZmxSessionLauncher.agentLabelCommand(zmxPath: Self.zmx, forNode: node))) #expect(await run(gate).succeeded) // A session running some *other* agent must still not satisfy a Codex gate — that is @@ -100,6 +100,98 @@ struct CodexReadinessGateTests { #expect(await !run(copilotGate).succeeded) } + @Test + func aLostStampIsRepairedRatherThanRelaunched() async throws { + guard ZmxLocator.isInstalled else { return } + let node = codexNode() + let name = try #require(await start(node)) + defer { Task { await kill(name) } } + + // A session that is alive and unlabelled — a stamp that failed, or one written by a + // build that predates stamping. The ensure adopts it instead of running `zmx run` + // against a live session, which would type the launch command into the agent. + let gate = ZmxSessionLauncher.daemonReadyCheckCommand( + zmxPath: Self.zmx, sessionName: name, agent: CLISessionBackendKind.codex.rawValue) + #expect(await !run(gate).succeeded) + + let repair = try #require( + ZmxSessionLauncher.adoptUnlabelledCommand(zmxPath: Self.zmx, forNode: node)) + #expect(await run(repair).succeeded) + #expect(await run(gate).succeeded) + } + + @Test + func aSessionLabelledForAnotherAgentIsNotAdopted() async throws { + guard ZmxLocator.isInstalled else { return } + let node = codexNode() + let name = try #require(await start(node)) + defer { Task { await kill(name) } } + + // Alive, but running something else. Relabelling it would make the gate lie; the + // ensure has to fall through to its relaunch branch instead, so the adopt must fail. + await run("\(Self.quoted(Self.zmx)) set \(Self.quoted(name)) agent=copilotCLI") + let repair = try #require( + ZmxSessionLauncher.adoptUnlabelledCommand(zmxPath: Self.zmx, forNode: node)) + #expect(await !run(repair).succeeded) + + let gate = ZmxSessionLauncher.daemonReadyCheckCommand( + zmxPath: Self.zmx, sessionName: name, agent: CLISessionBackendKind.codex.rawValue) + #expect(await !run(gate).succeeded) + } + + @Test + func theGateMatchesTheWholeLabelAndNotAPrefixOfIt() async throws { + guard ZmxLocator.isInstalled else { return } + let node = codexNode() + let name = try #require(await start(node)) + defer { Task { await kill(name) } } + + // No backend's rawValue is a prefix of another today, so this is a trap for whoever + // adds the one that is rather than a live bug — which is the moment to pin it. + await run("\(Self.quoted(Self.zmx)) set \(Self.quoted(name)) agent=codexFoo") + let gate = ZmxSessionLauncher.daemonReadyCheckCommand( + zmxPath: Self.zmx, sessionName: name, agent: CLISessionBackendKind.codex.rawValue) + #expect(await !run(gate).succeeded) + + // And still matches when the label is the last one on the row, where there is no + // trailing tab to anchor against. + await run("\(Self.quoted(Self.zmx)) set \(Self.quoted(name)) agent=codex") + #expect(await run(gate).succeeded) + } + + @Test + func aStampAgainstAMissingSessionFailsAndMustNotFailTheEnsure() async { + guard ZmxLocator.isInstalled else { return } + // The hazard the `|| true` exists for: an ensure that exits non-zero is retried, and + // the retry runs `zmx run` against a session that is by then live — which types the + // entire launch command into the agent's input, the very thing the ensure's atomic + // check-or-run exists to prevent. + guard let stamp = ZmxSessionLauncher.agentLabelCommand( + zmxPath: Self.zmx, forNode: codexNode()) + else { return } + #expect(await !run(stamp).succeeded) + #expect(await run("\(stamp) || true").succeeded) + } + + @Test + func nothingIsStampedOrRepairedForABackendTheGateNeverJudges() { + // Codex only. The gate reads a label for Codex alone (`readinessAgent`), so writing + // one anywhere else is a change to three backends that had no part in #272 — and the + // stamp is a shell fragment inside the ensure, which is not a place to spend risk. + for backend in CLISessionBackendKind.allCases where backend != .codex { + let node = LoopNode( + title: "Loop", loopType: .goalBased, goal: GoalSpec(summary: "work"), backend: backend) + #expect(ZmxSessionLauncher.readinessAgent(forNode: node) == nil) + #expect(ZmxSessionLauncher.agentLabelCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) == nil) + #expect( + ZmxSessionLauncher.adoptUnlabelledCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) + == nil) + // And the gate for such a node is the name check alone, exactly as before #272. + let command = ZmxSessionLauncher.aliveCheckCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) + #expect(!command.contains("agent=")) + } + } + @Test func theLabelTheDaemonWritesIsTheOneTheGateReads() { // One source for the write and the read. The gate used to take `executableName` @@ -107,7 +199,7 @@ struct CodexReadinessGateTests { // its case (`claudeCode` → `claude`) could have made the two halves disagree. let node = codexNode() let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName - let stamp = ZmxSessionLauncher.agentLabelCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) + let stamp = ZmxSessionLauncher.agentLabelCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) ?? "" let gate = ZmxSessionLauncher.daemonReadyCheckCommand( zmxPath: "/usr/local/bin/zmx", sessionName: name, agent: CLISessionBackendKind.codex.rawValue) diff --git a/graphcode/Tests/RemoteSessionResumeTests.swift b/graphcode/Tests/RemoteSessionResumeTests.swift index ab57647d..f4bc638d 100644 --- a/graphcode/Tests/RemoteSessionResumeTests.swift +++ b/graphcode/Tests/RemoteSessionResumeTests.swift @@ -166,8 +166,10 @@ struct RemoteSessionResumeTests { // assertion would pass with the write deleted entirely. let write = try #require(remoteCommand.range(of: "mkdir -p \"$HOME/.graphcode/hooks\"")) // And inside the create branch, not merely after the check — textual order alone - // would be satisfied by a fragment sitting outside the group. - let branch = try #require(remoteCommand.range(of: ">/dev/null 2>&1 || { ")) + // would be satisfied by a fragment sitting outside the group. The check and the + // group are no longer adjacent: the readiness repair sits between them (#272), so + // the group opener is what this anchors on. + let branch = try #require(remoteCommand.range(of: " || { ")) let run = try #require(remoteCommand.range(of: "'run'")) #expect(branch.lowerBound < write.lowerBound) #expect(write.lowerBound < run.lowerBound) From 7af70b149a5a09f0103e6fa77bd6c6f4f385f2a5 Mon Sep 17 00:00:00 2001 From: scgopi Date: Thu, 3 Sep 2026 21:46:52 -0700 Subject: [PATCH 5/6] Format the readiness tests to the gate Co-Authored-By: Claude Opus 5 (1M context) --- graphcode/Tests/CodexReadinessGateTests.swift | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/graphcode/Tests/CodexReadinessGateTests.swift b/graphcode/Tests/CodexReadinessGateTests.swift index 32552af7..336a837a 100644 --- a/graphcode/Tests/CodexReadinessGateTests.swift +++ b/graphcode/Tests/CodexReadinessGateTests.swift @@ -90,7 +90,8 @@ struct CodexReadinessGateTests { #expect(await !run(gate).succeeded) // The production stamp, run the way the daemon's ensure runs it. - await run(try #require(ZmxSessionLauncher.agentLabelCommand(zmxPath: Self.zmx, forNode: node))) + await run( + try #require(ZmxSessionLauncher.agentLabelCommand(zmxPath: Self.zmx, forNode: node))) #expect(await run(gate).succeeded) // A session running some *other* agent must still not satisfy a Codex gate — that is @@ -166,8 +167,9 @@ struct CodexReadinessGateTests { // the retry runs `zmx run` against a session that is by then live — which types the // entire launch command into the agent's input, the very thing the ensure's atomic // check-or-run exists to prevent. - guard let stamp = ZmxSessionLauncher.agentLabelCommand( - zmxPath: Self.zmx, forNode: codexNode()) + guard + let stamp = ZmxSessionLauncher.agentLabelCommand( + zmxPath: Self.zmx, forNode: codexNode()) else { return } #expect(await !run(stamp).succeeded) #expect(await run("\(stamp) || true").succeeded) @@ -182,12 +184,14 @@ struct CodexReadinessGateTests { let node = LoopNode( title: "Loop", loopType: .goalBased, goal: GoalSpec(summary: "work"), backend: backend) #expect(ZmxSessionLauncher.readinessAgent(forNode: node) == nil) - #expect(ZmxSessionLauncher.agentLabelCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) == nil) + #expect( + ZmxSessionLauncher.agentLabelCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) == nil) #expect( ZmxSessionLauncher.adoptUnlabelledCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) == nil) // And the gate for such a node is the name check alone, exactly as before #272. - let command = ZmxSessionLauncher.aliveCheckCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) + let command = ZmxSessionLauncher.aliveCheckCommand( + zmxPath: "/usr/local/bin/zmx", forNode: node) #expect(!command.contains("agent=")) } } @@ -199,7 +203,8 @@ struct CodexReadinessGateTests { // its case (`claudeCode` → `claude`) could have made the two halves disagree. let node = codexNode() let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName - let stamp = ZmxSessionLauncher.agentLabelCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) ?? "" + let stamp = + ZmxSessionLauncher.agentLabelCommand(zmxPath: "/usr/local/bin/zmx", forNode: node) ?? "" let gate = ZmxSessionLauncher.daemonReadyCheckCommand( zmxPath: "/usr/local/bin/zmx", sessionName: name, agent: CLISessionBackendKind.codex.rawValue) From f71d220f19663768d645d76f228086d1d90e54d1 Mon Sep 17 00:00:00 2001 From: scgopi Date: Thu, 3 Sep 2026 21:49:43 -0700 Subject: [PATCH 6/6] Name the symbol that exists, and the thing that writes it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `agentLabelStatement` was the in-script version, reverted for MAX_CANON before it shipped; two comments still pointed at it, and one still said the launch script stamps — contradicting the design the code next to it argues for. The daemon's ensure writes the label. Also points the repair's own comment at #276, filed for the residual it depends on: the repair lands only when an ensure runs, and nothing runs one periodically for a local project. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KMDUe3kmSwvx4Bth3xM6qF --- GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift index 9a1eed55..7189ca86 100644 --- a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift +++ b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift @@ -141,7 +141,9 @@ public enum ZmxSessionLauncher { /// local loop: the liveness sweep filters to remote projects, the local ensure runs at /// graph load, node creation and promotion, and a local send checks `sessionExists` /// rather than this gate. The loop would sit unattachable exactly as in #272, but - /// intermittently — which is worse than the deterministic bug it came from. + /// intermittently — which is worse than the deterministic bug it came from. The repair + /// still only lands when an ensure runs, and nothing runs one periodically for a local + /// project; that residual is issue #276. /// /// Only when there is no label at all, never over one that disagrees: a session already /// stamped with a different agent is a session running the wrong thing, and relaunching @@ -567,8 +569,8 @@ public enum ZmxSessionLauncher { agent: readinessAgent(forNode: node)) } - /// `agent` is a `CLISessionBackendKind.rawValue`, the same value the launch script - /// stamps (`agentLabelStatement`) — one source for the write and the read, so a backend + /// `agent` is a `CLISessionBackendKind.rawValue`, the same value the daemon's ensure + /// stamps (`agentLabelCommand`) — one source for the write and the read, so a backend /// whose binary is named differently from its case (`claudeCode` → `claude`) cannot /// make the two halves disagree. public static func daemonReadyCheckCommand( @@ -579,7 +581,7 @@ public enum ZmxSessionLauncher { RemoteProjectLocation.shellQuoted(zmxPath) + " ls 2>/dev/null | grep -v -e $'\\tended=' -e $'\\terr=' | grep -q " + name if let agent { - // The label graphcode writes (`agentLabelStatement`), not the process's own command + // The label graphcode writes (`agentLabelCommand`), not the process's own command // line: `zmx ls` never shows a `cmd=` for a `zmx run` session, so the grep this // replaced could not match however long it waited (issue #272). // Anchored on the label's own end — a tab, or the end of the line when it is the