fix(engram): make Antigravity plugin registration recoverable#1602
Closed
dnlrsls wants to merge 2 commits into
Closed
fix(engram): make Antigravity plugin registration recoverable#1602dnlrsls wants to merge 2 commits into
dnlrsls wants to merge 2 commits into
Conversation
📝 WalkthroughWalkthroughAntigravity Engram injection now reconciles global and plugin registrations, preserves ChangesAntigravity Engram injection
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant injectWithOptions
participant installAntigravityEngramPlugin
participant AntigravityGlobalConfig
participant EngramPluginFiles
injectWithOptions->>installAntigravityEngramPlugin: install Antigravity Engram integration
installAntigravityEngramPlugin->>AntigravityGlobalConfig: read and reconcile global MCP registration
installAntigravityEngramPlugin->>EngramPluginFiles: write MCP config, hooks, and plugin manifest
EngramPluginFiles-->>installAntigravityEngramPlugin: return convergence and write results
installAntigravityEngramPlugin-->>injectWithOptions: return staged paths and joined errors
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 1
🤖 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 `@internal/components/engram/inject.go`:
- Around line 302-308: Update the global configuration parsing around root so
zero-byte or whitespace-only global.data is normalized to an empty object before
json.Unmarshal. Preserve the existing parse error for non-empty invalid JSON and
the expected-object validation for other decoded values, allowing recoverable
empty mcp_config.json files to continue with {}.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 144c5cee-28de-4e1d-97aa-5794254181b4
⛔ Files ignored due to path filters (1)
testdata/golden/engram-antigravity-mcp.goldenis excluded by!testdata/**
📒 Files selected for processing (3)
internal/components/engram/inject.gointernal/components/engram/inject_test.gointernal/components/golden_test.go
19 tasks
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/components/engram/inject.go (2)
701-727: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winMake every Antigravity user-config write transactional.
The settings bootstrap and failure tests must prove that later activation failures cannot leave partial configuration behind.
internal/components/engram/inject.go#L701-L727: verify that a newly created settings file is removed, or an existing file is restored, when a later install step fails.internal/components/engram/inject_test.go#L712-L774: add failure cases and state assertions forsettings.json, pluginmcp_config.json, andhooks.json, not only the global config andplugin.json.As per path instructions, “Install/sync operations must be idempotent (running twice equals running once). Any write to user config must have a backup/restore path.”
🤖 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 `@internal/components/engram/inject.go` around lines 701 - 727, Make ensureAntigravitySettings participate in the install transaction by recording whether settingsPath existed and preserving its original contents before writing; expose or connect that backup to the later activation rollback so failures remove newly created settings.json or restore an existing file. Extend the failure cases in internal/components/engram/inject_test.go lines 712-774 to assert rollback for settings.json, plugin mcp_config.json, and hooks.json alongside the existing global config and plugin.json assertions.Source: Path instructions
346-354: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve the resolved engram binary for Antigravity.
preferredStableEngramCommand()collapses non-Homebrew paths to"engram", so downloaded installs lose the concrete binary path here and the generated MCP config depends on PATH inheritance.🤖 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 `@internal/components/engram/inject.go` around lines 346 - 354, Update the Antigravity command resolution around existingMergedEngramCommand and preferredStableEngramCommand so a downloaded or otherwise non-Homebrew install retains its resolved concrete binary path instead of being collapsed to "engram". Keep the stable Homebrew command behavior, then pass the preserved command through stableEngramCommandForExisting when generating the engram MCP configuration.
🤖 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.
Outside diff comments:
In `@internal/components/engram/inject.go`:
- Around line 701-727: Make ensureAntigravitySettings participate in the install
transaction by recording whether settingsPath existed and preserving its
original contents before writing; expose or connect that backup to the later
activation rollback so failures remove newly created settings.json or restore an
existing file. Extend the failure cases in
internal/components/engram/inject_test.go lines 712-774 to assert rollback for
settings.json, plugin mcp_config.json, and hooks.json alongside the existing
global config and plugin.json assertions.
- Around line 346-354: Update the Antigravity command resolution around
existingMergedEngramCommand and preferredStableEngramCommand so a downloaded or
otherwise non-Homebrew install retains its resolved concrete binary path instead
of being collapsed to "engram". Keep the stable Homebrew command behavior, then
pass the preserved command through stableEngramCommandForExisting when
generating the engram MCP configuration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: caad2ab2-c50d-431c-9316-79ff3984370d
📒 Files selected for processing (2)
internal/components/engram/inject.gointernal/components/engram/inject_test.go
This was referenced Jul 21, 2026
Contributor
Author
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.
Closes #797
Summary
Changes
internal/components/engram/inject.gointernal/components/engram/inject_test.gointernal/components/golden_test.gotestdata/golden/engram-antigravity-mcp.goldenTest Plan
go test -count=1 ./internal/components/engram ./internal/componentsgo test -count=1 ./...— incomplete because the repository-wide suite exceeded the execution timeout after 23 packages passed.Contributor Checklist
type:*label.Co-Authored-Byor AI attribution.Summary by CodeRabbit
New Features
argshandling.Bug Fixes
Tests