From ce86588ed38b62673390033b0a09b956aabb22b6 Mon Sep 17 00:00:00 2001 From: ziyang Date: Tue, 12 May 2026 19:57:53 +0800 Subject: [PATCH] fix: remove legacy codex hooks config --- Sources/CodeIsland/ConfigInstaller.swift | 36 ++++++++++++++-------- Sources/CodeIsland/RemoteInstaller.swift | 23 +++++++++----- Tests/CodeIslandTests/CodexHomeTests.swift | 14 +++++++++ 3 files changed, 52 insertions(+), 21 deletions(-) diff --git a/Sources/CodeIsland/ConfigInstaller.swift b/Sources/CodeIsland/ConfigInstaller.swift index 2c12156..c53d597 100644 --- a/Sources/CodeIsland/ConfigInstaller.swift +++ b/Sources/CodeIsland/ConfigInstaller.swift @@ -1627,25 +1627,35 @@ struct ConfigInstaller { contents = (try? String(contentsOfFile: configPath, encoding: .utf8)) ?? "" } - // Already set to true (non-commented) — don't touch - if contents.range(of: #"(?m)^\s*hooks\s*=\s*true"#, options: .regularExpression) != nil { - return true - } - - // Set to false (non-commented) — flip it to true in place - if contents.range(of: #"(?m)^\s*hooks\s*=\s*false"#, options: .regularExpression) != nil { + let currentHooksPattern = #"(?m)^\s*hooks\s*=\s*(true|false)\s*(#.*)?$"# + let hooksTruePattern = #"(?m)^\s*hooks\s*=\s*true\s*(#.*)?$"# + let hooksFalsePattern = #"(?m)^\s*hooks\s*=\s*false\s*(#.*)?$"# + let legacyHooksPattern = #"(?m)^\s*codex_hooks\s*=\s*(true|false)\s*(#.*)?$"# + let hasCurrentHooks = contents.range(of: currentHooksPattern, options: .regularExpression) != nil + let hasLegacyHooks = contents.range(of: legacyHooksPattern, options: .regularExpression) != nil + + // Remove the retired feature name used by older Codex releases. If the + // current flag is absent, turn the legacy flag into the current one. + if hasLegacyHooks { contents = contents.replacingOccurrences( - of: #"(?m)^\s*hooks\s*=\s*false"#, - with: "hooks = true", + of: legacyHooksPattern, + with: hasCurrentHooks ? "" : "hooks = true", options: .regularExpression ) - return fm.createFile(atPath: configPath, contents: contents.data(using: .utf8)) } - // Migrate the retired feature name used by older Codex releases. - if contents.range(of: #"(?m)^\s*codex_hooks\s*=\s*(true|false)"#, options: .regularExpression) != nil { + // Already set to true (non-commented) — don't touch beyond legacy cleanup. + if contents.range(of: hooksTruePattern, options: .regularExpression) != nil { + if hasLegacyHooks { + return fm.createFile(atPath: configPath, contents: contents.data(using: .utf8)) + } + return true + } + + // Set to false (non-commented) — flip it to true in place. + if contents.range(of: hooksFalsePattern, options: .regularExpression) != nil { contents = contents.replacingOccurrences( - of: #"(?m)^\s*codex_hooks\s*=\s*(true|false)"#, + of: hooksFalsePattern, with: "hooks = true", options: .regularExpression ) diff --git a/Sources/CodeIsland/RemoteInstaller.swift b/Sources/CodeIsland/RemoteInstaller.swift index c47d6e5..eab4bc6 100644 --- a/Sources/CodeIsland/RemoteInstaller.swift +++ b/Sources/CodeIsland/RemoteInstaller.swift @@ -529,15 +529,22 @@ def install_claude(): def ensure_toml_codex_hooks(path): content = path.read_text() if path.exists() else "" - if re.search(r"(?m)^\\s*hooks\\s*=\\s*true\\s*$", content): + current_hooks_pattern = r"(?m)^\\s*hooks\\s*=\\s*(true|false)\\s*(#.*)?$" + hooks_true_pattern = r"(?m)^\\s*hooks\\s*=\\s*true\\s*(#.*)?$" + hooks_false_pattern = r"(?m)^\\s*hooks\\s*=\\s*false\\s*(#.*)?$" + legacy_hooks_pattern = r"(?m)^\\s*codex_hooks\\s*=\\s*(true|false)\\s*(#.*)?$" + has_current_hooks = re.search(current_hooks_pattern, content) is not None + had_legacy_hooks = re.search(legacy_hooks_pattern, content) is not None + if re.search(legacy_hooks_pattern, content): + replacement = "" if has_current_hooks else "hooks = true" + content = re.sub(legacy_hooks_pattern, replacement, content) + if re.search(hooks_true_pattern, content): + if had_legacy_hooks: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(content.rstrip() + "\\n") return - if re.search(r"(?m)^\\s*hooks\\s*=\\s*false\\s*$", content): - content = re.sub(r"(?m)^\\s*hooks\\s*=\\s*false\\s*$", "hooks = true", content) - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(content.rstrip() + "\\n") - return - if re.search(r"(?m)^\\s*codex_hooks\\s*=\\s*(true|false)\\s*$", content): - content = re.sub(r"(?m)^\\s*codex_hooks\\s*=\\s*(true|false)\\s*$", "hooks = true", content) + if re.search(hooks_false_pattern, content): + content = re.sub(hooks_false_pattern, "hooks = true", content) path.parent.mkdir(parents=True, exist_ok=True) path.write_text(content.rstrip() + "\\n") return diff --git a/Tests/CodeIslandTests/CodexHomeTests.swift b/Tests/CodeIslandTests/CodexHomeTests.swift index 234c67a..0281228 100644 --- a/Tests/CodeIslandTests/CodexHomeTests.swift +++ b/Tests/CodeIslandTests/CodexHomeTests.swift @@ -99,6 +99,20 @@ final class CodexHomeTests: XCTestCase { XCTAssertFalse(contents.contains("codex_hooks")) } + func testEnableCodexHooksConfigRemovesLegacyFeatureNameWhenCurrentFeatureAlreadyEnabled() throws { + let codexHome = makeTemporaryCodexHome() + defer { try? FileManager.default.removeItem(at: codexHome) } + let config = codexHome.appendingPathComponent("config.toml") + try FileManager.default.createDirectory(at: codexHome, withIntermediateDirectories: true) + try "[features]\nhooks = true # current\ncodex_hooks = true # legacy\n".write(to: config, atomically: true, encoding: .utf8) + + XCTAssertTrue(ConfigInstaller.enableCodexHooksConfig(fm: .default)) + + let contents = try String(contentsOf: config, encoding: .utf8) + XCTAssertTrue(contents.contains("hooks = true")) + XCTAssertFalse(contents.contains("codex_hooks")) + } + private func makeTemporaryCodexHome() -> URL { let url = FileManager.default.temporaryDirectory .appendingPathComponent("codeisland-codex-home-\(UUID().uuidString)", isDirectory: true)