From e0a0043b638f8599956251b4b1ff943ccc112771 Mon Sep 17 00:00:00 2001 From: scgopi Date: Wed, 2 Sep 2026 19:58:55 -0700 Subject: [PATCH] fix: one project, however its path is spelled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Creating a child loop from inside a codespace added a second project to the sidebar and put the loop in it, where nothing was watching. Opening a project 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 with a trailing slash. Each became another project with its own graph, its own recents entry and its own sidebar row under the same name. A codespace turned that from possible into certain. A remote path cannot be checked against this filesystem, so unlike a local folder *every* spelling of one was openable: `codespace://cs/workspaces/repo/`, `…//repo`, and every subpath of it were four projects. Three changes, from the daemon outwards: - `canonicalize` now normalizes a remote path's spelling the way it has always normalized a local one. Trailing slashes, doubled separators and `.` segments collapse; different hosts stay different projects. - Routing a named path is now a policy rather than a lookup. For a client that is not the app, a folder inside a project that already exists *is* that project, and a remote path the daemon has never seen is refused — remote projects are added in the app, which validates the connection over ssh first. The app keeps both powers: opening a nested folder and adding a host are deliberate human acts there. - A refusal is now sent back as an error instead of dropped. Naming a path that was not a project used to be silence: the CLI waited out its socket timeout and then reported the command as *possibly applied*, when nothing past the open had been sent. Both CLIs read it and print the one line saying which path was wrong. The remote shim resolves before it opens. It is the one place that knows a bare absolute path is *this host's* spelling, so `/workspaces/repo` and a worktree under it now address the codespace project they belong to, and say so on stderr. Sidebar rows a pre-fix daemon left behind clear themselves out on the next launch, but only the empty ones: a twin that collected loops stays exactly where it is, because those loops are somebody's work and a merged-away row is one nobody can find again. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WYzCYpAW75pcCCDD1eq7wV --- .../Sources/Domain/BackendCapabilities.swift | 9 +- GraphcodeKit/Sources/GraphStore.swift | 9 - GraphcodeKit/Sources/ProjectRegistry.swift | 151 ++++++++++++++- .../Sources/Sessions/RemoteGraphAccess.swift | 107 +++++++++-- graphcode-cli/Sources/main.swift | 95 ++++------ .../Features/Project/ProjectFeature.swift | 9 +- graphcode/Tests/CompositeBackendTests.swift | 41 ---- graphcode/Tests/CopilotBackendTests.swift | 11 +- .../Tests/DuplicateProjectPathTests.swift | 177 ++++++++++++++++++ graphcode/Tests/GraphStoreTests.swift | 7 +- graphcode/Tests/NodeDraftTests.swift | 15 +- graphcode/Tests/RemoteCLIShimTests.swift | 39 +++- 12 files changed, 507 insertions(+), 163 deletions(-) delete mode 100644 graphcode/Tests/CompositeBackendTests.swift create mode 100644 graphcode/Tests/DuplicateProjectPathTests.swift 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 } }