fix(windows): fix Expand-Archive Windows Defender lock bug via native tar#131
fix(windows): fix Expand-Archive Windows Defender lock bug via native tar#131PunchingElm wants to merge 2 commits intosupermodeltools:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the npm postinstall flow on Windows to reduce install failures caused by antivirus file locks by preferring native tar extraction over PowerShell Expand-Archive.
Changes:
- Use Windows-native
tarto extract the downloaded.zipinto a temp directory. - Fall back to
Expand-Archivewith a retry loop iftarextraction fails.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| execSync(`tar -xf "${tmpArchive}" -C "${tmpDir}"`); | ||
| } catch { | ||
| const psCommand = `$RetryCount = 0; while ($RetryCount -lt 10) { try { Expand-Archive -Force -Path '${tmpArchive}' -DestinationPath '${tmpDir}'; break } catch { Start-Sleep -Seconds 1; $RetryCount++ } }`; | ||
| execSync( | ||
| `powershell -NoProfile -Command "${psCommand}"`, | ||
| ); | ||
| } | ||
| fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH); |
There was a problem hiding this comment.
The PowerShell retry loop swallows failures: if Expand-Archive keeps failing for all 10 retries, the script exits successfully and the next line throws a less-informative ENOENT on copyFileSync. Consider making the PowerShell command fail/throw after the final retry (non-zero exit code) and/or explicitly verify that the expected extracted file exists before proceeding so the user gets a clear installation error.
| try { | |
| execSync(`tar -xf "${tmpArchive}" -C "${tmpDir}"`); | |
| } catch { | |
| const psCommand = `$RetryCount = 0; while ($RetryCount -lt 10) { try { Expand-Archive -Force -Path '${tmpArchive}' -DestinationPath '${tmpDir}'; break } catch { Start-Sleep -Seconds 1; $RetryCount++ } }`; | |
| execSync( | |
| `powershell -NoProfile -Command "${psCommand}"`, | |
| ); | |
| } | |
| fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH); | |
| const extractedBinary = path.join(tmpDir, "supermodel.exe"); | |
| try { | |
| execSync(`tar -xf "${tmpArchive}" -C "${tmpDir}"`); | |
| } catch { | |
| const psCommand = `$RetryCount = 0; while ($RetryCount -lt 10) { try { Expand-Archive -Force -Path '${tmpArchive}' -DestinationPath '${tmpDir}'; return } catch { Start-Sleep -Seconds 1; $RetryCount++ } }; throw 'Failed to extract archive with Expand-Archive after 10 attempts.'`; | |
| execSync( | |
| `powershell -NoProfile -Command "${psCommand}"`, | |
| ); | |
| } | |
| if (!fs.existsSync(extractedBinary)) { | |
| fail(`Extraction failed: expected executable not found at ${extractedBinary}`); | |
| } | |
| fs.copyFileSync(extractedBinary, BIN_PATH); |
| execSync(`tar -xf "${tmpArchive}" -C "${tmpDir}"`); | ||
| } catch { | ||
| const psCommand = `$RetryCount = 0; while ($RetryCount -lt 10) { try { Expand-Archive -Force -Path '${tmpArchive}' -DestinationPath '${tmpDir}'; break } catch { Start-Sleep -Seconds 1; $RetryCount++ } }`; | ||
| execSync( | ||
| `powershell -NoProfile -Command "${psCommand}"`, | ||
| ); | ||
| } | ||
| fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH); | ||
| fs.rmSync(tmpDir, { recursive: true, force: true }); |
There was a problem hiding this comment.
If tar/Expand-Archive succeeds but copyFileSync throws (e.g., expected file not present), tmpDir won’t be removed because cleanup happens after the copy. Consider restructuring this block with try/finally so tmpDir is always removed, even when an error occurs.
| execSync(`tar -xf "${tmpArchive}" -C "${tmpDir}"`); | |
| } catch { | |
| const psCommand = `$RetryCount = 0; while ($RetryCount -lt 10) { try { Expand-Archive -Force -Path '${tmpArchive}' -DestinationPath '${tmpDir}'; break } catch { Start-Sleep -Seconds 1; $RetryCount++ } }`; | |
| execSync( | |
| `powershell -NoProfile -Command "${psCommand}"`, | |
| ); | |
| } | |
| fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH); | |
| fs.rmSync(tmpDir, { recursive: true, force: true }); | |
| try { | |
| execSync(`tar -xf "${tmpArchive}" -C "${tmpDir}"`); | |
| } catch { | |
| const psCommand = `$RetryCount = 0; while ($RetryCount -lt 10) { try { Expand-Archive -Force -Path '${tmpArchive}' -DestinationPath '${tmpDir}'; break } catch { Start-Sleep -Seconds 1; $RetryCount++ } }`; | |
| execSync( | |
| `powershell -NoProfile -Command "${psCommand}"`, | |
| ); | |
| } | |
| fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH); | |
| } finally { | |
| fs.rmSync(tmpDir, { recursive: true, force: true }); | |
| } |
…cleanup
- Extract extractZip() as a testable function with injectable execFn
- Move side-effectful module-level code inside require.main guard so
require('./install') no longer triggers a download
- Add try/finally to guarantee tmpDir cleanup even if copyFileSync throws
- Add 6 node:test tests covering tar path, PowerShell fallback, retry
loop structure, and path interpolation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WalkthroughThe installer script was refactored to export a testable Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (1)
npm/install.js (1)
79-93: Also cleanuptmpArchivein afinallyblock.If extraction/copy/chmod throws, the downloaded archive currently remains in temp storage.
Suggested cleanup flow
download(url, tmpArchive, () => { - if (ext === "tar.gz") { - execSync(`tar -xzf "${tmpArchive}" -C "${BIN_DIR}" supermodel`); - } else { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "supermodel-extract-")); - try { - extractZip(tmpArchive, tmpDir); - fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH); - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }); - } - } - if (process.platform !== "win32") fs.chmodSync(BIN_PATH, 0o755); - fs.unlinkSync(tmpArchive); - console.log(`[supermodel] Installed to ${BIN_PATH}`); + try { + if (ext === "tar.gz") { + execSync(`tar -xzf "${tmpArchive}" -C "${BIN_DIR}" supermodel`); + } else { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "supermodel-extract-")); + try { + extractZip(tmpArchive, tmpDir); + fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH); + } finally { + fs.rmSync(tmpDir, { recursive: true, force: true }); + } + } + if (process.platform !== "win32") fs.chmodSync(BIN_PATH, 0o755); + console.log(`[supermodel] Installed to ${BIN_PATH}`); + } finally { + fs.rmSync(tmpArchive, { force: true }); + } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@npm/install.js` around lines 79 - 93, Wrap the post-download cleanup and install steps for download(...) in a try/finally so the tmpArchive is always removed: move extraction (extractZip), copy (fs.copyFileSync), chmod (fs.chmodSync) and tmpDir cleanup (fs.rmSync) into a try where applicable and call fs.unlinkSync(tmpArchive) in the finally block; ensure the tar.gz branch still cleans up tmpArchive and that BIN_DIR/BIN_PATH handling and console.log(`[supermodel] Installed to ${BIN_PATH}`) remain after successful install.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@npm/install.js`:
- Around line 56-59: Escape single quotes in the archive and tmpDir values
before interpolating into the PowerShell command (replace ' with ''), and use
the -LiteralPath parameter instead of -Path in the Expand-Archive call; update
the construction of psCommand (the string passed to exec(`powershell -NoProfile
-Command "${psCommand}"`)) to interpolate the escaped archive and tmpDir and to
use -LiteralPath for both -DestinationPath/-LiteralPath as appropriate so paths
containing apostrophes (e.g., O'Brien) are handled safely.
---
Nitpick comments:
In `@npm/install.js`:
- Around line 79-93: Wrap the post-download cleanup and install steps for
download(...) in a try/finally so the tmpArchive is always removed: move
extraction (extractZip), copy (fs.copyFileSync), chmod (fs.chmodSync) and tmpDir
cleanup (fs.rmSync) into a try where applicable and call
fs.unlinkSync(tmpArchive) in the finally block; ensure the tar.gz branch still
cleans up tmpArchive and that BIN_DIR/BIN_PATH handling and
console.log(`[supermodel] Installed to ${BIN_PATH}`) remain after successful
install.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b1706888-9c6c-4060-a05c-e382fa2e70e1
📒 Files selected for processing (3)
npm/install.jsnpm/install.test.jsnpm/package.json
| `$RetryCount = 0; while ($RetryCount -lt 10) { try { Expand-Archive` + | ||
| ` -Force -Path '${archive}' -DestinationPath '${tmpDir}'; break }` + | ||
| ` catch { Start-Sleep -Seconds 1; $RetryCount++ } }`; | ||
| exec(`powershell -NoProfile -Command "${psCommand}"`); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In PowerShell, how should apostrophes be escaped inside single-quoted string literals, and is -LiteralPathpreferred for paths passed toExpand-Archive?
💡 Result:
In PowerShell, apostrophes (single quotes) inside single-quoted string literals are escaped by doubling them: use two consecutive single quotes (''') to represent a literal apostrophe. The backtick () escape character does not work in single-quoted strings, as they are treated literally with no substitutions or escapes except for the doubled single quote. For paths passed to Expand-Archive, -LiteralPath is preferred when the path contains special characters like [ or ] that could be interpreted as wildcards by the -Path parameter. -LiteralPath treats the path exactly as typed, without wildcard expansion. Official examples recommend -LiteralPath for such cases, e.g., Expand-Archive -LiteralPath 'C:\Archives\Draft[v1].zip' -DestinationPath C:\Reference.
Citations:
- 1: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_quoting_rules?view=powershell-5.1
- 2: https://stackoverflow.com/questions/22525381/escaping-single-quote-sign-in-powershell
- 3: https://stackoverflow.com/questions/11231410/can-i-use-a-single-quote-in-a-powershell-string
- 4: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.archive/expand-archive?view=powershell-7.4
- 5: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.archive/expand-archive?view=powershell-7.5
- 6: https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_quoting_rules?view=powershell-7.6
🏁 Script executed:
find . -name "install.js" -type f | head -20Repository: supermodeltools/cli
Length of output: 1852
Escape apostrophes in paths before passing to PowerShell.
Paths like C:\Users\O'Brien\Downloads\archive.zip would break the current code. When that path hits the single-quoted string (e.g., '-Path '${archive}'), the apostrophe closes the string early, and PowerShell fails to expand the archive.
The fix is straightforward: double any apostrophes before interpolating (PowerShell treats '' as a literal apostrophe in single-quoted strings). Also switch to -LiteralPath since it's designed exactly for this—it treats the path literally without trying to interpret special characters like brackets as wildcards.
Suggested fix
function extractZip(archive, tmpDir, execFn) {
const exec = execFn || execSync;
+ const psEscape = (value) => String(value).replace(/'/g, "''");
try {
exec(`tar -xf "${archive}" -C "${tmpDir}"`);
} catch {
+ const archivePs = psEscape(archive);
+ const tmpDirPs = psEscape(tmpDir);
const psCommand =
`$RetryCount = 0; while ($RetryCount -lt 10) { try { Expand-Archive` +
- ` -Force -Path '${archive}' -DestinationPath '${tmpDir}'; break }` +
+ ` -Force -LiteralPath '${archivePs}' -DestinationPath '${tmpDirPs}' -ErrorAction Stop; break }` +
` catch { Start-Sleep -Seconds 1; $RetryCount++ } }`;
exec(`powershell -NoProfile -Command "${psCommand}"`);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `$RetryCount = 0; while ($RetryCount -lt 10) { try { Expand-Archive` + | |
| ` -Force -Path '${archive}' -DestinationPath '${tmpDir}'; break }` + | |
| ` catch { Start-Sleep -Seconds 1; $RetryCount++ } }`; | |
| exec(`powershell -NoProfile -Command "${psCommand}"`); | |
| function extractZip(archive, tmpDir, execFn) { | |
| const exec = execFn || execSync; | |
| const psEscape = (value) => String(value).replace(/'/g, "''"); | |
| try { | |
| exec(`tar -xf "${archive}" -C "${tmpDir}"`); | |
| } catch { | |
| const archivePs = psEscape(archive); | |
| const tmpDirPs = psEscape(tmpDir); | |
| const psCommand = | |
| `$RetryCount = 0; while ($RetryCount -lt 10) { try { Expand-Archive` + | |
| ` -Force -LiteralPath '${archivePs}' -DestinationPath '${tmpDirPs}' -ErrorAction Stop; break }` + | |
| ` catch { Start-Sleep -Seconds 1; $RetryCount++ } }`; | |
| exec(`powershell -NoProfile -Command "${psCommand}"`); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@npm/install.js` around lines 56 - 59, Escape single quotes in the archive and
tmpDir values before interpolating into the PowerShell command (replace ' with
''), and use the -LiteralPath parameter instead of -Path in the Expand-Archive
call; update the construction of psCommand (the string passed to
exec(`powershell -NoProfile -Command "${psCommand}"`)) to interpolate the
escaped archive and tmpDir and to use -LiteralPath for both
-DestinationPath/-LiteralPath as appropriate so paths containing apostrophes
(e.g., O'Brien) are handled safely.
Replaced PowerShell Expand-Archive with native Windows tar to prevent ENOENT crashes caused by Windows Defender locking the downloaded archive during npm installation.
Summary by CodeRabbit
New Features
Bug Fixes
Tests