Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions GraphcodeKit/Sources/Domain/BackendCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
58 changes: 48 additions & 10 deletions GraphcodeKit/Sources/Sessions/PresenceHooks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down
97 changes: 74 additions & 23 deletions graphcode/Tests/CodexPresenceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,53 +23,103 @@ 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"))
}

@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
Expand All @@ -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))
}

Expand Down
6 changes: 5 additions & 1 deletion graphcode/Tests/RemoteLoopSurvivalTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 41 additions & 0 deletions graphcode/Tests/ZmxSessionLauncherTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading