feat(install): check for node availability and back up settings.json#58
Closed
jccidc wants to merge 1 commit intoJuliusBrussee:mainfrom
Closed
feat(install): check for node availability and back up settings.json#58jccidc wants to merge 1 commit intoJuliusBrussee:mainfrom
jccidc wants to merge 1 commit intoJuliusBrussee:mainfrom
Conversation
Two small safety improvements to hooks/install.sh: 1. Fail fast if node is missing. The script uses 'node -e' to merge the hook config into ~/.claude/settings.json, but does not check that node is on PATH before running. If the user doesn't have node installed (Claude Code ships it, but users running install.sh via curl pipe on a fresh machine may not), the script currently fails with a cryptic error from inside the heredoc. Add a 'command -v node' check at the top with a helpful message pointing to nodejs.org. 2. Back up settings.json before the node merge. The script now copies $SETTINGS to $SETTINGS.bak before running the node rewrite. If the merge fails mid-write (disk full, interrupted script, node crash), the user's config can be recovered from the backup instead of being lost. This is especially important now that the script wires up two hook entries (SessionStart + UserPromptSubmit) — more state at risk in a single write. Both changes are purely additive — no behavior change on the happy path, only on failure modes. Verified syntax with 'bash -n'.
There was a problem hiding this comment.
Pull request overview
Adds safety checks to the hook installer script to improve failure-mode UX when installing Claude Code “caveman” hooks.
Changes:
- Fail fast with a clear message when
nodeis not available onPATH. - Back up
~/.claude/settings.jsontosettings.json.bakbefore performing the Node-based merge.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+9
to
+13
| if ! command -v node >/dev/null 2>&1; then | ||
| echo "ERROR: 'node' is required to install the caveman hooks (used to merge" | ||
| echo " the hook config into ~/.claude/settings.json safely)." | ||
| echo " Install Node.js from https://nodejs.org and re-run this script." | ||
| exit 1 |
There was a problem hiding this comment.
These error messages are printed to stdout; for CLI scripts it’s better to send errors to stderr (e.g., echo ... >&2) so callers can separate normal output from failures when piping/redirecting.
Owner
|
Implemented in latest release! Thanks for raising the issue. |
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.
Two small safety improvements to
hooks/install.sh. Both purely additive — no behavior change on the happy path, only on failure modes.1. Fail fast if node is missing
The script uses
node -eto merge the hook config into~/.claude/settings.json, but doesn't check that node is on PATH first. Claude Code itself ships node, but users runninginstall.shvia curl pipe on a fresh machine (before installing Claude Code) hit a cryptic error from inside the heredoc.Adds a
command -v nodecheck at the top with a helpful message:2. Back up
settings.jsonbefore the node mergeThe script now copies
$SETTINGSto$SETTINGS.bakbefore running the node rewrite. If the merge fails mid-write (disk full, interrupted script, node crash, power loss), the user can recover their config from the backup instead of losing it.This matters more now that the script wires up two hook entries (SessionStart + UserPromptSubmit) in a single
node -ewrite. More state at risk per transaction.Scope
hooks/install.shbash -nSecond PR in a small batch today. Independent of the others — can be merged in any order. Close if it doesn't fit your vision. 🪨