fix(embed): show actionable message when npx is missing on PATH#1688
Closed
benfrank241 wants to merge 1 commit into
Closed
fix(embed): show actionable message when npx is missing on PATH#1688benfrank241 wants to merge 1 commit into
benfrank241 wants to merge 1 commit into
Conversation
The UI launcher previously raised a generic "Command Not Found" panel when npx wasn't installed (common on Windows without Node.js). This obscured the fact that the daemon was healthy and the API was usable. - Pre-flight check with shutil.which before subprocess.Popen so the failure is caught cleanly instead of relying on FileNotFoundError plumbing. - Reframe the panel: the daemon is running and usable now; the UI is optional; include the nodejs.org install link and the follow-up `hindsight-embed ui start` command. Also pulls in the docs-skill regeneration from the pre-commit hook.
Contributor
Author
|
Closing in favor of #1682 — that PR fixes the actual root cause (Windows detached subprocess not inheriting PATH), which is what TunaCookie originally hit. My messaging improvements here aren't worth carrying separately once the bug itself is gone. |
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.
Summary
npx(common on Windows without Node.js),start_uino longer surfaces the generic red "Command Not Found" panel. Daemon stays healthy and usable; the panel now reframes the UI as optional with concrete next steps (Node.js download link +hindsight-embed ui startfollow-up).shutil.whichso we catch the missing binary up-front instead of relying onFileNotFoundErrorfromsubprocess.Popen— that path is brittle on Windows where PATH resolution can diverge between Python and the OS loader.Motivation
User report on the community channel: daemon started fine, then this hard-failure panel scared them off — they thought the whole setup was broken. It wasn't; only the UI was unavailable because Node.js wasn't installed.
Test plan
uv run pytest tests/test_embed_manager.py -v— 12/12 pass, including newtest_start_ui_skips_spawn_when_npx_missing./scripts/hooks/lint.shcleannpx: confirm the new panel renders and the daemon URL is shown