✨ pull analyzer binary from package definition when not found #699
Conversation
Signed-off-by: David Zager <[email protected]>
WalkthroughAdds konveyor.fallbackAssets to vscode/package.json (baseUrl + per-platform file names and sha256 checksums for kai-analyzer-rpc v0.8.0-alpha.4). Adds ensureKaiAnalyzerBinary(context, logger) in vscode/src/paths.ts and calls it from ensurePaths; the helper downloads the platform-specific kai-analyzer-rpc binary when missing, verifies SHA-256, writes it to kai/{platform}-{arch}/, and sets executable permissions on non-Windows. New imports support hashing, streaming, file I/O, and permission changes. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![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 (4)
vscode/package.json (1)
697-721: Solid fallback assets config; consider widening platform coverage and pinning version metadata.This block looks correct and matches the logic in paths.ts. Two suggestions:
- Add entries for additional plausible platforms (e.g., win32-arm64) or handle them explicitly in code to provide clearer error guidance.
- Consider embedding the analyzer version alongside baseUrl to make it explicit in logs/UI and future-proof updates (helps when rotating releases).
vscode/src/paths.ts (3)
1-1: Nit: Prefer a distinct alias for fs-extra to avoid confusion with node:fs imports.You import from both node:fs and fs-extra in the same file. Using
import * as fse from "fs-extra"helps readers distinguish calls and avoid mental context switches.-import * as fs from "fs-extra"; +import * as fse from "fs-extra";Then update usages accordingly:
- fs.existsSync -> fse.existsSync
- (and any other fs-extra usages)
Also applies to: 7-13
58-147: Consider writing downloaded binaries under globalStorage rather than the extension install dir.Writing into the extension’s installation directory can be brittle (read-only scenarios, remote environments, and extension updates). A safer pattern is to place runtime/downloaded artifacts under
context.globalStorageUriorcontext.storageUriand teach the rest of the code to resolve the binary from there first, falling back to the packaged location.If you want, I can propose a small helper that resolves “effective analyzer path” with this precedence:
- user override (konveyor.analyzerPath)
- globalStorage downloaded binary (verified)
- packaged asset
- last resort: download using fallback config
190-192: Guard activation against network failures; surface a helpful message and allow deferred retry.A thrown error here will fail activation. Consider catching errors from
ensureKaiAnalyzerBinaryand showing a user-friendly notification with a retry action or guidance (e.g., “Configure konveyor.analyzerPath manually” or “Check proxy settings”), while allowing the extension to activate in a degraded mode.- // Ensure kai-analyzer-rpc binary exists - await ensureKaiAnalyzerBinary(context, logger); + // Ensure kai-analyzer-rpc binary exists + try { + await ensureKaiAnalyzerBinary(context, logger); + } catch (err) { + const msg = `Failed to ensure kai-analyzer-rpc binary: ${String(err)}`; + logger.error(msg); + // Don't block activation; inform the user and allow them to configure manually or retry later + void vscode.window.showErrorMessage( + msg, + "Retry download", + "Open Settings (konveyor.analyzerPath)", + ).then(async (choice) => { + if (choice === "Retry download") { + try { + await ensureKaiAnalyzerBinary(context, logger); + } catch (e) { + logger.error(`Retry failed: ${String(e)}`); + } + } else if (choice === "Open Settings (konveyor.analyzerPath)") { + void vscode.commands.executeCommand("workbench.action.openSettings", "konveyor.analyzerPath"); + } + }); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
vscode/package.json(1 hunks)vscode/src/paths.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
vscode/src/paths.ts (1)
scripts/_download.js (1)
downloadUrl(46-118)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build (windows)
- GitHub Check: Build (linux)
- GitHub Check: Build (macos)
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
vscode/src/paths.ts (1)
80-82: Optional: verify an existing binary before early return (stale/corrupt file risk).Today, an existing file bypasses verification and updates. Consider verifying the checksum and re-downloading on mismatch. This aligns with the linked issue’s integrity intent. Prior feedback suggested this as well.
- if (existsSync(kaiAnalyzerPath)) { - return; // Binary already exists - } - - logger.info(`kai-analyzer-rpc not found at ${kaiAnalyzerPath}, downloading...`); - - const fallbackConfig = packageJson["konveyor.fallbackAssets"]; + const fallbackConfig = packageJson["konveyor.fallbackAssets"]; if (!fallbackConfig) { throw new Error("No fallback asset configuration found in package.json"); } - - const assetConfig = fallbackConfig.assets[platformKey]; - + const assetConfig = fallbackConfig.assets[platformKey]; + + if (existsSync(kaiAnalyzerPath)) { + try { + const hash = createHash("sha256"); + await pipeline(createReadStream(kaiAnalyzerPath), hash); + const actualSha256 = hash.digest("hex"); + if (actualSha256 === assetConfig.sha256) { + return; // Existing binary is valid + } + logger.warn( + `Existing kai-analyzer-rpc at ${kaiAnalyzerPath} has sha256 ${actualSha256} (expected ${assetConfig.sha256}); re-downloading.`, + ); + } catch (e) { + logger.warn(`Failed to verify existing kai-analyzer-rpc, re-downloading: ${String(e)}`); + } + await unlink(kaiAnalyzerPath).catch(() => {}); + } + + logger.info(`kai-analyzer-rpc not found or invalid at ${kaiAnalyzerPath}, downloading...`);Also applies to: 86-95
🧹 Nitpick comments (2)
vscode/src/paths.ts (2)
108-147: Atomic download: write to a temp file, verify, then rename; also handle web streams and timeouts.This prevents partially downloaded/corrupt binaries becoming visible, improves robustness across Node versions by converting Web streams, and avoids hanging fetches.
Apply this focused refactor:
// Create target directory await mkdir(dirname(kaiAnalyzerPath), { recursive: true }); - // Download file - const response = await fetch(downloadUrl); + // Download to a temp path + const tmpPath = `${kaiAnalyzerPath}.tmp`; + try { + await unlink(tmpPath); + } catch {} + + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), 60_000); // 60s timeout + const response = await fetch(downloadUrl, { signal: controller.signal }); + clearTimeout(timeout); if (!response.ok) { throw new Error(`HTTP ${response.status}: ${response.statusText}`); } if (!response.body) { throw new Error("Response body is null"); } - const fileStream = createWriteStream(kaiAnalyzerPath); - await pipeline(response.body as any, fileStream); + // Stream download -> temp file + const out = createWriteStream(tmpPath); + await pipeline(Readable.fromWeb(response.body as any), out); progress.report({ message: "Verifying..." }); // Verify SHA256 const hash = createHash("sha256"); - const verifyStream = createReadStream(kaiAnalyzerPath); + const verifyStream = createReadStream(tmpPath); await pipeline(verifyStream, hash); const actualSha256 = hash.digest("hex"); if (actualSha256 !== assetConfig.sha256) { - try { - await unlink(kaiAnalyzerPath); - } catch (err) { - logger.warn(`Error deleting file: ${kaiAnalyzerPath}`, err); - } + await unlink(tmpPath).catch(() => {}); throw new Error( `SHA256 mismatch. Expected: ${assetConfig.sha256}, Actual: ${actualSha256}`, ); } - // Make executable on Unix systems - if (platform !== "win32") { - await chmod(kaiAnalyzerPath, 0o755); - } + // Atomically move into place, then set permissions + await rename(tmpPath, kaiAnalyzerPath); + if (platform !== "win32") { + await chmod(kaiAnalyzerPath, 0o755); + }Additional imports needed (outside this range):
+ import { rename, unlink } from "node:fs/promises"; + import { Readable } from "node:stream";Notes:
- If you adopt this refactor, it also resolves the deletion path robustness noted earlier.
97-99: Construct the download URL robustly (handles trailing/leading slashes).Small hardening to avoid accidental double/missing slashes.
- const downloadUrl = `${fallbackConfig.baseUrl}${assetConfig.file}`; + const downloadUrl = new URL(assetConfig.file, fallbackConfig.baseUrl).toString();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
vscode/src/paths.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build (macos)
- GitHub Check: Build (linux)
- GitHub Check: Build (windows)
🔇 Additional comments (1)
vscode/src/paths.ts (1)
195-197: Activation wiring LGTM.Calling ensureKaiAnalyzerBinary from ensurePaths keeps binary provisioning centralized and happens once per activation.
Signed-off-by: David Zager <[email protected]>
Signed-off-by: David Zager <[email protected]>
Fixes #695
Uploading Screen Recording 2025-08-20 at 13.57.34.mov…
Summary by CodeRabbit
New Features
Chores