From 773724ea7ae41d58f5683a7f0a3a7cb76d3f8621 Mon Sep 17 00:00:00 2001 From: scgopi Date: Tue, 15 Sep 2026 09:07:29 -0700 Subject: [PATCH 1/2] Brief Copilot through COPILOT_CUSTOM_INSTRUCTIONS_DIRS instead of a preamble Copilot searches the directories that variable lists for *.instructions.md files, not for AGENTS.md. The July measurement that concluded the variable was ignored ran on 1.0.75 with a working cwd control, but only tried AGENTS.md and .github/copilot-instructions.md. Re-measured on 1.0.75 through 1.0.84-5, an instructions/*.instructions.md under a listed directory lands in the system message every time. The briefing directory now carries that copy, every Copilot launch (fresh, resumed, local, remote, daemon and app) appends the directory to the user's own value, and the opening prompt no longer carries the read-this-file pointer. --add-dir stays. --- .../Sources/Domain/BackendCommand.swift | 30 ++--- .../Sources/Domain/SessionBriefing.swift | 55 ++++++--- .../Sources/Sessions/RemoteGraphAccess.swift | 4 + .../Sources/Sessions/ZmxSessionLauncher.swift | 64 +++++++--- .../Ghostty/GhosttyTerminalView+Remote.swift | 11 +- .../Ghostty/GhosttyTerminalView.swift | 28 +++-- .../Tests/AttachedSessionBriefingTests.swift | 33 +++++- .../CopilotInstructionsDeliveryTests.swift | 112 ++++++++++++++++++ graphcode/Tests/SessionBriefingTests.swift | 32 ++--- graphcode/Tests/SessionPromptTests.swift | 25 ++-- 10 files changed, 304 insertions(+), 90 deletions(-) create mode 100644 graphcode/Tests/CopilotInstructionsDeliveryTests.swift diff --git a/GraphcodeKit/Sources/Domain/BackendCommand.swift b/GraphcodeKit/Sources/Domain/BackendCommand.swift index ab4b7f18..a95ecbdd 100644 --- a/GraphcodeKit/Sources/Domain/BackendCommand.swift +++ b/GraphcodeKit/Sources/Domain/BackendCommand.swift @@ -83,10 +83,10 @@ extension CLISessionBackendKind { /// cosmetic. `claude` takes `--append-system-prompt-file `, which adds the file's /// contents to its system prompt and leaves the human's prompt as the only thing in the /// conversation. `copilot` has no equivalent — its custom instructions come from - /// `AGENTS.md` files it discovers on disk (hence `--no-custom-instructions` to switch - /// that off), which graphcode has no business writing into someone's repository. So - /// Copilot has no equivalent flag and no working equivalent mechanism, so it is told to - /// read the file by a preamble on the prompt and granted access to it with `--add-dir`. + /// instruction files it discovers on disk, which graphcode has no business writing into + /// someone's repository — so it is pointed at a copy in graphcode's own directory through + /// its environment instead (`briefingEnvironment`), which lands in its system prompt + /// just as Claude's flag does. /// /// Neither carries the prose on the command line. See `SessionBriefing` for why that is /// load-bearing rather than tidy: the launch command is typed into a terminal, and a @@ -125,20 +125,7 @@ extension CLISessionBackendKind { // for a prompt that actually is one, so an ordinary Copilot session keeps the // CLI's own defaults. let experimental = SessionPrompt.mentionsRecurrence(prompt) ? ["--experimental"] : [] - guard let briefingPath else { - return model + access + experimental + ["--interactive", prompt] - } - // And the preamble telling it the briefing is there to read. See - // `SessionBriefing.pointer` for why the tidier env-var route was abandoned. - // Ordered by `SessionPrompt`, not concatenated: a time-based node's prompt is the - // `/loop …` directive itself, and Copilot is the one backend that both hosts that - // loop type and receives its briefing as a preamble (issue #179). - return model + access + experimental - + [ - "--interactive", - SessionPrompt.composed( - preamble: SessionBriefing.pointer(toBriefingAt: briefingPath), prompt: prompt), - ] + return model + access + experimental + ["--interactive", prompt] case .codex: // Same shape as Claude Code — an interactive TUI taking its prompt positionally — // so the briefing rides the same way Copilot's does: `--add-dir` for access, a @@ -266,6 +253,13 @@ extension CLISessionBackendKind { /// is named by a config file, and `OPENCODE_CONFIG` is the one route that *merges over* /// the user's own config instead of replacing it (`OPENCODE_CONFIG_DIR` would drop /// their providers and plugins on the floor — read off the config loader, not the docs). + /// The environment that delivers the briefing at `briefingPath`, for the one backend + /// that takes it that way (`SessionBriefing.copilotInstructionsDirectoryVariable`). + public func briefingEnvironment(briefingPath: String?) -> [String: String] { + guard self == .copilotCLI, let briefingPath else { return [:] } + return SessionBriefing.copilotInstructionsEnvironment(briefingPath: briefingPath) + } + public func presenceEnvironment(hooksFile: URL?) -> [String: String] { switch self { case .openCode: diff --git a/GraphcodeKit/Sources/Domain/SessionBriefing.swift b/GraphcodeKit/Sources/Domain/SessionBriefing.swift index 2af0a1ff..051e249a 100644 --- a/GraphcodeKit/Sources/Domain/SessionBriefing.swift +++ b/GraphcodeKit/Sources/Domain/SessionBriefing.swift @@ -37,10 +37,40 @@ public enum SessionBriefing { directory.appendingPathComponent(slug(for: projectPath), isDirectory: true) } - /// The filename Copilot looks for when it searches a directory, and the file Claude Code - /// is handed by path. One file, two delivery mechanisms, no duplication. + /// The file Claude Code is handed by path and the other backends are pointed at. public static let fileName = "AGENTS.md" + /// The variable Copilot CLI reads for extra instruction directories, and the copy of the + /// briefing it finds there. + /// + /// **Copilot searches those directories for `*.instructions.md` files, not `AGENTS.md`.** + /// The first cut pointed the variable at a directory holding only `AGENTS.md`, nothing + /// loaded, and Copilot loops silently never fanned out (issue #2). The measurement that + /// followed was real — Copilot's own session logs from 2026-07-28 show CLI 1.0.75 and a + /// working-directory control that fired — but it tried only `AGENTS.md` and + /// `.github/copilot-instructions.md`, the two names Copilot looks for in a *repository*, + /// and concluded the variable was ignored. `copilot help environment` says only "custom + /// instructions files", which is how both readings survived. Re-measured on 2026-09-15 + /// against 1.0.75, 1.0.82, 1.0.83 and 1.0.84-1/-3/-5: an `*.instructions.md` anywhere + /// under a listed directory lands in the system message on every one of them, with or + /// without `applyTo` front matter, while those two names never load from there. + /// + /// Resuming rebuilds the system message from the resuming process's environment, so a + /// resumed Copilot session needs the variable as much as a fresh one does. + public static let copilotInstructionsDirectoryVariable = "COPILOT_CUSTOM_INSTRUCTIONS_DIRS" + public static let copilotInstructionsFile = "instructions/graphcode.instructions.md" + + /// Points Copilot at the directory holding `briefingPath`, *after* any directories the + /// user's own shell already listed — the variable is comma-separated and theirs first. + /// Expanded by the session's login shell, the only place that knows the user's value + /// and, for a remote `~/` path, the host's home directory. + public static func copilotInstructionsEnvironment(briefingPath: String) -> [String: String] { + let variable = copilotInstructionsDirectoryVariable + var directory = (briefingPath as NSString).deletingLastPathComponent + if directory.hasPrefix("~/") { directory = "$HOME/" + directory.dropFirst(2) } + return [variable: "${\(variable):+$\(variable),}\(directory)"] + } + /// The briefing for a node in `projectPath`'s graph, or `nil` when there's no path to /// tell it about — every command the briefing describes takes one, so a briefing /// without it would describe commands the session can't run. @@ -272,21 +302,10 @@ public enum SessionBriefing { """ } - /// How Copilot CLI is told about the briefing: a one-line preamble on the prompt, and - /// `--add-dir` so the session is allowed to read the file it names. - /// - /// **`COPILOT_CUSTOM_INSTRUCTIONS_DIRS` does not work.** `copilot help environment` - /// documents it as "additional directories to search for custom instructions files", and - /// it was the obvious right answer — a real system-level instruction, nothing in the - /// prompt, nothing written into anyone's repository. Measured against 1.0.75 it is simply - /// ignored: an `AGENTS.md` in a directory named by that variable has no effect, in either - /// the `AGENTS.md` or `.github/copilot-instructions.md` layout, while the identical file - /// in the working directory is picked up every time. Shipping on the documentation cost - /// a release where Copilot loops silently never fanned out (issue #2). + /// How a backend with no system-prompt channel is told about the briefing — Codex, + /// OpenCode and pi: a one-line preamble on the prompt. Copilot has a channel + /// (`copilotInstructionsDirectoryVariable`) and no longer takes one. /// - /// `--add-dir` is the half that is easy to miss. Copilot verifies file paths, so a - /// session told to read `~/.graphcode/briefings/…` cannot reach it — the pointer alone - /// looks like the agent ignoring an instruction when it is actually being denied. /// Deliberately ASCII-only, with plain words on both sides of the path. This string /// travels through more layers than any other prose graphcode emits — argv, zmx's /// typed command line, a canonical-mode tty, sometimes ssh — and an em dash sitting @@ -313,6 +332,10 @@ public enum SessionBriefing { try FileManager.default.createDirectory( at: directory, withIntermediateDirectories: true) try text.write(to: url, atomically: true, encoding: .utf8) + let instructions = directory.appendingPathComponent(copilotInstructionsFile) + try FileManager.default.createDirectory( + at: instructions.deletingLastPathComponent(), withIntermediateDirectories: true) + try text.write(to: instructions, atomically: true, encoding: .utf8) return url } catch { // A session with no briefing is the pre-briefing behaviour, which works. Failing the diff --git a/GraphcodeKit/Sources/Sessions/RemoteGraphAccess.swift b/GraphcodeKit/Sources/Sessions/RemoteGraphAccess.swift index 34194a11..39c590c5 100644 --- a/GraphcodeKit/Sources/Sessions/RemoteGraphAccess.swift +++ b/GraphcodeKit/Sources/Sessions/RemoteGraphAccess.swift @@ -71,6 +71,10 @@ public enum RemoteGraphAccess { briefingDirectory(forProjectPath: projectPath) + "/" + SessionBriefing.fileName } + public static func copilotInstructionsPath(forProjectPath projectPath: String) -> String { + briefingDirectory(forProjectPath: projectPath) + "/" + SessionBriefing.copilotInstructionsFile + } + /// The remote twin of `NodeMemory.directory(forProjectPath:nodeID:)`. Only the wake /// digest is delivered there — the log itself stays on the Mac, where the daemon /// appends to it; the digest is the budgeted, rebuildable view of it. diff --git a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift index 2b95933d..ab18e3b9 100644 --- a/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift +++ b/GraphcodeKit/Sources/Sessions/ZmxSessionLauncher.swift @@ -1223,8 +1223,11 @@ public enum ZmxSessionLauncher { + Self.loginShellInvocation( of: executable, arguments: resumeArgs, environment: Self.environment( - forBackend: node.backend, briefingPath: nil, hooksFile: hooksFile, - remoteHooksPath: remoteEnvironmentPath), + forBackend: node.backend, + briefingPath: Self.resumeBriefingPath( + forBackend: node.backend, projectPath: projectPath, isRemote: remote != nil, + settings: settings), + hooksFile: hooksFile, remoteHooksPath: remoteEnvironmentPath), scriptSuffix: remoteHooksSuffix) } @@ -1260,18 +1263,34 @@ public enum ZmxSessionLauncher { return paths } - /// Environment a session needs beyond what its shell provides. Copilot's briefing rides - /// on its argv (see `CLISessionBackendKind.launchArguments`) after the documented - /// environment route turned out not to work; OpenCode's presence plugin is the one - /// thing that genuinely has to travel this way (`presenceEnvironment`). + /// Environment a session needs beyond what its shell provides: Copilot's briefing + /// (`briefingEnvironment`) and OpenCode's presence plugin (`presenceEnvironment`). static func environment( forBackend backend: CLISessionBackendKind, briefingPath: String?, hooksFile: URL? = nil, remoteHooksPath: String? = nil ) -> [String: String] { + let briefing = backend.briefingEnvironment(briefingPath: briefingPath) if backend == .openCode, let remoteHooksPath { - return ["OPENCODE_CONFIG": remoteHooksPath] + return briefing.merging(["OPENCODE_CONFIG": remoteHooksPath]) { $1 } } - return backend.presenceEnvironment(hooksFile: hooksFile) + return briefing.merging(backend.presenceEnvironment(hooksFile: hooksFile)) { $1 } + } + + /// The briefing a resumed session is launched with. Only Copilot needs one: it rebuilds + /// its system prompt from the resuming process's environment, where every other + /// backend's briefing either rides in the conversation it restores or in a flag the + /// resume never carried. + static func resumeBriefingPath( + forBackend backend: CLISessionBackendKind, projectPath: String?, isRemote: Bool, + settings: GraphcodeSettings + ) -> String? { + guard backend == .copilotCLI, settings.briefsSessionsAboutTheGraph, let projectPath else { + return nil + } + return isRemote + ? SessionBriefing.text(projectPath: projectPath) + .map { _ in RemoteGraphAccess.briefingPath(forProjectPath: projectPath) } + : SessionBriefing.write(projectPath: projectPath)?.path } /// What a remote session's launch appends so its reporter loads — a `$HOME` path only @@ -1528,13 +1547,33 @@ public enum ZmxSessionLauncher { /// because the app's *attach* delivers too, before any node exists to have memory. /// Public for exactly that caller (`GhosttyTerminalView.remoteCommand`). public static func remoteDeliveryScript( - forNode node: LoopNode?, at location: RemoteProjectLocation, settings: GraphcodeSettings + forNode node: LoopNode?, backend: CLISessionBackendKind? = nil, + at location: RemoteProjectLocation, settings: GraphcodeSettings ) -> String? { + // The shim's receipt, written only once every file has landed — see + // `installerScript`. It is what lets a later ensure skip a delivery it doesn't need + // without ever claiming a shim the host never received. + RemoteGraphAccess.installerScript( + files: remoteDeliveryFiles(forNode: node, backend: backend, at: location, settings: settings), + receipt: (path: RemoteGraphAccess.shimStampPath, content: RemoteGraphAccess.cliShimStamp)) + } + + /// `remoteDeliveryScript`'s manifest: home-relative path → content. Copilot's copy of the + /// briefing (`SessionBriefing.copilotInstructionsFile`) goes only to a Copilot session, + /// named by `node` or, for the app's attach, by `backend`. + static func remoteDeliveryFiles( + forNode node: LoopNode?, backend: CLISessionBackendKind? = nil, + at location: RemoteProjectLocation, settings: GraphcodeSettings + ) -> [String: String] { var files = [RemoteGraphAccess.cliInstallPath: RemoteGraphAccess.cliShimSource] if settings.briefsSessionsAboutTheGraph, let text = SessionBriefing.text(projectPath: location.projectPath) { files[RemoteGraphAccess.briefingPath(forProjectPath: location.projectPath)] = text + if (node?.backend ?? backend) == .copilotCLI { + files[RemoteGraphAccess.copilotInstructionsPath(forProjectPath: location.projectPath)] = + text + } } if let node { let wakeURL = NodeMemory.directory( @@ -1555,12 +1594,7 @@ public enum ZmxSessionLauncher { promptText } } - // The shim's receipt, written only once every file above has landed — see - // `installerScript`. It is what lets a later ensure skip a delivery it doesn't need - // without ever claiming a shim the host never received. - return RemoteGraphAccess.installerScript( - files: files, - receipt: (path: RemoteGraphAccess.shimStampPath, content: RemoteGraphAccess.cliShimStamp)) + return files } /// `quotedCommand`, except that arguments naming graphcode's own remote files — diff --git a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView+Remote.swift b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView+Remote.swift index 90b6ffdf..782001f9 100644 --- a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView+Remote.swift +++ b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView+Remote.swift @@ -27,7 +27,8 @@ extension GhosttyTerminalView { RemoteProjectLocation.prepareControlSocketDirectory() let quoted = RemoteProjectLocation.shellQuoted let delivery = - ZmxSessionLauncher.remoteDeliveryScript(forNode: nil, at: location, settings: settings) + ZmxSessionLauncher.remoteDeliveryScript( + forNode: nil, backend: backend, at: location, settings: settings) .map { $0 + "; " } ?? "" let agentScripts = launchesClaudeCode @@ -209,7 +210,8 @@ extension GhosttyTerminalView { var script = preparation + "{ " let nodeID = SurfaceRef.nodeID(fromZmxSessionName: sessionName) let resumeLaunch = resumeCommand( - settings: settings, remoteSettingsPath: remotePresenceSettingsPath, isRemote: true) + settings: settings, briefingPath: remoteBriefingPath(settings: settings), + remoteSettingsPath: remotePresenceSettingsPath, isRemote: true) if let nodeID, let resumeLaunch { let idFile = PresenceHooks.remoteSessionIDExpression(forNodeID: nodeID) let attempt = @@ -243,8 +245,8 @@ extension GhosttyTerminalView { /// machine wrote. A resumed local session needs them for the same reason a fresh one /// does: without them the loop reports IDLE for as long as it runs. func resumeCommand( - settings: GraphcodeSettings, hooksFile: URL? = nil, remoteSettingsPath: String?, - isRemote: Bool = false + settings: GraphcodeSettings, briefingPath: String? = nil, hooksFile: URL? = nil, + remoteSettingsPath: String?, isRemote: Bool = false ) -> [String]? { guard backend.supportsResume, var parts = launchPrefix(settings: settings) else { return nil @@ -258,6 +260,7 @@ extension GhosttyTerminalView { .joined(separator: " ") if !presence.isEmpty { parts.append(presence) } addRemotePresenceSettings(remoteSettingsPath, to: &parts) + addBriefingEnvironment(briefingPath, to: &parts) parts += backend.resumeArguments( sessionID: "\"$\(ZmxSessionLauncher.remoteResumeIDVariable)\"") return Self.interactiveLoginShell(parts) diff --git a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift index e3a88509..96e12356 100644 --- a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift +++ b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView.swift @@ -193,10 +193,11 @@ struct GhosttyTerminalView: NSViewRepresentable { // delivers it for a daemon-started session — before this, only daemon-started loops // knew they could fan out, and a turn-based loop (which only ever starts here) asked // to create more loops improvised with its backend's own sub-agents instead (the - // Copilot shape of issue #2). Claude takes the file itself as a flag; Copilot and - // Codex are granted the directory and pointed at the file inside their opening - // prompt — see `sessionEnvironment`, where the pointer rides in the env var and so - // needs no shell quoting. + // Copilot shape of issue #2). Claude takes the file itself as a flag; Copilot is + // granted the directory and finds its copy through its environment + // (`briefingEnvironment`); Codex is granted the directory and pointed at the file + // inside its opening prompt — see `sessionEnvironment`, where the pointer rides in the + // env var and so needs no shell quoting. // A remote session's briefing path is `~/`-relative and deliberately unquoted // here: this whole string is the remote zsh's `-c` script, and that shell's own // tilde expansion is the only thing that knows the remote home directory. @@ -207,6 +208,7 @@ struct GhosttyTerminalView: NSViewRepresentable { parts.append("--add-dir \((briefingPath as NSString).deletingLastPathComponent)") } } + addBriefingEnvironment(briefingPath, to: &parts) if !prompt.isEmpty { if let flag = backend.promptFlag { parts.append(flag) } parts.append(prompt) @@ -237,6 +239,15 @@ struct GhosttyTerminalView: NSViewRepresentable { ["/bin/zsh", "-i", "-l", "-c", parts.joined(separator: " ")] } + /// `env` assignments ahead of the executable, double-quoted so the session's login shell + /// expands what `SessionBriefing.copilotInstructionsEnvironment` left for it to expand. + func addBriefingEnvironment(_ briefingPath: String?, to parts: inout [String]) { + let environment = backend.briefingEnvironment(briefingPath: briefingPath) + guard !environment.isEmpty else { return } + let assignments = environment.keys.sorted().map { "\($0)=\"\(environment[$0] ?? "")\"" } + parts.insert(contentsOf: ["env"] + assignments, at: 1) + } + func addRemotePresenceSettings(_ path: String?, to parts: inout [String]) { guard let path else { return } if backend == .claudeCode { @@ -287,7 +298,7 @@ struct GhosttyTerminalView: NSViewRepresentable { } /// What rides into the session through the environment: the opening prompt, carrying - /// for Copilot and Codex the pointer at the briefing file — on whichever side keeps a + /// for Codex, OpenCode and pi the pointer at the briefing file — on whichever side keeps a /// leading `/loop` directive leading (`SessionPrompt`). In the env var rather /// than on the command line because the pointer is prose — inside `"$VAR"` it needs no /// quoting and cannot break the shell string the command is joined into. Claude's @@ -295,7 +306,9 @@ struct GhosttyTerminalView: NSViewRepresentable { func sessionEnvironment(briefingPath: String?, hooksFile: URL? = nil) -> [String: String] { var environment = backend.presenceEnvironment(hooksFile: hooksFile) guard var prompt = initialPrompt else { return environment } - if backend != .claudeCode, let briefingPath { + if backend != .claudeCode, backend.briefingEnvironment(briefingPath: briefingPath).isEmpty, + let briefingPath + { prompt = SessionPrompt.composed( preamble: SessionBriefing.pointer(toBriefingAt: briefingPath), prompt: prompt) } @@ -384,7 +397,8 @@ struct GhosttyTerminalView: NSViewRepresentable { let settings = GraphcodeSettingsStore.load() guard SessionIDStore.load(forNodeID: nodeID) != nil, let resumeLaunch = resumeCommand( - settings: settings, hooksFile: presenceHooksFile(), remoteSettingsPath: nil) + settings: settings, briefingPath: briefingFile(settings: settings)?.path, + hooksFile: presenceHooksFile(), remoteSettingsPath: nil) else { // Nothing banked means there is no resume for this pane to make. It does *not* mean // a fresh launch is this pane's to make: `graphcoded` owns an unattended loop's diff --git a/graphcode/Tests/AttachedSessionBriefingTests.swift b/graphcode/Tests/AttachedSessionBriefingTests.swift index ce880da9..1c7b9fab 100644 --- a/graphcode/Tests/AttachedSessionBriefingTests.swift +++ b/graphcode/Tests/AttachedSessionBriefingTests.swift @@ -39,18 +39,38 @@ struct AttachedSessionBriefingTests { } @Test - func copilotIsGrantedTheDirectoryAndPointedAtTheFile() { + func copilotIsGrantedTheDirectoryAndFindsTheBriefingThroughItsEnvironment() { let command = shellCommand(.copilotCLI) // `--add-dir` is the half that is easy to miss: Copilot verifies paths, so without - // it the pointer reads as the agent ignoring an instruction (issue #2's shape). + // it a session asked about its briefing reads as ignoring it (issue #2's shape). #expect(command.contains("--add-dir /tmp/briefings/proj")) #expect(command.contains("--interactive")) + let variable = SessionBriefing.copilotInstructionsDirectoryVariable + #expect( + command.hasPrefix( + "exec env \(variable)=\"${\(variable):+$\(variable),}/tmp/briefings/proj\" copilot")) - // The pointer rides inside the env var, where prose needs no shell quoting. + // And the prompt is left alone, as Claude's is. let environment = surface(.copilotCLI).sessionEnvironment(briefingPath: briefing) - let prompt = environment["GRAPHCODE_TRIGGER_PROMPT"] ?? "" - #expect(prompt.contains(briefing)) - #expect(prompt.hasSuffix(" go")) + #expect(environment["GRAPHCODE_TRIGGER_PROMPT"] == "go") + } + + @Test + func aResumedCopilotSurfaceIsBriefedAgain() { + // Copilot rebuilds its system prompt from the resuming process's environment, so a + // resume without the variable is a session that has forgotten it is in a graph. + let settings = GraphcodeSettings() + let copilot = + surface(.copilotCLI).resumeCommand( + settings: settings, briefingPath: briefing, remoteSettingsPath: nil)?.last ?? "" + #expect(copilot.hasPrefix("exec env COPILOT_CUSTOM_INSTRUCTIONS_DIRS=")) + #expect(copilot.contains("--resume")) + for backend in [CLISessionBackendKind.claudeCode, .codex, .openCode, .pi] { + let resumed = + surface(backend).resumeCommand( + settings: settings, briefingPath: briefing, remoteSettingsPath: nil)?.last ?? "" + #expect(!resumed.contains("COPILOT_CUSTOM_INSTRUCTIONS_DIRS"), "\(backend)") + } } @Test @@ -87,6 +107,7 @@ struct AttachedSessionBriefingTests { surface(.copilotCLI).agentCommand( settings: GraphcodeSettings(), briefingPath: nil)?.last ?? "" #expect(!command.contains("--add-dir")) + #expect(!command.contains("COPILOT_CUSTOM_INSTRUCTIONS_DIRS")) let environment = surface(.copilotCLI).sessionEnvironment(briefingPath: nil) #expect(environment["GRAPHCODE_TRIGGER_PROMPT"] == "go") } diff --git a/graphcode/Tests/CopilotInstructionsDeliveryTests.swift b/graphcode/Tests/CopilotInstructionsDeliveryTests.swift new file mode 100644 index 00000000..30fe666d --- /dev/null +++ b/graphcode/Tests/CopilotInstructionsDeliveryTests.swift @@ -0,0 +1,112 @@ +import Foundation +import Testing + +@testable import GraphcodeKit + +/// Copilot takes the graph briefing through `COPILOT_CUSTOM_INSTRUCTIONS_DIRS` rather than +/// a read-this-file preamble. What every launch path has to carry for that to hold, since +/// the failure — a loop that never learns it is in a graph — is silent (issue #2). +@Suite +struct CopilotInstructionsDeliveryTests { + private let variable = SessionBriefing.copilotInstructionsDirectoryVariable + + private func goalNode(_ backend: CLISessionBackendKind) -> LoopNode { + LoopNode( + title: "Copilot", loopType: .goalBased, goal: GoalSpec(summary: "work"), backend: backend) + } + + @Test + func theVariableIsTheDocumentedPluralSpelling() { + // `COPILOT_CUSTOM_INSTRUCTION_DIRS` (singular) loads nothing, on every version measured. + #expect(variable == "COPILOT_CUSTOM_INSTRUCTIONS_DIRS") + #expect(SessionBriefing.copilotInstructionsFile.hasSuffix(".instructions.md")) + } + + @Test + func theLoginShellAppendsTheBriefingAfterTheUsersOwnDirectories() throws { + let value = try #require( + SessionBriefing.copilotInstructionsEnvironment( + briefingPath: "/b/briefings/p/AGENTS.md")[variable]) + func expanded(existing: String?) throws -> String { + let process = Process() + process.executableURL = URL(fileURLWithPath: "/bin/zsh") + process.arguments = ["-f", "-c", "print -r -- \"\(value)\""] + process.environment = existing.map { [variable: $0] } ?? [:] + let output = Pipe() + process.standardOutput = output + try process.run() + let data = output.fileHandleForReading.readDataToEndOfFile() + process.waitUntilExit() + return String(decoding: data, as: UTF8.self).trimmingCharacters(in: .newlines) + } + #expect(try expanded(existing: nil) == "/b/briefings/p") + #expect(try expanded(existing: "") == "/b/briefings/p") + #expect(try expanded(existing: "/mine,/theirs") == "/mine,/theirs,/b/briefings/p") + } + + @Test + func aRemoteBriefingIsFoundUnderTheHostsOwnHome() throws { + // A double-quoted `~` never expands, and nothing local knows the remote home. + let value = try #require( + SessionBriefing.copilotInstructionsEnvironment( + briefingPath: "~/.graphcode/briefings/p/AGENTS.md")[variable]) + #expect(value.hasSuffix(",}$HOME/.graphcode/briefings/p")) + } + + @Test + func onlyCopilotIsGivenTheVariable() { + for backend in CLISessionBackendKind.allCases where backend != .copilotCLI { + #expect(backend.briefingEnvironment(briefingPath: "/b/p/AGENTS.md").isEmpty, "\(backend)") + } + #expect(CLISessionBackendKind.copilotCLI.briefingEnvironment(briefingPath: nil).isEmpty) + } + + @Test + func theBriefingIsWrittenWhereCopilotSearches() throws { + let url = try #require(SessionBriefing.write(projectPath: "/tmp/copilot-instructions")) + let copy = url.deletingLastPathComponent() + .appendingPathComponent(SessionBriefing.copilotInstructionsFile) + #expect( + try String(contentsOf: copy, encoding: .utf8) == String(contentsOf: url, encoding: .utf8)) + } + + @Test + func aResumedCopilotLoopIsBriefedAgain() throws { + // Measured: `--resume` rebuilds the system message from the resuming process's + // environment, and drops the instructions when the variable is missing. + let briefed = GraphcodeSettings(briefsSessionsAboutTheGraph: true) + for projectPath in ["/tmp/copilot-instructions", "ssh://someone@box/~/project"] { + let resumed = try #require( + ZmxSessionLauncher.resumeArguments( + forNode: goalNode(.copilotCLI), sessionID: "abc", projectPath: projectPath, + settings: briefed)) + let script = try #require(resumed.first { $0.hasPrefix("exec ") }) + #expect(script.hasPrefix("exec env \(variable)=\""), "\(projectPath)") + #expect(ZmxSessionLauncher.fitsInATypedCommandLine(resumed), "\(projectPath)") + } + let unbriefed = try #require( + ZmxSessionLauncher.resumeArguments( + forNode: goalNode(.copilotCLI), sessionID: "abc", projectPath: "/tmp/copilot-instructions", + settings: GraphcodeSettings(briefsSessionsAboutTheGraph: false))) + #expect(!unbriefed.contains { $0.contains(variable) }) + } + + @Test + func aRemoteCopilotLoopIsDeliveredTheCopyItSearchesFor() throws { + let location = try #require( + RemoteProjectLocation.parse(projectPath: "ssh://someone@box/~/project")) + let settings = GraphcodeSettings(briefsSessionsAboutTheGraph: true) + let copy = RemoteGraphAccess.copilotInstructionsPath(forProjectPath: location.projectPath) + #expect( + ZmxSessionLauncher.remoteDeliveryFiles( + forNode: goalNode(.copilotCLI), at: location, settings: settings)[copy] != nil) + #expect( + ZmxSessionLauncher.remoteDeliveryFiles( + forNode: nil, backend: .copilotCLI, at: location, settings: settings)[copy] != nil) + for backend in CLISessionBackendKind.allCases where backend != .copilotCLI { + #expect( + ZmxSessionLauncher.remoteDeliveryFiles( + forNode: goalNode(backend), at: location, settings: settings)[copy] == nil) + } + } +} diff --git a/graphcode/Tests/SessionBriefingTests.swift b/graphcode/Tests/SessionBriefingTests.swift index ea45ff13..0357e202 100644 --- a/graphcode/Tests/SessionBriefingTests.swift +++ b/graphcode/Tests/SessionBriefingTests.swift @@ -40,8 +40,8 @@ struct SessionBriefingTests { defer { try? FileManager.default.removeItem(at: url) } let written = try String(contentsOf: url, encoding: .utf8) #expect(written.contains("graphcode node create \(Self.project)")) - // Named for Copilot's own discovery, which searches directories rather than taking a - // file — and sitting in a per-project directory readable enough to tell them apart. + // Sitting in a per-project directory readable enough to tell them apart, beside the + // copy Copilot discovers (`CopilotInstructionsDeliveryTests`). #expect(url.lastPathComponent == SessionBriefing.fileName) #expect(url.deletingLastPathComponent().lastPathComponent.contains("project")) } @@ -191,33 +191,35 @@ struct SessionBriefingTests { } @Test - func copilotIsBothToldToReadTheBriefingAndAllowedTo() throws { - // `copilot` has no `--append-system-prompt`, and the documented - // COPILOT_CUSTOM_INSTRUCTIONS_DIRS is ignored in 1.0.75 — measured, not assumed. + func copilotFindsTheBriefingThroughItsEnvironmentAndMayStillReadIt() throws { + // Copilot searches `COPILOT_CUSTOM_INSTRUCTIONS_DIRS` for `*.instructions.md` and puts + // what it finds in its system prompt — measured per version, not assumed + // (`SessionBriefing.copilotInstructionsDirectoryVariable`). let arguments = try #require( ZmxSessionLauncher.arguments( forNode: node(backend: .copilotCLI), projectPath: Self.project)) #expect(!arguments.contains("--append-system-prompt-file")) + let variable = SessionBriefing.copilotInstructionsDirectoryVariable + let script = try #require(arguments.first { $0.hasPrefix("exec ") }) + #expect(script.hasPrefix("exec env \(variable)=\"${\(variable):+$\(variable),}")) + #expect(script.contains("briefings")) - // Copilot needs *both* halves, and shipping only one is issue #2: a preamble telling - // it to read the briefing, and permission to actually open it. Copilot verifies file - // paths, so without the second the session is denied the file it was just told to - // open — which looks exactly like an agent ignoring its instructions. Under YOLO - // every path is already allowed; under the narrower tools-only mode the briefing's - // directory must be granted by name. The launcher reads the machine's real settings, - // so the assertion accepts whichever route this machine is on. + // The directory grant stays. Under YOLO every path is already allowed; under the + // narrower tools-only mode Copilot verifies paths, and a session asked about its + // briefing should be able to open the file. The launcher reads the machine's real + // settings, so the assertion accepts whichever route this machine is on. if !arguments.contains("--yolo") { let granted = zip(arguments, arguments.dropFirst()) .filter { $0.0 == "--add-dir" }.map(\.1) #expect(granted.contains { $0.contains("briefings") }) } - // A Copilot time loop opens as prose — run now, then arm /every — so the briefing - // pointer leads it, and the raw directive never fronts the message. + // Nothing about the briefing rides in the prompt, so a Copilot time loop opens with + // its own first pass and the directive that arms the schedule. let interactive = try #require(arguments.firstIndex(of: "--interactive")) let opening = arguments[interactive + 1] - #expect(opening.contains(".md")) + #expect(!opening.contains("briefing file")) #expect(opening.contains("Run this task now: Check")) #expect(opening.contains("/every 1h Check")) #expect(!opening.hasPrefix("/loop")) diff --git a/graphcode/Tests/SessionPromptTests.swift b/graphcode/Tests/SessionPromptTests.swift index e8f8d540..888d3451 100644 --- a/graphcode/Tests/SessionPromptTests.swift +++ b/graphcode/Tests/SessionPromptTests.swift @@ -19,12 +19,19 @@ struct SessionPromptTests { prompt: "/loop 1h Check the queue", tier: .standard, briefingPath: "/tmp/briefings/x/AGENTS.md") + // Copilot's briefing now arrives through its environment (`briefingEnvironment`), so + // nothing is left in the message to push the directive off the front. let interactive = try #require(arguments.firstIndex(of: "--interactive")) - let opening = arguments[interactive + 1] - #expect(opening.hasPrefix("/loop 1h Check the queue")) - // Trailing, not dropped: `/loop ` takes the rest of the line as the - // task, so the briefing reaches every scheduled pass rather than none of them. - #expect(opening.contains("/tmp/briefings/x/AGENTS.md")) + #expect(arguments[interactive + 1] == "/loop 1h Check the queue") + + // A backend that still takes the pointer trails it: `/loop ` takes + // the rest of the line as the task, so the briefing reaches every scheduled pass. + let opencode = CLISessionBackendKind.openCode.launchArguments( + prompt: "/loop 1h Check the queue", tier: .standard, + briefingPath: "/tmp/briefings/x/AGENTS.md") + let flag = try #require(opencode.firstIndex(of: "--prompt")) + #expect(opencode[flag + 1].hasPrefix("/loop 1h Check the queue")) + #expect(opencode[flag + 1].contains("/tmp/briefings/x/AGENTS.md")) } /// The other half of the same rule, and the one issue #2 bought: an ordinary prose @@ -32,13 +39,13 @@ struct SessionPromptTests { /// the work is a session that never fanned out. @Test func anOrdinaryPromptStillOpensWithTheBriefingPointer() throws { - let arguments = CLISessionBackendKind.copilotCLI.launchArguments( + let arguments = CLISessionBackendKind.openCode.launchArguments( prompt: "fix the failing test", tier: .standard, briefingPath: "/tmp/briefings/x/AGENTS.md") - let interactive = try #require(arguments.firstIndex(of: "--interactive")) - #expect(arguments[interactive + 1].hasPrefix("Before anything else")) - #expect(arguments[interactive + 1].hasSuffix("fix the failing test")) + let flag = try #require(arguments.firstIndex(of: "--prompt")) + #expect(arguments[flag + 1].hasPrefix("Before anything else")) + #expect(arguments[flag + 1].hasSuffix("fix the failing test")) } /// The wake digest is the second preamble a launched session carries (`NodeMemory`), From 45cd94779232555e6e07a26d8b72a3e45b0dd3ed Mon Sep 17 00:00:00 2001 From: scgopi Date: Tue, 15 Sep 2026 09:16:10 -0700 Subject: [PATCH 2/2] Format the delivery call and avoid a lossy Data-to-String conversion in the test --- .../Infrastructure/Ghostty/GhosttyTerminalView+Remote.swift | 3 ++- graphcode/Tests/CopilotInstructionsDeliveryTests.swift | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView+Remote.swift b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView+Remote.swift index 782001f9..4441a5a9 100644 --- a/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView+Remote.swift +++ b/graphcode/Sources/Infrastructure/Ghostty/GhosttyTerminalView+Remote.swift @@ -28,7 +28,8 @@ extension GhosttyTerminalView { let quoted = RemoteProjectLocation.shellQuoted let delivery = ZmxSessionLauncher.remoteDeliveryScript( - forNode: nil, backend: backend, at: location, settings: settings) + forNode: nil, backend: backend, at: location, settings: settings + ) .map { $0 + "; " } ?? "" let agentScripts = launchesClaudeCode diff --git a/graphcode/Tests/CopilotInstructionsDeliveryTests.swift b/graphcode/Tests/CopilotInstructionsDeliveryTests.swift index 30fe666d..dcd33fd1 100644 --- a/graphcode/Tests/CopilotInstructionsDeliveryTests.swift +++ b/graphcode/Tests/CopilotInstructionsDeliveryTests.swift @@ -37,7 +37,7 @@ struct CopilotInstructionsDeliveryTests { try process.run() let data = output.fileHandleForReading.readDataToEndOfFile() process.waitUntilExit() - return String(decoding: data, as: UTF8.self).trimmingCharacters(in: .newlines) + return (String(bytes: data, encoding: .utf8) ?? "").trimmingCharacters(in: .newlines) } #expect(try expanded(existing: nil) == "/b/briefings/p") #expect(try expanded(existing: "") == "/b/briefings/p")