mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(codex): write canonical hooks feature flag and migrate deprecated codex_hooks (#743)
## 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.
This commit is contained in:
Logan Kang
GitHub
parent
b7350aa29c
commit
dff6a19946
3 changed files with 400 additions and 71 deletions
|
|
@ -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"],
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue