fix(cli): warn when postinstall auto-update sync fails - #73
Open
ZayanKhan-12 wants to merge 1 commit into
Open
Conversation
The postinstall auto-update discarded the sync delegate's exit code with a ternary that returned 0 on both branches, so a failed sync was silently ignored. Thrown errors in the same path already emit a warning; non-zero exit codes now do too. Postinstall still exits 0 so a failed auto-update never breaks npm install. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Description
In
postinstall()inbin/metamask-skills.mjs, the exit code from the auto-update sync delegate was discarded by a ternary that returns0on both branches:So when the delegated
syncscript exits non-zero (e.g. the bash delegate fails, orspawnSynccan't start it anddelegatereturns1), the failure is silently ignored — while thrown errors in the same path do emit a warning via thecatchblock.This change keeps the intentional never-fail contract of postinstall (a failed auto-update should not break
npm install, so it still returns0), but surfaces the failure the same way thecatchpath does:Testing
node --test test/*.test.mjs— 36/36 passnode bin/metamask-skills.mjs --helpsmoke check passes🤖 Generated with Claude Code