Skip to content

fix: document the Codex PreToolUse hook as an intentional no-op#2185

Closed
Souptik96 wants to merge 1 commit into
Graphify-Labs:v8from
Souptik96:fix/2165-codex-hook-check-docs
Closed

fix: document the Codex PreToolUse hook as an intentional no-op#2185
Souptik96 wants to merge 1 commit into
Graphify-Labs:v8from
Souptik96:fix/2165-codex-hook-check-docs

Conversation

@Souptik96

Copy link
Copy Markdown
Contributor

Fixes #2165

What I found (the report's diagnosis is not quite right)

hook-check is a real command, and its no-op behavior is deliberate — not a stale
subcommand falling through. Verified against v8 (0.9.26):

  • graphify/cli.py dispatches it explicitly, with the reason inline:
    "Codex Desktop rejects hookSpecificOutput.additionalContext on PreToolUse. Keep this as a
    cross-platform no-op so installed hooks never break Bash tool calls. Graph guidance reaches
    the agent via AGENTS.md / skill instead."
  • graphify/__main__.py lists hook-check in _silent_cmds ("runs on every editor tool use
    and must be silent"), and CHANGELOG has a past fix specifically for its silence.
  • __main__.py also states the split directly: "Claude Code accepts additionalContext on
    PreToolUse (Codex Desktop does not — that path stays a no-op via hook-check)."

Two of the report's other points also don't reproduce on 0.9.26:

$ graphify totally-bogus-subcommand
error: unknown command 'totally-bogus-subcommand'
Run 'graphify --help' for usage.
$ echo $?      # -> 1

So an unrecognized subcommand already exits non-zero; hook-check returning instantly
with exit 0 is it working as designed, not falling through. And because
hook-check never reads stdin, it can't hang the way hook-guard does — so the missing
timeout field isn't a hazard on this entry.

I therefore did not repoint the Codex installer at hook-guard. That's what the issue
suggests, but hook-guard emits additionalContext, which is exactly what Codex Desktop
rejects — it would reintroduce the #522-class breakage this no-op exists to avoid.

The real problem: the docs and the installer over-promise

What legitimately misleads users (and produced this report) is that both the README and the
installer's own output describe the Codex hook as if it enforced graph usage:

  • README claimed the Codex hook "fires before every Bash tool call, same always-on
    mechanism as Claude Code" — it is not the same mechanism; on Codex the hook is inert and
    AGENTS.md does the work. Now stated accurately, with the reason.
  • _install_codex_hook printed PreToolUse hook registered (... hook-check) with no hint
    the entry is intentionally inert. It now says so.

Plus the guard the issue implicitly asks for

New test test_codex_hook_command_is_a_real_cli_subcommand reads the command back out of the
generated .codex/hooks.json and asserts its subcommand is one the CLI actually dispatches.
If a hook command is ever renamed out from under an installer, the suite fails instead of
shipping a permanently dead hook.

Verified it actually catches that (temporarily pointed the installer at hook-check-BOGUS):

E   AssertionError: codex hook registers 'hook-check-BOGUS', which the CLI does not dispatch
    (#2165). Known commands: ['add', 'affected', ..., 'hook', 'hook-check', 'hook-guard', ...]

Testing

$ uv run pytest tests/test_install.py::test_codex_hook_command_is_a_real_cli_subcommand -q
1 passed, 1 warning in 0.46s

$ uv run pytest tests/test_install.py tests/test_install_references.py tests/test_install_roundtrip.py tests/test_hooks.py -q
12 failed, 207 passed, 1 skipped, 1 warning in 24.09s

$ uv run ruff check --config pyproject.toml graphify/install.py tests/test_install.py
All checks passed!

$ uv run python -m tools.skillgen --check
check OK: 134 artifact(s) match committed output and expected/.

Real installer output after the change:

  .codex/hooks.json  ->  PreToolUse hook registered (…/graphify.EXE hook-check - intentional
  no-op; Codex Desktop rejects additionalContext on PreToolUse, so graph guidance comes from
  AGENTS.md)

On those 12 failures — they are pre-existing on Windows, not caused by this PR. My
baseline uv run pytest tests/ -q on unmodified v8 (66d8110) gives 45 failed, 3578
passed, 15 skipped
, of which exactly 12 are in these four files (test_install.py 2,
test_install_references.py 1, test_install_roundtrip.py 1, test_hooks.py 8) — the same
12, mostly WinError 2/32 and POSIX path-separator assertions. I did not touch them, since
they're outside this issue's scope. I don't have a Linux/macOS box to confirm they're green
there, so please read that count in that light.

…hify-Labs#2165)

`graphify codex install` registers `graphify hook-check` in .codex/hooks.json,
and Graphify-Labs#2165 reported that as a stale/unrecognized subcommand producing a silent
no-op. `hook-check` is in fact a real, deliberate no-op command: Codex Desktop
rejects hookSpecificOutput.additionalContext on PreToolUse, so the Codex hook
intentionally does nothing and AGENTS.md carries the always-on guidance
(cli.py dispatches `hook-check`; __main__.py lists it in _silent_cmds).
Repointing the installer at `hook-guard` would reintroduce the Graphify-Labs#522-class
breakage on Codex Desktop, so the behavior is left as is.

What actually misled the report was the documentation and the installer's own
output, which both describe the Codex hook as if it enforced graph usage:

- README: the Codex row claimed a PreToolUse hook that "fires before every Bash
  tool call, same always-on mechanism as Claude Code". It now states that the
  hook is a deliberate no-op, why (Codex Desktop rejects additionalContext), and
  that AGENTS.md is the always-on mechanism on this platform.
- `_install_codex_hook` printed "PreToolUse hook registered (... hook-check)"
  with no hint that the entry is inert. It now says so inline.

Also adds the regression guard the issue implicitly asks for: a test that reads
the command out of the generated .codex/hooks.json and asserts its subcommand is
one the CLI actually dispatches. A genuinely renamed/stale hook command now
fails the suite instead of shipping a permanently dead hook.

Note: contrary to the report, an unrecognized subcommand already exits non-zero
(`graphify totally-bogus-subcommand` -> "error: unknown command", exit 1), so no
change was needed there.
@safishamsi

Copy link
Copy Markdown
Collaborator

Thanks @Souptik96. Shipped in v0.9.27 (cherry-picked to v8). Closed-unmerged here, but it's in the release: https://github.com/Graphify-Labs/graphify/releases/tag/v0.9.27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

codex install registers graphify hook-check, which is not a command in 0.9.25 (silent no-op)

2 participants