Conversation
When native `nmem` isn't on PATH (common in WSL), hooks now define a bash function that bridges to `nmem.cmd` via Windows interop. This lets SessionStart, compaction, UserPromptSubmit, and Stop hooks work transparently in WSL without requiring the shim to be pre-installed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaces direct nmem invocations in hooks with a cross-platform shell wrapper that checks PATH and defines a fallback Changes
Sequence Diagram(s)sequenceDiagram
participant HookRunner as Hook Runner
participant Shell as Shell/Script
participant Nmem as nmem (native)
participant NmemCmd as nmem.cmd (Windows proxy)
HookRunner->>Shell: execute hook command
Shell->>Shell: check if `nmem` in PATH
alt nmem found
Shell->>Nmem: call nmem with args (read/compact/save/etc)
Nmem-->>Shell: results
else nmem missing (Windows)
Shell->>NmemCmd: define / proxy to cmd.exe and call nmem.cmd
NmemCmd-->>Shell: results
end
Shell-->>HookRunner: complete hook flow (memory read, fallback markdown, compaction, save)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
nowledge-mem-claude-code-plugin/hooks/hooks.json (1)
1-45: Consider extracting the WSL wrapper to reduce duplication.The fallback pattern is repeated across multiple hooks. A cleaner approach might be sourcing a shared script or using a top-level environment setup. This would also make it easier to add the
uvx --from nmem-cli nmemfallback mentioned in project learnings for non-Windows environments.Additionally, note that commands in
commands/save.mdandskills/save-thread/SKILL.mdinvokenmemdirectly without the hook wrapper—those paths will still fail in WSL without the manual shim from the README.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-claude-code-plugin/hooks/hooks.json` around lines 1 - 45, Multiple hooks (SessionStart, UserPromptSubmit, Stop) repeat the WSL nmem wrapper string ("command -v nmem >/dev/null 2>&1 || nmem(){ cmd.exe /s /c \"\\\"nmem.cmd\\\" $*\"; };") in their "command" values; extract that fallback into a single reusable shim (e.g., source a top-level script or set an environment variable) and replace each inline wrapper with a short call to that shim, update the shim to include the additional uvx fallback ("uvx --from nmem-cli nmem") for non-Windows environments, and ensure commands referenced in commands/save.md and skills/save-thread/SKILL.md also call the same shim so they work under WSL.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nowledge-mem-claude-code-plugin/hooks/hooks.json`:
- Line 19: The shell wrapper definition for the nmem function in the "command"
string uses $* which does not preserve argument boundaries; update the nmem()
wrapper to use "$@" instead (i.e., nmem(){ cmd.exe /s /c "\"nmem.cmd\"" "$@";
};) so all arguments, especially those with spaces, are passed intact to
nmem.cmd; locate the "command" value containing the nmem() definition and
replace the $* instance with "$@" consistently.
- Line 29: The hook prints nmem usage but fails to define the nmem() fallback,
so in WSL users will see commands that won't run; update the hooks.json entry to
include the same fallback wrapper function used on lines with the other hooks
(i.e., define a shell function named nmem() that invokes nmem.cmd when nmem is
absent), referencing the existing pattern for the nmem() fallback so suggested
commands like `nmem --json m search "query"` will actually work on systems where
only nmem.cmd exists.
- Around line 39-40: The command string defines a shell wrapper function named
nmem that forwards arguments using unquoted $*, which can break with spaces;
update the wrapper in the JSON ("command" value) to forward arguments as "$@"
(properly escaped in the JSON string) so the nmem function uses "\"nmem.cmd\"
\"$@\"" when invoked; keep the rest of the line including the async: true and
the trailing "|| true" unchanged.
- Line 10: The inline fallback shell function nmem(){ ... } uses $* which loses
argument boundaries and breaks commands with spaces (e.g., nmem m add "multi
word insight"); update that fallback to preserve argument quoting by replacing
$* with "$@" or implement the README's quoting loop pattern so the nmem()
function forwards arguments exactly as received to nmem.cmd; locate the
"command" string in hooks.json and adjust the nmem(){ ... } definition
accordingly to use "$@" (or the quoting loop) before invoking nmem --json wm
read.
---
Nitpick comments:
In `@nowledge-mem-claude-code-plugin/hooks/hooks.json`:
- Around line 1-45: Multiple hooks (SessionStart, UserPromptSubmit, Stop) repeat
the WSL nmem wrapper string ("command -v nmem >/dev/null 2>&1 || nmem(){ cmd.exe
/s /c \"\\\"nmem.cmd\\\" $*\"; };") in their "command" values; extract that
fallback into a single reusable shim (e.g., source a top-level script or set an
environment variable) and replace each inline wrapper with a short call to that
shim, update the shim to include the additional uvx fallback ("uvx --from
nmem-cli nmem") for non-Windows environments, and ensure commands referenced in
commands/save.md and skills/save-thread/SKILL.md also call the same shim so they
work under WSL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7a27ccf7-129c-4ae9-bb71-c21c2735d257
📒 Files selected for processing (1)
nowledge-mem-claude-code-plugin/hooks/hooks.json
Address review feedback: - Replace $* with "$@" for-loop quoting to preserve argument boundaries (e.g. `nmem m add "hello world"` no longer flattens into two args) - Gate function definition on `command -v nmem.cmd` so the fallback is only defined in WSL — prevents false positives on plain Linux - UserPromptSubmit now defines the wrapper function so the agent can actually execute the nmem commands shown in the nudge text Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When native
nmemisn't on PATH (common in WSL), hooks now define a bash function that bridges tonmem.cmdvia Windows interop. This lets SessionStart, compaction, UserPromptSubmit, and Stop hooks work transparently in WSL without requiring the shim to be pre-installed.Summary by CodeRabbit