Merge pull request #349 from scgopi/fix/codex-briefing-fits

Launch Codex loops with their graph briefing

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: scgopi

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`.

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,

graphcode/Tests/CodexPresenceTests.swift

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

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

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