Repository navigation
Conversation
Codex edits through apply_patch rather than Edit or Write. Its hook payload
carries the whole patch as text and never sets tool_input.file_path, so
analyze.sh and format.sh fired on every edit and did nothing: the field they
read was empty, and they exited 0 having analyzed and formatted nothing.
Both now read their files through a shared payload_file_paths helper. It
takes tool_input.file_path when present, as Claude Code sends it, and
otherwise the "A path" and "M path" lines of an apply_patch tool_response,
which is what Codex reports after the tool has run. A deleted file ("D path")
is left out, a rename reports its new name as "M", and a relative path is
resolved against the payload's cwd, since Codex echoes the path exactly as
the model wrote it. Codex's tool_response is a string and Claude Code's is an
object, so the scan is guarded with `strings` and the Claude Code path is
unchanged.
A patch touching several files analyzes and formats each. analyze.sh keeps
going after a failure so the agent sees every issue at once, then exits 2.
Both scripts gain the tests they never had. CI has no Dart SDK, so the tests
stub `dart` on PATH and assert which files it was asked to handle, the way the
existing tests stub very_good. The Codex fixtures use the tool_response text
captured from codex-cli 0.153.4.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The "without jq" cases gave the hook a PATH of the dart stub plus /bin, counting on /bin having cat but not jq. That holds on macOS and fails on Ubuntu, where /bin is a symlink to /usr/bin and so still reaches jq; the hook found it, ran, and the assertions that it stood aside failed in CI. The PATH is now built from a directory holding only a symlink to cat, the one coreutil the hook uses before it checks for jq, so what is on it no longer depends on how the host lays out /bin. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`env -i PATH=... bash "$HOOK"` resolves bash with the PATH it was just given. That worked while /bin was on it and broke the moment the no-jq PATH was narrowed to a directory holding only cat: env could not find bash at all and exited 127, which the "exits 0 when jq is missing" assertions caught. bash is now captured once by absolute path and invoked that way, so the hook's PATH can be as narrow as a case needs without taking the interpreter with it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 of 6 tasks
Contributor
Author
|
Superseded by #175: the Codex payload parsing moves to vgv_ai_cli's translator, and the scripts take their files as arguments instead. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Codex edits through
apply_patch, notEdit/Write. Its payload carries the patch as text and never setstool_input.file_path, soanalyze.shandformat.shfired on every Codex edit and did nothing.Both now read their files through one shared
payload_file_pathshelper:tool_input.file_pathwhen present — Claude Code, unchangedA path/M pathlines of anapply_patchtool_response— CodexD pathis skipped, a rename reports its new name asMcwd, since Codex echoes it exactly as the model wrote ittool_responseis a string on Codex and an object on Claude; the scan is guarded withstringsA patch touching several files now analyzes and formats each.
analyze.shkeeps going after a failure so the agent sees every issue, then exits 2.Both scripts gain tests. CI has no Dart SDK, so they stub
dartand assert which files it was asked to handle, like the existing tests stubvery_good. Codex fixtures usetool_responsetext captured from codex-cli 0.153.4.Verified
dartagainst real captured Codex payloads: the relative-path Add reports an unused local and exits 2; the Update reformats the file.PostToolUse, which is where both hooks run.Context: VeryGoodOpenSource/vgv_ai_cli#51 translates the hooks into
~/.codex/hooks.json; with this change the scripts work there without a payload wrapper.Type of Change
feat)fix)refactor)docs)ci)chore)🤖 Generated with Claude Code