diff --git a/GraphcodeKit/Sources/CLI/GraphcodeCommand.swift b/GraphcodeKit/Sources/CLI/GraphcodeCommand.swift index c554d42c..5c08ac6d 100644 --- a/GraphcodeKit/Sources/CLI/GraphcodeCommand.swift +++ b/GraphcodeKit/Sources/CLI/GraphcodeCommand.swift @@ -816,7 +816,10 @@ extension GraphcodeCommand { return lines.joined(separator: "\n") } for node in graph.nodes { - var line = " \(node.id) \(node.displayState) \(node.loopType) \(node.title)" + // A finished loop's session can still sit at its prompt; the presence it reports + // must not hide the ending, or the loop reads as one that can still be restarted. + let state = node.isResolved ? node.state : node.displayState + var line = " \(node.id) \(state) \(node.loopType) \(node.title)" if let exitCode = node.presence?.exitCode { line += " ← session exited (\(exitCode))" } else if let reason = AttentionRollup.reason(for: node) { diff --git a/GraphcodeKit/Sources/GraphStore.swift b/GraphcodeKit/Sources/GraphStore.swift index 9124dabd..d9fc5a9b 100644 --- a/GraphcodeKit/Sources/GraphStore.swift +++ b/GraphcodeKit/Sources/GraphStore.swift @@ -1317,7 +1317,7 @@ public actor GraphStore { await stopNode(nodeID) case .restartNode(let nodeID): - await restartNode(nodeID) + await restartNode(nodeID, from: connectionID) case .restartSessions: await restartSessions() @@ -3252,9 +3252,9 @@ public actor GraphStore { /// a stopped one was told to stay down, so both are refused rather than revived — except /// a loop stopped for a missing CLI, which nobody told to stay down and whose restart /// is exactly what its dialog asks the human for once the CLI is installed. - private func restartNode(_ nodeID: UUID) async { + private func restartNode(_ nodeID: UUID, from connectionID: UUID?) async { guard let node = graph.nodes[id: nodeID] else { - announceError("no loop \(nodeID) in this graph") + await refuse("no loop \(nodeID) in this graph", to: connectionID) return } if node.state == .stopped, let failure = node.launchFailure { @@ -3262,7 +3262,8 @@ public actor GraphStore { return } guard !node.isResolved else { - announceError("\(node.title) has finished — there is no session to restart") + await refuse( + "\(node.title) has finished — there is no session to restart", to: connectionID) return } if node.loopType == .composite { @@ -4508,6 +4509,19 @@ public actor GraphStore { pendingErrors.append(message) } + /// A refusal that is one client's business: changing nothing, it has no `.graphChanged` + /// to acknowledge it, so the asker's CLI hears only this — and a bystander parked on + /// its own verdict would take a broadcast copy for one (`mailroomInbox` does the same). + /// Without a v1 requester to answer, it falls back to the broadcast. + private func refuse(_ message: String, to connectionID: UUID?) async { + guard let connectionID, connections[connectionID] != nil else { + announceError(message) + return + } + _ = await send(.errorOccurred(message), to: connectionID) + onAnnounceError?(message) + } + private func reject( _ message: String, broadcastErrors: Bool diff --git a/graphcode-cli/Sources/main.swift b/graphcode-cli/Sources/main.swift index 8b9c1ff0..07430ffa 100644 --- a/graphcode-cli/Sources/main.swift +++ b/graphcode-cli/Sources/main.swift @@ -214,9 +214,14 @@ func runAndPrintGraph(projectPath: String, _ commands: [DaemonCommand]) throws { try sendCommand(command) } phase = "waiting for the daemon to acknowledge the command" + // A refused command changes nothing, so its error is the only answer that will come. let event = try client.waitForEvent { - if case .graphChanged = $0 { return true } else { return false } + switch $0 { + case .graphChanged, .errorOccurred: return true + default: return false + } } + if case .errorOccurred(let message) = event { fail(message) } if case .graphChanged(let graph) = event { print(GraphcodeCommand.render(graph, mailroomReader: mailroomReader)) } diff --git a/graphcode/Tests/GraphcodeCommandTests.swift b/graphcode/Tests/GraphcodeCommandTests.swift index aa7bb025..ad46df88 100644 --- a/graphcode/Tests/GraphcodeCommandTests.swift +++ b/graphcode/Tests/GraphcodeCommandTests.swift @@ -345,6 +345,20 @@ struct GraphcodeCommandTests { #expect(output.contains("session exited (1)")) } + @Test + func renderingAFinishedLoopShowsItsEndingOverItsOpenSession() { + let node = LoopNode( + title: "Naming check", loopType: .goalBased, goal: GoalSpec(summary: "work"), + presence: PresenceReading(presence: .awaitingInput, confidence: .scanned), + state: .succeeded) + let graph = LoopGraph( + project: ProjectRef(path: "/tmp/x", name: "x"), nodes: [node]) + + let output = GraphcodeCommand.render(graph) + + #expect(output.contains("\(node.id) succeeded goalBased Naming check")) + } + @Test func renderingAnEmptyGraphSaysSoRatherThanPrintingNothing() { let output = GraphcodeCommand.render( diff --git a/graphcode/Tests/SessionRestartTests.swift b/graphcode/Tests/SessionRestartTests.swift index 556028f3..0a2ae120 100644 --- a/graphcode/Tests/SessionRestartTests.swift +++ b/graphcode/Tests/SessionRestartTests.swift @@ -70,6 +70,65 @@ struct SessionRestartTests { #expect(errors.value.count == 1) } + /// A refused restart changes nothing, so no `.graphChanged` acknowledges it: the + /// asking CLI hears only the refusal, and a bystander's CLI, parked on a verdict of its + /// own, must not take a broadcast copy for one. + @Test + func aRefusedRestartAnswersOnlyTheConnectionThatAsked() async throws { + let store = GraphStore(onRestartSession: { _, _ in true }) + await store.handle(.createNode(draft("Done"))) + let id = await store.graph.nodes[0].id + await store.handle(.nodeCheckApproved(id)) + var asker: [Int32] = [0, 0] + var bystander: [Int32] = [0, 0] + #expect(socketpair(AF_UNIX, SOCK_STREAM, 0, &asker) == 0) + #expect(socketpair(AF_UNIX, SOCK_STREAM, 0, &bystander) == 0) + defer { + for descriptor in [asker[0], asker[1], bystander[0], bystander[1]] { close(descriptor) } + } + let askerID = UUID() + await store.addConnection(id: askerID, fileDescriptor: asker[0]) + await store.addConnection(id: UUID(), fileDescriptor: bystander[0]) + guard case .graphChanged = try nextEvent(from: asker[1]), + case .graphChanged = try nextEvent(from: bystander[1]) + else { + Issue.record("expected each connection's joining snapshot") + return + } + + await store.handle(.restartNode(id), from: askerID) + + guard case .errorOccurred(let message) = try nextEvent(from: asker[1]) else { + Issue.record("the asker heard nothing, so its CLI waits out its timeout") + return + } + #expect(message == "Done has finished — there is no session to restart") + var reached: [String] = [] + while let event = try nextEvent(from: bystander[1], waiting: 1) { + if case .errorOccurred(let message) = event { reached.append(message) } + } + #expect(reached.isEmpty) + } + + /// Blocking read with a deadline: `OutboundChannels` writes off the actor, so a + /// non-blocking read straight after `handle` races the delivery. + private func nextEvent(from descriptor: Int32, waiting seconds: Int32 = 2) throws + -> DaemonEvent? + { + var timeout = timeval(tv_sec: Int(seconds), tv_usec: 0) + setsockopt( + descriptor, SOL_SOCKET, SO_RCVTIMEO, &timeout, socklen_t(MemoryLayout.size)) + var length: UInt32 = 0 + var header = [UInt8](repeating: 0, count: 4) + guard recv(descriptor, &header, 4, MSG_WAITALL) == 4 else { return nil } + withUnsafeMutableBytes(of: &length) { $0.copyBytes(from: header) } + var payload = [UInt8](repeating: 0, count: Int(UInt32(bigEndian: length))) + guard recv(descriptor, &payload, payload.count, MSG_WAITALL) == payload.count else { + return nil + } + return try JSONDecoder().decode(DaemonEvent.self, from: Data(payload)) + } + @Test func restartingEverySessionSkipsTheFinishedOnes() async { let restarted = LockIsolated>([])