Disable hooks for nested Codex helpers#194
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c15eb64d35
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [[ "$(cat "$_CODEX_FEATURE_CACHE")" == "yes" ]] && CODEX_DISABLE_HOOKS_ARGS=(--disable hooks --disable plugin_hooks --disable codex_hooks) | ||
| elif codex --help 2>&1 | grep -q -- '--disable'; then | ||
| CODEX_DISABLE_HOOKS_ARGS=(--disable codex_hooks) | ||
| CODEX_DISABLE_HOOKS_ARGS=(--disable hooks --disable plugin_hooks --disable codex_hooks) |
There was a problem hiding this comment.
Probe feature names before disabling them
When the installed Codex is an older supported build that has --disable but only knows the codex_hooks feature, this unconditional list makes every nested codex exec/codex review fail before it can do any work. Codex v0.114 already exposes --disable and validates each feature name, but its registry key is codex_hooks and does not include hooks or plugin_hooks, so the new probe passes and the subsequent invocation errors with an unknown feature. Please probe codex features list/feature support or fall back per feature instead of treating --disable support as support for all names; the same copied pattern in ask-codex.sh and bitlesson-select.sh needs the same guard.
Useful? React with 👍 / 👎.
|
can you
|
Summary
codex exec/codex review,/humanize:ask-codex, and BitLesson selectiondevbranch compatibility fixes while preserving hook recursion protectiondevand keep plugin version metadata at1.17.0Root Cause
Humanize already tried to prevent Stop-hook recursion by launching nested Codex with
--disable codex_hooks. That covered older Codex builds where the native hook feature was namedcodex_hooks.Recent Codex builds expose hook features under stable names such as
hooksandplugin_hooks. In those builds, disabling onlycodex_hooksmay not disable the active native/plugin hook paths. When Humanize's Stop hook launches a nestedcodex execorcodex review, that nested Codex process can load the same Stop hook again, which can re-enter the RLCR loop during review.At the same time, some older supported Codex builds expose
--disablebut reject unknown feature names. Passing every known feature unconditionally can make nestedcodex exec/codex reviewfail before doing useful work.Fix
When the installed Codex CLI supports
--disable, Humanize now probes each known hook feature name before using it:Only supported feature names are passed as repeated
--disable <feature>arguments. This keeps compatibility with oldercodex_hooksbuilds while also disabling the currently active hook feature names in newer Codex builds.The same protection is applied to
ask-codex.shandbitlesson-select.sh, since both are nested helper-style Codex invocations and can otherwise inherit the caller's hook environment.Validation
bash -n hooks/loop-codex-stop-hook.sh scripts/bitlesson-select.sh scripts/ask-codex.sh tests/test-disable-nested-codex-hooks.sh tests/test-bitlesson-select-routing.sh tests/test-ask-codex.shbash tests/test-disable-nested-codex-hooks.shbash tests/test-bitlesson-select-routing.shbash tests/test-ask-codex.shgit diff --check