chore(plugin): v4.5.0 — restore MCP + disable noisy context inject (BREAKING)#177
Merged
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
WalkthroughВерсия плагина engram обновлена с 4.4.0 до 4.5.0; добавлена переменная окружения Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 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: 1
🧹 Nitpick comments (1)
plugin/engram/hooks/session-start.js (1)
196-199: Следующий шаг: урезать payload на уровне API, а не только рендер.Сейчас ненужные поля всё ещё тянутся по сети. Имеет смысл добавить server-side фильтрацию секций (например, only
always_inject) для снижения latency/нагрузки.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin/engram/hooks/session-start.js` around lines 196 - 199, В коде плагина (plugin/engram/hooks/session-start.js) нужно не только отключить рендеринг <engram-context>, <project-briefing> и <engram-guidance>, но и отфильтровать их на стороне API при формировании payload: измените логику, которая собирает ответ для "inject GET" (использует result.always_inject) чтобы исключать ненужные секции из исходящего JSON, оставляя только элементы с result.always_inject === true; обновите соответствующую сборку ответа/функцию формирования payload в том же файле чтобы эти поля не передавались по сети и добавьте/обновите тесты или контракт ответа, подтверждающие что отключённые теги полностью удаляются из API-ответа.
🤖 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/scripts/run-engram.js`:
- Around line 27-34: The current check uses !process.env.ENGRAM_URL which treats
strings of only whitespace as truthy; update the logic where
process.env.ENGRAM_URL/ENGRAM_URL_LEGACY are handled so you normalize and test
for non-empty trimmed values (use process.env.ENGRAM_URL &&
process.env.ENGRAM_URL.trim().length or equivalent) before assigning or deciding
the warning path; specifically change the assignment/fallback and the subsequent
emptiness check that writes to process.stderr to use trimmed checks on
process.env.ENGRAM_URL and process.env.ENGRAM_URL_LEGACY so whitespace-only
values trigger the fallback and WARNING correctly.
---
Nitpick comments:
In `@plugin/engram/hooks/session-start.js`:
- Around line 196-199: В коде плагина (plugin/engram/hooks/session-start.js)
нужно не только отключить рендеринг <engram-context>, <project-briefing> и
<engram-guidance>, но и отфильтровать их на стороне API при формировании
payload: измените логику, которая собирает ответ для "inject GET" (использует
result.always_inject) чтобы исключать ненужные секции из исходящего JSON,
оставляя только элементы с result.always_inject === true; обновите
соответствующую сборку ответа/функцию формирования payload в том же файле чтобы
эти поля не передавались по сети и добавьте/обновите тесты или контракт ответа,
подтверждающие что отключённые теги полностью удаляются из API-ответа.
🪄 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: c190796d-a6df-40a1-ab6b-79987b5deea2
📒 Files selected for processing (4)
plugin/engram/.claude-plugin/plugin.jsonplugin/engram/.mcp.jsonplugin/engram/hooks/session-start.jsplugin/engram/scripts/run-engram.js
Comment on lines
+27
to
+34
| if (!process.env.ENGRAM_URL && process.env.ENGRAM_URL_LEGACY) { | ||
| process.env.ENGRAM_URL = process.env.ENGRAM_URL_LEGACY; | ||
| } | ||
|
|
||
| // Visible diagnostic: warn to stderr if both ended up empty so the user has a signal, | ||
| // not a silent gRPC dial failure on every tool call. | ||
| if (!process.env.ENGRAM_URL) { | ||
| process.stderr.write( |
There was a problem hiding this comment.
Усилите проверку “пустого” URL (пробелы сейчас проходят).
Сейчас !process.env.ENGRAM_URL не ловит значения вида " ", из-за чего fallback и WARN могут быть пропущены.
Предлагаемый патч
-if (!process.env.ENGRAM_URL && process.env.ENGRAM_URL_LEGACY) {
- process.env.ENGRAM_URL = process.env.ENGRAM_URL_LEGACY;
+const currentUrl = (process.env.ENGRAM_URL || "").trim();
+const legacyUrl = (process.env.ENGRAM_URL_LEGACY || "").trim();
+
+if (!currentUrl && legacyUrl) {
+ process.env.ENGRAM_URL = legacyUrl;
}
// Visible diagnostic: warn to stderr if both ended up empty so the user has a signal,
// not a silent gRPC dial failure on every tool call.
-if (!process.env.ENGRAM_URL) {
+if (!(process.env.ENGRAM_URL || "").trim()) {
process.stderr.write(📝 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 (!process.env.ENGRAM_URL && process.env.ENGRAM_URL_LEGACY) { | |
| process.env.ENGRAM_URL = process.env.ENGRAM_URL_LEGACY; | |
| } | |
| // Visible diagnostic: warn to stderr if both ended up empty so the user has a signal, | |
| // not a silent gRPC dial failure on every tool call. | |
| if (!process.env.ENGRAM_URL) { | |
| process.stderr.write( | |
| const currentUrl = (process.env.ENGRAM_URL || "").trim(); | |
| const legacyUrl = (process.env.ENGRAM_URL_LEGACY || "").trim(); | |
| if (!currentUrl && legacyUrl) { | |
| process.env.ENGRAM_URL = legacyUrl; | |
| } | |
| // Visible diagnostic: warn to stderr if both ended up empty so the user has a signal, | |
| // not a silent gRPC dial failure on every tool call. | |
| if (!(process.env.ENGRAM_URL || "").trim()) { | |
| process.stderr.write( |
🤖 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 27 - 34, The current check
uses !process.env.ENGRAM_URL which treats strings of only whitespace as truthy;
update the logic where process.env.ENGRAM_URL/ENGRAM_URL_LEGACY are handled so
you normalize and test for non-empty trimmed values (use process.env.ENGRAM_URL
&& process.env.ENGRAM_URL.trim().length or equivalent) before assigning or
deciding the warning path; specifically change the assignment/fallback and the
subsequent emptiness check that writes to process.stderr to use trimmed checks
on process.env.ENGRAM_URL and process.env.ENGRAM_URL_LEGACY so whitespace-only
values trigger the fallback and WARNING correctly.
…REAKING) Consolidates the BREAKING CHANGE from PR #176 (legacy MCP/CLI removal) with two user-visible plugin tactical fixes into a single coherent v4.5.0 release. ## BREAKING CHANGES (carried over from PR #176) The 'engram-mcp' and 'engram-mcp-stdio-proxy' release artifacts no longer ship. Bare-metal users who installed via tarball or scripts/install.sh and relied on the engram-mcp binary must migrate to the plugin marketplace install path: /plugin marketplace add thebtf/engram-marketplace /plugin install engram Plugin marketplace users are unaffected — ensure-binary.js downloads only the 'engram' stdio-proxy binary, not engram-mcp. cmd/mcp/, cmd/engram-cli/, the Dockerfile client image stage, the engram-mcp Makefile + goreleaser + install.sh entries, and the bin/mcp-server / bin/mcp-stdio-proxy COMPONENTS.md sections are all gone. ## Fix #1 — MCP startup reliability after CC plugin update Smoking gun (.agent/reports/plugin-tactical-fix-triage.md): when CC updates the plugin from v4.3.0 to v4.4.x it does NOT migrate plugin-level user_config (server_url + api_token). .mcp.json then expands ${user_config.server_url} to an empty string, the binary spawns silently, and every MCP tool call fails with an opaque gRPC dial error against an empty target. Fix: - plugin/engram/.mcp.json: env block now also passes "ENGRAM_URL_LEGACY": "${ENGRAM_URL}" so users who had ENGRAM_URL in their shell from the v4.3.0 setup still get a working MCP server. - plugin/engram/scripts/run-engram.js: before spawning, fall back to ENGRAM_URL_LEGACY if ENGRAM_URL is empty, AND emit a visible WARN to stderr if both are empty so the failure mode is no longer silent. ## Fix #2 — disable noisy context injection Per .agent/reports/phase-2-synthesis.md fix #16 + the entity audits, the session-start hook currently injects 100 raw observations unioned with 10 semantic hits, plus project briefing, plus learned guidance. Reported as noise rather than relevant context. plugin/engram/hooks/session-start.js now keeps the inject GET (still needed for result.always_inject) but renders ONLY: - result.always_inject -> <user-behavior-rules> - GET /api/issues -> <engram-issues> - GET /api/issues resolved -> <engram-resolved-issues> Skipped: result.observations / result.full_count / result.project_briefing / result.guidance. mark-injected scoped to always_inject IDs only so citation tracking does not log false positives. Misleading observation- count log line removed. Save / recall MCP tools (recall_memory, store_memory, find_by_file, observations, store, feedback, vault, issues) are completely independent of this hook — separate gRPC path, unaffected. This is tactical, NOT a redesign. Phase 2 strategic work continues: citation session_id="" smoking gun, 100-obs cap policy, hook curated-render redesign — all in .agent/reports/phase-2-synthesis.md. ## Version bump Per Constitution §15 the daemon version and plugin version move together: - cmd/engram/main.go daemonVersion "v4.4.0" -> "v4.5.0" - plugin/engram/.claude-plugin/plugin.json "4.4.0" -> "4.5.0" ## Verification - JSON parse on .mcp.json + plugin.json: clean - node --check on run-engram.js + session-start.js: clean - go build ./... clean - 3-OS matrix CI green on the prior 4.4.1 commit (re-runs on amend) ## Files 4 plugin/daemon files + 1 daemon version constant. Net: -76 LOC (mostly removed render blocks in session-start.js).
thebtf
force-pushed
the
chore/plugin-tactical-fix-v4.4.1
branch
from
73f95a0 to
d14ddd1
Compare
thebtf
changed the title
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.
v4.5.0 release
Consolidates the BREAKING CHANGE from PR #176 (legacy MCP/CLI removal) with two user-visible plugin tactical fixes into one coherent release. Single marketplace-sync cycle, single tag, single set of release notes.
The
engram-mcpandengram-mcp-stdio-proxyrelease artifacts no longer ship.Bare-metal users who installed via tarball or
scripts/install.shand relied on theengram-mcpbinary must migrate to the plugin marketplace install path:Plugin marketplace users are unaffected —
ensure-binary.jsdownloads only theengramstdio-proxy binary, notengram-mcp.Fix #1 — MCP startup reliability after CC plugin update
Smoking gun (
.agent/reports/plugin-tactical-fix-triage.md): when CC updates the plugin from v4.3.0 to v4.4.x it does NOT migrate plugin-leveluserConfig(server_url+api_token)..mcp.jsonthen expands${user_config.server_url}to an empty string, the binary spawns silently, and every MCP tool call fails with an opaque gRPC dial error against an empty target.Changes:
plugin/engram/.mcp.json— env block now also passes"ENGRAM_URL_LEGACY": "${ENGRAM_URL}"so users who hadENGRAM_URLin their shell from the v4.3.0 setup still get a working MCP server.plugin/engram/scripts/run-engram.js— before spawning, fall back toENGRAM_URL_LEGACYifENGRAM_URLis empty, AND emit a visible WARN to stderr if both are empty so the failure mode is no longer silent.Fix #2 — Disable noisy context injection
Per
.agent/reports/phase-2-synthesis.mdfix #16 + the entity audits, the session-start hook currently injects 100 raw observations unioned with 10 semantic hits, plus project briefing, plus learned guidance. Reported as noise rather than relevant context.Field-level classification (kept verbatim from
plugin-tactical-fix-triage.md):result.observations/results<engram-context>result.full_count<engram-context>cutoffresult.project_briefing<project-briefing>result.guidance<engram-guidance>result.always_inject<user-behavior-rules>GET /api/issues(open)<engram-issues>GET /api/issues(resolved)<engram-resolved-issues>Changes in
plugin/engram/hooks/session-start.js:<engram-context>(~lines 209-243),<project-briefing>(~245-248),<engram-guidance>(~251-273)result.always_injectlives in that response and we still render itmark-injectedAPI call toalwaysInjectIDs only so citation tracking does not log false positivesNet diff in this hook: −91 lines.
What stays working (safety guarantees)
recall,recall_memory,store,store_memory,find_by_file,observations,feedback,vault,issues, etc. Separate gRPC path (internal/mcp/tools_*.go), zero coupling to inject hook./api/issuesHTTP calls, untouched.always_injecttier still rendered.engram-serverredeploy required.Version bump
Per Constitution §15 the daemon version and plugin version move together:
cmd/engram/main.godaemonVersion"v4.4.0" → "v4.5.0"plugin/engram/.claude-plugin/plugin.jsonversion"4.4.0" → "4.5.0"Verification
.mcp.json+plugin.json— cleannode --checkonrun-engram.js+session-start.js— cleango build ./...— cleanOut of scope (Phase 2 strategic work continues)
This is tactical only. Phase 2 redesign continues in parallel:
session_id=""smoking gun fix (synthesis fix feat: self-learning utility signals #1).agent/reports/phase-2-synthesis.mdThis PR keeps the user productive while that redesign happens.
Files in this PR
5 changed:
plugin/engram/.mcp.jsonplugin/engram/scripts/run-engram.jsplugin/engram/hooks/session-start.jsplugin/engram/.claude-plugin/plugin.jsoncmd/engram/main.go(daemonVersion bump)Net −76 LOC (mostly removed render blocks).
Summary by CodeRabbit
Примечания к выпуску
New Features
Chores