Repository navigation
Conversation
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
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.
Fixes #170
The plugin's hooks added the cargo bin directory to the end of PATH, so any other
troddenalready on the agent's PATH won. When Claude Code runs with a PATH that lacks~/.cargo/bin(started from a GUI, an IDE or a shell that never sourced~/.cargo/env) and an oldertroddensits in, say,/usr/local/bin, every hook ran the old binary. Aftercargo install, the user's terminal ran the new one while the plugin kept running the old one, and its errors only showed up in the hook log.Moving the cargo bin directory to the front every time would just flip the problem: someone who installed a prebuilt binary or used Homebrew, and still has an old
cargo installlying around, would then get the stale cargo copy even though their PATH says otherwise. So the hooks now do what rustup's~/.cargo/envdoes. If${CARGO_HOME:-$HOME/.cargo}/binis already on PATH, PATH is left alone and its order decides, the same as in the user's shell. If it is missing, it is added at the front, which is where the user's shell setup would have put it. Atroddenthat is only found on PATH, whatever installed it, still runs, and with notroddenanywhere the hooks still exit quietly.What this does not cover on purpose: if the user's own PATH lists an older
troddenahead of the cargo bin directory, that one runs, because that is also whattroddenruns in their terminal.trodden doctoralready prints whichtroddenis on PATH.To reproduce, I put a fresh build in a scratch
CARGO_HOME/bin, a fake oldtroddenthat fails with "unrecognized subcommand 'hook'" in another directory, and ran each plugin hook command withsh -cand a UserPromptSubmit payload, with only the fake's directory on PATH. Before, every hook ran the fake and exited 2. After, every hook runs the cargo build and exits 0. With both directories on PATH and the fake first, the fake still runs, as intended.The new test in
crates/trodden/tests/claude_code_plugin.rsreads the plugin's hooks.json and runs every hook command against fake binaries. It checks that the cargo install wins when PATH lacks the cargo bin directory (this one fails on main), that a PATH listing the cargo bin directory keeps its order, that atroddenfound only on PATH runs, and that the hooks stay quiet when there is notrodden.