Handle Windows installs#129
Conversation
JustSuperHuman
requested review from
greynewell and
jonathanpopham
as code owners
WalkthroughThis PR updates the CLI installation documentation to clarify platform-specific install options (Linux/Mac via curl, npm cross-platform) and modifies the Windows installer to use PowerShell's Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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: 2
🧹 Nitpick comments (1)
npm/install.js (1)
68-69: Please avoid pejorative wording in source comments.Line 68’s comment text can age poorly in shared codebases; neutral phrasing keeps docs professional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@npm/install.js` around lines 68 - 69, The inline comment that currently reads "Windows (peasants): Expand-Archive extracts everything, so extract to a temp dir and copy only the binary." uses pejorative language; replace it with a neutral, professional comment that explains the Windows behavior and the workaround (for example: "Windows: Expand-Archive extracts all files, so extract to a temporary directory and copy only the binary."). Update the comment near the Windows extraction handling in install.js (the comment starting "Windows (peasants):") to the neutral wording.
🤖 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 70-75: The code currently uses a fixed temp directory (tmpDir =
path.join(os.tmpdir(), "supermodel-extract")) which can collide across parallel
installs; change the creation of tmpDir in npm/install.js to a unique per-run
temp directory (e.g., use fs.mkdtempSync or fs.promises.mkdtemp with a prefix
like path.join(os.tmpdir(), "supermodel-extract-")) and then use that tmpDir for
the Expand-Archive command, the fs.copyFileSync(path.join(tmpDir,
"supermodel.exe"), BIN_PATH) call, and the final fs.rmSync cleanup so each
process has its own distinct extraction folder and is still properly removed
afterwards.
In `@README.md`:
- Around line 7-13: The README currently jumps from H1 to H3 (lines showing "###
Linux / Mac" and "### npm (cross-platform)"); update those section headings to
H2 by changing "### Linux / Mac" and "### npm (cross-platform)" to "## Linux /
Mac" and "## npm (cross-platform)" so the heading hierarchy is consistent and
satisfies markdownlint MD001.
---
Nitpick comments:
In `@npm/install.js`:
- Around line 68-69: The inline comment that currently reads "Windows
(peasants): Expand-Archive extracts everything, so extract to a temp dir and
copy only the binary." uses pejorative language; replace it with a neutral,
professional comment that explains the Windows behavior and the workaround (for
example: "Windows: Expand-Archive extracts all files, so extract to a temporary
directory and copy only the binary."). Update the comment near the Windows
extraction handling in install.js (the comment starting "Windows (peasants):")
to the neutral wording.
🪄 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: ca93748b-cf15-437f-8ed0-7f2b15b269d8
📒 Files selected for processing (2)
README.mdnpm/install.js
| const tmpDir = path.join(os.tmpdir(), "supermodel-extract"); | ||
| execSync( | ||
| `powershell -NoProfile -Command "Expand-Archive -Force -Path '${tmpArchive}' -DestinationPath '${tmpDir}'"`, | ||
| ); | ||
| fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH); | ||
| fs.rmSync(tmpDir, { recursive: true, force: true }); |
There was a problem hiding this comment.
Use a unique temp extraction directory to avoid install collisions.
Lines 70-75 use a fixed temp folder name. Parallel installs can race on the same directory and fail intermittently. Use a unique temp dir per run.
Proposed fix
- const tmpDir = path.join(os.tmpdir(), "supermodel-extract");
- execSync(
- `powershell -NoProfile -Command "Expand-Archive -Force -Path '${tmpArchive}' -DestinationPath '${tmpDir}'"`,
- );
- fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH);
- fs.rmSync(tmpDir, { recursive: true, force: true });
+ const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "supermodel-extract-"));
+ try {
+ execSync(
+ `powershell -NoProfile -Command "Expand-Archive -Force -Path '${tmpArchive}' -DestinationPath '${tmpDir}'"`,
+ );
+ fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH);
+ } finally {
+ fs.rmSync(tmpDir, { recursive: true, force: true });
+ }📝 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.
| const tmpDir = path.join(os.tmpdir(), "supermodel-extract"); | |
| execSync( | |
| `powershell -NoProfile -Command "Expand-Archive -Force -Path '${tmpArchive}' -DestinationPath '${tmpDir}'"`, | |
| ); | |
| fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH); | |
| fs.rmSync(tmpDir, { recursive: true, force: true }); | |
| const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "supermodel-extract-")); | |
| try { | |
| execSync( | |
| `powershell -NoProfile -Command "Expand-Archive -Force -Path '${tmpArchive}' -DestinationPath '${tmpDir}'"`, | |
| ); | |
| fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH); | |
| } finally { | |
| fs.rmSync(tmpDir, { recursive: true, 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 70 - 75, The code currently uses a fixed temp
directory (tmpDir = path.join(os.tmpdir(), "supermodel-extract")) which can
collide across parallel installs; change the creation of tmpDir in
npm/install.js to a unique per-run temp directory (e.g., use fs.mkdtempSync or
fs.promises.mkdtemp with a prefix like path.join(os.tmpdir(),
"supermodel-extract-")) and then use that tmpDir for the Expand-Archive command,
the fs.copyFileSync(path.join(tmpDir, "supermodel.exe"), BIN_PATH) call, and the
final fs.rmSync cleanup so each process has its own distinct extraction folder
and is still properly removed afterwards.
| ### Linux / Mac | ||
|
|
||
| ```bash | ||
| curl -fsSL https://supermodeltools.com/install.sh | sh | ||
| ``` | ||
|
|
||
| ### npm (cross-platform) |
There was a problem hiding this comment.
Fix heading hierarchy to satisfy markdownlint (MD001).
Line 7 jumps from H1 to H3. Promote these section headers to H2 to keep levels consistent.
Proposed fix
-### Linux / Mac
+## Linux / Mac
@@
-### npm (cross-platform)
+## npm (cross-platform)📝 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.
| ### Linux / Mac | |
| ```bash | |
| curl -fsSL https://supermodeltools.com/install.sh | sh | |
| ``` | |
| ### npm (cross-platform) | |
| ## Linux / Mac | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 7-7: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 7 - 13, The README currently jumps from H1 to H3
(lines showing "### Linux / Mac" and "### npm (cross-platform)"); update those
section headings to H2 by changing "### Linux / Mac" and "### npm
(cross-platform)" to "## Linux / Mac" and "## npm (cross-platform)" so the
heading hierarchy is consistent and satisfies markdownlint MD001.
1 participant
Summary by CodeRabbit
Documentation
Bug Fixes