remove engram - #161
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe change adds extensive product, API, architecture, data-model, security, deployment, operations, testing, and known-issues documentation. It also renames a local task, updates an audit test fixture, narrows uninstall cleanup, and applies minor documentation formatting changes. ChangesDocumentation and maintenance updates
Estimated code review effort: 2 (Simple) | ~15 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (18)
docs/product-overview.md-231-231 (1)
231-231: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language identifier to the project-structure fence.
markdownlintreports MD040 for this block. Mark it astextor another suitable language.🤖 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 `@docs/product-overview.md` at line 231, Update the project-structure fenced code block in the product overview documentation to include an explicit language identifier, using text or another suitable language, so it satisfies markdownlint MD040.Source: Linters/SAST tools
docs/security.md-243-243 (1)
243-243: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd language identifiers to fenced code blocks.
markdownlintreports MD040 for these fences. Mark the wire-format block astextand the lifecycle/checksum blocks with suitable identifiers.Also applies to: 300-300, 315-315
🤖 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 `@docs/security.md` at line 243, Update the fenced code blocks at the referenced locations in docs/security.md to include language identifiers: use text for the wire-format block and appropriate identifiers for the lifecycle and checksum blocks, resolving markdownlint MD040 without changing their contents.Source: Linters/SAST tools
docs/testing.md-90-90 (1)
90-90: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd a language identifier to the directory-layout fence.
markdownlintreports MD040 for this block. Mark it astext.🤖 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 `@docs/testing.md` at line 90, Update the directory-layout fenced code block in the testing documentation to specify the text language identifier, resolving the markdownlint MD040 warning while preserving its contents.Source: Linters/SAST tools
docs/api-sdk-reference.md-134-134 (1)
134-134: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd language identifiers to fenced code blocks.
markdownlintreports MD040 for these fences. Add identifiers such asbash,json,rust, ortextto keep documentation lint-clean.Also applies to: 171-171, 201-201, 218-218, 237-237, 264-264, 274-274, 307-307, 420-420, 459-459, 486-486, 554-554, 585-585, 795-795, 817-817
🤖 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 `@docs/api-sdk-reference.md` at line 134, Update all fenced code blocks in docs/api-sdk-reference.md, including the listed locations, to specify an appropriate language identifier such as bash, json, rust, or text. Preserve each block’s existing content and choose identifiers that match the enclosed examples so markdownlint MD040 passes.Source: Linters/SAST tools
docs/architecture-system-design.md-30-30 (1)
30-30: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd language identifiers to fenced code blocks.
markdownlintreports MD040 for these fences. Specify the appropriate language (text,text/tree, or similar) for each block.Also applies to: 482-482, 534-534
🤖 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 `@docs/architecture-system-design.md` at line 30, Update the fenced code blocks in the architecture documentation, including the blocks near the referenced locations, to specify appropriate language identifiers such as text or tree. Ensure every fence has a valid identifier so markdownlint MD040 passes without changing the block contents.Source: Linters/SAST tools
docs/security.md-272-272 (1)
272-272: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the reported platform-name capitalization.
LanguageTool flags the references to GitHub in these lines. Use the official capitalization consistently.
Also applies to: 323-323
🤖 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 `@docs/security.md` at line 272, Correct the platform-name capitalization in the GitHub references within the security documentation, including the additional occurrence noted by the review. Use the official “GitHub” capitalization consistently without changing the surrounding security or update details.Source: Linters/SAST tools
docs/api-sdk-reference.md-171-188 (1)
171-188: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDocument the supported
session-endhook event.
src/cli/hook.rsdefinesSessionEnd, but this reference lists only three events and omits the supported no-op command. Update the command syntax and event table so the CLI contract is complete.🤖 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 `@docs/api-sdk-reference.md` around lines 171 - 188, Update the hook event documentation to include the supported session-end event defined by SessionEnd in src/cli/hook.rs. Add session-end to the EVENT parameter’s allowed values and document it in the Events table as the supported no-op command, while preserving the existing event descriptions and hook protocol notes.docs/testing.md-525-531 (1)
525-531: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReconcile the pricing-entry count.
This says the embedded table contains 22 model entries, conflicting with the 19-entry count elsewhere. Use the actual data file as the single source of truth and update all documentation references.
🤖 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 `@docs/testing.md` around lines 525 - 531, Update the “Embedded Test Data” documentation to use the actual entry count from data/anthropic-pricing.json, and reconcile every other pricing-entry count reference in the documentation to that same value. Treat the data file, not the existing prose, as the source of truth.docs/data-model.md-422-422 (1)
422-422: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReconcile the pricing-entry count across documentation.
This section says the pricing table contains 19 model entries, while the API and testing references use different counts. Choose the count produced by the actual pricing data and update all references consistently.
🤖 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 `@docs/data-model.md` at line 422, Reconcile the pricing-entry count in the documentation by verifying the actual pricing data and updating this section and every related API/testing reference to use that single count. Preserve the listed aliases, tier pricing, and modifier metadata details.docs/deployment-and-operations.md-359-360 (1)
359-360: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winResolve the reported documentation-lint findings.
LanguageTool reports platform-name capitalization and wording issues, while
markdownlintreports a missing fenced-block language. Correct these lines before merge.Also applies to: 429-429, 562-562, 629-629
🤖 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 `@docs/deployment-and-operations.md` around lines 359 - 360, Resolve the documentation lint findings in the deployment and operations document: apply the reported LanguageTool capitalization and wording corrections to the affected table entries and corresponding lines, and add an appropriate language identifier to the fenced code block flagged by markdownlint. Preserve the existing deployment instructions and technical meaning.Source: Linters/SAST tools
docs/business-overview.md-249-249 (1)
249-249: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClean up the reported documentation wording.
LanguageTool reports spelling, compound-adjective, platform-name, and hyphenation issues in these lines. Please correct them before merging so the new reference documentation remains consistent and polished.
Also applies to: 283-283, 339-339, 374-374
🤖 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 `@docs/business-overview.md` at line 249, Clean up the reported wording in the documentation entries around “Cache token tracking” and the additional referenced sections, correcting LanguageTool’s spelling, compound-adjective, platform-name, and hyphenation findings. Preserve the documented meaning while applying consistent polished terminology across all affected lines.Source: Linters/SAST tools
KNOWN_ISSUES.md-270-278 (1)
270-278: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the Technical Debt count in the summary.
The section contains seven entries, including “Inconsistent error handling patterns,” but the summary reports six. Update the count or explain which entry is intentionally excluded.
🤖 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 `@KNOWN_ISSUES.md` around lines 270 - 278, Update the Technical Debt count in the summary table to reflect all seven listed entries, including “Inconsistent error handling patterns,” or explicitly document which entry is intentionally excluded.KNOWN_ISSUES.md-18-22 (1)
18-22: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUse “orphaned/running child processes,” not “zombie processes.”
A child that remains running because the timeout path never kills it is a process leak; “zombie” specifically describes an exited child that has not been reaped. The current wording can mislead remediation and incident triage.
🤖 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 `@KNOWN_ISSUES.md` around lines 18 - 22, Update the “Hung child process on version probe timeout” section in KNOWN_ISSUES.md to replace “zombie processes” with “orphaned/running child processes,” while preserving the existing description of the timeout leak and its impact.KNOWN_ISSUES.md-236-244 (1)
236-244: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCorrect the number of state stores.
This section says “three separate state stores” but enumerates four:
state.json,runtime-state.json,auth.db, andanalytics.db.🤖 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 `@KNOWN_ISSUES.md` around lines 236 - 244, Update the “State management is split across JSON and SQLite” section in KNOWN_ISSUES.md to state that the codebase uses four separate state stores, matching the four enumerated files. Leave the listed stores and consolidation recommendation unchanged.KNOWN_ISSUES.md-94-110 (1)
94-110: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winClarify MSRV vs toolchain pin
rust-version = "1.91"sets the minimum supported Rust version, not a pinned compiler. There’s norust-toolchain*file here, so if you want to describe a toolchain pin, cite the actual CI channel or add a toolchain file; otherwise call this an MSRV note and add the audit date for the dependency-freshness claim.🤖 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 `@KNOWN_ISSUES.md` around lines 94 - 110, Update the Rust toolchain statement in the dependency-status section to describe rust-version = "1.91" as the minimum supported Rust version rather than a pinned compiler. Do not call it pinned unless an actual rust-toolchain file or CI channel is referenced, and add the audit date to the dependency-freshness claim.README.md-126-133 (1)
126-133: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFix the lean-ctx install wording in README.md. It should say the native installer path (
curl | shorbrew+lean-ctx onboard) instead ofnpm install -g lean-ctx-bin, which implies a Node/npm dependency that the installer doesn’t use.🤖 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 `@README.md` around lines 126 - 133, Update the lean-ctx installation wording in the README’s “Each run” description to use the supported native installer path—curl pipe-to-shell or Homebrew followed by lean-ctx onboard—instead of npm install -g lean-ctx-bin. Preserve the existing detection-first and hook/MCP behavior descriptions.src/uninstall.rs-105-112 (1)
105-112: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep
lean-ctxin the OpenCode cleanup filter.The function still deletes
lean-ctx.mdat Lines 98-100, but this filter removes only Exa and Git references. A config containing onlylean-ctx.mdwill retain a dangling instruction, and Line 105 will not even enter the cleanup path.Suggested fix
- if content.contains("agentflare") || content.contains("exa.md") { + if content.contains("agentflare") + || content.contains("exa.md") + || content.contains("git.md") + || content.contains("lean-ctx.md") + { ... !s.contains("exa.md") && !s.contains("git.md") + && !s.contains("lean-ctx.md")🤖 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 `@src/uninstall.rs` around lines 105 - 112, Update the OpenCode cleanup condition and instruction-retention filter in the uninstall flow to include “lean-ctx” alongside the existing Exa and Git patterns. Ensure configurations containing only lean-ctx.md enter the cleanup path and remove their corresponding instruction without changing handling for the existing patterns.src/mcp_prompts.rs-171-171 (1)
171-171: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAlign the handoff destination wording.
The empty-state text says facts and decisions belong in “artifact memory,” while the command branch says they go to artifacts; the schema text also says they belong in memory, not artifacts. Make these consistent so both prompt paths point agents to the same destination.
🤖 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 `@src/mcp_prompts.rs` at line 171, Align the destination wording in the empty-state prompt, command branch, and schema text so they all consistently direct facts and decisions to artifacts, using the existing artifact terminology throughout.
🧹 Nitpick comments (1)
src/init.rs (1)
523-533: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a positive-path stale-rule test
The current test only covers the no-superseded-version case.
rule_text::superseded()still includesexa.md,git.md, andlean-ctx.md, so the stale → prompt → refresh path should have coverage too.🤖 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 `@src/init.rs` around lines 523 - 533, Add a test alongside is_stale_rule_false_for_rule_with_no_superseded_versions that creates a temporary rule file using one of the entries still returned by rule_text::superseded(), such as rule_text::GIT, with superseded content, then asserts is_stale_rule returns true and covers the stale-rule refresh/prompt path as appropriate.
🤖 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 `@crates/gateway-registry/src/audit.rs`:
- Around line 67-79: The audit test’s redaction assertion is disconnected from
its input fixture. Update the `args` value in the test around `record` so it
contains the sensitive `"do-not-leak-me"` value being checked, while preserving
the existing record calls and assertions.
In `@docs/api-sdk-reference.md`:
- Around line 616-624: Update the MCP Server “Capabilities” list in the
documentation to include prompts alongside tools and resources, matching the
implementation’s exposed list_prompts and get_prompt functionality.
- Line 372: Update the Encryption documentation to state that an absent
AGENTFLARE_VAULT_PASSPHRASE triggers an interactive TTY prompt, while
non-interactive operations without a passphrase store files in plaintext; retain
the existing encryption algorithm and file-format details.
In `@docs/architecture-system-design.md`:
- Around line 106-140: Remove the stale COMP --> ENGRAM_INSTALL edge and its
corresponding Engram node from the architecture diagram. In the Component
Registry section, change “Six components per host” to “Five components per host”
while preserving the existing five component entries and descriptions.
- Around line 465-472: The Cache Strategies table’s Dedup keys invalidation
entry incorrectly says they are never invalidated. Update that entry to state
that dedup_keys are removed during the full cache rebuild triggered by a
pricing-fingerprint change, while preserving the exact-key deduplication
behavior otherwise.
In `@docs/deployment-and-operations.md`:
- Line 486: Update the recovery runbook’s vault documentation and related
instructions to use ~/.local/share/agentflare/vault/ as the filesystem directory
containing profile files, and identify auth.db separately as the SQLite
database. Replace references to ~/.agentflare/vault/ and ensure backup and
deletion steps cover both storage locations appropriately.
In `@docs/product-overview.md`:
- Around line 56-61: Update the Supported agents table so Cline and Continue
appear only in the Extension row, remove them from the CLI row, and replace the
duplicated “VS Code Cline/Continue” entries with the correct extension-tier
names.
- Around line 144-150: Update the “MCP Server Integration” section to describe
only the supported session/nudge resources and routing/session-health tools.
Remove claims that MCP can toggle a global on/off flag or provide cost data,
while retaining the accurate stdio transport and MCP integration details.
- Around line 193-211: Rewrite the lifecycle steps in the product overview to
match the implemented hook behavior: describe non-consent component
auto-application during session start, standard/routing context injection during
PromptSubmit, and batching plus schedule-wakeup nudges during PreToolUse.
Correct the default-off escape-hatch description so it does not claim the active
flag disables every hook path, while preserving the documented init and
consent-gated installation behavior.
In `@docs/testing.md`:
- Around line 313-318: Update the end-to-end integration-point list in the
testing documentation to reference the analytics database at
~/.agentflare/analytics.db instead of ~/.local/share/agentflare/rollup.db,
leaving the other listed integration points unchanged.
In `@SECURITY.md`:
- Around line 14-22: Reconcile the lean-ctx installation claims in SECURITY.md
with the actual tool_install/mise_install behavior and the leanctx component
description in components.rs. Remove the inaccurate npm and go install
statements, and accurately describe the native curl/brew installation and any
subprocesses the implementation actually invokes.
In `@src/uninstall.rs`:
- Around line 154-172: The clean_mcp_entry function must also remove legacy
Engram MCP configuration during uninstall. Extend its matching and removal logic
to recognize the Engram entry and delete the engram server key and corresponding
.continue/mcpServers/engram.json file, while keeping Engram excluded from new
registration.
---
Minor comments:
In `@docs/api-sdk-reference.md`:
- Line 134: Update all fenced code blocks in docs/api-sdk-reference.md,
including the listed locations, to specify an appropriate language identifier
such as bash, json, rust, or text. Preserve each block’s existing content and
choose identifiers that match the enclosed examples so markdownlint MD040
passes.
- Around line 171-188: Update the hook event documentation to include the
supported session-end event defined by SessionEnd in src/cli/hook.rs. Add
session-end to the EVENT parameter’s allowed values and document it in the
Events table as the supported no-op command, while preserving the existing event
descriptions and hook protocol notes.
In `@docs/architecture-system-design.md`:
- Line 30: Update the fenced code blocks in the architecture documentation,
including the blocks near the referenced locations, to specify appropriate
language identifiers such as text or tree. Ensure every fence has a valid
identifier so markdownlint MD040 passes without changing the block contents.
In `@docs/business-overview.md`:
- Line 249: Clean up the reported wording in the documentation entries around
“Cache token tracking” and the additional referenced sections, correcting
LanguageTool’s spelling, compound-adjective, platform-name, and hyphenation
findings. Preserve the documented meaning while applying consistent polished
terminology across all affected lines.
In `@docs/data-model.md`:
- Line 422: Reconcile the pricing-entry count in the documentation by verifying
the actual pricing data and updating this section and every related API/testing
reference to use that single count. Preserve the listed aliases, tier pricing,
and modifier metadata details.
In `@docs/deployment-and-operations.md`:
- Around line 359-360: Resolve the documentation lint findings in the deployment
and operations document: apply the reported LanguageTool capitalization and
wording corrections to the affected table entries and corresponding lines, and
add an appropriate language identifier to the fenced code block flagged by
markdownlint. Preserve the existing deployment instructions and technical
meaning.
In `@docs/product-overview.md`:
- Line 231: Update the project-structure fenced code block in the product
overview documentation to include an explicit language identifier, using text or
another suitable language, so it satisfies markdownlint MD040.
In `@docs/security.md`:
- Line 243: Update the fenced code blocks at the referenced locations in
docs/security.md to include language identifiers: use text for the wire-format
block and appropriate identifiers for the lifecycle and checksum blocks,
resolving markdownlint MD040 without changing their contents.
- Line 272: Correct the platform-name capitalization in the GitHub references
within the security documentation, including the additional occurrence noted by
the review. Use the official “GitHub” capitalization consistently without
changing the surrounding security or update details.
In `@docs/testing.md`:
- Line 90: Update the directory-layout fenced code block in the testing
documentation to specify the text language identifier, resolving the
markdownlint MD040 warning while preserving its contents.
- Around line 525-531: Update the “Embedded Test Data” documentation to use the
actual entry count from data/anthropic-pricing.json, and reconcile every other
pricing-entry count reference in the documentation to that same value. Treat the
data file, not the existing prose, as the source of truth.
In `@KNOWN_ISSUES.md`:
- Around line 270-278: Update the Technical Debt count in the summary table to
reflect all seven listed entries, including “Inconsistent error handling
patterns,” or explicitly document which entry is intentionally excluded.
- Around line 18-22: Update the “Hung child process on version probe timeout”
section in KNOWN_ISSUES.md to replace “zombie processes” with “orphaned/running
child processes,” while preserving the existing description of the timeout leak
and its impact.
- Around line 236-244: Update the “State management is split across JSON and
SQLite” section in KNOWN_ISSUES.md to state that the codebase uses four separate
state stores, matching the four enumerated files. Leave the listed stores and
consolidation recommendation unchanged.
- Around line 94-110: Update the Rust toolchain statement in the
dependency-status section to describe rust-version = "1.91" as the minimum
supported Rust version rather than a pinned compiler. Do not call it pinned
unless an actual rust-toolchain file or CI channel is referenced, and add the
audit date to the dependency-freshness claim.
In `@README.md`:
- Around line 126-133: Update the lean-ctx installation wording in the README’s
“Each run” description to use the supported native installer path—curl
pipe-to-shell or Homebrew followed by lean-ctx onboard—instead of npm install -g
lean-ctx-bin. Preserve the existing detection-first and hook/MCP behavior
descriptions.
In `@src/mcp_prompts.rs`:
- Line 171: Align the destination wording in the empty-state prompt, command
branch, and schema text so they all consistently direct facts and decisions to
artifacts, using the existing artifact terminology throughout.
In `@src/uninstall.rs`:
- Around line 105-112: Update the OpenCode cleanup condition and
instruction-retention filter in the uninstall flow to include “lean-ctx”
alongside the existing Exa and Git patterns. Ensure configurations containing
only lean-ctx.md enter the cleanup path and remove their corresponding
instruction without changing handling for the existing patterns.
---
Nitpick comments:
In `@src/init.rs`:
- Around line 523-533: Add a test alongside
is_stale_rule_false_for_rule_with_no_superseded_versions that creates a
temporary rule file using one of the entries still returned by
rule_text::superseded(), such as rule_text::GIT, with superseded content, then
asserts is_stale_rule returns true and covers the stale-rule refresh/prompt path
as appropriate.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f0cb918d-26a6-4d46-9da9-d18a1939a5b4
📒 Files selected for processing (29)
AGENTS.mdCONTRIBUTING.mdKNOWN_ISSUES.mdREADME.mdSECURITY.mdcrates/agentflare-artifacts/CHANGELOG.mdcrates/gateway-registry/src/audit.rsdocs/api-sdk-reference.mddocs/architecture-system-design.mddocs/business-overview.mddocs/data-model.mddocs/deployment-and-operations.mddocs/product-overview.mddocs/security.mddocs/testing.mdmise.local.tomlsrc/cli/hook.rssrc/components.rssrc/config.rssrc/engram_install.rssrc/hook.rssrc/init.rssrc/main.rssrc/mcp_prompts.rssrc/mcp_server.rssrc/mise_install.rssrc/rule_text.rssrc/tool_install.rssrc/uninstall.rs
💤 Files with no reviewable changes (4)
- AGENTS.md
- src/engram_install.rs
- src/main.rs
- src/config.rs
| AUTH --> AUTH_DB | ||
| AUTH --> AUTH_CRYPT | ||
| AUTH --> AUTH_RUNNER | ||
|
|
||
| COMP --> ENGRAM_INSTALL | ||
| COMP --> RULE_TEXT | ||
| COMP --> STATE | ||
|
|
||
| MCP --> OPTIMIZE | ||
| ``` | ||
|
|
||
| ## Core Components | ||
|
|
||
| ### 1. CLI Dispatch (`src/main.rs`) | ||
|
|
||
| The entry point. Uses `clap` with `#[derive(Parser, Subcommand)]` for a fully typed command tree. Each subcommand dispatches to a module's `run()` or `cli_*()` function. Error handling uses `color_eyre` for human-readable backtraces. | ||
|
|
||
| **Subcommands**: `init`, `hook` (with `session-start`/`prompt-submit`/`pre-tool-use`), `cost`, `coaching`, `mcp`, `agents` (with `list`/`doctor`/`install`/`update`/`uninstall`/`launch`), `auth` (with 15+ sub-subcommands), `alias`, `update`, `uninstall`. | ||
|
|
||
| ### 2. Component Registry (`src/components.rs`) | ||
|
|
||
| The architectural linchpin. A `Component` is a tuple of `(id, needs_consent, describe, check, apply)`: | ||
|
|
||
| - **`check`**: Returns `true` if the component is already satisfied (idempotent). | ||
| - **`apply`**: Runs the fix and returns a human-readable result string. | ||
| - **`needs_consent`**: If `true`, the component is only installed during explicit `init` (user consent). If `false`, it can auto-heal during `hook session-start`. | ||
|
|
||
| Six components per host: | ||
| | Component | Consent | What it does | | ||
| |-----------|---------|--------------| | ||
| | `rules` | No | Writes Exa/Git/lean-ctx rule files per host | | ||
| | `leanctx` | Yes | `npm install -g lean-ctx-bin && lean-ctx onboard` | | ||
| | `ponytail-plugin` | Yes | Claude Code only: installs Ponytail plugin | | ||
| | `ponytail-mode` | No | Pins Ponytail to "ultra" mode (Claude Code only) | | ||
| | `caveman-mode` | No | Pins Caveman to "ultra" mode (Claude Code only) | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the stale Engram edge and correct the component count.
The diagram still contains COMP --> ENGRAM_INSTALL, contradicting the Engram-removal objective. The following section also claims six components while listing only five. Remove the Engram node and update the count/table to match the current registry.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 134-134: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 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 `@docs/architecture-system-design.md` around lines 106 - 140, Remove the stale
COMP --> ENGRAM_INSTALL edge and its corresponding Engram node from the
architecture diagram. In the Component Registry section, change “Six components
per host” to “Five components per host” while preserving the existing five
component entries and descriptions.
master already carried its own engram removal (PR #157) plus several unrelated advances since this branch diverged: built-in memory docs, agentflare run/mise refactor, generalized merge_opencode_mcp, and more thorough uninstall cleanup (engram.md/engram.json entries this branch's version missed). In every conflict, master's side was the superset - kept it throughout, dropping only this branch's now-superseded duplicate engram-removal edits.
- deployment-and-operations.md: vault/auth.db paths corrected to ~/.local/share/agentflare/ (were stated as ~/.agentflare/) - architecture-system-design.md: leanctx component description was still describing the old npm-based install, now matches the native installer - 24 fenced code blocks across 8 docs files were missing a language identifier (markdownlint MD040) - tagged as `text` The other 9 of the original 12 actionable comments were already resolved by this branch's prior "fix: address all CodeRabbit inline comments" commit; verified each against current source (auth.rs, auth_db.rs, rollup.rs, mcp_server.rs) rather than trusting the stale review, since it predates that commit.
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)
src/uninstall.rs (1)
52-55: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winContinue deleting legacy Engram rule files during uninstall.
Both cleanup lists omit
engram.md, so existing Claude/OpenCode installations can retain the removed Engram instructions after uninstall. Add the legacy filename to both lists.Suggested fix
- for f in &["exa.md", "git.md", "lean-ctx.md"] { + for f in &["exa.md", "git.md", "lean-ctx.md", "engram.md"] {Apply this in both
clean_claude_codeandclean_opencode.Also applies to: 105-108
🤖 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 `@src/uninstall.rs` around lines 52 - 55, Add "engram.md" to the filename lists iterated by both clean_claude_code and clean_opencode, ensuring uninstall removes the legacy Engram rule file for Claude and OpenCode installations while preserving the existing cleanup behavior.
🤖 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 `@src/uninstall.rs`:
- Around line 117-120: Update the content guard in the uninstall cleanup flow to
also match the managed instruction markers “git.md” and “lean-ctx.md”, alongside
the existing “agentflare”, “exa.md”, and “engram.md” checks, so configurations
containing only either marker reach the serde_json cleanup path.
---
Outside diff comments:
In `@src/uninstall.rs`:
- Around line 52-55: Add "engram.md" to the filename lists iterated by both
clean_claude_code and clean_opencode, ensuring uninstall removes the legacy
Engram rule file for Claude and OpenCode installations while preserving the
existing cleanup behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f355c64-ab37-4588-ab1c-60e660a28c13
📒 Files selected for processing (12)
crates/agentflare-artifacts/CHANGELOG.mdcrates/gateway-registry/src/audit.rsdocs/api-sdk-reference.mddocs/architecture-system-design.mddocs/deployment-and-operations.mddocs/feature-agent-lifecycle-cli.mddocs/feature-caam-auth-vault.mddocs/product-overview.mddocs/security.mddocs/testing.mdsrc/components.rssrc/uninstall.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/product-overview.md
- docs/testing.md
- docs/architecture-system-design.md
- docs/deployment-and-operations.md
- docs/api-sdk-reference.md
- docs/security.md
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
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)
src/uninstall.rs (1)
52-55: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winContinue deleting legacy Engram rule files during uninstall.
Both cleanup lists omit
engram.md, so existing Claude/OpenCode installations can retain the removed Engram instructions after uninstall. Add the legacy filename to both lists.Suggested fix
- for f in &["exa.md", "git.md", "lean-ctx.md"] { + for f in &["exa.md", "git.md", "lean-ctx.md", "engram.md"] {Apply this in both
clean_claude_codeandclean_opencode.Also applies to: 105-108
🤖 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 `@src/uninstall.rs` around lines 52 - 55, Add "engram.md" to the filename lists iterated by both clean_claude_code and clean_opencode, ensuring uninstall removes the legacy Engram rule file for Claude and OpenCode installations while preserving the existing cleanup behavior.
🤖 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 `@src/uninstall.rs`:
- Around line 117-120: Update the content guard in the uninstall cleanup flow to
also match the managed instruction markers “git.md” and “lean-ctx.md”, alongside
the existing “agentflare”, “exa.md”, and “engram.md” checks, so configurations
containing only either marker reach the serde_json cleanup path.
---
Outside diff comments:
In `@src/uninstall.rs`:
- Around line 52-55: Add "engram.md" to the filename lists iterated by both
clean_claude_code and clean_opencode, ensuring uninstall removes the legacy
Engram rule file for Claude and OpenCode installations while preserving the
existing cleanup behavior.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f355c64-ab37-4588-ab1c-60e660a28c13
📒 Files selected for processing (12)
crates/agentflare-artifacts/CHANGELOG.mdcrates/gateway-registry/src/audit.rsdocs/api-sdk-reference.mddocs/architecture-system-design.mddocs/deployment-and-operations.mddocs/feature-agent-lifecycle-cli.mddocs/feature-caam-auth-vault.mddocs/product-overview.mddocs/security.mddocs/testing.mdsrc/components.rssrc/uninstall.rs
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/product-overview.md
- docs/testing.md
- docs/architecture-system-design.md
- docs/deployment-and-operations.md
- docs/api-sdk-reference.md
- docs/security.md
🛑 Comments failed to post (1)
src/uninstall.rs (1)
117-120: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Include all managed instruction markers in the OpenCode cleanup guard.
The body removes
git.mdandlean-ctx.md, but this guard does not check for either marker. A config containing only one of them is skipped and remains unchanged.Suggested fix
if (content.contains("agentflare") || content.contains("exa.md") + || content.contains("git.md") + || content.contains("lean-ctx.md") || content.contains("engram.md"))📝 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.if (content.contains("agentflare") || content.contains("exa.md") || content.contains("git.md") || content.contains("lean-ctx.md") || content.contains("engram.md")) && let Ok(mut config) = serde_json::from_str::<Value>(&content)🤖 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 `@src/uninstall.rs` around lines 117 - 120, Update the content guard in the uninstall cleanup flow to also match the managed instruction markers “git.md” and “lean-ctx.md”, alongside the existing “agentflare”, “exa.md”, and “engram.md” checks, so configurations containing only either marker reach the serde_json cleanup path.
1 participant
Summary
Remove the engram cross-session memory integration (Gentleman-Programming/engram) from agentflare. Deletes engram_install.rs, the handoff-on-exit feature, config.rs (handoff-only), and all references in source + docs.
Changes
Test plan
Notes for reviewers
Summary by CodeRabbit
Documentation
Bug Fixes
engram.mdrule files.Chores