diff --git a/GraphcodeKit/Sources/Domain/BackendCommand.swift b/GraphcodeKit/Sources/Domain/BackendCommand.swift index 3f749fbb..a6eadac0 100644 --- a/GraphcodeKit/Sources/Domain/BackendCommand.swift +++ b/GraphcodeKit/Sources/Domain/BackendCommand.swift @@ -286,17 +286,15 @@ extension CLISessionBackendKind { case .copilotCLI: return sessionName.map { ["--name", $0] } ?? [] case .codex: - // Codex reports only the *end* of a turn, through `notify`, and needs no file to do - // it — just somewhere to write, which is why this is the one backend that takes the - // `zmx` path rather than a path to something graphcode wrote. Its other edge is + // Codex reports only the *end* of a turn, through `notify`. Its other edge is // covered without asking Codex anything: see `ZmxSessionLauncher.codexPresence`. - return zmxPath.map { - [ - "-c", - PresenceHooks.codexNotifyOverride( - zmxPath: $0, sessionsDirectory: sessionsDirectory), - ] - } ?? [] + // A remote launch has no local hooks file and names the one its ensure wrote on + // the host, which only a remote `sessionsDirectory` distinguishes from a local + // launch whose write failed. + if let hooksFile { + return ["-c", PresenceHooks.codexNotifyOverride(scriptPath: hooksFile.path)] + } + return sessionsDirectory == nil ? [] : ["-c", PresenceHooks.remoteCodexNotifyOverride] case .openCode: // Reports through a plugin, which rides in the environment rather than the argv — // see `presenceEnvironment`. diff --git a/GraphcodeKit/Sources/Sessions/PresenceHooks.swift b/GraphcodeKit/Sources/Sessions/PresenceHooks.swift index 8ce3fffb..ffd78092 100644 --- a/GraphcodeKit/Sources/Sessions/PresenceHooks.swift +++ b/GraphcodeKit/Sources/Sessions/PresenceHooks.swift @@ -57,6 +57,12 @@ public enum PresenceHooks { directory.appendingPathComponent("notification.sh") } + /// Codex's `notify` program, a file so the `-c` override that names it stays a path — + /// inlined, the script alone overran the typed line once a briefing rode beside it. + public static var codexNotifyScriptFile: URL { + directory.appendingPathComponent("codex-notify.sh") + } + /// Which of a backend's lifecycle events mean what, in the backend's own event names. /// /// `nil` for a backend with no hook mechanism at all, which is the honest answer for @@ -356,6 +362,7 @@ public enum PresenceHooks { public static let remotePiExtensionExpression = "\"\(remotePiExtensionPath)\"" public static let remoteOpenCodePluginExpression = "\"$HOME/.graphcode/hooks/opencode-presence.js\"" + public static let remoteCodexNotifyScriptExpression = "\"$HOME/.graphcode/hooks/codex-notify.sh\"" /// The remote twin of `SessionIDStore.file(forNodeID:)` — the file the remote /// `SessionStart` hook wrote, as a shell expression the ensure dial can `cat`. @@ -424,6 +431,7 @@ public enum PresenceHooks { public static func write(forBackend backend: CLISessionBackendKind) -> URL? { guard ZmxLocator.isInstalled else { return nil } if backend == .pi { return writePiExtension() } + if backend == .codex { return writeCodexNotifyScript() } guard let json = json(forBackend: backend, zmxPath: ZmxLocator.binaryURL.path) else { return nil } let url = file(forBackend: backend) @@ -469,6 +477,18 @@ public enum PresenceHooks { } } + /// Codex's reporter, written where `codexNotifyOverride(scriptPath:)` names it. + private static func writeCodexNotifyScript() -> URL? { + do { + try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true) + try codexNotifyScript(zmxPath: ZmxLocator.binaryURL.path) + .write(to: codexNotifyScriptFile, atomically: true, encoding: .utf8) + return codexNotifyScriptFile + } catch { + return nil + } + } + // MARK: - Remote sessions /// Where the hooks land on a remote host — a `$HOME` expression rather than a path, @@ -507,6 +527,12 @@ public enum PresenceHooks { + " && printf '%s' \(singleQuoted(PiPresenceExtension.remoteSource(zmxPath: "zmx")))" + " > \(remotePiExtensionExpression); } 2>/dev/null || true" } + if backend == .codex { + let script = codexNotifyScript(zmxPath: "zmx", sessionsDirectory: remoteSessionsExpression) + return "{ mkdir -p \"$HOME/.graphcode/hooks\"" + + " && printf '%s' \(singleQuoted(script))" + + " > \(remoteCodexNotifyScriptExpression); } 2>/dev/null || true" + } guard backend == .claudeCode else { return nil } guard let json = json( @@ -545,22 +571,34 @@ public enum PresenceHooks { /// Validated as a real key rather than assumed: `codex --strict-config` rejects an /// invented field outright ("unknown configuration field") and accepts this one. /// - /// The value is TOML, parsed by Codex out of one argv element. Codex appends its event - /// JSON as a further argument, which `sh -c` puts in `$0`; its thread ID is persisted - /// under the node ID so only that node can resume it after the zmx session disappears. - public static func codexNotifyOverride( - zmxPath: String, sessionsDirectory: String? = nil - ) -> String { + /// The value is TOML, parsed by Codex out of one argv element, and names a script file + /// rather than carrying the script: `zmx` types the launch into a `MAX_CANON`-capped line, + /// and the inline script left no room for the briefing, so every Codex loop launched + /// unbriefed. Codex appends its event JSON as a further argument, `$1` to the script. + public static func codexNotifyOverride(scriptPath: String) -> String { + "notify=[\"/bin/sh\",\(tomlString(scriptPath))]" + } + + /// The remote twin: only a shell on that host can expand `$HOME`, so `sh -c` does it + /// there and forwards the event JSON (its `$0`) as the script's `$1`. + public static let remoteCodexNotifyCommand = + "exec /bin/sh \(remoteCodexNotifyScriptExpression) \"$0\"" + + public static var remoteCodexNotifyOverride: String { + "notify=[\"/bin/sh\",\"-c\",\(tomlString(remoteCodexNotifyCommand))]" + } + + /// What `notify` runs: its thread ID is persisted under the node ID so only that node + /// can resume it after the zmx session disappears, then the turn end is reported. + static func codexNotifyScript(zmxPath: String, sessionsDirectory: String? = nil) -> String { let sessions = sessionsDirectory ?? localSessionsExpression - let script = - "i=$(printf '%s' \"$0\"|sed -n " + return "i=$(printf '%s' \"$1\"|sed -n " + "'s/.*\"thread-id\"[[:space:]]*:[[:space:]]*\"\\([^\"]*\\)\".*/\\1/p'); " + "n=\"${ZMX_SESSION#\(SurfaceRef.zmxSessionPrefix)}\"; d=\(sessions); " + "if [ -n \"$i\" ]&&[ \"$n\" != \"$ZMX_SESSION\" ];then mkdir -p \"$d\"; " + "printf '%s %s %s\\n' \"$(date +%s)\" \"$i\" \"$PWD\" " + ">>\"$d/$n.history\" 2>/dev/null; printf %s \"$i\">\"$d/$n.id\"; fi; " - + "\(singleQuoted(zmxPath)) set \"$ZMX_SESSION\" presence=idle >/dev/null 2>&1; exit 0" - return "notify=[\"/bin/sh\",\"-c\",\(tomlString(script))]" + + "\(singleQuoted(zmxPath)) set \"$ZMX_SESSION\" presence=idle >/dev/null 2>&1; exit 0\n" } /// A TOML basic string. Only the two escapes this can actually produce are handled, diff --git a/graphcode/Tests/CodexPresenceTests.swift b/graphcode/Tests/CodexPresenceTests.swift index be2c7b02..d200da82 100644 --- a/graphcode/Tests/CodexPresenceTests.swift +++ b/graphcode/Tests/CodexPresenceTests.swift @@ -23,42 +23,88 @@ struct CodexPresenceTests { @Test func theTurnEndIsReportedThroughTheOneChannelCodexHas() { - let override = PresenceHooks.codexNotifyOverride(zmxPath: zmx) + let script = PresenceHooks.codexNotifyScript(zmxPath: zmx) - #expect(override.hasPrefix("notify=[")) - #expect(override.contains("presence=idle")) + #expect(script.contains("presence=idle")) // Same session-owned label store the other two backends report into, so one reader // serves all three. - #expect(override.contains(#"$ZMX_SESSION"#)) - #expect(override.contains("thread-id")) - #expect(override.contains(".history")) - #expect(override.contains(".id")) + #expect(script.contains(#"$ZMX_SESSION"#)) + #expect(script.contains("thread-id")) + #expect(script.contains(".history")) + #expect(script.contains(".id")) + #expect(script.contains("'\(zmx)'")) } @Test func theOverrideIsValidTOMLForAnAwkwardPath() { - // The value is TOML parsed out of one argv element, so the inner quotes around - // `$ZMX_SESSION` have to survive as escapes rather than closing the string early. - let override = PresenceHooks.codexNotifyOverride(zmxPath: "/Users/o'brien/bin/zmx") - - #expect(override.contains(#"\"$ZMX_SESSION\""#)) - // Two layers, and the doubled backslash is both of them doing their job: shell - // quoting turns the apostrophe into `'\''`, then TOML escapes that backslash to - // `\\`. Codex decodes the TOML back to `'\''`, which is what the shell must see. - #expect(override.contains(#"o'\\''brien"#)) - // Three array elements: the program, its flag, and the script. - #expect(override.hasPrefix(#"notify=["/bin/sh","-c",""#)) - #expect(override.hasSuffix("]")) + // The value is TOML parsed out of one argv element, so a quote in the script's path + // has to survive as an escape rather than closing the string early. + let override = PresenceHooks.codexNotifyOverride(scriptPath: #"/Users/o"brien/codex-notify.sh"#) + + #expect(override == #"notify=["/bin/sh","/Users/o\"brien/codex-notify.sh"]"#) + // The remote form's `$HOME` and `$0` stay escaped for the shell on the host to expand. + #expect( + PresenceHooks.remoteCodexNotifyOverride + == #"notify=["/bin/sh","-c","exec /bin/sh \"$HOME/.graphcode/hooks/codex-notify.sh\" \"$0\""]"# + ) + #expect(PresenceHooks.codexNotifyScript(zmxPath: "/Users/o'brien/zmx").contains(#"o'\''brien"#)) + } + + @Test + func theNotifyScriptBanksTheThreadAndReportsIdleWhenCodexRunsIt() async throws { + // Run the way Codex runs it: the script by path, the event JSON appended as one more + // argument — then again through the remote form's `$HOME` hop. + let root = FileManager.default.temporaryDirectory + .appendingPathComponent("codex-notify-\(UUID().uuidString)", isDirectory: true) + defer { try? FileManager.default.removeItem(at: root) } + let hooks = root.appendingPathComponent(".graphcode/hooks", isDirectory: true) + try FileManager.default.createDirectory(at: hooks, withIntermediateDirectories: true) + let calls = root.appendingPathComponent("zmx-calls") + let fakeZmx = root.appendingPathComponent("zmx") + try "#!/bin/sh\necho \"$@\" >> '\(calls.path)'\n".write( + to: fakeZmx, atomically: true, encoding: .utf8) + try FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: fakeZmx.path) + let sessions = root.appendingPathComponent("sessions", isDirectory: true) + let script = hooks.appendingPathComponent("codex-notify.sh") + try PresenceHooks.codexNotifyScript( + zmxPath: fakeZmx.path, sessionsDirectory: PresenceHooks.singleQuoted(sessions.path) + ).write(to: script, atomically: true, encoding: .utf8) + let nodeID = UUID().uuidString + let event = #"{"type":"agent-turn-complete","thread-id":"t-42"}"# + let environment = [ + "HOME": root.path, "PATH": "/usr/bin:/bin", "ZMX_SESSION": "graphcode-\(nodeID)", + ] + + #expect(try await run(["/bin/sh", script.path, event], environment: environment) == 0) + #expect( + try String(contentsOf: sessions.appendingPathComponent("\(nodeID).id"), encoding: .utf8) + == "t-42") + #expect( + try await run( + ["/bin/sh", "-c", PresenceHooks.remoteCodexNotifyCommand, event], environment: environment) + == 0) + let reports = try String(contentsOf: calls, encoding: .utf8) + #expect(reports == String(repeating: "set graphcode-\(nodeID) presence=idle\n", count: 2)) + } + + private func run(_ argv: [String], environment: [String: String]) async throws -> Int32 { + let process = Process() + process.executableURL = URL(fileURLWithPath: argv[0]) + process.arguments = Array(argv.dropFirst()) + process.environment = environment + return try await withCheckedThrowingContinuation { continuation in + process.terminationHandler = { continuation.resume(returning: $0.terminationStatus) } + do { try process.run() } catch { continuation.resume(throwing: error) } + } } @Test func codexTakesTheFlagAndNothingMeantForTheOthers() { let arguments = CLISessionBackendKind.codex.presenceArguments( - hooksFile: URL(fileURLWithPath: "/tmp/hooks.json"), + hooksFile: URL(fileURLWithPath: "/tmp/codex-notify.sh"), sessionName: "graphcode-A", zmxPath: zmx) - #expect(arguments.first == "-c") - #expect(arguments.count == 2) + #expect(arguments == ["-c", #"notify=["/bin/sh","/tmp/codex-notify.sh"]"#]) // Codex has no `--settings` to layer hooks into and no `--name` to label a session. #expect(!arguments.contains("--settings")) #expect(!arguments.contains("--name")) @@ -66,10 +112,14 @@ struct CodexPresenceTests { @Test func nowhereToReportMeansNoFlagRatherThanABrokenOne() { - // A machine with no zmx cannot accept a presence or resume-ID report. + // A machine with no zmx writes no script, and a local launch must not fall back to + // naming the remote one. #expect( CLISessionBackendKind.codex.presenceArguments( hooksFile: nil, sessionName: "graphcode-A", zmxPath: nil) == []) + #expect( + CLISessionBackendKind.codex.presenceArguments( + hooksFile: nil, sessionName: "graphcode-A", zmxPath: zmx) == []) } @Test @@ -86,6 +136,7 @@ struct CodexPresenceTests { #expect(command.contains("notify=")) #expect(command.contains("thread-id")) #expect(command.contains("$HOME/.graphcode/sessions")) + #expect(command.contains("codex-notify.sh")) #expect(!command.contains(ZmxLocator.binaryURL.path)) } diff --git a/graphcode/Tests/RemoteLoopSurvivalTests.swift b/graphcode/Tests/RemoteLoopSurvivalTests.swift index d68a0ab9..2b2f9331 100644 --- a/graphcode/Tests/RemoteLoopSurvivalTests.swift +++ b/graphcode/Tests/RemoteLoopSurvivalTests.swift @@ -437,7 +437,11 @@ struct RemoteLoopSurvivalTests { .resumeCommand(settings: GraphcodeSettings(), remoteSettingsPath: nil, isRemote: true)? .joined(separator: " ") #expect(codex?.contains(#"resume "$GRAPHCODE_RESUME_ID""#) == true) - #expect(codex?.contains("$HOME/.graphcode/sessions") == true) + // The notifier is a script the restore's hooks write puts on the host. + #expect(codex?.contains("$HOME/.graphcode/hooks/codex-notify.sh") == true) + #expect( + PresenceHooks.remoteWriteFragment(forBackend: .codex)? + .contains("$HOME/.graphcode/sessions") == true) } // MARK: - Remote presence hooks diff --git a/graphcode/Tests/ZmxSessionLauncherTests.swift b/graphcode/Tests/ZmxSessionLauncherTests.swift index ad5fcbb6..e6f3d3e9 100644 --- a/graphcode/Tests/ZmxSessionLauncherTests.swift +++ b/graphcode/Tests/ZmxSessionLauncherTests.swift @@ -540,6 +540,47 @@ struct ZmxSessionLauncherTests { /// The probe's own failures, kept out of the suite body above only because swiftlint's /// `type_body_length` is at its limit there. extension ZmxSessionLauncherTests { + @Test + func aCodexLoopLaunchesBriefedAtEveryGoalLength() throws { + // Codex's `notify` override carried its whole reporter script inline, so its launch + // was ~650 bytes before any goal and the briefing never fit: every Codex loop, local + // or remote, short goal or long, launched with no idea it was in a graph. The medium + // goal is sized from the measured briefed baseline so it fills the line to its edge. + try #require(ZmxLocator.isInstalled) + let settings = GraphcodeSettings(briefsSessionsAboutTheGraph: true) + let budget = ZmxSessionLauncher.maximumTypedCommandBytes + let remote = "ssh://someone@box/~/project" + for projectPath in ["/tmp", remote] { + let briefing = + projectPath == remote + ? RemoteGraphAccess.briefingPath(forProjectPath: remote) + : SessionBriefing.directory(forProjectPath: projectPath) + .appendingPathComponent(SessionBriefing.fileName).path + func launch(_ goal: String) throws -> [String] { + let node = LoopNode( + title: "Codex", loopType: .goalBased, goal: GoalSpec(summary: goal), backend: .codex) + defer { NodeMemory.remove(projectPath: projectPath, nodeID: node.id) } + return try #require( + ZmxSessionLauncher.arguments(forNode: node, projectPath: projectPath, settings: settings)) + } + let baseline = try launch("x").reduce(0) { $0 + $1.utf8.count + 3 } + // Measured 235 bytes of room locally with the file; main left none once briefed. + #expect(budget - baseline >= 200, "\(projectPath): \(baseline) bytes before the goal") + let medium = String(repeating: "m", count: max(budget - baseline - 8, 1)) + let long = String(repeating: "Resolve the conflict before moving on. ", count: 103) + + for (length, goal) in [("short", "Fix the flaky test"), ("medium", medium), ("long", long)] { + let arguments = try launch(goal) + let context = "\(length) goal at \(projectPath)" + #expect(ZmxSessionLauncher.fitsInATypedCommandLine(arguments), "\(context)") + #expect(arguments.last?.contains(briefing) == true, "\(context)") + #expect(arguments.contains { $0.hasPrefix("notify=[") }, "\(context)") + } + #expect(try launch(medium).last?.contains(medium) == true, "\(projectPath)") + #expect(try launch(long).last?.contains(NodeMemory.promptFileName) == true, "\(projectPath)") + } + } + @Test func aMediumGoalMovesToAFileBeforeTheBriefingIsDropped() throws { // Issue #345: a ~600-byte goal overran the line only once the briefing was added, and