Fix npx-thunderid failing to start when install directory exists but pack contents are deleted#3075
Fix npx-thunderid failing to start when install directory exists but pack contents are deleted#3075rajithacharith wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR updates ChangesInstallation Detection Validation
🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
tools/npx-thunderid/src/index.tsESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/npx-thunderid/src/index.ts`:
- Line 70: The current alreadyInstalled check calls
findThunderRoot(versionState.installPath) which can throw ENOENT because
findThunderRoot -> findSetupScript calls fs.readdirSync on a missing path;
update either findSetupScript or findThunderRoot to guard filesystem access with
a try/catch or explicit fs.existsSync check before calling fs.readdirSync, and
return null on failure, or alternatively add an existence guard before invoking
findThunderRoot in the alreadyInstalled computation (referencing
alreadyInstalled, versionState.installPath, findThunderRoot, findSetupScript) so
a deleted install directory yields null instead of throwing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1063dc87-d45d-4b50-87b6-1a296e5469c6
📒 Files selected for processing (1)
tools/npx-thunderid/src/index.ts
| const state = readState(); | ||
| const versionState = state.installs[VERSION]; | ||
| const alreadyInstalled = Boolean(versionState?.installPath && fs.existsSync(versionState.installPath)); | ||
| const alreadyInstalled = Boolean(versionState?.installPath && findThunderRoot(versionState.installPath) !== null); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate setup.ts and inspect findThunderRoot: confirm it's exported and guards missing paths.
fd 'setup.ts' --type f
echo "=== findThunderRoot (function-declaration form) ==="
ast-grep --pattern 'export function findThunderRoot($$$) { $$$ }'
echo "=== findThunderRoot (arrow form) ==="
ast-grep --pattern 'export const findThunderRoot = ($$$) => { $$$ }'
echo "=== Context: implementation + fs calls used inside ==="
rg -nP --type=ts -C10 'function findThunderRoot|findThunderRoot\s*='Repository: thunder-id/thunderid
Length of output: 1891
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== tools/npx-thunderid/src/index.ts around line 70 ==="
nl -ba tools/npx-thunderid/src/index.ts | sed -n '50,100p'
echo "=== tools/npx-thunderid/src/index.ts around main() and its invocation ==="
nl -ba tools/npx-thunderid/src/index.ts | sed -n '140,210p'
echo "=== tools/npx-thunderid/src/setup.ts around findSetupScript + findThunderRoot ==="
nl -ba tools/npx-thunderid/src/setup.ts | sed -n '1,120p'
echo "=== All references to findThunderRoot in tools/npx-thunderid/src ==="
rg -n "findThunderRoot\(" tools/npx-thunderid/srcRepository: thunder-id/thunderid
Length of output: 181
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== tools/npx-thunderid/src/index.ts around line 70 ==="
cat -n tools/npx-thunderid/src/index.ts | sed -n '60,90p'
echo "=== tools/npx-thunderid/src/index.ts around main() and its invocation ==="
cat -n tools/npx-thunderid/src/index.ts | sed -n '150,220p'
echo "=== tools/npx-thunderid/src/setup.ts around findSetupScript + findThunderRoot ==="
cat -n tools/npx-thunderid/src/setup.ts | sed -n '1,140p'
echo "=== All references to findThunderRoot in tools/npx-thunderid/src ==="
rg -n "findThunderRoot\(" tools/npx-thunderid/srcRepository: thunder-id/thunderid
Length of output: 6545
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== tools/npx-thunderid/src/index.ts beginning (main / top-level error handling) ==="
cat -n tools/npx-thunderid/src/index.ts | sed -n '1,120p'
echo "=== tools/npx-thunderid/src/index.ts around alreadyInstalled usage ==="
cat -n tools/npx-thunderid/src/index.ts | sed -n '90,170p'Repository: thunder-id/thunderid
Length of output: 7993
Fix ENOENT risk in alreadyInstalled check (findThunderRoot can throw).
tools/npx-thunderid/src/index.ts computes alreadyInstalled at line 70 by calling findThunderRoot(versionState.installPath) outside any try/catch. But findThunderRoot → findSetupScript calls fs.readdirSync(installPath) without guarding that installPath exists, so deleting the whole install directory will throw (ENOENT) instead of returning null, preventing the intended graceful re-download. Add an existence guard/try-catch inside findSetupScript/findThunderRoot (or check fs.existsSync(versionState.installPath) before calling findThunderRoot).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/npx-thunderid/src/index.ts` at line 70, The current alreadyInstalled
check calls findThunderRoot(versionState.installPath) which can throw ENOENT
because findThunderRoot -> findSetupScript calls fs.readdirSync on a missing
path; update either findSetupScript or findThunderRoot to guard filesystem
access with a try/catch or explicit fs.existsSync check before calling
fs.readdirSync, and return null on failure, or alternatively add an existence
guard before invoking findThunderRoot in the alreadyInstalled computation
(referencing alreadyInstalled, versionState.installPath, findThunderRoot,
findSetupScript) so a deleted install directory yields null instead of throwing.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2 participants
Purpose
When a user manually deletes the downloaded Thunder pack contents but leaves the install directory on disk,
npx thunderidfails to start with:The
alreadyInstalledcheck usedfs.existsSyncon the install path, which only confirms the directory exists — not that the Thunder files inside are intact.Approach
Replace the shallow
fs.existsSync(versionState.installPath)check withfindThunderRoot(versionState.installPath) !== nullintools/npx-thunderid/src/index.ts.findThunderRootlooks forsetup.sh(orsetup.ps1on Windows) inside the directory, which is the true signal that the installation is usable. A missing setup script now correctly marks the install as absent and triggers a fresh download.Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
Bug Fixes
Chores