fix(plugin): config-file credential fallback for harness-independent setup (v6.4.15) - #240
Conversation
…setup (v6.4.15) Codex 0.139 stopped forwarding [shell_environment_policy.set] values to plugin MCP server children (openai/codex#24401). No supported env-injection path exists for plugin MCP servers in Codex. Prior versions relied on ENGRAM_URL/ENGRAM_TOKEN via shell_environment_policy.set, silently broken for Codex >= 0.139. Resolution chain (env always wins over file): 1. ENGRAM_URL / ENGRAM_TOKEN explicit env 2. CLAUDE_PLUGIN_OPTION_server_url / CLAUDE_PLUGIN_OPTION_api_token (CC userConfig) 3. ENGRAM_CLAUDE_USERCONFIG_URL / ENGRAM_CLAUDE_USERCONFIG_TOKEN (legacy aliases) 4. Config file: ENGRAM_CONFIG_FILE -> <pluginData>/config.json -> ~/.engram/config.json Bug fixed: duplicate const os = require(os) at lib.js line 538 caused SyntaxError on module load, failing all 7 plugin test suites (23 pass / 7 fail -> 55 pass / 0 fail).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughВерсия 6.4.15 добавляет поддержку файлового fallback для учётных данных Engram, восстанавливая функциональность, нарушенную в Codex ≥0.139. Реализована приоритизированная цепочка резолва credentials, новые утилиты конфиг-файла, обновлена диагностика с указанием статуса конфига, и добавлены соответствующие тесты. ChangesConfig-file fallback для учётных данных (v6.4.15)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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 docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
|
@codex review |
✅ Action performedReview finished.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0fae925d4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin/engram/scripts/run-engram.js (1)
239-251:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winСинхронизируйте startup-диагностику с реально поддерживаемыми env-ключами.
Резолв URL/токена поддерживает
CLAUDE_PLUGIN_OPTION_SERVER_URLиCLAUDE_PLUGIN_OPTION_API_TOKEN, но вformatStartupDiagnosticони не выводятся. Это дает вводящую в заблуждение диагностику при troubleshooting.Предлагаемый патч
const keys = [ ["ENGRAM_URL", false], ["ENGRAM_TOKEN", true], ["ENGRAM_SERVER_URL", false], ["CLAUDE_PLUGIN_OPTION_server_url", false], + ["CLAUDE_PLUGIN_OPTION_SERVER_URL", false], ["CLAUDE_PLUGIN_OPTION_api_token", true], + ["CLAUDE_PLUGIN_OPTION_API_TOKEN", true], ["ENGRAM_CLAUDE_USERCONFIG_URL", false], ["ENGRAM_CLAUDE_USERCONFIG_TOKEN", true],🤖 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 `@plugin/engram/scripts/run-engram.js` around lines 239 - 251, The startup diagnostic's keys list is out of sync with the actual env names used by the resolver: update the keys array used by formatStartupDiagnostic to include the real supported names CLAUDE_PLUGIN_OPTION_SERVER_URL and CLAUDE_PLUGIN_OPTION_API_TOKEN (matching casing), and ensure their sensitivity booleans are correct (true for tokens, false for URLs), replacing the existing entries ["CLAUDE_PLUGIN_OPTION_server_url", false] and ["CLAUDE_PLUGIN_OPTION_api_token", true]; verify other related names (e.g. CLAUDE_PLUGIN_ROOT/DATA) remain unchanged so diagnostics accurately reflect supported env keys.
🤖 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 `@CHANGELOG.md`:
- Around line 10-35: Обновите секцию ссылок версий внизу CHANGELOG.md: добавьте
сравнения/ссылки для v6.4.15, v6.4.14 и v6.4.13 и замените текущую ссылку
"v6.4.12...HEAD" на корректные диапазоны (например v6.4.15...HEAD и отдельные
ссылки v6.4.15/v6.4.14/v6.4.13), чтобы каждая пометка релиза ([6.4.15],
[6.4.14], [6.4.13]) имела соответствующую ссылку; проверьте синтаксис ссылок
внизу файла и обновите все версии/хеши, чтобы навигация по релизам снова
работала корректно.
---
Outside diff comments:
In `@plugin/engram/scripts/run-engram.js`:
- Around line 239-251: The startup diagnostic's keys list is out of sync with
the actual env names used by the resolver: update the keys array used by
formatStartupDiagnostic to include the real supported names
CLAUDE_PLUGIN_OPTION_SERVER_URL and CLAUDE_PLUGIN_OPTION_API_TOKEN (matching
casing), and ensure their sensitivity booleans are correct (true for tokens,
false for URLs), replacing the existing entries
["CLAUDE_PLUGIN_OPTION_server_url", false] and
["CLAUDE_PLUGIN_OPTION_api_token", true]; verify other related names (e.g.
CLAUDE_PLUGIN_ROOT/DATA) remain unchanged so diagnostics accurately reflect
supported env keys.
🪄 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: 117d0796-3391-4787-a343-95a359a76077
📒 Files selected for processing (10)
CHANGELOG.mddocs/release-notes/v6.4.15.mdinternal/version/version.goplugin/engram/.claude-plugin/plugin.jsonplugin/engram/.codex-plugin/plugin.jsonplugin/engram/commands/setup.mdplugin/engram/hooks/lib.jsplugin/engram/hooks/session-start.jsplugin/engram/scripts/run-engram.jsplugin/engram/scripts/run-engram.test.js
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a JSON config file credential fallback (~/.engram/config.json) for Codex >= 0.139 and other environments that do not forward environment variables to plugin children. The feedback suggests several robustness and performance improvements: using the configuredPluginEnv helper in lib.js to avoid un-interpolated placeholders in the config file path, optimizing getEngramConfig to avoid reading the config file twice when credentials are missing, and updating describeConfigFile (along with adding a corresponding test) to distinguish between a missing config file and a malformed one.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- run-engram.js: resolveConfigFilePath now falls through to
~/.engram/config.json when pluginData is set but pluginData/config.json
does not exist; fixes users who only create the documented home-dir path
(P1 codex finding)
- run-engram.js: describeConfigFile now distinguishes missing file from
malformed JSON — reports config_file=malformed(...) instead of
config_file=missing(...) when the file exists but is invalid
- run-engram.test.js: update resolveConfigFilePath test to create the
pluginData/config.json file so it exists (required by new behaviour);
add new fall-through test and malformed-JSON diagnostic test (now 57 tests)
- lib.js: resolveConfigFilePath filters ${...} placeholder values via
configuredPluginEnv() for consistency with run-engram.js; also falls
through to home dir when pluginData candidate file is absent
- lib.js: getEngramConfig reads config file at most once (single read when
either credential is missing, avoids double readFileSync)
- lib.js: RunHook calls getEngramConfig() at entry to hydrate ENGRAM_URL /
ENGRAM_TOKEN from the config file for every hook process; each hook runs
in its own Node process so config-file-only setups now work in all hooks
(P2 codex finding)
- CHANGELOG.md: add missing version comparison links for 6.4.13/6.4.14/
6.4.15 and update [Unreleased] to v6.4.15...HEAD
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e552e149a9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (pluginData && typeof pluginData === "string" && pluginData.trim()) { | ||
| const candidate = path.join(pluginData.trim(), "config.json"); | ||
| if (fs.existsSync(candidate)) { | ||
| return candidate; |
There was a problem hiding this comment.
Fresh evidence: this version still returns <pluginData>/config.json based only on fs.existsSync() before readEngramConfigFile() parses it. In an install where that plugin-data file exists but is malformed or incomplete while the documented ~/.engram/config.json is valid, the wrapper never checks the home fallback and exits as unconfigured; the duplicate resolver in hooks/lib.js has the same behavior, so hook processes would also ignore the valid home config in that scenario.
Useful? React with 👍 / 👎.
1 participant
Problem
Codex 0.139 stopped forwarding
[shell_environment_policy.set]values andPLUGIN_ROOT/PLUGIN_DATAto plugin-bundled MCP server children (undocumented behavior that ≤0.136 had; changelogs silent). Codex has no supported env-injection path for plugin MCP servers —plugins.<plugin>.mcp_servers.<server>only carries enabled/tools policy (openai/codex#24401). Result: the engram plugin fails on Codex 0.139 withconnection closed: initialize responsebecauseENGRAM_TOKENnever reaches the wrapper.Historical context: pre-plugin engram worked because it was a user-defined
[mcp_servers.engram]with first-class[mcp_servers.engram.env]— the plugin migration moved onto a channel that does not exist for plugins.Fix
Config-file credential fallback, independent of harness env forwarding. Resolution chain (env always wins):
ENGRAM_URL/ENGRAM_TOKEN(host env — Claude settings.json path unchanged)ENGRAM_SERVER_URL,CLAUDE_PLUGIN_OPTION_*, legacyENGRAM_CLAUDE_USERCONFIG_*$ENGRAM_CONFIG_FILE→<pluginData>/config.json→~/.engram/config.json— JSON{"server_url","api_token"}Malformed/missing file = silent skip to the existing FATAL diagnostics, which now name the config path. Startup diagnostic gains
config_file=present/missing(token never logged).setup.mdCodex section rewritten around the config file;shell_environment_policy.setdocumented as legacy/broken on 0.139+.Verification
Summary by CodeRabbit
Новые возможности
Исправления ошибок
Документация
Безопасность