From 8f9f1cb20a1471b574ec21c2ae9a7cb62674a3d9 Mon Sep 17 00:00:00 2001 From: Danny Gershman Date: Thu, 11 Jun 2026 19:09:43 -0400 Subject: [PATCH] Fix Codex hook config: rename to [features].hooks, drop async (CROW-494) Codex v0.139.0+ renamed `[features].codex_hooks` to `[features].hooks` and silently skips any hook marked `async: true` because its runtime is sync-only. The deprecation warning was loud; the async-skip was silent, breaking Crow's session-state detection (auto-respond, card-color, etc.) on every Codex session because `PostToolUse` and `Stop` never reached `CodexSignalSource`. - Empty out `asyncEvents` in `CodexHookConfigWriter` so no entry is written with `async: true`. - Switch the TOML key from `codex_hooks` to `hooks`. - Add `removeTomlSectionLine` helper + a one-shot migration step in `installGlobalTomlConfig` that strips a legacy `codex_hooks` line under `[features]` so users converging from older installs end up with a single, current entry. Idempotent. - Update existing tests + add tests covering the migration path and the no-async invariant. Closes #494 --- .../CrowCodex/CodexHookConfigWriter.swift | 57 ++++++++++++-- .../CodexHookConfigWriterTests.swift | 74 ++++++++++++++++++- 2 files changed, 124 insertions(+), 7 deletions(-) diff --git a/Packages/CrowCodex/Sources/CrowCodex/CodexHookConfigWriter.swift b/Packages/CrowCodex/Sources/CrowCodex/CodexHookConfigWriter.swift index d7624c6c..99ea3be8 100644 --- a/Packages/CrowCodex/Sources/CrowCodex/CodexHookConfigWriter.swift +++ b/Packages/CrowCodex/Sources/CrowCodex/CodexHookConfigWriter.swift @@ -22,8 +22,12 @@ public struct CodexHookConfigWriter: HookConfigWriter { "PermissionRequest", ] - /// Post-execution events safe to run async (fire-and-forget). - private static let asyncEvents: Set = ["PostToolUse", "Stop"] + /// Events that should run async (fire-and-forget). Codex's hook runtime + /// is sync-only as of v0.139.0 — declaring `async = true` causes the + /// entry to be silently skipped on startup, which breaks Crow's + /// session-state detection. Keep this empty until/unless Codex grows + /// real async support upstream. + private static let asyncEvents: Set = [] public init() {} @@ -89,9 +93,15 @@ public struct CodexHookConfigWriter: HookConfigWriter { } /// Install or update `/config.toml` with the - /// `features.codex_hooks = true` flag and the Crow `notify` line. + /// `features.hooks = true` flag and the Crow `notify` line. /// Preserves any other user-authored config — minimal line-oriented merge /// avoids pulling in a TOML dependency for two simple keys. + /// + /// Also runs a one-shot migration: legacy installs (Crow before this + /// fix, or older Codex versions) wrote `codex_hooks = true` under + /// `[features]`. Codex v0.139.0+ renamed the key to `hooks` and emits + /// a deprecation warning for the old one — strip it so users converging + /// from older configs end up with a single, current entry. public static func installGlobalTomlConfig(codexHome: String, crowPath: String) throws { try FileManager.default.createDirectory(atPath: codexHome, withIntermediateDirectories: true) let tomlPath = (codexHome as NSString).appendingPathComponent("config.toml") @@ -104,11 +114,14 @@ public struct CodexHookConfigWriter: HookConfigWriter { let notifyLine = "notify = [\"\(escapeTomlString(crowPath))\", \"codex-notify\"]" content = upsertTomlLine(content, key: "notify", line: notifyLine) + // Strip the deprecated `codex_hooks` key before writing the modern + // `hooks` key so we don't leave both lines behind on migration. + content = removeTomlSectionLine(content, section: "features", key: "codex_hooks") content = upsertTomlSectionLine( content, section: "features", - key: "codex_hooks", - line: "codex_hooks = true" + key: "hooks", + line: "hooks = true" ) try content.write(toFile: tomlPath, atomically: true, encoding: .utf8) @@ -200,6 +213,40 @@ public struct CodexHookConfigWriter: HookConfigWriter { return lines.joined(separator: "\n") } + /// Remove `key = …` from inside `[section]` if present. Returns the + /// content unchanged when the section or key is absent — idempotent and + /// safe to chain before an `upsertTomlSectionLine` call. + static func removeTomlSectionLine( + _ content: String, + section: String, + key: String + ) -> String { + var lines = content.components(separatedBy: "\n") + var sectionStart: Int? = nil + var sectionEnd: Int = lines.count + let sectionHeader = "[\(section)]" + for (i, raw) in lines.enumerated() { + let trimmed = raw.trimmingCharacters(in: .whitespaces) + if trimmed == sectionHeader { + sectionStart = i + continue + } + if let _ = sectionStart, trimmed.hasPrefix("[") && trimmed.hasSuffix("]") { + sectionEnd = i + break + } + } + + guard let start = sectionStart else { return content } + for i in (start + 1).. String? { diff --git a/Packages/CrowCodex/Tests/CrowCodexTests/CodexHookConfigWriterTests.swift b/Packages/CrowCodex/Tests/CrowCodexTests/CodexHookConfigWriterTests.swift index 6bdcd561..8bfefa42 100644 --- a/Packages/CrowCodex/Tests/CrowCodexTests/CodexHookConfigWriterTests.swift +++ b/Packages/CrowCodex/Tests/CrowCodexTests/CodexHookConfigWriterTests.swift @@ -102,7 +102,8 @@ struct CodexHookConfigWriterTests { let toml = try String(contentsOf: codexHome.appendingPathComponent("config.toml")) #expect(toml.contains("notify = [\"/opt/homebrew/bin/crow\", \"codex-notify\"]")) #expect(toml.contains("[features]")) - #expect(toml.contains("codex_hooks = true")) + #expect(toml.contains("hooks = true")) + #expect(!toml.contains("codex_hooks"), "deprecated codex_hooks key must not be written") } @Test func installGlobalTomlConfigPreservesUserSettings() throws { @@ -132,6 +133,75 @@ struct CodexHookConfigWriterTests { #expect(toml.contains("memories = true")) // Crow entries added. #expect(toml.contains("notify = ")) - #expect(toml.contains("codex_hooks = true")) + #expect(toml.contains("hooks = true")) + #expect(!toml.contains("codex_hooks"), "deprecated codex_hooks key must not be written") + } + + @Test func installGlobalTomlConfigMigratesLegacyCodexHooksKey() throws { + let codexHome = try makeTempCodexHome() + defer { try? FileManager.default.removeItem(at: codexHome) } + + // Pre-seed with the deprecated `codex_hooks` key that pre-fix + // installs left behind. The migration should strip it and replace + // it with the current `hooks` key. + let legacy = """ + model = "gpt-4o" + + [features] + codex_hooks = true + memories = true + """ + try legacy.write( + toFile: codexHome.appendingPathComponent("config.toml").path, + atomically: true, encoding: .utf8 + ) + + try CodexHookConfigWriter.installGlobalTomlConfig( + codexHome: codexHome.path, + crowPath: "/usr/local/bin/crow" + ) + + let toml = try String(contentsOf: codexHome.appendingPathComponent("config.toml")) + #expect(toml.contains("hooks = true"), "modern hooks key should be present") + #expect(!toml.contains("codex_hooks"), "deprecated codex_hooks key should be stripped") + // Unrelated user entries survive. + #expect(toml.contains("model = \"gpt-4o\"")) + #expect(toml.contains("memories = true")) + + // Migration is idempotent — re-running produces the same content. + try CodexHookConfigWriter.installGlobalTomlConfig( + codexHome: codexHome.path, + crowPath: "/usr/local/bin/crow" + ) + let second = try String(contentsOf: codexHome.appendingPathComponent("config.toml")) + #expect(toml == second) + } + + @Test func installGlobalConfigEmitsNoAsyncHooks() throws { + // Codex's hook runtime is sync-only; declaring async causes the + // entry to be silently skipped at startup, which breaks Crow's + // session-state detection. Guard against the regression. + let codexHome = try makeTempCodexHome() + defer { try? FileManager.default.removeItem(at: codexHome) } + try CodexHookConfigWriter.installGlobalConfig( + codexHome: codexHome.path, + crowPath: "/usr/local/bin/crow" + ) + + let data = try Data(contentsOf: codexHome.appendingPathComponent("hooks.json")) + let json = try JSONSerialization.jsonObject(with: data) as! [String: Any] + let hooks = json["hooks"] as! [String: Any] + for (event, value) in hooks { + let entries = value as! [[String: Any]] + for outer in entries { + let inner = outer["hooks"] as! [[String: Any]] + for entry in inner { + #expect( + entry["async"] == nil, + "event \(event) has async flag; Codex silently skips async hooks" + ) + } + } + } } }