feat(plugin): auto-download binary + userConfig onboarding#149
Merged
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
- ensure-binary.sh: SessionStart hook downloads correct platform binary from GitHub Releases to CLAUDE_PLUGIN_DATA/bin/, version-pinned to plugin.json version. Atomic replace, cached until version changes. - userConfig: prompts for server_url and api_token on plugin install (no manual env var setup needed for MCP server) - .mcp.json: uses CLAUDE_PLUGIN_DATA binary path + userConfig env vars - Plugin version 4.0.1
thebtf
force-pushed
the
feat/plugin-auto-binary
branch
from
502eca8 to
2dc7834
Compare
ОбзорПлагин refactored для динамической загрузки и управления бинарным файлом engram через Node.js-скрипты. Версия обновлена до 4.0.1, добавлена конфигурация пользователя для URL сервера и токена API, интегрирована автоматическая проверка и установка бинарного файла при запуске сессии. Изменения
Диаграммы последовательностиsequenceDiagram
participant Claude as Claude Code Plugin
participant EnsureBin as ensure-binary.js
participant FS as Файловая система
participant GitHub as GitHub Releases
Claude->>EnsureBin: Запуск при SessionStart (тайм-аут 60s)
EnsureBin->>EnsureBin: Прочитать версию из plugin.json
EnsureBin->>FS: Проверить наличие бинарного файла и .version
alt Бинарный файл не существует или устарел
EnsureBin->>EnsureBin: Определить платформу/архитектуру
EnsureBin->>GitHub: Загрузить нужный релиз (HTTP/HTTPS с редиректами)
GitHub-->>EnsureBin: Бинарный файл
EnsureBin->>FS: Записать временный файл (.tmp)
EnsureBin->>FS: Атомарно заменить существующий бинарный файл
EnsureBin->>FS: Записать .version файл
EnsureBin->>FS: Установить права на выполнение (non-Windows)
else Бинарный файл актуален
EnsureBin->>EnsureBin: Пропустить загрузку
end
EnsureBin-->>Claude: Завершить (ошибки обработаны как non-fatal)
sequenceDiagram
participant MCP as MCP Configuration
participant RunEngram as run-engram.js
participant FS as Файловая система
participant Engram as engram binary
participant User as Пользовательский процесс
MCP->>RunEngram: Запуск через node с переменными окружения
RunEngram->>RunEngram: Прочитать CLAUDE_PLUGIN_DATA
RunEngram->>FS: Определить путь к бинарному файлу (с расширением .exe на Windows)
RunEngram->>FS: Проверить существование файла
alt Бинарный файл найден
RunEngram->>Engram: Запустить процесс с CLI аргументами
Engram->>User: Выполнить операцию (унаследованные stdin/stdout)
User-->>Engram: Результат выполнения
Engram-->>RunEngram: Код выхода
RunEngram-->>MCP: Передать код выхода
else Бинарный файл не найден
RunEngram->>RunEngram: Вывести ошибку с инструкцией
RunEngram-->>MCP: Выход с кодом 1
end
Оценка сложности кода🎯 4 (Сложный) | ⏱️ ~45 минут Стихотворение
🚥 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 docstrings
🧪 Generate unit tests (beta)
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: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugin/engram/.mcp.json`:
- Around line 7-10: Remove the hardcoded insecure flag and make ENGRAM_INSECURE
opt-in: stop unconditionally setting "ENGRAM_INSECURE": "1" in .mcp.json;
instead populate ENGRAM_INSECURE only when the user explicitly enables an
insecure option (e.g., user_config.insecure === true) or when validating that
ENGRAM_URL is a non-HTTPS scheme (starts with "http://") and the user has
consented; reference the existing keys ENGRAM_URL and ENGRAM_API_TOKEN and
implement the change where env is built so the token is not sent alongside an
implicitly enabled insecure flag.
In `@plugin/engram/scripts/ensure-binary.js`:
- Around line 81-85: The downloaded executable (tmpPath) must be
integrity-verified before being used: fetch the expected checksum or signature
for the release (e.g., a .sha256 asset or detached signature from the same
GitHub release) and verify the downloaded file after download(url, tmpPath) by
computing its SHA-256 (using crypto) and comparing to the expected value, or by
verifying the signature with the release public key; if verification fails,
remove tmpPath and throw an error so the binary is never moved/used. Locate the
download call and variables url, tmpPath, desiredVersion, suffix, and binaryPath
to implement the verification step and proper cleanup on failure.
- Around line 46-55: The platform/arch mapping currently falls back to amd64 for
any arch that's not arm64, which will download incompatible binaries for
ia32/arm/Windows ARM; update the logic in ensure-binary.js to only map known
supported triples (e.g., for darwin: arch === "arm64" -> "darwin-arm64", arch
=== "x64" -> "darwin-amd64"; for linux: arch === "arm64" -> "linux-arm64", arch
=== "x64" -> "linux-amd64"; for win32: allow only arch === "x64" ->
"windows-amd64.exe" and arch === "arm64" -> "windows-arm64.exe" if you ship it)
and otherwise write an explicit unsupported platform/arch message using
process.stderr.write and return; change the variables referenced (platform,
arch, suffix, binaryName) and the conditional blocks that currently set
suffix/binaryName to implement these strict checks.
- Around line 94-100: The current replace in ensure-binary.js deletes binaryPath
before ensuring tmpPath is moved, risking loss if fs.renameSync(tmpPath,
binaryPath) fails; change to an atomic swap with rollback: first rename
binaryPath to a backup name (e.g., backupPath = binaryPath + '.bak') only if it
exists, then rename tmpPath to binaryPath; if the second rename fails, restore
the backup by renaming backupPath back to binaryPath and surface the original
error; on success remove the backup. Use the existing variables binaryPath and
tmpPath and perform all operations inside the try/catch so failures trigger the
rollback.
In `@plugin/engram/scripts/run-engram.js`:
- Around line 26-32: The spawnSync call may return a result with result.error
instead of throwing; after calling spawnSync (in run-engram.js where spawnSync
and binaryPath are used) check if result.error is present, log the error (e.g.,
console.error with a clear message and the error.stack or message) and exit with
a non-zero code (process.exit(1)); otherwise continue to use
process.exit(result.status ?? 0) for normal termination. Ensure the check runs
before calling process.exit so failed process spawn is diagnosed.
🪄 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: 23f3defb-557b-45c2-bc1e-5169539dd642
📒 Files selected for processing (5)
plugin/engram/.claude-plugin/plugin.jsonplugin/engram/.mcp.jsonplugin/engram/hooks/hooks.jsonplugin/engram/scripts/ensure-binary.jsplugin/engram/scripts/run-engram.js
Comment on lines
+7
to
+10
| "env": { | ||
| "ENGRAM_URL": "${user_config.server_url}", | ||
| "ENGRAM_API_TOKEN": "${user_config.api_token}", | ||
| "ENGRAM_INSECURE": "1" |
There was a problem hiding this comment.
Не включайте ENGRAM_INSECURE без явного opt-in.
На Line 10 небезопасный режим принудительно включается для всех установок, включая случаи с обычным HTTPS endpoint. Это особенно рискованно, потому что в тот же процесс передаётся ENGRAM_API_TOKEN. Лучше выводить этот флаг из отдельной пользовательской настройки или включать его только для явно разрешённых http:///self-signed сценариев.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/engram/.mcp.json` around lines 7 - 10, Remove the hardcoded insecure
flag and make ENGRAM_INSECURE opt-in: stop unconditionally setting
"ENGRAM_INSECURE": "1" in .mcp.json; instead populate ENGRAM_INSECURE only when
the user explicitly enables an insecure option (e.g., user_config.insecure ===
true) or when validating that ENGRAM_URL is a non-HTTPS scheme (starts with
"http://") and the user has consented; reference the existing keys ENGRAM_URL
and ENGRAM_API_TOKEN and implement the change where env is built so the token is
not sent alongside an implicitly enabled insecure flag.
Comment on lines
+46
to
+55
| if (platform === "win32") { | ||
| suffix = "windows-amd64.exe"; | ||
| binaryName = "engram.exe"; | ||
| } else if (platform === "darwin") { | ||
| suffix = arch === "arm64" ? "darwin-arm64" : "darwin-amd64"; | ||
| } else if (platform === "linux") { | ||
| suffix = arch === "arm64" ? "linux-arm64" : "linux-amd64"; | ||
| } else { | ||
| process.stderr.write(`[engram] unsupported platform: ${platform}\n`); | ||
| return; |
There was a problem hiding this comment.
Не подменяйте неподдерживаемые архитектуры на amd64.
Сейчас любой process.arch, кроме arm64, на macOS/Linux уходит в *-amd64, а Windows всегда получает windows-amd64.exe. На ia32/arm/Windows ARM это скачает несовместимый бинарник вместо явного отказа.
💡 Пример правки
if (platform === "win32") {
- suffix = "windows-amd64.exe";
- binaryName = "engram.exe";
+ if (arch === "x64") {
+ suffix = "windows-amd64.exe";
+ binaryName = "engram.exe";
+ } else {
+ process.stderr.write(`[engram] unsupported architecture: ${platform}/${arch}\n`);
+ return;
+ }
} else if (platform === "darwin") {
- suffix = arch === "arm64" ? "darwin-arm64" : "darwin-amd64";
+ if (arch === "arm64") suffix = "darwin-arm64";
+ else if (arch === "x64") suffix = "darwin-amd64";
+ else {
+ process.stderr.write(`[engram] unsupported architecture: ${platform}/${arch}\n`);
+ return;
+ }
} else if (platform === "linux") {
- suffix = arch === "arm64" ? "linux-arm64" : "linux-amd64";
+ if (arch === "arm64") suffix = "linux-arm64";
+ else if (arch === "x64") suffix = "linux-amd64";
+ else {
+ process.stderr.write(`[engram] unsupported architecture: ${platform}/${arch}\n`);
+ return;
+ }
} else {📝 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.
Suggested change
| if (platform === "win32") { | |
| suffix = "windows-amd64.exe"; | |
| binaryName = "engram.exe"; | |
| } else if (platform === "darwin") { | |
| suffix = arch === "arm64" ? "darwin-arm64" : "darwin-amd64"; | |
| } else if (platform === "linux") { | |
| suffix = arch === "arm64" ? "linux-arm64" : "linux-amd64"; | |
| } else { | |
| process.stderr.write(`[engram] unsupported platform: ${platform}\n`); | |
| return; | |
| if (platform === "win32") { | |
| if (arch === "x64") { | |
| suffix = "windows-amd64.exe"; | |
| binaryName = "engram.exe"; | |
| } else { | |
| process.stderr.write(`[engram] unsupported architecture: ${platform}/${arch}\n`); | |
| return; | |
| } | |
| } else if (platform === "darwin") { | |
| if (arch === "arm64") suffix = "darwin-arm64"; | |
| else if (arch === "x64") suffix = "darwin-amd64"; | |
| else { | |
| process.stderr.write(`[engram] unsupported architecture: ${platform}/${arch}\n`); | |
| return; | |
| } | |
| } else if (platform === "linux") { | |
| if (arch === "arm64") suffix = "linux-arm64"; | |
| else if (arch === "x64") suffix = "linux-amd64"; | |
| else { | |
| process.stderr.write(`[engram] unsupported architecture: ${platform}/${arch}\n`); | |
| return; | |
| } | |
| } else { | |
| process.stderr.write(`[engram] unsupported platform: ${platform}\n`); | |
| return; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/engram/scripts/ensure-binary.js` around lines 46 - 55, The
platform/arch mapping currently falls back to amd64 for any arch that's not
arm64, which will download incompatible binaries for ia32/arm/Windows ARM;
update the logic in ensure-binary.js to only map known supported triples (e.g.,
for darwin: arch === "arm64" -> "darwin-arm64", arch === "x64" ->
"darwin-amd64"; for linux: arch === "arm64" -> "linux-arm64", arch === "x64" ->
"linux-amd64"; for win32: allow only arch === "x64" -> "windows-amd64.exe" and
arch === "arm64" -> "windows-arm64.exe" if you ship it) and otherwise write an
explicit unsupported platform/arch message using process.stderr.write and
return; change the variables referenced (platform, arch, suffix, binaryName) and
the conditional blocks that currently set suffix/binaryName to implement these
strict checks.
Comment on lines
+81
to
+85
| const url = `https://github.com/${REPO}/releases/download/v${desiredVersion}/engram-${suffix}`; | ||
| const tmpPath = binaryPath + ".tmp"; | ||
|
|
||
| try { | ||
| await download(url, tmpPath); |
There was a problem hiding this comment.
Скачанный executable нужно проверять по checksum или подписи.
Здесь hook автоматически скачивает бинарник, который затем будет исполнен в следующем шаге. Сейчас доверие строится только на HTTPS и redirect'ах GitHub; без SHA-256/подписей компрометация release asset превращается в тихое выполнение произвольного кода при SessionStart.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/engram/scripts/ensure-binary.js` around lines 81 - 85, The downloaded
executable (tmpPath) must be integrity-verified before being used: fetch the
expected checksum or signature for the release (e.g., a .sha256 asset or
detached signature from the same GitHub release) and verify the downloaded file
after download(url, tmpPath) by computing its SHA-256 (using crypto) and
comparing to the expected value, or by verifying the signature with the release
public key; if verification fails, remove tmpPath and throw an error so the
binary is never moved/used. Locate the download call and variables url, tmpPath,
desiredVersion, suffix, and binaryPath to implement the verification step and
proper cleanup on failure.
Comment on lines
+94
to
+100
| // Atomic replace | ||
| try { | ||
| if (fs.existsSync(binaryPath)) fs.unlinkSync(binaryPath); | ||
| fs.renameSync(tmpPath, binaryPath); | ||
| } catch (err) { | ||
| process.stderr.write(`[engram] install failed: ${err.message}\n`); | ||
| return; |
There was a problem hiding this comment.
Замена бинарника сейчас не атомарна и может удалить рабочую версию.
На Line 96 текущий бинарник удаляется до того, как новый гарантированно оказался на месте. Если renameSync на Line 97 падает, плагин остаётся вообще без бинарника. Нужен хотя бы rollback на предыдущий файл или схема, где last-known-good не удаляется до успешной установки.
💡 Более безопасный вариант с rollback
// Atomic replace
+ const backupPath = binaryPath + ".bak";
try {
- if (fs.existsSync(binaryPath)) fs.unlinkSync(binaryPath);
+ if (fs.existsSync(binaryPath)) fs.renameSync(binaryPath, backupPath);
fs.renameSync(tmpPath, binaryPath);
+ if (fs.existsSync(backupPath)) fs.unlinkSync(backupPath);
} catch (err) {
+ try {
+ if (!fs.existsSync(binaryPath) && fs.existsSync(backupPath)) {
+ fs.renameSync(backupPath, binaryPath);
+ }
+ } catch {}
process.stderr.write(`[engram] install failed: ${err.message}\n`);
return;
}📝 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.
Suggested change
| // Atomic replace | |
| try { | |
| if (fs.existsSync(binaryPath)) fs.unlinkSync(binaryPath); | |
| fs.renameSync(tmpPath, binaryPath); | |
| } catch (err) { | |
| process.stderr.write(`[engram] install failed: ${err.message}\n`); | |
| return; | |
| // Atomic replace | |
| const backupPath = binaryPath + ".bak"; | |
| try { | |
| if (fs.existsSync(binaryPath)) fs.renameSync(binaryPath, backupPath); | |
| fs.renameSync(tmpPath, binaryPath); | |
| if (fs.existsSync(backupPath)) fs.unlinkSync(backupPath); | |
| } catch (err) { | |
| try { | |
| if (!fs.existsSync(binaryPath) && fs.existsSync(backupPath)) { | |
| fs.renameSync(backupPath, binaryPath); | |
| } | |
| } catch {} | |
| process.stderr.write(`[engram] install failed: ${err.message}\n`); | |
| return; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/engram/scripts/ensure-binary.js` around lines 94 - 100, The current
replace in ensure-binary.js deletes binaryPath before ensuring tmpPath is moved,
risking loss if fs.renameSync(tmpPath, binaryPath) fails; change to an atomic
swap with rollback: first rename binaryPath to a backup name (e.g., backupPath =
binaryPath + '.bak') only if it exists, then rename tmpPath to binaryPath; if
the second rename fails, restore the backup by renaming backupPath back to
binaryPath and surface the original error; on success remove the backup. Use the
existing variables binaryPath and tmpPath and perform all operations inside the
try/catch so failures trigger the rollback.
Comment on lines
+26
to
+32
| try { | ||
| const { spawnSync } = require("child_process"); | ||
| const result = spawnSync(binaryPath, process.argv.slice(2), { | ||
| stdio: "inherit", | ||
| env: process.env, | ||
| }); | ||
| process.exit(result.status ?? 1); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Node.js child_process.spawnSync, when the child process cannot be started, does it throw or return a result object with an error property?
💡 Result:
In Node.js child_process.spawnSync, when the child process cannot be started (e.g., command not found, resulting in ENOENT), it returns a result object that includes an error property, rather than throwing an exception. The result object has fields like error (populated with the spawn failure Error), status: null, signal: null, output: null, stdout: null, stderr: null, and pid (a number).
Citations:
- 1: https://stackoverflow.com/questions/37125619/spawnsyncnpm-install-gives-error-spawnsync-npm-enoent
- 2: doc: child_process.SpawnSync output, stdout, stderr can be null nodejs/node#33458
- 3: https://nodejs.org/api/child_process.html
Добавьте проверку result.error после spawnSync.
Когда дочерний процесс не может быть запущен, spawnSync возвращает объект с свойством error, а не выбрасывает исключение. Текущий код не проверяет эту ошибку и просто завершается с кодом 1 без диагностики.
Пример правки
try {
const { spawnSync } = require("child_process");
const result = spawnSync(binaryPath, process.argv.slice(2), {
stdio: "inherit",
env: process.env,
});
+ if (result.error) {
+ process.stderr.write(`[engram] exec failed: ${result.error.message}\n`);
+ process.exit(1);
+ }
process.exit(result.status ?? 1);
} catch (err) {
process.stderr.write(`[engram] exec failed: ${err.message}\n`);
process.exit(1);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugin/engram/scripts/run-engram.js` around lines 26 - 32, The spawnSync call
may return a result with result.error instead of throwing; after calling
spawnSync (in run-engram.js where spawnSync and binaryPath are used) check if
result.error is present, log the error (e.g., console.error with a clear message
and the error.stack or message) and exit with a non-zero code (process.exit(1));
otherwise continue to use process.exit(result.status ?? 0) for normal
termination. Ensure the check runs before calling process.exit so failed process
spawn is diagnosed.
This was referenced Apr 13, 2026
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.
Changes
${CLAUDE_PLUGIN_DATA}/bin/. Version-pinned to plugin.json version. Cached until version changes.server_urlandapi_tokenon plugin enable (zero manual env setup for MCP server)${CLAUDE_PLUGIN_DATA}/bin/engrambinary path +user_config.*env varsOnboarding flow (user perspective)
/plugin install engramTest plan
Summary by CodeRabbit
Примечания к выпуску
Новые функции
Обновления