From 6bf099f6a65fd465ad7c281f68b22507cd387193 Mon Sep 17 00:00:00 2001 From: scgopi Date: Mon, 2 Nov 2026 19:00:32 -0800 Subject: [PATCH 1/2] Rename live Copilot sessions and find titled ones remotely A blank-titled Copilot loop launched as graphcode- and kept that name, because its suggested title arrives after launch and only Claude Code was sent /rename. Later renames were lost the same way. Copilot now gets /rename with the node's short id kept, since the name is also its lookup key. The remote lookups still matched the bare name exactly, so they never found a session named after its loop. All four now accept both forms. Co-Authored-By: Claude Opus 5.5 Signed-off-by: scgopi --- .../Sources/Domain/BackendCommand.swift | 21 ++++- GraphcodeKit/Sources/GraphStore.swift | 21 ++--- .../Sources/Sessions/CopilotSessionLog.swift | 8 +- .../Sources/Sessions/SessionTransplant.swift | 2 +- .../Tests/RemoteSessionExportTests.swift | 3 +- graphcode/Tests/SessionNamingTests.swift | 92 ++++++++++++++++++- 6 files changed, 126 insertions(+), 21 deletions(-) diff --git a/GraphcodeKit/Sources/Domain/BackendCommand.swift b/GraphcodeKit/Sources/Domain/BackendCommand.swift index 72a2cae8a..def394f7b 100644 --- a/GraphcodeKit/Sources/Domain/BackendCommand.swift +++ b/GraphcodeKit/Sources/Domain/BackendCommand.swift @@ -374,6 +374,18 @@ extension CLISessionBackendKind { } } + /// What to type into a live session to show a loop's new title, for the backends that + /// apply it mid-turn without interrupting. `nil` when the session can't be renamed. + public func renameCommand(displayName: String?, sessionName: String) -> String? { + guard let name = displayName.flatMap(Self.sessionLabel) else { return nil } + switch self { + case .claudeCode: return "/rename \(name)" + case .copilotCLI: + return "/rename \(Self.copilotSessionName(title: name, sessionName: sessionName))" + case .codex, .openCode, .pi, .nod: return nil + } + } + /// OpenCode's TUI has no title flag (`--title` is `opencode run`'s alone), so the title /// rides to graphcode's plugin, which sets it when the session is created /// (`OpenCodePresencePlugin`). A resumed session keeps the title it already has. @@ -396,7 +408,7 @@ extension CLISessionBackendKind { /// Copilot's name for a titled session: the title, then enough of the node's id for /// `CopilotSessionLog` to find it again. Copilot refuses `--name` beside `--resume`, so - /// this is fixed at first launch and outlives any later rename of the loop. + /// a later rename of the loop can only reach a live session (`renameCommand`). public static func copilotSessionName(title: String, sessionName: String) -> String { guard let label = sessionLabel(title) else { return sessionName } return "\(label) · \(copilotNameSuffix(of: sessionName))" @@ -413,6 +425,13 @@ extension CLISessionBackendKind { return unquoted.hasSuffix(" · \(copilotNameSuffix(of: sessionName))") } + /// `isCopilotSessionName` for a remote host's shell, as a `grep` still to be given its + /// file. `.?` is YAML's closing quote, whichever one the writer picked: a literal one + /// would have to survive the single quotes every remote script is wrapped in. + public static func copilotNameMatcher(for sessionName: String) -> String { + "grep -qE '^name: (\(sessionName)|.* · \(copilotNameSuffix(of: sessionName)).?)$'" + } + private static func copilotNameSuffix(of sessionName: String) -> String { let id = sessionName.hasPrefix("graphcode-") diff --git a/GraphcodeKit/Sources/GraphStore.swift b/GraphcodeKit/Sources/GraphStore.swift index 21780fc96..63479fcf5 100644 --- a/GraphcodeKit/Sources/GraphStore.swift +++ b/GraphcodeKit/Sources/GraphStore.swift @@ -2031,19 +2031,18 @@ public actor GraphStore { graph.nodes[id: nodeID]?.title = trimmed } - /// Carries a new title into a live Claude Code session as `/rename`, which takes effect - /// even mid-turn without interrupting it. Without this, a blank-titled loop's session - /// kept its launch-time name for good: the title is suggested only after the session - /// has started. A resolved loop is renamed too: its session often stays up for - /// follow-up questions, and the send's own liveness gate skips one that is gone. Never - /// staged to memory — a name a resume will pass anyway (`--name`) is not something the - /// loop should read at its next wake. + /// Carries a new title into a live session as `/rename`, which takes effect even + /// mid-turn without interrupting it. Without this, a blank-titled loop's session kept + /// its launch-time name for good: the title is suggested only after the session has + /// started. A resolved loop is renamed too: its session often stays up for follow-up + /// questions, and the send's own liveness gate skips one that is gone. Never staged to + /// memory — a rename is not something the loop should read at its next wake. private func renameSession(of node: LoopNode) { - guard node.backend == .claudeCode, let onDeliverMessage, - let name = SessionNaming.displayName(forTitle: node.title) - .flatMap(CLISessionBackendKind.sessionLabel) + guard let onDeliverMessage, + let command = node.backend.renameCommand( + displayName: SessionNaming.displayName(forTitle: node.title), + sessionName: SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName) else { return } - let command = "/rename \(name)" let path = graph.project.path guard subGraphDepth == 0 else { Task { _ = await onDeliverMessage(node, command, path) } diff --git a/GraphcodeKit/Sources/Sessions/CopilotSessionLog.swift b/GraphcodeKit/Sources/Sessions/CopilotSessionLog.swift index e7a8910a6..aa16331e2 100644 --- a/GraphcodeKit/Sources/Sessions/CopilotSessionLog.swift +++ b/GraphcodeKit/Sources/Sessions/CopilotSessionLog.swift @@ -312,7 +312,8 @@ public enum CopilotSessionLog { let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName let find = "F=''; for d in $(ls -t \"$HOME/.copilot/session-state/\" 2>/dev/null); do " - + "if grep -qx 'name: \(name)' \"$HOME/.copilot/session-state/$d/workspace.yaml\" 2>/dev/null; " + + "if \(CLISessionBackendKind.copilotNameMatcher(for: name)) " + + "\"$HOME/.copilot/session-state/$d/workspace.yaml\" 2>/dev/null; " + "then F=\"$HOME/.copilot/session-state/$d/events.jsonl\"; break; fi; done" let keep = "grep -aE '\"type\":\"(user\\.message|assistant\\.message|tool\\.execution_start" @@ -433,7 +434,7 @@ public enum CopilotSessionLog { let logged = DialLog.fragment(session: name, dial: "bank", event: "copilot-id") return "{ [ -s \(idFile) ] || { gc_sid=''; " + "for gc_d in $(ls -t \"$HOME/.copilot/session-state/\" 2>/dev/null); do " - + "if grep -qx 'name: \(name)' " + + "if \(CLISessionBackendKind.copilotNameMatcher(for: name)) " + "\"$HOME/.copilot/session-state/$gc_d/workspace.yaml\" 2>/dev/null; then " + "gc_sid=\"$gc_d\"; break; fi; done; " + "if [ -n \"$gc_sid\" ]; then mkdir -p \(sessions); " @@ -450,7 +451,8 @@ public enum CopilotSessionLog { let findDir = "D=''; for d in $(ls -t \"$HOME/.copilot/session-state/\" 2>/dev/null); do " - + "if grep -qx 'name: \(name)' \"$HOME/.copilot/session-state/$d/workspace.yaml\" 2>/dev/null; then " + + "if \(CLISessionBackendKind.copilotNameMatcher(for: name)) " + + "\"$HOME/.copilot/session-state/$d/workspace.yaml\" 2>/dev/null; then " + "D=\"$HOME/.copilot/session-state/$d\"; break; fi; done" // The keep-list carries `lifecycleBoundaries` as well as the mapped events, so diff --git a/GraphcodeKit/Sources/Sessions/SessionTransplant.swift b/GraphcodeKit/Sources/Sessions/SessionTransplant.swift index eb960535f..ee934c164 100644 --- a/GraphcodeKit/Sources/Sessions/SessionTransplant.swift +++ b/GraphcodeKit/Sources/Sessions/SessionTransplant.swift @@ -249,7 +249,7 @@ public enum SessionTransplant { let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName return "S=$(cat \(idFile) 2>/dev/null); if [ -z \"$S\" ]; then " + "for d in $(ls -t \"$HOME/.copilot/session-state/\" 2>/dev/null); do " - + "if grep -qx 'name: \(name)' " + + "if \(CLISessionBackendKind.copilotNameMatcher(for: name)) " + "\"$HOME/.copilot/session-state/$d/workspace.yaml\" 2>/dev/null; " + "then S=\"$d\"; break; fi; done; fi; " + "[ -n \"$S\" ] && [ -d \"$HOME/.copilot/session-state/$S\" ] || exit 0; " diff --git a/graphcode/Tests/RemoteSessionExportTests.swift b/graphcode/Tests/RemoteSessionExportTests.swift index 085f7f1ef..aecf5ba63 100644 --- a/graphcode/Tests/RemoteSessionExportTests.swift +++ b/graphcode/Tests/RemoteSessionExportTests.swift @@ -67,7 +67,8 @@ struct RemoteSessionExportTests { // Banked id first, then the same `workspace.yaml` walk the ensure's bank fragment // does — Copilot has no hook to bank its own id, so a session may be unbanked. let bank = try #require(script.range(of: "cat \"$HOME/.graphcode/sessions\"")) - let walk = try #require(script.range(of: "grep -qx 'name: \(name)'")) + let walk = try #require( + script.range(of: CLISessionBackendKind.copilotNameMatcher(for: name))) #expect(bank.lowerBound < walk.lowerBound) #expect(script.contains("/.copilot/session-state/$d/workspace.yaml")) #expect(script.hasSuffix("exec tar -cf - -C \"$HOME/.copilot/session-state\" \"$S\"")) diff --git a/graphcode/Tests/SessionNamingTests.swift b/graphcode/Tests/SessionNamingTests.swift index 4f2339f19..cce531a27 100644 --- a/graphcode/Tests/SessionNamingTests.swift +++ b/graphcode/Tests/SessionNamingTests.swift @@ -226,11 +226,95 @@ struct SessionNamingTests { #expect(await typedAfterRenaming(.claudeCode, state: state).typed == ["/rename Fix login"]) } + /// Copilot 1.0.96 applies a typed `/rename` idle and mid-turn, and the short id has to + /// survive it: the name is how `CopilotSessionLog` finds the session afterwards. @Test - func aRenameReachesNoSessionItCannotName() async { - #expect(await typedAfterRenaming(.claudeCode, ramp: false).typed.isEmpty) - #expect(await typedAfterRenaming(.copilotCLI).typed.isEmpty) + func aRenamedCopilotLoopKeepsItsLookupKey() async throws { + let result = await typedAfterRenaming(.copilotCLI) + #expect(result.typed == ["/rename Fix login · 11111111"]) + #expect(result.memory.isEmpty) + let name = try #require(result.typed.first?.dropFirst("/rename ".count)) + #expect(CLISessionBackendKind.isCopilotSessionName(String(name), for: sessionName)) + } + + @Test(arguments: [LoopState.succeeded, .failed]) + func aFinishedCopilotLoopsOpenSessionIsRenamedToo(_ state: LoopState) async { #expect( - await typedAfterRenaming(.claudeCode, to: NodeDraft.untitledFallback).typed.isEmpty) + await typedAfterRenaming(.copilotCLI, state: state).typed + == ["/rename Fix login · 11111111"]) + } + + @Test(arguments: [CLISessionBackendKind.claudeCode, .copilotCLI]) + func aRenameWaitsForTheRampAndARealTitle(_ backend: CLISessionBackendKind) async { + #expect(await typedAfterRenaming(backend, ramp: false).typed.isEmpty) + #expect(await typedAfterRenaming(backend, to: NodeDraft.untitledFallback).typed.isEmpty) + } + + @Test(arguments: [CLISessionBackendKind.codex, .openCode, .pi, .nod]) + func aRenameReachesNoSessionItCannotName(_ backend: CLISessionBackendKind) async { + #expect(await typedAfterRenaming(backend).typed.isEmpty) + } + + /// Every name Copilot can record for a node, in the spellings its YAML writer picks, as + /// the remote walk reads them — a titled session it cannot find has no banked id, and a + /// remote loop with no banked id restarts from scratch. + @Test(arguments: [ + (locale: "en_US.UTF-8", name: "graphcode-11111111-2222-3333-4444-555555555555"), + (locale: "en_US.UTF-8", name: "Fix login · 11111111"), + (locale: "en_US.UTF-8", name: "'Fix: login · 11111111'"), + (locale: "en_US.UTF-8", name: "\"Fix: login · 11111111\""), + (locale: "C", name: "Fix login · 11111111"), + (locale: "C", name: "'Fix: login · 11111111'"), + ]) + func theRemoteWalkFindsEveryNameOfTheNode(locale: String, name: String) throws { + let workspace = "client_name: github/cli\nname: \(name)\n" + #expect(try bankedID(recording: workspace, locale: locale) != nil) + } + + @Test(arguments: [ + "name: Fix login · 99999999", + "name: Fix login", + "name: graphcode-99999999-2222-3333-4444-555555555555", + "client_name: Fix login · 11111111", + ]) + func theRemoteWalkFindsNoOtherSession(line: String) throws { + #expect(try bankedID(recording: "\(line)\n", locale: "en_US.UTF-8") == nil) + } + + /// The pattern's quotes, `$` and `·` have to arrive intact through the one wrapper every + /// remote script rides in. + @Test + func theRemoteWalkSurvivesTheLoginShellQuoting() throws { + let location = RemoteProjectLocation(host: "box", remotePath: "/srv/widget") + let workspace = "name: 'Fix: login · 11111111'\n" + #expect( + try bankedID( + recording: workspace, locale: "en_US.UTF-8", wrap: location.remoteLoginShellCommand) + != nil) + } + + /// Runs the ensure's bank fragment under `/bin/sh` against a fake home, and reads back + /// the id it banked. + private func bankedID( + recording workspace: String, locale: String, wrap: (String) -> String = { $0 } + ) throws -> String? { + let home = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("copilot-walk-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager.default.removeItem(at: home) } + let session = home.appendingPathComponent(".copilot/session-state/the-session") + try FileManager.default.createDirectory(at: session, withIntermediateDirectories: true) + try workspace.write( + to: session.appendingPathComponent("workspace.yaml"), atomically: true, encoding: .utf8) + + let process = Process() + process.executableURL = URL(fileURLWithPath: "/bin/sh") + process.arguments = ["-c", wrap(CopilotSessionLog.remoteIDBankFragment(forNodeID: nodeID))] + process.environment = ["HOME": home.path, "PATH": "/usr/bin:/bin", "LC_ALL": locale] + try process.run() + process.waitUntilExit() + #expect(process.terminationStatus == 0) + let banked = home.appendingPathComponent( + ".graphcode/sessions/\(nodeID.uuidString).id") + return try? String(contentsOf: banked, encoding: .utf8) } } From f317f24d9321b5d59d5fe21864b676d13fef7c98 Mon Sep 17 00:00:00 2001 From: scgopi Date: Mon, 2 Nov 2026 19:38:25 -0800 Subject: [PATCH 2/2] Hold a Copilot rename until typing it is safe The review of this PR found that /rename shared the session with things it could collide with. The first pass types outside GraphStore's chain, so the two could merge into one name and lose the lookup key. A liveness gate alone also let Enter land on a permission dialog, or /rename answer an open ask_user. A Copilot rename now waits, for up to three minutes, until no first pass is in flight, no question is open and the session has opened. If the wait runs out, the name stays stale and nothing is typed. Also keep titles inside what Copilot's rename accepts, drop control characters from session labels, tighten the remote matcher to a real closing quote, and pin the summary and presence reads with tests. Co-Authored-By: Claude Opus 5.5 Signed-off-by: scgopi --- .../Sources/Domain/BackendCommand.swift | 21 +- GraphcodeKit/Sources/GraphStore.swift | 19 +- GraphcodeKit/Sources/ProjectRegistry.swift | 5 + .../Sources/Sessions/CLISessionBackend.swift | 6 + .../Sources/Sessions/CopilotSessionLog.swift | 95 ++++++ .../Sources/Sessions/ZmxSessionLauncher.swift | 29 ++ graphcode/Tests/CopilotRenameTests.swift | 288 ++++++++++++++++++ graphcode/Tests/SessionNamingTests.swift | 63 ---- 8 files changed, 455 insertions(+), 71 deletions(-) create mode 100644 graphcode/Tests/CopilotRenameTests.swift diff --git a/GraphcodeKit/Sources/Domain/BackendCommand.swift b/GraphcodeKit/Sources/Domain/BackendCommand.swift index def394f7b..52135b70c 100644 --- a/GraphcodeKit/Sources/Domain/BackendCommand.swift +++ b/GraphcodeKit/Sources/Domain/BackendCommand.swift @@ -401,7 +401,8 @@ extension CLISessionBackendKind { /// A loop's title as one line a session picker can show, or `nil` when nothing is left. public static func sessionLabel(_ title: String) -> String? { - let line = title.components(separatedBy: .newlines).joined(separator: " ") + let line = title.components(separatedBy: CharacterSet.newlines.union(.controlCharacters)) + .joined(separator: " ") .trimmingCharacters(in: .whitespaces) return line.isEmpty ? nil : line } @@ -410,8 +411,14 @@ extension CLISessionBackendKind { /// `CopilotSessionLog` to find it again. Copilot refuses `--name` beside `--resume`, so /// a later rename of the loop can only reach a live session (`renameCommand`). public static func copilotSessionName(title: String, sessionName: String) -> String { - guard let label = sessionLabel(title) else { return sessionName } - return "\(label) · \(copilotNameSuffix(of: sessionName))" + // Copilot 1.0.96 rejects a name with a double quote or over 100 characters and keeps + // the old one. + let usable = title.filter { $0 != "\"" } + guard let label = sessionLabel(usable) else { return sessionName } + let suffix = " · \(copilotNameSuffix(of: sessionName))" + let fitted = label.prefix(copilotNameLimit - suffix.count) + .trimmingCharacters(in: .whitespaces) + return fitted + suffix } /// Whether `recorded`, a `name:` read back out of Copilot's `workspace.yaml`, belongs to @@ -426,12 +433,14 @@ extension CLISessionBackendKind { } /// `isCopilotSessionName` for a remote host's shell, as a `grep` still to be given its - /// file. `.?` is YAML's closing quote, whichever one the writer picked: a literal one - /// would have to survive the single quotes every remote script is wrapped in. + /// file. The closing bracket's class is YAML's closing quote, whichever one the writer + /// picked. public static func copilotNameMatcher(for sessionName: String) -> String { - "grep -qE '^name: (\(sessionName)|.* · \(copilotNameSuffix(of: sessionName)).?)$'" + "grep -qE '^name: (\(sessionName)|.* · \(copilotNameSuffix(of: sessionName))[\"'\\'']?)$'" } + private static let copilotNameLimit = 100 + private static func copilotNameSuffix(of sessionName: String) -> String { let id = sessionName.hasPrefix("graphcode-") diff --git a/GraphcodeKit/Sources/GraphStore.swift b/GraphcodeKit/Sources/GraphStore.swift index 63479fcf5..9124dabdf 100644 --- a/GraphcodeKit/Sources/GraphStore.swift +++ b/GraphcodeKit/Sources/GraphStore.swift @@ -107,6 +107,7 @@ public actor GraphStore { /// stays for the `until`-guard and for every test that stubs a bare yes/no. private let onCheckPredicate: (@Sendable (ShellPredicate) async -> PredicateOutcome?)? private let onDeliverMessage: (@Sendable (LoopNode, String, String?) async -> Bool)? + private let onAwaitRenameWindow: (@Sendable (LoopNode, String?) async -> Bool)? private let onCaptureScript: (@Sendable (ShellPredicate) async -> String?)? private let onReadUsage: (@Sendable (LoopNode, String?) async -> UsageSample?)? private let onReadGoalVerdict: (@Sendable (LoopNode, String?) async -> GoalVerdict?)? @@ -403,6 +404,7 @@ public actor GraphStore { onEvaluatePredicate: (@Sendable (ShellPredicate) async -> Bool)? = nil, onCheckPredicate: (@Sendable (ShellPredicate) async -> PredicateOutcome?)? = nil, onDeliverMessage: (@Sendable (LoopNode, String, String?) async -> Bool)? = nil, + onAwaitRenameWindow: (@Sendable (LoopNode, String?) async -> Bool)? = nil, onCaptureScript: (@Sendable (ShellPredicate) async -> String?)? = nil, onReadUsage: (@Sendable (LoopNode, String?) async -> UsageSample?)? = nil, onReadActivity: (@Sendable (LoopNode, String?) async -> String?)? = nil, @@ -451,6 +453,7 @@ public actor GraphStore { self.onEvaluatePredicate = onEvaluatePredicate self.onCheckPredicate = onCheckPredicate self.onDeliverMessage = onDeliverMessage + self.onAwaitRenameWindow = onAwaitRenameWindow self.onCaptureScript = onCaptureScript self.onReadUsage = onReadUsage self.onReadActivity = onReadActivity @@ -1577,6 +1580,7 @@ public actor GraphStore { onEvaluatePredicate: onEvaluatePredicate, onCheckPredicate: onCheckPredicate, onDeliverMessage: onDeliverMessage, + onAwaitRenameWindow: onAwaitRenameWindow, onCaptureScript: onCaptureScript, onAppendMemory: onAppendMemory, onRemoveMemory: onRemoveMemory, @@ -2044,15 +2048,25 @@ public actor GraphStore { sessionName: SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName) else { return } let path = graph.project.path + let window = node.backend == .copilotCLI ? onAwaitRenameWindow : nil guard subGraphDepth == 0 else { - Task { _ = await onDeliverMessage(node, command, path) } + Task { + if let window, await !window(node, path) { return } + _ = await onDeliverMessage(node, command, path) + } return } let previous = sessionTyping[node.id]?.task let token = UUID() let task = Task { [self] in await previous?.value - _ = await withDeadline(deliveryDeadline) { await onDeliverMessage(node, command, path) } + var open = true + if let window { + open = await window(node, path) && graph.nodes[id: node.id]?.title == node.title + } + if open { + _ = await withDeadline(deliveryDeadline) { await onDeliverMessage(node, command, path) } + } if sessionTyping[node.id]?.token == token { sessionTyping.removeValue(forKey: node.id) } @@ -4630,6 +4644,7 @@ public actor GraphStore { onEvaluatePredicate: onEvaluatePredicate, onCheckPredicate: onCheckPredicate, onDeliverMessage: onDeliverMessage, + onAwaitRenameWindow: onAwaitRenameWindow, onCaptureScript: onCaptureScript, onReadUsage: onReadUsage, onReadPresence: onReadPresence, diff --git a/GraphcodeKit/Sources/ProjectRegistry.swift b/GraphcodeKit/Sources/ProjectRegistry.swift index 60b51c056..d2df44636 100644 --- a/GraphcodeKit/Sources/ProjectRegistry.swift +++ b/GraphcodeKit/Sources/ProjectRegistry.swift @@ -71,6 +71,7 @@ public actor ProjectRegistry { private let evaluatePredicate: (@Sendable (ShellPredicate) async -> Bool)? private let checkPredicate: (@Sendable (ShellPredicate) async -> PredicateOutcome?)? private let deliverMessage: (@Sendable (LoopNode, String, String?) async -> Bool)? + private let awaitRenameWindow: (@Sendable (LoopNode, String?) async -> Bool)? private let captureScript: (@Sendable (ShellPredicate) async -> String?)? private let readUsage: (@Sendable (LoopNode, String?) async -> UsageSample?)? private let readGoalVerdict: (@Sendable (LoopNode, String?) async -> GoalVerdict?)? @@ -135,6 +136,8 @@ public actor ProjectRegistry { ShellPredicateEvaluator.check, deliverMessage: (@Sendable (LoopNode, String, String?) async -> Bool)? = CLISessionBackend.deliverMessage, + awaitRenameWindow: (@Sendable (LoopNode, String?) async -> Bool)? = + CLISessionBackend.awaitRenameWindow, captureScript: (@Sendable (ShellPredicate) async -> String?)? = ShellPredicateEvaluator.capture, readUsage: (@Sendable (LoopNode, String?) async -> UsageSample?)? = CLISessionBackend.readUsage, @@ -183,6 +186,7 @@ public actor ProjectRegistry { self.evaluatePredicate = evaluatePredicate self.checkPredicate = checkPredicate self.deliverMessage = deliverMessage + self.awaitRenameWindow = awaitRenameWindow self.captureScript = captureScript self.readUsage = readUsage self.readGoalVerdict = readGoalVerdict @@ -1296,6 +1300,7 @@ public actor ProjectRegistry { onEvaluatePredicate: evaluatePredicate, onCheckPredicate: checkPredicate, onDeliverMessage: deliverMessage, + onAwaitRenameWindow: awaitRenameWindow, onCaptureScript: captureScript, onReadUsage: readUsage, onReadActivity: readActivity, diff --git a/GraphcodeKit/Sources/Sessions/CLISessionBackend.swift b/GraphcodeKit/Sources/Sessions/CLISessionBackend.swift index 99bc06b3c..94f76b26a 100644 --- a/GraphcodeKit/Sources/Sessions/CLISessionBackend.swift +++ b/GraphcodeKit/Sources/Sessions/CLISessionBackend.swift @@ -340,6 +340,12 @@ extension CLISessionBackend { await backend(for: node).restart(node, path) } + /// The rename gate `GraphStore` is wired with — see `ZmxSessionLauncher.awaitRenameWindow`. + public static let awaitRenameWindow: @Sendable (LoopNode, String?) async -> Bool = { + node, projectPath in + await ZmxSessionLauncher.awaitRenameWindow(of: node, projectPath: projectPath) + } + /// The `.message` transport `GraphStore` is wired with — routed through the *target's* /// backend, since it's the target's session being typed into. public static let deliverMessage: @Sendable (LoopNode, String, String?) async -> Bool = { diff --git a/GraphcodeKit/Sources/Sessions/CopilotSessionLog.swift b/GraphcodeKit/Sources/Sessions/CopilotSessionLog.swift index aa16331e2..a1337437b 100644 --- a/GraphcodeKit/Sources/Sessions/CopilotSessionLog.swift +++ b/GraphcodeKit/Sources/Sessions/CopilotSessionLog.swift @@ -226,6 +226,101 @@ public enum CopilotSessionLog { } } + /// Whether typing into this session now could answer a question instead of reaching + /// the composer: Enter on a permission dialog approves the tool, and an open `ask_user` + /// takes the text as its answer. A session still opening counts too: it can sit at a + /// prompt for minutes, measured at two under `ask`, before its log says anything. Yes + /// when the log can't be read, because a keystroke nobody asked for can wait and a wrong + /// answer can't be taken back. A session that isn't live is a no: the send's own gate + /// skips it. + static func awaitingDecision(of node: LoopNode, projectPath: String?) async -> Bool { + if let projectPath, let location = RemoteProjectLocation.parse(projectPath: projectPath) { + let (succeeded, output) = await ZmxSessionLauncher.collectRemoteOutput( + remoteDecisionInvocation(forNode: node, at: location), location: location) + return remoteDecisionPending( + succeeded: succeeded, output: output, expectingOpening: node.sessionPrompt != nil) + } + guard await ZmxSessionLauncher.sendGate(node) else { return false } + let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName + guard let directory = directory(forSessionNamed: name) else { return true } + return holdsADecision( + tailLines(ofLogAt: directory.appendingPathComponent("events.jsonl")), + expectingOpening: node.sessionPrompt != nil) + } + + /// Newest first, stopping where the running session began: an older run's unanswered + /// dialog is gone. Lines may be cut short, so ids are read as text rather than as JSON. + static func holdsADecision( + _ lines: [Line], expectingOpening: Bool = false + ) -> Bool { + var sawUserMessage = false + var permissionSettled = false + var finishedCalls: Set = [] + for line in lines.reversed() { + if line.contains(#""type":"session.start""#) { return expectingOpening && !sawUserMessage } + if line.contains(#""type":"session.resume""#) { return false } + if line.contains(#""type":"user.message""#) { + sawUserMessage = true + } else if line.contains(#""type":"permission.completed""#) { + permissionSettled = true + } else if line.contains(#""type":"permission.requested""#) { + if !permissionSettled { return true } + } else if line.contains(#""type":"tool.execution_complete""#) { + toolCallID(in: line).map { _ = finishedCalls.insert($0) } + } else if line.contains(#""type":"tool.execution_start""#), + line.contains(#""toolName":"ask_user""#), + let call = toolCallID(in: line), !finishedCalls.contains(call) + { + return true + } + } + return false + } + + private static func toolCallID(in line: Line) -> String? { + let key = #""toolCallId":""# + guard let start = line.range(of: key)?.upperBound, + let end = line[start...].firstIndex(of: "\"") + else { return nil } + return String(line[start.. [String] { + let name = SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName + let check = ZmxSessionLauncher.quotedCommand(["zmx", "get", name]) + let marker = ZmxSessionLauncher.remoteProbeMarker + let script = + "if \(check) >/dev/null 2>&1; then " + + "D=''; for d in $(ls -t \"$HOME/.copilot/session-state/\" 2>/dev/null); do " + + "if \(CLISessionBackendKind.copilotNameMatcher(for: name)) " + + "\"$HOME/.copilot/session-state/$d/workspace.yaml\" 2>/dev/null; then " + + "D=\"$HOME/.copilot/session-state/$d\"; break; fi; done; " + + "if [ -n \"$D\" ]; then " + + "tail -c 65536 \"$D/events.jsonl\" 2>/dev/null | grep -aE " + + "'\"type\":\"(session\\.start|session\\.resume|user\\.message|permission\\.requested" + + "|permission\\.completed|tool\\.execution_complete)\"|\"toolName\":\"ask_user\"'" + + " | cut -c1-300; echo '\(marker) read'; else echo '\(marker) booting'; fi; " + + "else echo '\(marker) absent'; fi" + return location.sshInvocation(remoteCommand: location.remoteLoginShellCommand(script)) + } + + static func remoteDecisionPending( + succeeded: Bool, output: String, expectingOpening: Bool = false + ) -> Bool { + let marker = ZmxSessionLauncher.remoteProbeMarker + let lines = output.split(whereSeparator: \.isNewline) + guard succeeded, let verdict = lines.last(where: { $0.hasPrefix(marker) }) else { return true } + switch verdict.dropFirst(marker.count).trimmingCharacters(in: .whitespaces) { + case "absent": return false + case "read": return holdsADecision(lines, expectingOpening: expectingOpening) + default: return true + } + } + /// What this node's Copilot session is doing, or `nil` when nothing says. /// /// Local only, deliberately. The remote twin of this would have to carry a JSON record diff --git a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift index 8dbc0277c..eec8351d8 100644 --- a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift +++ b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift @@ -3085,6 +3085,34 @@ public enum ZmxSessionLauncher { } } + /// Holds a `/rename` until typing it can't collide with anything. The first pass types + /// outside `GraphStore`'s per-session chain, and its text would land in the name; a + /// question Copilot is waiting on would take the keystrokes as its answer + /// (`CopilotSessionLog.awaitingDecision`). `false` once the wait runs out: a name that + /// stays stale costs far less than either collision. + static func awaitRenameWindow( + of node: LoopNode, projectPath: String?, polls: Int = renameWindowPolls, + interval: Duration = renameWindowInterval + ) async -> Bool { + for poll in 0.. 0 { try? await Task.sleep(for: interval) } + if await !FirstPassTickets.shared.isClaimed(node.id), + await !CopilotSessionLog.awaitingDecision(of: node, projectPath: projectPath) + { + return true + } + } + DialLog.record( + session: SurfaceRef(id: node.id, launchesClaudeCode: true).zmxSessionName, + dial: "rename", event: "window-closed") + return false + } + + /// Eighteen reads over three minutes: longer than a whole first pass or a slow opening, + /// and few enough that a remote dialog nobody answers costs little. + static let renameWindowPolls = 18 + static let renameWindowInterval: Duration = .seconds(10) + /// The verification window per attempt is generous next to the instant write Copilot /// actually does, and the attempt count is small: three swallowed sends in a row means /// something structural, not something a fourth send fixes. @@ -3144,6 +3172,7 @@ public enum ZmxSessionLauncher { private var claimed: Set = [] func claim(_ id: UUID) -> Bool { claimed.insert(id).inserted } func release(_ id: UUID) { claimed.remove(id) } + func isClaimed(_ id: UUID) -> Bool { claimed.contains(id) } } /// How long a resumed session has to still be there before its launch counts as taken. diff --git a/graphcode/Tests/CopilotRenameTests.swift b/graphcode/Tests/CopilotRenameTests.swift new file mode 100644 index 000000000..d69efa430 --- /dev/null +++ b/graphcode/Tests/CopilotRenameTests.swift @@ -0,0 +1,288 @@ +import ComposableArchitecture +import Foundation +import Testing + +@testable import GraphcodeKit +@testable import graphcode + +/// What stands between a loop's new title and a live Copilot session: the window `/rename` +/// waits for, and the name the remote reads have to find afterwards. +@Suite +struct CopilotRenameTests { + private let nodeID = UUID(uuidString: "11111111-2222-3333-4444-555555555555") ?? UUID() + + private var sessionName: String { + SurfaceRef(id: nodeID, launchesClaudeCode: true).zmxSessionName + } + + private func node(_ backend: CLISessionBackendKind, title: String = "Fix login") -> LoopNode { + LoopNode( + id: nodeID, title: title, loopType: .goalBased, goal: GoalSpec(summary: "Tests pass"), + backend: backend) + } + + /// Copilot's first pass and its open questions both have to get out of the way first, + /// and a rename that never gets a window is dropped rather than typed. + @Test + func aCopilotRenameWaitsForItsWindow() async { + let typed = LockIsolated<[String]>([]) + let asked = LockIsolated(0) + let store = GraphStore( + graph: LoopGraph( + project: ProjectRef(path: "/tmp/naming", name: "naming"), + nodes: [node(.copilotCLI, title: NodeDraft.untitledFallback)]), + onDeliverMessage: { _, text, _ in + typed.withValue { $0.append(text) } + return true + }, + onAwaitRenameWindow: { _, _ in + asked.withValue { $0 += 1 } + return asked.value > 1 + }) + await SessionNaming.$rampOverride.withValue(true) { + await store.handle(.renameNode(nodeID, title: "Closed")) + await store.handle(.renameNode(nodeID, title: "Open")) + await store.finishSessionTyping() + } + #expect(asked.value == 2) + #expect(typed.value == ["/rename Open · 11111111"]) + } + + /// The title may have moved on by the time the window opens; only the newest is typed. + @Test + func aHeldRenameThatWasSupersededIsNotTyped() async { + let typed = LockIsolated<[String]>([]) + let store = GraphStore( + graph: LoopGraph( + project: ProjectRef(path: "/tmp/naming", name: "naming"), + nodes: [node(.copilotCLI, title: NodeDraft.untitledFallback)]), + onDeliverMessage: { _, text, _ in + typed.withValue { $0.append(text) } + return true + }, + onAwaitRenameWindow: { _, _ in + try? await Task.sleep(for: .milliseconds(50)) + return true + }) + await SessionNaming.$rampOverride.withValue(true) { + await store.handle(.renameNode(nodeID, title: "Stale")) + await store.handle(.renameNode(nodeID, title: "Fresh")) + await store.finishSessionTyping() + } + #expect(typed.value == ["/rename Fresh · 11111111"]) + } + + /// Claude Code's rename has no such wait, so its timing stays what #703 shipped. + @Test + func aClaudeRenameAsksForNoWindow() async { + let asked = LockIsolated(0) + let store = GraphStore( + graph: LoopGraph( + project: ProjectRef(path: "/tmp/naming", name: "naming"), + nodes: [node(.claudeCode, title: NodeDraft.untitledFallback)]), + onDeliverMessage: { _, _, _ in true }, + onAwaitRenameWindow: { _, _ in + asked.withValue { $0 += 1 } + return true + }) + await SessionNaming.$rampOverride.withValue(true) { + await store.handle(.renameNode(nodeID, title: "Fix login")) + await store.finishSessionTyping() + } + #expect(asked.value == 0) + } + + @Test(arguments: [ + (lines: [#"{"type":"permission.requested","data":{}}"#], held: true), + ( + lines: [#"{"type":"permission.requested"}"#, #"{"type":"permission.completed"}"#], + held: false + ), + ( + lines: [#"{"type":"permission.requested"}"#, #"{"type":"session.resume"}"#], + held: false + ), + ( + lines: [#"{"type":"tool.execution_start","data":{"toolCallId":"a","toolName":"ask_user"}}"#], + held: true + ), + ( + lines: [ + #"{"type":"tool.execution_start","data":{"toolCallId":"a","toolName":"ask_user"}}"#, + #"{"type":"tool.execution_complete","data":{"toolCallId":"b"}}"#, + ], + held: true + ), + ( + lines: [ + #"{"type":"tool.execution_start","data":{"toolCallId":"a","toolName":"ask_user"}}"#, + #"{"type":"tool.execution_complete","data":{"toolCallId":"a"}}"#, + ], + held: false + ), + ( + lines: [#"{"type":"tool.execution_start","data":{"toolCallId":"a","toolName":"bash"}}"#], + held: false + ), + ( + lines: [#"{"type":"assistant.message","data":{"content":"permission.requested"}}"#], + held: false + ), + ]) + func aDecisionIsOnlyAnOpenQuestion(lines: [String], held: Bool) { + #expect(CopilotSessionLog.holdsADecision(lines) == held) + } + + @Test + func aRemoteRenameOnlyProceedsOnAProvenAnswer() { + let marker = ZmxSessionLauncher.remoteProbeMarker + let pending = { CopilotSessionLog.remoteDecisionPending(succeeded: $0, output: $1) } + #expect(pending(false, "\(marker) read")) + #expect(pending(true, "a banner from zshrc")) + #expect(pending(true, "\(marker) booting")) + #expect(!pending(true, "\(marker) absent")) + #expect(!pending(true, "banner\n\(marker) read")) + #expect(pending(true, "{\"type\":\"permission.requested\"}\n\(marker) read")) + } + + /// Runs the remote decision read against a fake home and a fake `zmx`, through the same + /// wrapper a real dial uses. + @Test + func theRemoteDecisionReadFindsAnOpenDialogUnderATitledName() throws { + let home = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("copilot-decision-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager.default.removeItem(at: home) } + let session = home.appendingPathComponent(".copilot/session-state/the-session") + let bin = home.appendingPathComponent("bin") + for directory in [session, bin] { + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + } + try "name: 'Fix: login · 11111111'\n".write( + to: session.appendingPathComponent("workspace.yaml"), atomically: true, encoding: .utf8) + try "{\"type\":\"permission.requested\"}\n".write( + to: session.appendingPathComponent("events.jsonl"), atomically: true, encoding: .utf8) + let zmx = bin.appendingPathComponent("zmx") + try "#!/bin/sh\nexit 0\n".write(to: zmx, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: zmx.path) + + let location = RemoteProjectLocation(host: "box", remotePath: "/srv/widget") + let invocation = CopilotSessionLog.remoteDecisionInvocation( + forNode: node(.copilotCLI), at: location) + let process = Process() + let pipe = Pipe() + process.executableURL = URL(fileURLWithPath: "/bin/sh") + process.arguments = ["-c", try #require(invocation.last)] + process.environment = [ + "HOME": home.path, "PATH": "\(bin.path):/usr/bin:/bin", "LC_ALL": "en_US.UTF-8", + ] + process.standardOutput = pipe + try process.run() + let output = String( + decoding: pipe.fileHandleForReading.readDataToEndOfFile(), as: UTF8.self) + process.waitUntilExit() + #expect(output.contains("\(ZmxSessionLauncher.remoteProbeMarker) read")) + #expect(CopilotSessionLog.remoteDecisionPending(succeeded: true, output: output)) + } + + @Test + func copilotsNameFitsWhatItsRenameAccepts() throws { + let name = CLISessionBackendKind.copilotSessionName( + title: "A \"quoted\"\ttitle " + String(repeating: "x", count: 200), sessionName: sessionName) + #expect(name.count <= 100) + #expect(!name.contains("\"")) + #expect(!name.contains("\t")) + #expect(CLISessionBackendKind.isCopilotSessionName(name, for: sessionName)) + } + + @Test + func everyRemoteCopilotReadMatchesTheTitledName() { + let location = RemoteProjectLocation(host: "box", remotePath: "/srv/widget") + let node = node(.copilotCLI) + let scripts = [ + CopilotSessionLog.remotePresenceInvocation(forNode: node, at: location), + CopilotSessionLog.remoteSummaryInvocation(forNode: node, at: location, since: nil), + ].map { $0.joined(separator: " ") } + for script in scripts { + #expect(script.contains(".* · 11111111")) + #expect(!script.contains("grep -qx")) + } + } + + /// Every name Copilot can record for a node, in the spellings its YAML writer picks, as + /// the remote walk reads them — a titled session it cannot find has no banked id, and a + /// remote loop with no banked id restarts from scratch. + @Test(arguments: [ + (locale: "en_US.UTF-8", name: "graphcode-11111111-2222-3333-4444-555555555555"), + (locale: "en_US.UTF-8", name: "Fix login · 11111111"), + (locale: "en_US.UTF-8", name: "'Fix: login · 11111111'"), + (locale: "en_US.UTF-8", name: "\"Fix: login · 11111111\""), + (locale: "C", name: "Fix login · 11111111"), + (locale: "C", name: "'Fix: login · 11111111'"), + ]) + func theRemoteWalkFindsEveryNameOfTheNode(locale: String, name: String) throws { + let workspace = "client_name: github/cli\nname: \(name)\n" + #expect(try bankedID(recording: workspace, locale: locale) != nil) + } + + @Test(arguments: [ + "name: Fix login · 99999999", + "name: Fix login", + "name: graphcode-99999999-2222-3333-4444-555555555555", + "client_name: Fix login · 11111111", + "name: Fix login · 11111111x", + ]) + func theRemoteWalkFindsNoOtherSession(line: String) throws { + #expect(try bankedID(recording: "\(line)\n", locale: "en_US.UTF-8") == nil) + } + + /// The pattern's quotes, `$` and `·` have to arrive intact through the one wrapper every + /// remote script rides in. + @Test + func theRemoteWalkSurvivesTheLoginShellQuoting() throws { + let location = RemoteProjectLocation(host: "box", remotePath: "/srv/widget") + let workspace = "name: 'Fix: login · 11111111'\n" + #expect( + try bankedID( + recording: workspace, locale: "en_US.UTF-8", wrap: location.remoteLoginShellCommand) + != nil) + } + + /// Runs the ensure's bank fragment under `/bin/sh` against a fake home, and reads back + /// the id it banked. + private func bankedID( + recording workspace: String, locale: String, wrap: (String) -> String = { $0 } + ) throws -> String? { + let home = URL(fileURLWithPath: NSTemporaryDirectory()) + .appendingPathComponent("copilot-walk-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager.default.removeItem(at: home) } + let session = home.appendingPathComponent(".copilot/session-state/the-session") + try FileManager.default.createDirectory(at: session, withIntermediateDirectories: true) + try workspace.write( + to: session.appendingPathComponent("workspace.yaml"), atomically: true, encoding: .utf8) + + let process = Process() + process.executableURL = URL(fileURLWithPath: "/bin/sh") + process.arguments = ["-c", wrap(CopilotSessionLog.remoteIDBankFragment(forNodeID: nodeID))] + process.environment = ["HOME": home.path, "PATH": "/usr/bin:/bin", "LC_ALL": locale] + try process.run() + process.waitUntilExit() + #expect(process.terminationStatus == 0) + let banked = home.appendingPathComponent( + ".graphcode/sessions/\(nodeID.uuidString).id") + return try? String(contentsOf: banked, encoding: .utf8) + } + + /// `ask` held a real session at a prompt for two minutes with nothing in its log but + /// `session.start`, so a session with an opening prompt counts as busy until it shows up. + @Test + func aSessionStillOpeningHoldsTheRename() { + let start = #"{"type":"session.start"}"# + let message = #"{"type":"user.message"}"# + #expect(CopilotSessionLog.holdsADecision([start], expectingOpening: true)) + #expect(!CopilotSessionLog.holdsADecision([start, message], expectingOpening: true)) + #expect(!CopilotSessionLog.holdsADecision([start], expectingOpening: false)) + #expect( + !CopilotSessionLog.holdsADecision( + [start, #"{"type":"session.resume"}"#], expectingOpening: true)) + } +} diff --git a/graphcode/Tests/SessionNamingTests.swift b/graphcode/Tests/SessionNamingTests.swift index cce531a27..42ada4724 100644 --- a/graphcode/Tests/SessionNamingTests.swift +++ b/graphcode/Tests/SessionNamingTests.swift @@ -254,67 +254,4 @@ struct SessionNamingTests { func aRenameReachesNoSessionItCannotName(_ backend: CLISessionBackendKind) async { #expect(await typedAfterRenaming(backend).typed.isEmpty) } - - /// Every name Copilot can record for a node, in the spellings its YAML writer picks, as - /// the remote walk reads them — a titled session it cannot find has no banked id, and a - /// remote loop with no banked id restarts from scratch. - @Test(arguments: [ - (locale: "en_US.UTF-8", name: "graphcode-11111111-2222-3333-4444-555555555555"), - (locale: "en_US.UTF-8", name: "Fix login · 11111111"), - (locale: "en_US.UTF-8", name: "'Fix: login · 11111111'"), - (locale: "en_US.UTF-8", name: "\"Fix: login · 11111111\""), - (locale: "C", name: "Fix login · 11111111"), - (locale: "C", name: "'Fix: login · 11111111'"), - ]) - func theRemoteWalkFindsEveryNameOfTheNode(locale: String, name: String) throws { - let workspace = "client_name: github/cli\nname: \(name)\n" - #expect(try bankedID(recording: workspace, locale: locale) != nil) - } - - @Test(arguments: [ - "name: Fix login · 99999999", - "name: Fix login", - "name: graphcode-99999999-2222-3333-4444-555555555555", - "client_name: Fix login · 11111111", - ]) - func theRemoteWalkFindsNoOtherSession(line: String) throws { - #expect(try bankedID(recording: "\(line)\n", locale: "en_US.UTF-8") == nil) - } - - /// The pattern's quotes, `$` and `·` have to arrive intact through the one wrapper every - /// remote script rides in. - @Test - func theRemoteWalkSurvivesTheLoginShellQuoting() throws { - let location = RemoteProjectLocation(host: "box", remotePath: "/srv/widget") - let workspace = "name: 'Fix: login · 11111111'\n" - #expect( - try bankedID( - recording: workspace, locale: "en_US.UTF-8", wrap: location.remoteLoginShellCommand) - != nil) - } - - /// Runs the ensure's bank fragment under `/bin/sh` against a fake home, and reads back - /// the id it banked. - private func bankedID( - recording workspace: String, locale: String, wrap: (String) -> String = { $0 } - ) throws -> String? { - let home = URL(fileURLWithPath: NSTemporaryDirectory()) - .appendingPathComponent("copilot-walk-\(UUID().uuidString)", isDirectory: true) - defer { try? FileManager.default.removeItem(at: home) } - let session = home.appendingPathComponent(".copilot/session-state/the-session") - try FileManager.default.createDirectory(at: session, withIntermediateDirectories: true) - try workspace.write( - to: session.appendingPathComponent("workspace.yaml"), atomically: true, encoding: .utf8) - - let process = Process() - process.executableURL = URL(fileURLWithPath: "/bin/sh") - process.arguments = ["-c", wrap(CopilotSessionLog.remoteIDBankFragment(forNodeID: nodeID))] - process.environment = ["HOME": home.path, "PATH": "/usr/bin:/bin", "LC_ALL": locale] - try process.run() - process.waitUntilExit() - #expect(process.terminationStatus == 0) - let banked = home.appendingPathComponent( - ".graphcode/sessions/\(nodeID.uuidString).id") - return try? String(contentsOf: banked, encoding: .utf8) - } }