diff --git a/GraphcodeKit/Sources/Domain/BackendCapabilities.swift b/GraphcodeKit/Sources/Domain/BackendCapabilities.swift index f97b8dbd..fdb158c6 100644 --- a/GraphcodeKit/Sources/Domain/BackendCapabilities.swift +++ b/GraphcodeKit/Sources/Domain/BackendCapabilities.swift @@ -102,18 +102,11 @@ extension CLISessionBackendKind { // timer of its own. Copilot has since grown one. Worth knowing if a recurring loop // runs once and stops: that is the symptom of a Copilot too old to have it, and // `copilot help commands` on the machine running the loop is where to check. - // - // `supportsSubAgents` was false for the same reason and flipped the same way: read - // off 1.0.80's `copilot help commands`, which lists `/fleet` ("enable fleet mode for - // parallel subagent execution"), `/tasks` ("view and manage tasks (subagents and - // shell commands)") and `/subagents`, plus `--agent ` on the launch line. - // That is the fan-out a composite leans on. The same age caveat applies: a - // composite whose Copilot workers never fan out is a Copilot older than that. return BackendCapabilities( supportsGoalMode: true, supportsHooks: false, supportsStructuredOutput: true, - supportsSubAgents: true, + supportsSubAgents: false, supportsMCP: true, supportsMidSessionInput: true, supportsInSessionRecurrence: true) diff --git a/GraphcodeKit/Sources/GraphStore.swift b/GraphcodeKit/Sources/GraphStore.swift index d13d4ff5..e7c6d08d 100644 --- a/GraphcodeKit/Sources/GraphStore.swift +++ b/GraphcodeKit/Sources/GraphStore.swift @@ -811,15 +811,6 @@ public actor GraphStore { goalCache: goalCache, recurrence: effects.recurrence, subGraphDepth: subGraphDepth + 1) - // A loop added inside a composite with no backend named runs on the composite's — - // a Copilot composite must produce Copilot workers, the same rule `createNode` - // applies to a loop fanning out from inside its own session. A creator the tree - // can find still wins, exactly as it would at the top level. - var command = command - if case .createNode(var draft) = command, draft.backend == nil { - draft.backend = draft.createdBy.flatMap { stored($0)?.backend } ?? node.backend - command = .createNode(draft) - } await child.handle(command) // Settled before the write-back and roll-up below, so a client sees the refusal // ahead of the broadcast it would otherwise time out against, and an update's diff --git a/GraphcodeKit/Sources/ProjectRegistry.swift b/GraphcodeKit/Sources/ProjectRegistry.swift index 03165f8f..d4916eec 100644 --- a/GraphcodeKit/Sources/ProjectRegistry.swift +++ b/GraphcodeKit/Sources/ProjectRegistry.swift @@ -278,8 +278,12 @@ public actor ProjectRegistry { send(.recentProjectsListed(persistence.loadRecentProjects()), to: fileDescriptor) case .openProject(let path): - guard Self.isOpenable(path) else { break } - await open(Self.canonicalize(path), for: connectionID, fileDescriptor: fileDescriptor) + switch routing(for: path, isSidebar: sidebarConnections.contains(connectionID)) { + case .project(let canonicalPath): + await open(canonicalPath, for: connectionID, fileDescriptor: fileDescriptor) + case .refused(let reason): + send(.errorOccurred(reason), to: fileDescriptor) + } case .restoreOpenProjects: // Each of these broadcasts a `.graphChanged` exactly as `.openProject` would, so @@ -294,7 +298,7 @@ public actor ProjectRegistry { // it is joined to projects *other* clients open, so `graphcode status ` // puts a row in a running app instead of one that only appears next launch. sidebarConnections.insert(connectionID) - for path in persistence.loadOpenProjects() where Self.isWellFormedProjectPath(path) { + for path in prunedOpenProjects() where Self.isWellFormedProjectPath(path) { await open(path, for: connectionID, fileDescriptor: fileDescriptor) } @@ -308,6 +312,7 @@ public actor ProjectRegistry { let canonicalPath = Self.canonicalize(path) await close(canonicalPath, for: connectionID) persistence.forgetProject(path: canonicalPath) + if path != canonicalPath { persistence.forgetProject(path: path) } case .deleteProjectGraph(let path): let canonicalPath = Self.canonicalize(path) @@ -331,8 +336,20 @@ public actor ProjectRegistry { persistence.deleteGraph(path: canonicalPath) case .graphCommand(let path, let inner): - guard let store = stores[Self.canonicalize(path)] else { return } - await store.handle(inner) + // Routed the same way the open was, so a client that had its path redirected to the + // project containing it addresses that project here too. Without the second half, + // the open would land on one graph and every command after it on nothing at all — + // silently, which is how a `node create` could look like it hung. + switch routing(for: path, isSidebar: sidebarConnections.contains(connectionID)) { + case .project(let canonicalPath): + guard let store = stores[canonicalPath] else { + send(.errorOccurred("\(path) isn't open — open it first."), to: fileDescriptor) + return + } + await store.handle(inner) + case .refused(let reason): + send(.errorOccurred(reason), to: fileDescriptor) + } } } @@ -378,7 +395,38 @@ public actor ProjectRegistry { await store.removeConnection(connectionID) } connectionProjectPaths[connectionID]?.remove(canonicalPath) - persistence.saveOpenProjects(persistence.loadOpenProjects().filter { $0 != canonicalPath }) + // Compared canonically, not literally: a project added before remote paths were + // normalized is stored under the spelling it arrived with, and closing it sends that + // spelling back through `canonicalize`. Filtering on the raw string left those rows + // in the open set and un-closable. + persistence.saveOpenProjects( + persistence.loadOpenProjects().filter { Self.canonicalize($0) != canonicalPath }) + } + + /// Clears out the empty twins a pre-normalization daemon left in the sidebar: a stored + /// path that is only another stored path spelled differently — a trailing slash, a + /// doubled separator — and whose graph never received a loop or a board post. + /// + /// Deliberately timid. A twin with anything in it is left exactly where it is: it is + /// somebody's work, and folding it into the project it duplicates would make those + /// loops vanish rather than be found. Its graph file is kept either way; only the + /// sidebar entry and the recents row go, and re-opening the path brings both back. + private func prunedOpenProjects() -> [String] { + let stored = persistence.loadOpenProjects() + let kept = stored.filter { path in + let canonical = Self.canonicalize(path) + // Only ever a *later* twin, so the first spelling of a project always survives even + // when every stored spelling of it is a variant. + guard path != canonical, + stored.prefix(while: { $0 != path }).contains(where: { Self.canonicalize($0) == canonical }) + else { return true } + let graph = persistence.loadGraph(path: path) + let isEmpty = (graph?.nodesAtAnyDepth.isEmpty ?? true) && (graph?.artifactory.isEmpty ?? true) + if isEmpty { persistence.forgetProject(path: path) } + return !isEmpty + } + if kept != stored { persistence.saveOpenProjects(kept) } + return kept } /// Append rather than insert-at-front: the sidebar should come back in the order it @@ -397,6 +445,82 @@ public actor ProjectRegistry { return true } + // MARK: - Which project a named path belongs to + + enum PathRouting: Equatable { + case project(String) + case refused(String) + } + + /// Where a path a client named should be routed, and whether it may become a *new* + /// project rather than an existing one. + /// + /// Opening is create-if-missing, because that is how a folder becomes a project at all: + /// `graphcode status ` from a shell is a supported way to add one. What that + /// missed is that most paths a *loop* names are not new projects — they are its own + /// worktree, its working directory, or its project's path spelled slightly differently. + /// Each of those quietly became a second project: its own graph, its own recents entry, + /// its own row in the sidebar under the same name, with the loops the agent then created + /// inside it where nobody was looking. A codespace made it trivial to hit, since a + /// remote path is never checked against a filesystem: every spelling of one was openable. + /// + /// So two kinds of path are never a new project when a shell client names them: + /// + /// - **A folder inside a project that already exists** is that project — a worktree + /// under the repository, a subdirectory, the remote path of a codespace already added. + /// - **A remote path this daemon has never seen.** Remote projects are added in the app, + /// which validates the connection over ssh first; nothing typed at a shell can be + /// checked that way, so an unknown one is a typo or a spelling variant of a known one. + /// + /// The app is exempt from both, and is told apart by having asked for the whole open set + /// (`sidebarConnections`): opening a nested folder or adding a remote host is a + /// deliberate human act there, and refusing it would break Add Folder. + func routing(for path: String, isSidebar: Bool) -> PathRouting { + guard Self.isWellFormedProjectPath(path) else { + return .refused( + "\(path) isn't a project path — name an absolute folder, an ssh:// or codespace:// " + + "project, or \(LoopGraphScope.globalPath).") + } + let canonicalPath = Self.canonicalize(path) + guard canonicalPath != LoopGraphScope.globalPath else { return .project(canonicalPath) } + let known = knownProjectPaths() + if known.contains(canonicalPath) { return .project(canonicalPath) } + if !isSidebar, let container = Self.project(containing: canonicalPath, in: known) { + return .project(container) + } + if RemoteProjectLocation.parse(projectPath: canonicalPath) != nil { + guard isSidebar else { + return .refused( + "graphcode doesn't know a project at \(canonicalPath). Run `graphcode projects` " + + "for the exact path; a remote repository or codespace is added in the app.") + } + return .project(canonicalPath) + } + guard Self.isOpenable(canonicalPath) else { + return .refused( + "there's no folder at \(canonicalPath). Run `graphcode projects` for the paths " + + "graphcode knows.") + } + return .project(canonicalPath) + } + + /// Every project this daemon knows about, canonically spelled: what the sidebar has + /// open, what recents remembers, and whatever is resident. + private func knownProjectPaths() -> Set { + var paths = Set(persistence.loadOpenProjects().map(Self.canonicalize)) + paths.formUnion(persistence.loadRecentProjects().map { Self.canonicalize($0.path) }) + paths.formUnion(stores.keys) + return paths + } + + /// The deepest known project a path lies inside — deepest so that a nested project a + /// human deliberately opened wins over the repository around it. + static func project(containing path: String, in known: Set) -> String? { + known + .filter { $0 != LoopGraphScope.globalPath && path.hasPrefix($0 + "/") } + .max { $0.count < $1.count } + } + // MARK: - The global Orchestrator Graph /// Loads the one always-resident global graph @@ -551,9 +675,18 @@ public actor ProjectRegistry { /// project it asked for by the path a folder picker handed it comes back named by this, /// and `/tmp` vs `/private/tmp` is enough to make the two look like different projects. public static func canonicalize(_ path: String) -> String { - guard path != LoopGraphScope.globalPath, - RemoteProjectLocation.parse(projectPath: path) == nil - else { return path } + guard path != LoopGraphScope.globalPath else { return path } + // A remote path gets the textual half of the same treatment. It cannot be resolved + // against this filesystem — the directory is on another machine — but the spellings + // that fork one project into two are all textual: a trailing slash, a doubled + // separator, a `.` segment. Left unnormalized, `codespace://cs/workspaces/repo/` and + // `codespace://cs/workspaces/repo` were two projects, two graphs, and two rows in the + // sidebar with the same name. + if let remote = RemoteProjectLocation.parse(projectPath: path) { + var normalized = remote + normalized.remotePath = RemoteProjectLocation.normalizedPath(remote.remotePath) + return normalized.projectPath + } return URL(fileURLWithPath: path).resolvingSymlinksInPath().path } diff --git a/GraphcodeKit/Sources/Sessions/RemoteGraphAccess.swift b/GraphcodeKit/Sources/Sessions/RemoteGraphAccess.swift index 99785133..acaa018e 100644 --- a/GraphcodeKit/Sources/Sessions/RemoteGraphAccess.swift +++ b/GraphcodeKit/Sources/Sessions/RemoteGraphAccess.swift @@ -297,9 +297,16 @@ public enum RemoteGraphAccess { def open_project(self, path): self.send({"openProject": {"path": path}}) - _, value = self.wait_for(["graphChanged"]) + key, value = self.wait_for(["graphChanged", "errorOccurred"]) + if key == "errorOccurred": + fail(value["_0"]) return value["_0"] + def known_projects(self): + self.send({"listRecentProjects": {}}) + _, value = self.wait_for(["recentProjectsListed"]) + return [p.get("path") for p in (value["_0"] or []) if p.get("path")] + def self_node_id(): name = os.environ.get("ZMX_SESSION", "") @@ -355,21 +362,90 @@ public enum RemoteGraphAccess { return {"graphCommand": {"projectPath": project, "command": command}} - def run_and_print(project, commands): + def normalized(path): + parts = [] + for segment in path.split("/"): + if not segment or segment == ".": + continue + if segment == "..": + if parts: + parts.pop() + continue + parts.append(segment) + return "/" + "/".join(parts) + + + def remote_parts(project): + # (host, path) for a project this daemon reaches over ssh, else (None, None). + for scheme in ("ssh://", "codespace://"): + if project.startswith(scheme): + rest = project[len(scheme):] + separator = rest.find("/") + if separator < 1: + return None, None + authority = rest[:separator].split("@")[-1].split(":")[0] + return authority, normalized(rest[separator:]) + return None, None + + + def this_host_names(): + names = [os.environ.get("CODESPACE_NAME", "")] + try: + names.append(os.uname().nodename) + except Exception: + pass + return [name for name in names if name] + + + def resolve_project(daemon, project): + # A path spelled the way *this host* sees it -- the session's working directory, + # its worktree -- named as the project it belongs to. The Mac keys graphs by the + # ssh:// or codespace:// URI, and opening one is create-if-missing, so a local + # spelling used to add a second project with the same name and put the loops in + # there rather than in the graph the human is watching. + if not project.startswith("/"): + return project + here = normalized(project) + matches = [] + for known in daemon.known_projects(): + host, path = remote_parts(known) + if path is None: + continue + if here == path or here.startswith(path.rstrip("/") + "/"): + matches.append((known, host)) + if not matches: + return project + if len(matches) > 1: + local = this_host_names() + preferred = [match for match in matches if match[1] in local] + if len(preferred) != 1: + fail("%s is inside more than one project graphcode knows (%s). Name the " + "one you mean -- your briefing states it exactly." + % (project, ", ".join(sorted(match[0] for match in matches)))) + matches = preferred + resolved = matches[0][0] + sys.stderr.write("graphcode: %s is this host's path for %s\n" % (project, resolved)) + return resolved + + + def run_and_print(project, inner=None): daemon = Daemon() + project = resolve_project(daemon, project) graph = daemon.open_project(project) - for command in commands: - daemon.send(command) - if commands: - _, value = daemon.wait_for(["graphChanged"]) + if inner is not None: + daemon.send(graph_command(project, inner)) + key, value = daemon.wait_for(["graphChanged", "errorOccurred"]) + if key == "errorOccurred": + fail(value["_0"]) graph = value["_0"] print(render(graph)) - def run_with_verdict(project, command, acknowledgement): + def run_with_verdict(project, inner, acknowledgement): daemon = Daemon() + project = resolve_project(daemon, project) daemon.open_project(project) - daemon.send(command) + daemon.send(graph_command(project, inner)) key, value = daemon.wait_for(["graphChanged", "errorOccurred"]) if key == "errorOccurred": fail(value["_0"]) @@ -462,7 +538,7 @@ public enum RemoteGraphAccess { if verb == "status": if not arguments: fail("missing project-path") - run_and_print(arguments[0], []) + run_and_print(arguments[0]) return if verb != "node": fail("unknown or Mac-only command: %s (see `graphcode help`)" % verb) @@ -483,7 +559,7 @@ public enum RemoteGraphAccess { if flags.get("into"): into = parse_uuid(flags["into"], "--into") create = {"subGraphCommand": {"nodeID": into, "command": create}} - run_and_print(project, [graph_command(project, create)]) + run_and_print(project, create) return if subverb not in ("stop", "restart", "delete", "send", "memo"): fail("node %s runs from the Mac's own shell, not from a remote host" % subverb) @@ -491,11 +567,11 @@ public enum RemoteGraphAccess { fail("missing node-id") node_id = parse_uuid(arguments.pop(0), "node-id") if subverb == "stop": - run_and_print(project, [graph_command(project, {"stopNode": {"_0": node_id}})]) + run_and_print(project, {"stopNode": {"_0": node_id}}) elif subverb == "restart": - run_and_print(project, [graph_command(project, {"restartNode": {"_0": node_id}})]) + run_and_print(project, {"restartNode": {"_0": node_id}}) elif subverb == "delete": - run_and_print(project, [graph_command(project, {"deleteNode": {"_0": node_id}})]) + run_and_print(project, {"deleteNode": {"_0": node_id}}) else: follow_up = False if subverb == "send" and arguments and arguments[0] == "--follow-up": @@ -511,12 +587,11 @@ public enum RemoteGraphAccess { if subverb == "send": if follow_up: payload["followUp"] = True - run_with_verdict(project, graph_command(project, {"messageNode": payload}), + run_with_verdict(project, {"messageNode": payload}, "accepted — typed in when the loop next goes idle" if follow_up else "delivered") else: - run_with_verdict(project, graph_command(project, {"memoNode": payload}), - "noted") + run_with_verdict(project, {"memoNode": payload}, "noted") main(sys.argv[1:]) diff --git a/graphcode-cli/Sources/main.swift b/graphcode-cli/Sources/main.swift index f59c9898..597332f4 100644 --- a/graphcode-cli/Sources/main.swift +++ b/graphcode-cli/Sources/main.swift @@ -107,6 +107,27 @@ defer { client.closeConnection() } let artifactoryReader = SurfaceRef.nodeID( fromZmxSessionName: ProcessInfo.processInfo.environment["ZMX_SESSION"] ?? "") +/// Joins the project every verb addresses, and stops here when the daemon refuses it. +/// +/// A refusal — a path that names no folder, a remote project graphcode has never been +/// told about, a spelling of one it has — used to arrive as an event nobody was reading: +/// the wait for `.graphChanged` ran to the socket timeout and then reported the command +/// as *possibly applied*, when in fact nothing past the open had been sent. Reading the +/// error is what turns that into one line saying which path was wrong. +@discardableResult +func openProject(_ projectPath: String) throws -> LoopGraph? { + try client.send(.openProject(path: projectPath)) + let opened = try client.waitForEvent { + switch $0 { + case .graphChanged, .errorOccurred: return true + case .recentProjectsListed: return false + } + } + if case .errorOccurred(let message) = opened { fail(message) } + if case .graphChanged(let graph) = opened { return graph } + return nil +} + /// Every mutating verb waits for the `.graphChanged` broadcast its own command caused, /// then prints the resulting graph. That's the daemon's only acknowledgement — it has no /// request/response correlation — and it doubles as useful output. @@ -118,14 +139,11 @@ let artifactoryReader = SurfaceRef.nodeID( /// other loop — would arrive and be mistaken for the acknowledgement, so `status` /// appeared to work and hung only on quiet projects. func runAndPrintGraph(projectPath: String, _ commands: [DaemonCommand]) throws { - try client.send(.openProject(path: projectPath)) - let opened = try client.waitForEvent { - if case .graphChanged = $0 { return true } else { return false } - } + let opened = try openProject(projectPath) guard !commands.isEmpty else { - if case .graphChanged(let graph) = opened { - print(GraphcodeCommand.render(graph, artifactoryReader: artifactoryReader)) + if let opened { + print(GraphcodeCommand.render(opened, artifactoryReader: artifactoryReader)) } return } @@ -220,8 +238,7 @@ do { // loop, ZMX_SESSION names the sender, and the target sees who's talking. let sender = SurfaceRef.nodeID( fromZmxSessionName: ProcessInfo.processInfo.environment["ZMX_SESSION"] ?? "") - try client.send(.openProject(path: projectPath)) - _ = try client.waitForEvent { if case .graphChanged = $0 { return true } else { return false } } + try openProject(projectPath) try client.send( .graphCommand( projectPath: projectPath, @@ -246,13 +263,10 @@ do { var attributed = update attributed.updatedBy = SurfaceRef.nodeID( fromZmxSessionName: ProcessInfo.processInfo.environment["ZMX_SESSION"] ?? "") - try client.send(.openProject(path: projectPath)) - let opened = try client.waitForEvent { - if case .graphChanged = $0 { return true } else { return false } - } + let opened = try openProject(projectPath) // The same advice `node create` prints — turning the flag on from `update` is the // same surprise. Best-effort: the node must be visible at the top level. - if case .graphChanged(let graph) = opened { + if let graph = opened { for warning in GraphcodeCommand.updateWarnings( for: attributed, currentNode: graph.nodes.first(where: { $0.id == nodeID })) { @@ -280,8 +294,7 @@ do { // the daemon refuse a loop handing itself a stop condition through promotion. let promoter = SurfaceRef.nodeID( fromZmxSessionName: ProcessInfo.processInfo.environment["ZMX_SESSION"] ?? "") - try client.send(.openProject(path: projectPath)) - _ = try client.waitForEvent { if case .graphChanged = $0 { return true } else { return false } } + try openProject(projectPath) try client.send( .graphCommand( projectPath: projectPath, @@ -302,8 +315,7 @@ do { case .memoNode(let projectPath, let nodeID, let text): let author = SurfaceRef.nodeID( fromZmxSessionName: ProcessInfo.processInfo.environment["ZMX_SESSION"] ?? "") - try client.send(.openProject(path: projectPath)) - _ = try client.waitForEvent { if case .graphChanged = $0 { return true } else { return false } } + try openProject(projectPath) try client.send( .graphCommand( projectPath: projectPath, command: .memoNode(nodeID, text: text, from: author))) @@ -319,8 +331,7 @@ do { case .refineNode(let projectPath, let nodeID, let text): let refiner = SurfaceRef.nodeID( fromZmxSessionName: ProcessInfo.processInfo.environment["ZMX_SESSION"] ?? "") - try client.send(.openProject(path: projectPath)) - _ = try client.waitForEvent { if case .graphChanged = $0 { return true } else { return false } } + try openProject(projectPath) try client.send( .graphCommand( projectPath: projectPath, command: .refineNode(nodeID, text: text, from: refiner))) @@ -336,8 +347,7 @@ do { case .rollbackRefinement(let projectPath, let nodeID): let requester = SurfaceRef.nodeID( fromZmxSessionName: ProcessInfo.processInfo.environment["ZMX_SESSION"] ?? "") - try client.send(.openProject(path: projectPath)) - _ = try client.waitForEvent { if case .graphChanged = $0 { return true } else { return false } } + try openProject(projectPath) try client.send( .graphCommand( projectPath: projectPath, command: .rollbackRefinement(nodeID, from: requester))) @@ -367,8 +377,7 @@ do { // the board. let author = SurfaceRef.nodeID( fromZmxSessionName: ProcessInfo.processInfo.environment["ZMX_SESSION"] ?? "") - try client.send(.openProject(path: projectPath)) - _ = try client.waitForEvent { if case .graphChanged = $0 { return true } else { return false } } + try openProject(projectPath) try client.send( .graphCommand( projectPath: projectPath, @@ -396,10 +405,7 @@ do { "artifactory sync needs a loop identity — run it from inside a loop's session " + "($ZMX_SESSION); a human reading the board wants `graphcode artifactory list`") } - try client.send(.openProject(path: projectPath)) - let opened = try client.waitForEvent { - if case .graphChanged = $0 { return true } else { return false } - } + let opened = try openProject(projectPath) try client.send( .graphCommand(projectPath: projectPath, command: .artifactorySync(from: reader))) let syncVerdict = try client.waitForEvent { event in @@ -416,7 +422,7 @@ do { // The window is one round-trip wide and a watcher would have heard the post live // anyway; fixing it properly means syncing to the highest *printed* id rather // than to latest, which nothing so far has needed. - if case .graphChanged(let graph) = opened { + if let graph = opened { if json { print(GraphcodeCommand.renderArtifactoryJSON(graph, unreadFor: reader)) } else if mark { @@ -443,11 +449,7 @@ do { // Read-only: the post rides the snapshot, no command is sent, no cursor moves — // the deep-read half of `sync --headlines` triage, priced at one line of context // per post a loop actually decides to care about. - try client.send(.openProject(path: projectPath)) - let read = try client.waitForEvent { - if case .graphChanged = $0 { return true } else { return false } - } - if case .graphChanged(let graph) = read { + if let graph = try openProject(projectPath) { guard let post = graph.artifactory.first(where: { $0.id == postID }) else { fail( "no post #\(postID) on this board — `graphcode artifactory list \(projectPath)` " @@ -461,11 +463,7 @@ do { // snapshot is waited for, and no cursor moves. This is the human's window onto // the board; `sync` is the loop's. `--search` filters what is shown, never what // is remembered. - try client.send(.openProject(path: projectPath)) - let opened = try client.waitForEvent { - if case .graphChanged = $0 { return true } else { return false } - } - if case .graphChanged(let graph) = opened { + if let graph = try openProject(projectPath) { if json { print(GraphcodeCommand.renderArtifactoryJSON(graph, search: search)) } else { @@ -483,8 +481,7 @@ do { "artifactory watch needs a loop identity — run it from inside a loop's session " + "($ZMX_SESSION); the mail is delivered to the loop that watches") } - try client.send(.openProject(path: projectPath)) - _ = try client.waitForEvent { if case .graphChanged = $0 { return true } else { return false } } + try openProject(projectPath) try client.send( .graphCommand( projectPath: projectPath, @@ -510,8 +507,7 @@ do { case .usage(let projectPath): // Refresh first: usage is pulled on demand rather than polled, so printing without // asking would show whatever was last read, which could be nothing at all. - try client.send(.openProject(path: projectPath)) - _ = try client.waitForEvent { if case .graphChanged = $0 { return true } else { return false } } + try openProject(projectPath) try client.send(.graphCommand(projectPath: projectPath, command: .refreshUsage)) let event = try client.waitForEvent { if case .graphChanged = $0 { return true } else { return false } @@ -521,11 +517,7 @@ do { } case .exportNode(let projectPath, let nodeID, let output, let includeChildren): - try client.send(.openProject(path: projectPath)) - let opened = try client.waitForEvent { - if case .graphChanged = $0 { return true } else { return false } - } - guard case .graphChanged(let graph) = opened else { fail("Could not load graph") } + guard let graph = try openProject(projectPath) else { fail("Could not load graph") } let persistence = ProjectPersistence(baseDirectory: SupportDirectory.url) guard @@ -547,11 +539,7 @@ do { print("Memory logs: \(bundle.memoryByNodeID.count)") case .exportGraph(let projectPath, let output): - try client.send(.openProject(path: projectPath)) - let opened = try client.waitForEvent { - if case .graphChanged = $0 { return true } else { return false } - } - guard case .graphChanged(let graph) = opened else { fail("Could not load graph") } + guard let graph = try openProject(projectPath) else { fail("Could not load graph") } let persistence = ProjectPersistence(baseDirectory: SupportDirectory.url) let bundle = persistence.createFullGraphExportBundle( @@ -595,8 +583,7 @@ do { guard let (request, resumingSessions) = box.value else { fail("the bundle contains no loops") } - try client.send(.openProject(path: projectPath)) - _ = try client.waitForEvent { if case .graphChanged = $0 { return true } else { return false } } + try openProject(projectPath) try client.send( .graphCommand(projectPath: projectPath, command: .importNodes(request))) let verdict = try client.waitForEvent { event in diff --git a/graphcode/Sources/Features/Project/ProjectFeature.swift b/graphcode/Sources/Features/Project/ProjectFeature.swift index bd7585b1..a1baf1a6 100644 --- a/graphcode/Sources/Features/Project/ProjectFeature.swift +++ b/graphcode/Sources/Features/Project/ProjectFeature.swift @@ -867,12 +867,9 @@ extension ProjectFeature { state.draftSchedule = .daily state.draftScheduleTime = "09:00" state.draftSubGraph = nil - // The parent's backend when there is one, then the open composite's — its workers - // run on what it runs on — and the human's default otherwise (Settings → Sessions), - // never a hardcoded one. - state.draftBackend = - backend ?? state.openCompositeID.flatMap { state.graph.nodes[id: $0]?.backend } - ?? GraphcodeSettingsStore.load().defaultBackend + // The parent's backend when there is one; the human's default otherwise + // (Settings → Sessions), never a hardcoded one. + state.draftBackend = backend ?? GraphcodeSettingsStore.load().defaultBackend state.draftWorktree = .none state.draftBranch = "" state.draftParentNodeID = parentNodeID diff --git a/graphcode/Tests/CompositeBackendTests.swift b/graphcode/Tests/CompositeBackendTests.swift deleted file mode 100644 index c9da67af..00000000 --- a/graphcode/Tests/CompositeBackendTests.swift +++ /dev/null @@ -1,41 +0,0 @@ -import Foundation -import IdentifiedCollections -import Testing - -@testable import GraphcodeKit - -/// Which backend a composite's workers run on. Its own suite because -/// `CompositeAndGlobalGraphTests` sits at the lint budget's type-body limit. -@Suite -struct CompositeBackendTests { - @Test - func aLoopAddedInsideACompositeRunsOnTheCompositesBackend() async { - // A Copilot composite must produce Copilot workers. The draft the app or the CLI - // sends for a loop inside a composite names no backend unless the human picked one, - // and `NodeDraft.effectiveBackend` would have fallen to Claude Code — a composite - // labelled Copilot whose every worker ran a different agent. - let composite = LoopNode( - title: "Triage inbox", loopType: .composite, backend: .copilotCLI, - subGraph: LoopGraph(project: ProjectRef(path: "sub", name: "sub"), nodes: [])) - let store = GraphStore( - graph: LoopGraph(project: ProjectRef(path: "/tmp/p", name: "p"), nodes: [composite])) - - await store.handle( - .subGraphCommand( - nodeID: composite.id, - command: .createNode( - NodeDraft(title: "Classify", loopType: .goalBased, goal: GoalSpec(summary: "sorted"))))) - await store.handle( - .subGraphCommand( - nodeID: composite.id, - command: .createNode( - NodeDraft( - title: "Explicit", loopType: .goalBased, goal: GoalSpec(summary: "sorted"), - backend: .codex)))) - - let workers = await store.graph.nodes[id: composite.id]?.subGraph?.nodes - #expect(workers?.first(where: { $0.title == "Classify" })?.backend == .copilotCLI) - // A backend the human named still wins over the composite's. - #expect(workers?.first(where: { $0.title == "Explicit" })?.backend == .codex) - } -} diff --git a/graphcode/Tests/CopilotBackendTests.swift b/graphcode/Tests/CopilotBackendTests.swift index 501dbd6c..332a6684 100644 --- a/graphcode/Tests/CopilotBackendTests.swift +++ b/graphcode/Tests/CopilotBackendTests.swift @@ -83,7 +83,7 @@ struct CopilotBackendTests { } @Test - func copilotHostsEveryLoopType() { + func copilotHostsEverythingButComposites() { // Goal-based works because a goal is just a prompt plus a predicate the *daemon* // polls from outside — nothing about it needs a skill the agent has to own. #expect(CLISessionBackendKind.copilotCLI.canHost(.turnBased)) @@ -92,12 +92,9 @@ struct CopilotBackendTests { // Copilot had no `/loop` equivalent when this row was first written against 1.0.75; // it has one now, which is what allows this pairing (issue #3). #expect(CLISessionBackendKind.copilotCLI.canHost(.timeBased)) - // A composite is a graph of loops running inside one node and leans on sub-agent - // fan-out, which was unverified when this row was first written. 1.0.80 lists - // `/fleet`, `/tasks` and `/subagents`, so the last refused pairing is allowed too. - #expect(CLISessionBackendKind.copilotCLI.canHost(.composite)) - #expect(CLISessionBackendKind.hosting(.composite).contains(.copilotCLI)) - #expect(CLISessionBackendKind.copilotCLI.canHost(.sketch)) + // A composite is a graph of loops running inside one node, and sub-agent fan-out is + // still unverified here — the one row that stays refused. + #expect(!CLISessionBackendKind.copilotCLI.canHost(.composite)) } @Test diff --git a/graphcode/Tests/DuplicateProjectPathTests.swift b/graphcode/Tests/DuplicateProjectPathTests.swift new file mode 100644 index 00000000..9e937cf8 --- /dev/null +++ b/graphcode/Tests/DuplicateProjectPathTests.swift @@ -0,0 +1,177 @@ +import Foundation +import Testing + +@testable import GraphcodeKit + +/// One project, named several ways, has to stay one project. +/// +/// Opening is create-if-missing — that is how `graphcode status ` adds a folder +/// from a shell — and every path a client named went straight through it. A loop names +/// paths constantly: its own worktree, its working directory, its project spelled with a +/// trailing slash. Each of those became a second project with its own graph, its own +/// recents entry and its own sidebar row under the same name, and the child loops the +/// agent created went into it, where nothing was watching. +/// +/// A codespace made it certain rather than possible. A remote path cannot be checked +/// against this filesystem, so *every* spelling of one was openable. +@Suite +struct DuplicateProjectPathTests { + private static let codespace = "codespace://curly-space-guide/workspaces/widget" + + private func makeRegistryAndPersistence() -> (ProjectRegistry, ProjectPersistence) { + let directory = FileManager.default.temporaryDirectory + .appendingPathComponent("graphcode-tests-\(UUID().uuidString)", isDirectory: true) + return ( + ProjectRegistry(persistenceDirectory: directory), ProjectPersistence(baseDirectory: directory) + ) + } + + @Test + func remoteSpellingsOfOnePathCanonicalizeTogether() { + for spelling in [ + "codespace://curly-space-guide/workspaces/widget/", + "codespace://curly-space-guide/workspaces//widget", + "codespace://curly-space-guide/workspaces/./widget", + "codespace://curly-space-guide/workspaces/other/../widget", + ] { + #expect(ProjectRegistry.canonicalize(spelling) == Self.codespace) + } + #expect( + ProjectRegistry.canonicalize("ssh://dev@build-box:2222/home/dev/widget/") + == "ssh://dev@build-box:2222/home/dev/widget") + // Different hosts are different projects, however alike the directory looks. + #expect( + ProjectRegistry.canonicalize("codespace://other-space/workspaces/widget") != Self.codespace) + } + + @Test + func threeSpellingsOfOneCodespaceAreOneProject() async { + let (registry, persistence) = makeRegistryAndPersistence() + let app = UUID() + await registry.addConnection(id: app, fileDescriptor: -1) + await registry.handle(.restoreOpenProjects, connectionID: app) + await registry.handle(.openProject(path: Self.codespace), connectionID: app) + + let shell = UUID() + await registry.addConnection(id: shell, fileDescriptor: -1) + for spelling in ["\(Self.codespace)/", "codespace://curly-space-guide/workspaces//widget"] { + await registry.handle(.openProject(path: spelling), connectionID: shell) + } + + #expect(persistence.loadRecentProjects().map(\.path) == [Self.codespace]) + #expect(persistence.loadOpenProjects() == [Self.codespace]) + } + + @Test + func aLoopsWorktreeIsItsProjectRatherThanANewOne() async { + let (registry, persistence) = makeRegistryAndPersistence() + let app = UUID() + await registry.addConnection(id: app, fileDescriptor: -1) + await registry.handle(.restoreOpenProjects, connectionID: app) + await registry.handle(.openProject(path: Self.codespace), connectionID: app) + + let shell = UUID() + await registry.addConnection(id: shell, fileDescriptor: -1) + await registry.handle( + .openProject(path: "\(Self.codespace)/worktrees/fix-215"), connectionID: shell) + + #expect(persistence.loadRecentProjects().map(\.path) == [Self.codespace]) + } + + @Test + func aRemoteProjectNobodyAddedIsRefusedFromAShell() async { + let (registry, persistence) = makeRegistryAndPersistence() + let shell = UUID() + await registry.addConnection(id: shell, fileDescriptor: -1) + await registry.handle(.openProject(path: Self.codespace), connectionID: shell) + + #expect(persistence.loadRecentProjects().isEmpty) + + guard + case .refused(let reason) = await registry.routing(for: Self.codespace, isSidebar: false) + else { + Issue.record("a remote project the daemon has never seen should be refused") + return + } + #expect(reason.contains("graphcode projects")) + + // The app adds it — validated over ssh first — and then the same shell can reach it. + #expect( + await registry.routing(for: Self.codespace, isSidebar: true) == .project(Self.codespace)) + } + + @Test + func theAppStillOpensANestedFolderAsItsOwnProject() async throws { + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("nested-\(UUID().uuidString)", isDirectory: true) + let nested = root.appendingPathComponent("packages/api", isDirectory: true) + try FileManager.default.createDirectory(at: nested, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: root) } + + let (registry, persistence) = makeRegistryAndPersistence() + let app = UUID() + await registry.addConnection(id: app, fileDescriptor: -1) + await registry.handle(.restoreOpenProjects, connectionID: app) + await registry.handle(.openProject(path: root.path), connectionID: app) + await registry.handle(.openProject(path: nested.path), connectionID: app) + + #expect(persistence.loadOpenProjects().count == 2) + + // The same folder named by a shell client is the project it sits in, not a third one. + let shell = UUID() + await registry.addConnection(id: shell, fileDescriptor: -1) + await registry.handle( + .openProject(path: nested.appendingPathComponent("src").path), connectionID: shell) + #expect(persistence.loadOpenProjects().count == 2) + } + + @Test + func aFolderThatIsNotThereIsRefusedRatherThanIgnored() async { + let (registry, _) = makeRegistryAndPersistence() + guard + case .refused(let reason) = await registry.routing( + for: "/workspaces/widget", isSidebar: false) + else { + Issue.record("a path naming no folder should be refused") + return + } + #expect(reason.contains("/workspaces/widget")) + } + + /// What a daemon that ran before this fix left behind: the same codespace in the + /// sidebar three times. The empty ones go on the next launch; one that collected loops + /// stays, because those loops are somebody's work and a merged-away row is a row nobody + /// can find again. + @Test + func emptyTwinsAreClearedOutOnTheNextLaunchAndOnesWithLoopsAreNot() async { + let (registry, persistence) = makeRegistryAndPersistence() + let twinWithLoops = "\(Self.codespace)//" + persistence.saveOpenProjects([Self.codespace, "\(Self.codespace)/", twinWithLoops]) + persistence.saveGraph( + LoopGraph( + project: ProjectRef(path: twinWithLoops, name: "widget"), + nodes: [ + LoopNode( + title: "Child", loopType: .turnBased, checkDescription: "Sound?", + firstInstruction: "Work") + ])) + + let app = UUID() + await registry.addConnection(id: app, fileDescriptor: -1) + await registry.handle(.restoreOpenProjects, connectionID: app) + + #expect(persistence.loadOpenProjects() == [Self.codespace, twinWithLoops]) + } + + @Test + func theDeepestKnownProjectWins() { + let known: Set = ["/repo", "/repo/packages/api"] + #expect( + ProjectRegistry.project(containing: "/repo/packages/api/src", in: known) + == "/repo/packages/api") + #expect(ProjectRegistry.project(containing: "/repo/docs", in: known) == "/repo") + #expect(ProjectRegistry.project(containing: "/elsewhere", in: known) == nil) + // A prefix that isn't a path boundary is not containment. + #expect(ProjectRegistry.project(containing: "/repository/docs", in: known) == nil) + } +} diff --git a/graphcode/Tests/GraphStoreTests.swift b/graphcode/Tests/GraphStoreTests.swift index dc524a50..a6f47d98 100644 --- a/graphcode/Tests/GraphStoreTests.swift +++ b/graphcode/Tests/GraphStoreTests.swift @@ -339,12 +339,11 @@ struct GraphStoreTests { // it properly afterward (see `TitleSuggestionClient`). await store.handle(.createNode(NodeDraft(title: "No goal", loopType: .goalBased))) await store.handle(.createNode(NodeDraft(title: "Bare", loopType: .timeBased))) - // A composite on Codex: sub-agent fan-out is the one capability still unverified - // there, so this is the pairing that stays impossible. (Copilot's was, until 1.0.80 - // grew `/fleet`.) + // A composite on Copilot: sub-agent fan-out is the one capability still unverified + // there, so this is the pairing that stays impossible now that Codex is spiked. await store.handle( .createNode( - NodeDraft(title: "Wrong backend", loopType: .composite, backend: .codex))) + NodeDraft(title: "Wrong backend", loopType: .composite, backend: .copilotCLI))) #expect(await store.graph.nodes.isEmpty) } diff --git a/graphcode/Tests/NodeDraftTests.swift b/graphcode/Tests/NodeDraftTests.swift index da08f7d9..f00bc156 100644 --- a/graphcode/Tests/NodeDraftTests.swift +++ b/graphcode/Tests/NodeDraftTests.swift @@ -171,16 +171,15 @@ struct NodeDraftTests { title: "Ship", loopType: .goalBased, goal: GoalSpec(summary: "Tests pass"), backend: .copilotCLI ).isValid) - // Time-based on Copilot is allowed now that it can re-trigger its own session, and - // a composite since 1.0.80 grew `/fleet`; a composite on Codex is the pairing that - // stays refused, since sub-agent fan-out is unverified there. + // Time-based on Copilot is allowed now that it can re-trigger its own session; a + // composite is the pairing that stays refused, since sub-agent fan-out is unverified. #expect( NodeDraft( title: "Poll", loopType: .timeBased, triggerPrompt: "/loop 1h Check", backend: .copilotCLI ).isValid) - #expect(NodeDraft(title: "Triage", loopType: .composite, backend: .copilotCLI).isValid) - #expect(!NodeDraft(title: "Triage", loopType: .composite, backend: .codex).isValid) + #expect( + !NodeDraft(title: "Triage", loopType: .composite, backend: .copilotCLI).isValid) } @Test @@ -228,9 +227,9 @@ struct NodeDraftTests { #expect( CLISessionBackendKind.hosting(.timeBased) == [.claudeCode, .copilotCLI, .codex, .openCode]) - // A composite still needs sub-agent fan-out, which Claude Code and (since 1.0.80's - // `/fleet`) Copilot have been shown to do. - #expect(CLISessionBackendKind.hosting(.composite) == [.claudeCode, .copilotCLI]) + // A composite still needs sub-agent fan-out, which only Claude Code has been shown + // to do. + #expect(CLISessionBackendKind.hosting(.composite) == [.claudeCode]) } @Test diff --git a/graphcode/Tests/RemoteCLIShimTests.swift b/graphcode/Tests/RemoteCLIShimTests.swift index bf77e758..7acbcc91 100644 --- a/graphcode/Tests/RemoteCLIShimTests.swift +++ b/graphcode/Tests/RemoteCLIShimTests.swift @@ -76,6 +76,39 @@ struct RemoteCLIShimTests { command: .messageNode(nodeID, text: "the API changed", from: nil, followUp: nil))) } + /// The path a loop *on that host* naturally types — its working directory, or the + /// worktree it was told to work in. The daemon keys graphs by the `ssh://` URI and + /// opening one is create-if-missing, so this used to add a second project named after + /// the worktree and put the child loop inside it. The shim is the one place that knows + /// this is a remote host's spelling, so it is where the two are matched up. + @Test + func aLocalPathOnTheRemoteHostResolvesToItsProject() throws { + let run = try runShim([ + "node", "create", "/home/dev/widget/worktrees/fix-215", + "--title", "Fix 215", "--type", "goal", "--goal", "tests pass", + ]) + + #expect(run.status == 0) + #expect(run.commands.first == .listRecentProjects) + #expect(run.commands.dropFirst().first == .openProject(path: Self.project)) + guard case .graphCommand(let path, .createNode) = run.commands.last else { + Issue.record("expected a createNode against the resolved project, got \(run.commands)") + return + } + #expect(path == Self.project) + #expect(run.stderr.contains(Self.project)) + } + + /// A path on that host belonging to no known project stays as it was typed: the daemon + /// answers with the error naming it, rather than the shim inventing a project. + @Test + func anUnrelatedLocalPathIsLeftForTheDaemonToRefuse() throws { + let run = try runShim(["status", "/home/dev/somewhere-else"]) + + #expect(run.commands.first == .listRecentProjects) + #expect(run.commands.dropFirst().first == .openProject(path: "/home/dev/somewhere-else")) + } + // MARK: - Harness private struct ShimRun { @@ -145,7 +178,11 @@ struct RemoteCLIShimTests { guard let command = try? JSONDecoder().decode(DaemonCommand.self, from: data) else { return } received.append(command) - guard let reply = try? JSONEncoder().encode(DaemonEvent.graphChanged(graph)), + let event: DaemonEvent = + command == .listRecentProjects + ? .recentProjectsListed([ProjectRef(path: Self.project, name: "widget")]) + : .graphChanged(graph) + guard let reply = try? JSONEncoder().encode(event), (try? FramedMessageIO.writeFrame(reply, to: client)) != nil else { return } }