fix(codex): write hook config to the location codex expects#1729
fix(codex): write hook config to the location codex expects#1729entire[bot] wants to merge 3 commits into
Conversation
entire enable --agent codex wrote a project-local .codex/config.toml to flip on the hooks feature. When the repo lives inside <CODEX_HOME>/agents (the reserved tree Codex recursively scans for custom agent-role TOML files), that config.toml is picked up by Codex's role-file scanner and rejected at startup as a malformed agent role definition. Scope the feature flag into the global config.toml instead, under [projects."<repoRoot>".features], whenever repoRoot falls inside that reserved tree, so no TOML file is ever written there. hooks.json (JSON, not scanned) keeps installing at the normal project-local path. A stale project-local config.toml left behind by an older entire version is removed once it contains nothing but the feature-flag content this package manages. Fixes #842
There was a problem hiding this comment.
Pull request overview
This PR adjusts how the Codex agent hook feature flag is written so entire enable --agent codex doesn’t create a repo-local .codex/config.toml in the reserved <CODEX_HOME>/agents/... tree (which Codex scans for agent-role TOML files), avoiding Codex’s “malformed agent role definition” startup warning.
Changes:
- Route feature-flag writes to
<CODEX_HOME>/config.tomlunder a per-project[projects."<repoRoot>".features]table when the repo lives under<CODEX_HOME>/agents. - Keep installing
.codex/hooks.jsonproject-locally, and add a self-heal cleanup for stale.codex/config.tomlthat contains only Entire-managed flag content. - Add regression tests covering reserved-tree behavior, idempotency, preservation of unrelated config, and stale-file cleanup.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cmd/entire/cli/agent/codex/hooks.go | Adds reserved-tree detection and global scoped config write path; adds cleanup of stale local config; introduces small TOML/section-edit helpers. |
| cmd/entire/cli/agent/codex/hooks_test.go | Adds tests for the reserved agents tree scenario and cleanup/idempotency behaviors. |
Follow-up hardening on the #842 fix after audit. - Escape all TOML-significant characters (quote, backslash, newline, control chars) when embedding repoRoot into the scoped [projects."<repoRoot>".features] key. A path containing a quote or a newline could otherwise terminate the quoted key early and inject arbitrary lines into the shared global config.toml; a newline also split the header across physical lines and defeated the line-based idempotency check. - Serialize the global config.toml read-modify-write under a cross-process advisory lock and write it atomically. Two concurrent `entire enable` runs for different repos under CODEX_HOME/agents could otherwise lose each other's edits or expose a truncated file. - Detect the stale project-local config.toml with a line-anchored match instead of substring stripping, so self-heal never deletes a file carrying any content this package did not write. - Fix isUnderCodexAgentsDir so a directory literally named "..foo" under the agents tree is correctly detected as inside it. Verified end-to-end against codex-cli 0.139.0: a repo under CODEX_HOME/agents no longer triggers the "malformed agent role definition" startup warning, the scoped hooks feature is honored, and existing global keys are preserved.
|
Rethinking the approach here: per the current Codex docs (https://developers.openai.com/codex/hooks), hooks are now enabled by default — So what we really need is much simpler: stop creating What to keep from this PR: the stale-file self-heal ( Two things to verify before landing that simplification:
|
|
Closing as duplicate — superseded by #1760, which implements the simpler approach from the last review comment: hooks are enabled by default since codex-cli 0.124.0 (openai/codex#19012), so the flag write is dropped entirely instead of being scoped into the global config. The stale-file self-heal from this PR carries over (applied everywhere, line-anchored detection). Both open verification items were resolved there: the default flipped in rust-v0.124.0, and hooks were live-verified to fire on codex-cli 0.144.4 with only .codex/hooks.json and no feature flag anywhere. |
Trail: https://entire.io/gh/entireio/cli/trails/836
Summary
entire enable --agent codexwrites a project-local.codex/config.tomlto enable the hooks feature. When the repo lives inside<CODEX_HOME>/agents/...(the tree Codex recursively scans for custom agent-role TOML files), that config.toml gets picked up by Codex's role-file scanner and rejected at startup as a malformed agent role definition (missingdeveloper_instructions).config.tomlinstead, under[projects."<repoRoot>".features], wheneverrepoRootfalls inside that reserved tree, so no TOML file is ever written there.hooks.json(JSON, not scanned by Codex's role discovery) keeps installing at the normal project-local path..codex/config.tomlleft behind by an older entire version is removed once it contains nothing but the feature-flag content this package manages; files with unrelated content are left alone.Testing
Live-reproduced and verified against the real
codexbinary (codex-cli 0.139.0), using an isolated tempHOME/CODEX_HOMEfor every run — real~/.codexwas never touched (confirmed unchanged before/after).Before fix — repo at
~/.codex/agents/repos/project, runentire enable --agent codex, thencodex exec:After fix — same repo layout and same
codex execinvocation: no.codex/config.tomlis created (only.codex/hooks.json), the flag lands in<CODEX_HOME>/config.tomlunder[projects."...".features], and the warning is gone.Also verified:
config.tomlcarrying unrelated custom keys and another project's own scoped section — both preserved untouched.config.tomlleft by an older entire version (the exact bug artifact from the issue) is cleaned up on next enable; a localconfig.tomlcarrying unrelated user content is left alone.New regression tests cover all of the above in
cmd/entire/cli/agent/codex/hooks_test.go.gofmt -lcleangolangci-lint run(full repo) — 0 issuesgo test -race ./...— all packages passgo test ./cmd/entire/cli/agent/codex/...Audit hardening
A follow-up audit pass tightened three sharp edges, each with mutation-verified tests:
repoRootis a filesystem path embedded into[projects."<repoRoot>".features]. The key now escapes every TOML-significant character (quote, backslash, newline, other control chars). Previously a path containing a"or a newline could terminate the quoted key early and inject arbitrary lines into the shared globalconfig.toml; a newline also split the header across physical lines and broke idempotency. Reproduced live: a repo directory whose name contained a newline injected a rogue line into the global config before the fix, and no longer does.internal/flock) and swapped in atomically (jsonutil.WriteFileAtomic). Two concurrententire enableruns for different repos underCODEX_HOME/agentscould previously clobber each other's edits; a concurrent stress test reproduces the lost update without the lock and passes with it (green under-race).[features]header) instead of substring stripping, which could collapse unrelated content to whitespace and delete a file this package never wrote.isUnderCodexAgentsDirso a directory literally named..foounder the agents tree is correctly detected as inside it.Re-verified end-to-end against
codex-cli 0.139.0in an isolated tempHOME/CODEX_HOME: fresh install, existing-keys-preserved, idempotent re-enable, stale-heal, and unrelated-content-preserved all behave correctly, and the real~/.codexwas confirmed untouched (byte-identicalconfig.toml/agentstree before and after).Fixes #842