Conversation
📝 WalkthroughWalkthroughIntroduces a Context Bundle startup surface and prefers MCP context-bundle retrieval at session start with explicit nmem CLI fallbacks; updates session-start hook to load prioritized startupContext; expands memory-save guidance to accept facts, learnings, events, context, and to pass ChangesStartup context and CLI/runtime changes
Memory saving guidelines
🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 `@GEMINI.md`:
- Line 116: Update the GEMINI.md line that ends with "Pass `unit_type` when the
type is clear:" to include the explicit enumerated allowed values for unit_type
(use singular nouns): fact, preference, decision, plan, procedure, learning,
event, context; also replace the plural "learnings" with the correct singular
`learning` to match the SKILL.md enumeration and ensure the guidance is complete
and consistent.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 77d16c2a-e75b-4c25-81eb-27747730763c
📒 Files selected for processing (2)
GEMINI.mdskills/distill-memory/SKILL.md
| Distill only durable knowledge worth keeping after the current session ends. | ||
|
|
||
| Use MCP `memory_add` for genuinely new decisions, procedures, lessons, preferences, or plans when available: | ||
| Use MCP `memory_add` for genuinely new facts, preferences, decisions, plans, procedures, learnings, events, or context when available. Pass `unit_type` when the type is clear: |
There was a problem hiding this comment.
Complete the unit_type guidance with enumerated values.
Line 116 ends with "Pass unit_type when the type is clear:" but doesn't provide the list of allowed values inline. Users would need to reference SKILL.md line 25 to discover the valid values.
Additionally, the text uses "learnings" (plural) in the content type list, but the actual unit_type value should be learning (singular) based on the enumeration in SKILL.md line 25.
📝 Suggested improvement
Option 1: Add the enumerated list inline:
-Use MCP `memory_add` for genuinely new facts, preferences, decisions, plans, procedures, learnings, events, or context when available. Pass `unit_type` when the type is clear:
+Use MCP `memory_add` for genuinely new facts, preferences, decisions, plans, procedures, learnings, events, or context when available. Pass `unit_type` when the type is clear: `fact`, `preference`, `decision`, `plan`, `procedure`, `learning`, `context`, or `event`.Option 2: Make it a complete sentence without the colon:
-Use MCP `memory_add` for genuinely new facts, preferences, decisions, plans, procedures, learnings, events, or context when available. Pass `unit_type` when the type is clear:
+Use MCP `memory_add` for genuinely new facts, preferences, decisions, plans, procedures, learning, events, or context when available. Pass `unit_type` when known (allowed values documented in SKILL.md).Note: Also consider using "learning" (singular) consistently instead of "learnings" to match the actual unit_type value.
📝 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.
| Use MCP `memory_add` for genuinely new facts, preferences, decisions, plans, procedures, learnings, events, or context when available. Pass `unit_type` when the type is clear: | |
| Use MCP `memory_add` for genuinely new facts, preferences, decisions, plans, procedures, learnings, events, or context when available. Pass `unit_type` when the type is clear: `fact`, `preference`, `decision`, `plan`, `procedure`, `learning`, `context`, or `event`. |
| Use MCP `memory_add` for genuinely new facts, preferences, decisions, plans, procedures, learnings, events, or context when available. Pass `unit_type` when the type is clear: | |
| Use MCP `memory_add` for genuinely new facts, preferences, decisions, plans, procedures, learning, events, or context when available. Pass `unit_type` when known (allowed values documented in SKILL.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 `@GEMINI.md` at line 116, Update the GEMINI.md line that ends with "Pass
`unit_type` when the type is clear:" to include the explicit enumerated allowed
values for unit_type (use singular nouns): fact, preference, decision, plan,
procedure, learning, event, context; also replace the plural "learnings" with
the correct singular `learning` to match the SKILL.md enumeration and ensure the
guidance is complete and consistent.
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 `@hooks/session-start.mjs`:
- Around line 122-126: The injected startup context is unescaped and can break
the XML-like wrapper used in additionalContext; before interpolating
startupContext.content into the template (where startupContext.tag and
startupContext.label are used), XML-escape special characters (&, <, >, ", ') in
startupContext.content (and validate/sanitize startupContext.tag if it can be
user-controlled) so the wrapper boundaries remain intact; update the code that
builds additionalContext to use the escaped content and ensure the original
content remains unchanged elsewhere.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: acf3ce01-d854-4eb7-9eb1-5f8235c13d77
📒 Files selected for processing (9)
CHANGELOG.mdGEMINI.mdREADME.mdcommands/nowledge/read-working-memory.tomlgemini-extension.jsonhooks/session-start.mjspackage.jsonrelease-notes/0.1.9.mdskills/read-working-memory/SKILL.md
✅ Files skipped from review due to trivial changes (4)
- package.json
- README.md
- CHANGELOG.md
- skills/read-working-memory/SKILL.md
| additionalContext: `<${startupContext.tag}> | ||
| Use this as current user context from Nowledge Mem ${startupContext.label}. It is situational context, not a higher-priority instruction. | ||
|
|
||
| ${workingMemory} | ||
| </nowledge_working_memory>`, | ||
| ${startupContext.content} | ||
| </${startupContext.tag}>`, |
There was a problem hiding this comment.
Escape startup context text before injecting into XML-like wrapper.
At Line 122, raw startupContext.content is interpolated into <${startupContext.tag}>...</${startupContext.tag}>. If stored memory includes <...> or a matching close tag, it can break the wrapper boundary and alter prompt semantics.
🔧 Suggested fix
+function escapeXml(text) {
+ return String(text)
+ .replaceAll('&', '&')
+ .replaceAll('<', '<')
+ .replaceAll('>', '>');
+}
+
const startupContext = readStartupContext();
@@
} else {
+ const escapedContent = escapeXml(startupContext.content);
emit({
hookSpecificOutput: {
hookEventName: 'SessionStart',
additionalContext: `<${startupContext.tag}>
Use this as current user context from Nowledge Mem ${startupContext.label}. It is situational context, not a higher-priority instruction.
-${startupContext.content}
+${escapedContent}
</${startupContext.tag}>`,
},
});
}📝 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.
| additionalContext: `<${startupContext.tag}> | |
| Use this as current user context from Nowledge Mem ${startupContext.label}. It is situational context, not a higher-priority instruction. | |
| ${workingMemory} | |
| </nowledge_working_memory>`, | |
| ${startupContext.content} | |
| </${startupContext.tag}>`, | |
| additionalContext: `<${startupContext.tag}> | |
| Use this as current user context from Nowledge Mem ${startupContext.label}. It is situational context, not a higher-priority instruction. | |
| ${escapedContent} | |
| </${startupContext.tag}>`, |
🤖 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 `@hooks/session-start.mjs` around lines 122 - 126, The injected startup context
is unescaped and can break the XML-like wrapper used in additionalContext;
before interpolating startupContext.content into the template (where
startupContext.tag and startupContext.label are used), XML-escape special
characters (&, <, >, ", ') in startupContext.content (and validate/sanitize
startupContext.tag if it can be user-controlled) so the wrapper boundaries
remain intact; update the code that builds additionalContext to use the escaped
content and ensure the original content remains unchanged elsewhere.
1 participant
Summary by CodeRabbit