From dff6a19946b8f96bb8b16fa945b69a1ed09709af Mon Sep 17 00:00:00 2001 From: Logan Kang Date: Sat, 13 Jun 2026 02:49:34 +0900 Subject: [PATCH] fix(codex): write canonical hooks feature flag and migrate deprecated codex_hooks (#743) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description `headroom init codex` writes the hooks feature flag into `.codex/config.toml` under the key `codex_hooks`. Codex renamed the canonical key to `hooks` and kept `codex_hooks` as a legacy alias (openai/codex#20522). Current Codex builds warn about `[features].codex_hooks` and tell users to use `[features].hooks` instead, so configs written by headroom should stop emitting the deprecated key. This PR switches headroom to write the canonical `hooks` key and **migrates existing configs in place**. The migration is the tricky part: a config can already contain `codex_hooks`, `hooks`, or both, in any order, inside or outside headroom's marker block — and a naive replace can emit a *duplicate* `hooks` key, which is invalid TOML that Codex rejects outright. The fix strips every `codex_hooks` line up front (any value, anywhere) — mirroring the existing top-level key cleanup in `_ensure_codex_provider` (#260) — then guarantees `hooks` is present without ever duplicating it, and respects a user-managed `hooks` value that lives outside our marker block. Fixes: N/A (no tracking issue — surfaced while aligning with Codex >= 0.129; related upstream context: openai/codex#20522 and the warning behavior discussed in openai/codex#22148) ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - Write the canonical `hooks` key instead of the deprecated `codex_hooks` in `_ensure_codex_feature_flag` (`headroom/cli/init.py`). - Strip any `codex_hooks` line (any value, inside or outside the marker block) before ensuring the flag, so re-running `init` migrates a legacy config instead of leaving a stale key or producing a duplicate `hooks` key (invalid TOML). - Respect a user-managed `hooks` value found outside headroom's marker block (e.g. `hooks = false`); only the deprecated alias is removed. - Make the insert/create paths match `_replace_marker_block`'s normalisation so re-running `init` is byte-idempotent. - Extract a `_codex_feature_block()` helper to remove the 4x duplicated marker block assembly. - Add regression tests for the previously-broken edge cases. ## Testing - [x] Unit tests pass (`pytest`) — affected module fully green (see output) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom/cli/init.py`) - [x] New tests added for new functionality - [x] Manual testing performed (reproduced each edge case against the patched function via `tomllib.loads`) ## Test Output ``` $ pytest -v tests/test_cli/test_init_cli.py -k "feature_flag or hooks_feature" tests/test_cli/test_init_cli.py::test_init_codex_merges_feature_flag_into_existing_table PASSED tests/test_cli/test_init_cli.py::test_init_codex_creates_hooks_feature_flag_on_first_init PASSED tests/test_cli/test_init_cli.py::test_ensure_codex_feature_flag_replaces_existing_marker PASSED tests/test_cli/test_init_cli.py::test_ensure_codex_feature_flag_migrates_legacy_codex_hooks_key PASSED tests/test_cli/test_init_cli.py::test_ensure_codex_feature_flag_migrates_when_both_keys_present PASSED tests/test_cli/test_init_cli.py::test_ensure_codex_feature_flag_migrates_when_keys_reversed PASSED tests/test_cli/test_init_cli.py::test_ensure_codex_feature_flag_drops_legacy_key_outside_marker PASSED tests/test_cli/test_init_cli.py::test_ensure_codex_feature_flag_is_idempotent PASSED tests/test_cli/test_init_cli.py::test_ensure_codex_feature_flag_creates_features_section_when_missing PASSED ======================= 9 passed, 45 deselected in 0.24s ======================= $ pytest -q tests/test_cli/test_init_cli.py 54 passed $ ruff check . All checks passed! $ mypy headroom/cli/init.py Success: no issues found in 1 source file ``` Note: the broader `tests/test_cli/` run has one unrelated failure (`test_wrap_copilot_auto_detects_running_proxy_backend`) caused by a real proxy already bound to port 8787 in the local environment — it fails identically on a clean checkout without this change. ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation (none required) - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [ ] I have updated the CHANGELOG.md if applicable (managed by release-please; generated from the conventional commit, not edited by hand) ## Screenshots (if applicable) N/A ## Additional Notes - **Why the duplicate-key path matters:** TOML forbids duplicate keys, so a `[features]` table containing both `codex_hooks` and `hooks` (which the old in-place migration could produce) makes Codex reject `config.toml` entirely. The new "strip then ensure" approach can never emit two `hooks` lines. - **Version provenance:** the `codex_hooks` -> `hooks` rename landed in openai/codex#20522, first shipped in Codex `rust-v0.129.0`. `codex_hooks` remains a working legacy alias, but current Codex builds can warn users to move to `[features].hooks`. - **Idempotency:** running `headroom init codex` repeatedly now produces a byte-stable `config.toml`, so there is no churn on re-init. --- e2e/init/run.py | 20 ++- headroom/cli/init.py | 201 +++++++++++++++++-------- tests/test_cli/test_init_cli.py | 250 ++++++++++++++++++++++++++++++-- 3 files changed, 400 insertions(+), 71 deletions(-) diff --git a/e2e/init/run.py b/e2e/init/run.py index cb6820d67..b28c8315b 100644 --- a/e2e/init/run.py +++ b/e2e/init/run.py @@ -25,6 +25,11 @@ import json import sys from pathlib import Path +try: + import tomllib +except ModuleNotFoundError: # Python < 3.11 + import tomli as tomllib # type: ignore[no-redef] + # Add repo root to sys.path so the harness import works whether the file is # invoked as ``python e2e/init/run.py`` or ``python -m e2e.init.run``. _REPO_ROOT = Path(__file__).resolve().parents[2] @@ -66,6 +71,15 @@ def _read_manifest(home: Path, profile: str) -> dict[str, object]: return json.loads(path.read_text(encoding="utf-8")) +def _expect_codex_hooks_feature(config: str) -> None: + parsed = tomllib.loads(config) + features = parsed.get("features") + if not isinstance(features, dict) or features.get("hooks") is not True: + raise AssertionError("Codex config should enable hooks") + if "codex_hooks" in features: + raise AssertionError("Codex config should not keep deprecated codex_hooks") + + # ----- existing-flow assertions (ported verbatim from the old run.py) --------- @@ -166,8 +180,7 @@ def _verify_codex_local(ctx: CaseContext) -> None: raise AssertionError("Codex local init missing 'supports_websockets = true'") if config.count("[features]") != 1: raise AssertionError("Codex config should keep a single [features] table") - if "codex_hooks = true" not in config: - raise AssertionError("Codex config should enable codex_hooks") + _expect_codex_hooks_feature(config) command = hooks["hooks"]["SessionStart"][0]["hooks"][0]["command"] _expect_hook_command(command, profile) @@ -206,8 +219,7 @@ def _verify_codex_global(ctx: CaseContext) -> None: ) if "supports_websockets = true" not in config: raise AssertionError("Codex global init missing 'supports_websockets = true'") - if "codex_hooks = true" not in config: - raise AssertionError("Codex user config should enable codex_hooks") + _expect_codex_hooks_feature(config) hooks = json.loads((ctx.home / ".codex" / "hooks.json").read_text(encoding="utf-8")) _expect_hook_command( hooks["hooks"]["SessionStart"][0]["hooks"][0]["command"], diff --git a/headroom/cli/init.py b/headroom/cli/init.py index b9d71d33e..267a5d40c 100644 --- a/headroom/cli/init.py +++ b/headroom/cli/init.py @@ -5,6 +5,7 @@ from __future__ import annotations import json import logging import os +import re import shlex import shutil import subprocess @@ -15,6 +16,11 @@ from hashlib import sha1 from pathlib import Path from typing import Any +try: + import tomllib +except ModuleNotFoundError: # Python < 3.11 + import tomli as tomllib # type: ignore[no-redef] + import click from headroom.install.models import ConfigScope, InstallPreset, RuntimeKind, SupervisorKind @@ -51,6 +57,16 @@ _SUPPORTED_TARGETS = ("claude", "copilot", "codex", "openclaw") _LOCAL_TARGETS = {"claude", "codex"} _GLOBAL_TARGETS = {"claude", "copilot", "codex", "openclaw"} _STARTUP_READY_TIMEOUT_SECONDS = 15 +_TOML_TABLE_HEADER_RE = re.compile(r"^[ \t]*(?:\[\[[^\]\r\n]+\]\]|\[[^\]\r\n]+\])[ \t]*(?:#.*)?$") +_TOML_FEATURES_NAME_RE = r"(?:features|\"features\"|'features')" +_TOML_CODEX_HOOKS_NAME_RE = r"(?:codex_hooks|\"codex_hooks\"|'codex_hooks')" +_CODEX_FEATURES_TABLE_RE = re.compile( + rf"^[ \t]*\[[ \t]*{_TOML_FEATURES_NAME_RE}[ \t]*\][ \t]*(?:#.*)?$" +) +_CODEX_FEATURES_DOTTED_LEGACY_RE = re.compile( + rf"^[ \t]*{_TOML_FEATURES_NAME_RE}[ \t]*\.[ \t]*{_TOML_CODEX_HOOKS_NAME_RE}[ \t]*=" +) +_CODEX_FEATURES_LEGACY_KEY_RE = re.compile(rf"^[ \t]*{_TOML_CODEX_HOOKS_NAME_RE}[ \t]*=") def _command_string(parts: list[str]) -> str: @@ -210,10 +226,7 @@ def _ensure_copilot_hooks(path: Path, profile: str) -> None: def _replace_marker_block( content: str, marker_start: str, marker_end: str, block: str, *, at_root: bool = False ) -> str: - if marker_start in content and marker_end in content: - start = content.index(marker_start) - end = content.index(marker_end) + len(marker_end) - content = content[:start].rstrip() + "\n\n" + content[end:].lstrip() + content = _remove_marker_block(content, marker_start, marker_end) block = block.strip() if at_root: # The block carries top-level keys, so it must sit above the first table @@ -221,8 +234,7 @@ def _replace_marker_block( # into that table and Codex rejects the config (#260). lines = content.splitlines() for index, line in enumerate(lines): - stripped = line.strip() - if stripped.startswith("[") and stripped.endswith("]"): + if _TOML_TABLE_HEADER_RE.search(line): head = "\n".join(lines[:index]).rstrip() tail = "\n".join(lines[index:]).lstrip("\n") prefix = f"{head}\n\n" if head else "" @@ -230,6 +242,14 @@ def _replace_marker_block( return (content.rstrip() + "\n\n" + block + "\n").lstrip() +def _remove_marker_block(content: str, marker_start: str, marker_end: str) -> str: + if marker_start not in content or marker_end not in content: + return content + start = content.index(marker_start) + end = content.index(marker_end) + len(marker_end) + return content[:start].rstrip() + "\n\n" + content[end:].lstrip() + + def _strip_codex_init_block(content: str) -> str: """Remove all Headroom init-managed blocks and orphan keys from a Codex config.toml string.""" import re @@ -296,59 +316,126 @@ def _ensure_codex_provider(path: Path, port: int) -> None: path.write_text(content, encoding="utf-8") -def _ensure_codex_feature_flag(path: Path) -> None: - content = path.read_text(encoding="utf-8") if path.exists() else "" - if _CODEX_FEATURE_MARKER_START in content and _CODEX_FEATURE_MARKER_END in content: - block = f"{_CODEX_FEATURE_MARKER_START}\ncodex_hooks = true\n{_CODEX_FEATURE_MARKER_END}" - content = _replace_marker_block( - content, - _CODEX_FEATURE_MARKER_START, - _CODEX_FEATURE_MARKER_END, - block, - ) - elif "[features]" in content: +def _codex_feature_block() -> str: + return f"{_CODEX_FEATURE_MARKER_START}\nhooks = true\n{_CODEX_FEATURE_MARKER_END}" + + +def _codex_dotted_feature_block() -> str: + return f"{_CODEX_FEATURE_MARKER_START}\nfeatures.hooks = true\n{_CODEX_FEATURE_MARKER_END}" + + +def _codex_features_table_index(lines: list[str]) -> int | None: + return next( + (index for index, line in enumerate(lines) if _CODEX_FEATURES_TABLE_RE.search(line)), + None, + ) + + +def _codex_features(content: str) -> dict[str, Any] | None: + if not content.strip(): + return None + try: + parsed = tomllib.loads(content) + except tomllib.TOMLDecodeError: + return None + features = parsed.get("features") + return features if isinstance(features, dict) else None + + +def _codex_features_has_hooks(content: str) -> bool: + features = _codex_features(content) + if features is None: + # Keep init resilient for already-invalid user configs; this fallback + # only needs to avoid adding a second obvious hooks line. lines = content.splitlines() - inserted = False - for index, line in enumerate(lines): - if line.strip() != "[features]": - continue - section_end = index + 1 - while section_end < len(lines) and not ( - lines[section_end].startswith("[") and lines[section_end].endswith("]") - ): - if "codex_hooks" in lines[section_end]: - inserted = True - break - section_end += 1 - if not inserted: - lines[index + 1 : index + 1] = [ - _CODEX_FEATURE_MARKER_START, - "codex_hooks = true", - _CODEX_FEATURE_MARKER_END, - ] - inserted = True - break - content = "\n".join(lines).rstrip() + "\n" - if not inserted: - content = ( - content.rstrip() - + "\n\n[features]\n" - + _CODEX_FEATURE_MARKER_START - + "\n" - + "codex_hooks = true\n" - + _CODEX_FEATURE_MARKER_END - + "\n" - ) + features_index = _codex_features_table_index(lines) + if features_index is None: + return False + for line in lines[features_index + 1 :]: + if _TOML_TABLE_HEADER_RE.search(line): + break + if re.search(r"^[ \t]*hooks[ \t]*=", line): + return True + return False + + return "hooks" in features + + +def _strip_codex_legacy_feature_flag(content: str) -> str: + lines = content.splitlines(keepends=True) + retained: list[str] = [] + in_features = False + in_root = True + + for line in lines: + if _TOML_TABLE_HEADER_RE.search(line): + in_root = False + in_features = bool(_CODEX_FEATURES_TABLE_RE.search(line)) + retained.append(line) + continue + if (in_root and _CODEX_FEATURES_DOTTED_LEGACY_RE.search(line)) or ( + in_features and _CODEX_FEATURES_LEGACY_KEY_RE.search(line) + ): + continue + retained.append(line) + + return "".join(retained) + + +def _ensure_codex_feature_flag(path: Path) -> None: + """Ensure Codex's ``[features].hooks`` flag is enabled in config.toml. + + ``hooks`` is the canonical key. ``codex_hooks`` was the original key name and + still resolves as a deprecated alias, but Codex >= 0.129 emits a deprecation + warning for it (renamed in openai/codex#20522). Any legacy + ``[features].codex_hooks`` line is removed, whether inside or outside our + marker block, so a migrated config drops the deprecated key and never + collides with a duplicate ``hooks`` key. A user-managed ``hooks`` value + outside our marker block is left untouched. + """ + content = path.read_text(encoding="utf-8") if path.exists() else "" + # Drop the deprecated alias key from [features]. Mirrors the top-level key + # cleanup in _ensure_codex_provider (#260) so re-running init migrates a + # legacy config rather than producing a duplicate `hooks` key, while leaving + # unrelated user tables untouched. + content = _strip_codex_legacy_feature_flag(content) + if _CODEX_FEATURE_MARKER_START in content and _CODEX_FEATURE_MARKER_END in content: + # init owns its marker block; remove it first, then reinsert under the + # correct TOML scope below. + content = _remove_marker_block( + content, _CODEX_FEATURE_MARKER_START, _CODEX_FEATURE_MARKER_END + ) + + if _codex_features_has_hooks(content): + # A user-managed `[features].hooks` key already exists outside our + # marker block; respect their value. Clearing the legacy key above was + # the only work. + pass else: - content = ( - content.rstrip() - + "\n\n[features]\n" - + _CODEX_FEATURE_MARKER_START - + "\n" - + "codex_hooks = true\n" - + _CODEX_FEATURE_MARKER_END - + "\n" - ).lstrip() + lines = content.splitlines() + features_index = _codex_features_table_index(lines) + if features_index is not None: + # Leading blank line matches the normalisation _replace_marker_block + # applies on later runs, so re-running init is byte-idempotent. + lines[features_index + 1 : features_index + 1] = [ + "", + *_codex_feature_block().splitlines(), + ] + content = "\n".join(lines).rstrip() + "\n" + elif _codex_features(content) is not None: + # The user expressed [features] via dotted keys, so adding a new + # table would duplicate it. Keep this key at the document root. + content = _replace_marker_block( + content, + _CODEX_FEATURE_MARKER_START, + _CODEX_FEATURE_MARKER_END, + _codex_dotted_feature_block(), + at_root=True, + ) + else: + content = ( + content.rstrip() + "\n\n[features]\n\n" + _codex_feature_block() + "\n" + ).lstrip() path.parent.mkdir(parents=True, exist_ok=True) path.write_text(content, encoding="utf-8") diff --git a/tests/test_cli/test_init_cli.py b/tests/test_cli/test_init_cli.py index 99a4ce5dc..3039e94eb 100644 --- a/tests/test_cli/test_init_cli.py +++ b/tests/test_cli/test_init_cli.py @@ -209,13 +209,28 @@ def test_init_codex_merges_feature_flag_into_existing_table(monkeypatch, tmp_pat content = config_path.read_text(encoding="utf-8") assert 'base_url = "http://127.0.0.1:9000/v1"' in content assert content.count("[features]") == 1 - assert "codex_hooks = true" in content + assert "hooks = true" in content assert 'env_key = "OPENAI_API_KEY"' not in content hooks = json.loads((tmp_path / ".codex" / "hooks.json").read_text(encoding="utf-8")) assert "--profile init-local-demo" in hooks["hooks"]["SessionStart"][0]["hooks"][0]["command"] assert "init hook ensure" in hooks["hooks"]["SessionStart"][0]["hooks"][0]["command"] +def test_init_codex_creates_hooks_feature_flag_on_first_init( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + monkeypatch.chdir(tmp_path) + + init_cli._init_codex(global_scope=False, profile="init-local-demo", port=9000) + + content = (tmp_path / ".codex" / "config.toml").read_text(encoding="utf-8") + parsed = tomllib.loads(content) + assert parsed["model_provider"] == "headroom" + assert parsed["features"]["hooks"] is True + assert "codex_hooks" not in content + + def test_init_claude_uses_custom_port(monkeypatch, tmp_path: Path) -> None: init_cli, _ = _load_init_module(monkeypatch) monkeypatch.chdir(tmp_path) @@ -482,7 +497,7 @@ def test_ensure_codex_provider_keeps_root_keys_above_existing_table( """ init_cli, _ = _load_init_module(monkeypatch) path = tmp_path / "config.toml" - path.write_text("[features]\ncodex_hooks = true\n", encoding="utf-8") + path.write_text("[features]\nhooks = true\n", encoding="utf-8") init_cli._ensure_codex_provider(path, 8787) @@ -492,7 +507,7 @@ def test_ensure_codex_provider_keeps_root_keys_above_existing_table( assert "model_provider" not in parsed["features"] assert "openai_base_url" not in parsed["features"] # The user's existing table is preserved. - assert parsed["features"]["codex_hooks"] is True + assert parsed["features"]["hooks"] is True assert parsed["model_providers"]["headroom"]["base_url"] == "http://127.0.0.1:8787/v1" @@ -505,13 +520,13 @@ def test_ensure_codex_provider_replaces_existing_model_provider( """ init_cli, _ = _load_init_module(monkeypatch) path = tmp_path / "config.toml" - path.write_text('model_provider = "openai"\n[features]\ncodex_hooks = true\n', encoding="utf-8") + path.write_text('model_provider = "openai"\n[features]\nhooks = true\n', encoding="utf-8") init_cli._ensure_codex_provider(path, 8787) parsed = tomllib.loads(path.read_text(encoding="utf-8")) # raises on a duplicate key assert parsed["model_provider"] == "headroom" - assert parsed["features"]["codex_hooks"] is True + assert parsed["features"]["hooks"] is True def test_ensure_codex_feature_flag_replaces_existing_marker(monkeypatch, tmp_path: Path) -> None: @@ -526,11 +541,35 @@ def test_ensure_codex_feature_flag_replaces_existing_marker(monkeypatch, tmp_pat content = path.read_text(encoding="utf-8") assert content.count(init_cli._CODEX_FEATURE_MARKER_START) == 1 - assert "codex_hooks = true" in content + assert "hooks = true" in content + assert "codex_hooks" not in content -def test_ensure_codex_feature_flag_skips_duplicate_existing_setting( - monkeypatch, tmp_path: Path +def test_ensure_codex_feature_flag_replaces_marker_inside_features_scope( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + path.write_text( + "[features]\n" + f"{init_cli._CODEX_FEATURE_MARKER_START}\n" + "hooks = true\n" + f"{init_cli._CODEX_FEATURE_MARKER_END}\n" + "\n[tools]\nhooks = false\n", + encoding="utf-8", + ) + + init_cli._ensure_codex_feature_flag(path) + + content = path.read_text(encoding="utf-8") + parsed = tomllib.loads(content) + assert parsed["features"]["hooks"] is True + assert parsed["tools"]["hooks"] is False + assert content.count(init_cli._CODEX_FEATURE_MARKER_START) == 1 + + +def test_ensure_codex_feature_flag_migrates_legacy_codex_hooks_key( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path ) -> None: init_cli, _ = _load_init_module(monkeypatch) path = tmp_path / "config.toml" @@ -539,10 +578,201 @@ def test_ensure_codex_feature_flag_skips_duplicate_existing_setting( init_cli._ensure_codex_feature_flag(path) content = path.read_text(encoding="utf-8") - assert content.count("codex_hooks = true") == 1 + parsed = tomllib.loads(content) + assert parsed["features"]["hooks"] is True + assert parsed["features"]["shell_tool"] is True + assert "codex_hooks" not in parsed["features"] + assert content.count("hooks = true") == 1 + assert content.count(init_cli._CODEX_FEATURE_MARKER_START) == 1 + + +def test_ensure_codex_feature_flag_migrates_dotted_legacy_codex_hooks_key( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + path.write_text("features.codex_hooks = true\nfeatures.shell_tool = true\n", encoding="utf-8") + + init_cli._ensure_codex_feature_flag(path) + + content = path.read_text(encoding="utf-8") + parsed = tomllib.loads(content) + assert parsed["features"]["hooks"] is True + assert parsed["features"]["shell_tool"] is True + assert "codex_hooks" not in parsed["features"] + assert "features.hooks = true" in content + + +def test_ensure_codex_feature_flag_migrates_when_both_keys_present( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + # A config that carried both the legacy and the correct key must not produce + # a duplicate `hooks` key (which Codex would reject as invalid TOML). + path.write_text("[features]\ncodex_hooks = true\nhooks = false\n", encoding="utf-8") + + init_cli._ensure_codex_feature_flag(path) + + content = path.read_text(encoding="utf-8") + parsed = tomllib.loads(content) + assert "codex_hooks" not in parsed["features"] + # The user's explicit `hooks` value is respected; only the legacy key is removed. + assert parsed["features"]["hooks"] is False + + +def test_ensure_codex_feature_flag_migrates_when_keys_reversed( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + path.write_text("[features]\nhooks = false\ncodex_hooks = true\n", encoding="utf-8") + + init_cli._ensure_codex_feature_flag(path) + + content = path.read_text(encoding="utf-8") + parsed = tomllib.loads(content) + assert "codex_hooks" not in parsed["features"] + assert parsed["features"]["hooks"] is False + + +def test_ensure_codex_feature_flag_ignores_hooks_outside_features( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + path.write_text( + "[features]\nshell_tool = true\n\n[some_other_table]\nhooks = true\n", + encoding="utf-8", + ) + + init_cli._ensure_codex_feature_flag(path) + + content = path.read_text(encoding="utf-8") + parsed = tomllib.loads(content) + assert parsed["features"]["hooks"] is True + assert parsed["features"]["shell_tool"] is True + assert parsed["some_other_table"]["hooks"] is True + assert content.count(init_cli._CODEX_FEATURE_MARKER_START) == 1 + + +def test_ensure_codex_feature_flag_ignores_hooks_after_commented_table_header( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + path.write_text( + "[features]\nshell_tool = true\n\n[some_other_table] # comment\nhooks = true\n", + encoding="utf-8", + ) + + init_cli._ensure_codex_feature_flag(path) + + parsed = tomllib.loads(path.read_text(encoding="utf-8")) + assert parsed["features"]["hooks"] is True + assert parsed["features"]["shell_tool"] is True + assert parsed["some_other_table"]["hooks"] is True + + +def test_ensure_codex_feature_flag_respects_commented_features_header_and_quoted_key( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + path.write_text('[features] # comment\n"hooks" = false\n', encoding="utf-8") + + init_cli._ensure_codex_feature_flag(path) + + content = path.read_text(encoding="utf-8") + parsed = tomllib.loads(content) + assert parsed["features"]["hooks"] is False assert init_cli._CODEX_FEATURE_MARKER_START not in content +def test_ensure_codex_feature_flag_migrates_quoted_legacy_codex_hooks_key( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + path.write_text('[features]\n"codex_hooks" = true\n', encoding="utf-8") + + init_cli._ensure_codex_feature_flag(path) + + parsed = tomllib.loads(path.read_text(encoding="utf-8")) + assert parsed["features"]["hooks"] is True + assert "codex_hooks" not in parsed["features"] + + +def test_ensure_codex_feature_flag_respects_root_dotted_feature_key( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + path.write_text("features.hooks = false\n", encoding="utf-8") + + init_cli._ensure_codex_feature_flag(path) + + content = path.read_text(encoding="utf-8") + parsed = tomllib.loads(content) + assert parsed["features"]["hooks"] is False + assert init_cli._CODEX_FEATURE_MARKER_START not in content + + +def test_ensure_codex_feature_flag_preserves_legacy_key_outside_features( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + path.write_text("[some_other_table]\ncodex_hooks = true\n", encoding="utf-8") + + init_cli._ensure_codex_feature_flag(path) + + parsed = tomllib.loads(path.read_text(encoding="utf-8")) + assert parsed["features"]["hooks"] is True + assert parsed["some_other_table"]["codex_hooks"] is True + + +def test_ensure_codex_feature_flag_drops_legacy_key_outside_marker( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + path.write_text( + "[features]\n" + "codex_hooks = true\n" + f"{init_cli._CODEX_FEATURE_MARKER_START}\n" + "hooks = true\n" + f"{init_cli._CODEX_FEATURE_MARKER_END}\n", + encoding="utf-8", + ) + + init_cli._ensure_codex_feature_flag(path) + + content = path.read_text(encoding="utf-8") + parsed = tomllib.loads(content) + assert "codex_hooks" not in parsed["features"] + assert parsed["features"]["hooks"] is True + assert content.count("hooks = true") == 1 + + +def test_ensure_codex_feature_flag_is_idempotent( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + init_cli, _ = _load_init_module(monkeypatch) + path = tmp_path / "config.toml" + path.write_text("[features]\ncodex_hooks = true\n", encoding="utf-8") + + init_cli._ensure_codex_feature_flag(path) + first = path.read_text(encoding="utf-8") + init_cli._ensure_codex_feature_flag(path) + second = path.read_text(encoding="utf-8") + + assert first == second + parsed = tomllib.loads(second) + assert parsed["features"]["hooks"] is True + assert second.count("hooks = true") == 1 + + def test_ensure_codex_feature_flag_creates_features_section_when_missing( monkeypatch, tmp_path: Path ) -> None: @@ -554,7 +784,7 @@ def test_ensure_codex_feature_flag_creates_features_section_when_missing( content = path.read_text(encoding="utf-8") assert "[features]" in content - assert "codex_hooks = true" in content + assert "hooks = true" in content def test_manifest_changed_detects_differences(monkeypatch) -> None: