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:
📝 WalkthroughWalkthroughOpenClaw and multiple integrations/plugins were extended with optional ambient "spaces" (lanes) and space-resolution plumbing; OpenClaw session lifecycle now tracks conversation roots to derive stable thread IDs, distinguishing Changes
Sequence Diagram(s)sequenceDiagram
participant OC as OpenClaw Runtime
participant Hook as Session Lifecycle Hooks
participant Root as Conversation Root Tracker
participant Thread as Thread ID Resolver
participant Mem as Nowledge Mem Client/API
rect rgba(100,150,200,0.5)
Note over OC,Hook: Session start / resume
OC->>Hook: emit session_start(event)
Hook->>Root: registerSessionStartConversation(event, ctx)
Root->>Root: map sessionId → conversationRoot
OC->>Thread: buildStableThreadId(event, ctx)
Thread->>Root: resolveConversationRoot(sessionId)
Root-->>Thread: conversationRoot
Thread->>Mem: derive stable_thread_id (hash(root))
Mem-->>OC: use stable_thread_id for reads/writes
end
rect rgba(150,100,100,0.5)
Note over OC,Hook: Session end — compaction
OC->>Hook: emit session_end(reason:"compaction", nextSessionId)
Hook->>Root: registerSessionEndConversation(event)
Root->>Root: preserve root for nextSessionId
OC->>Mem: continue using same thread id (no fork)
end
rect rgba(150,150,100,0.5)
Note over OC,Hook: Session end — new/reset
OC->>Hook: emit session_end(reason:"new", nextSessionId)
Hook->>Root: registerSessionEndConversation(event)
Root->>Root: switch root for nextSessionId
OC->>Thread: buildStableThreadId(nextEvent, ctx)
Thread->>Root: resolveConversationRoot(nextSessionId)
Root-->>Thread: new root → different thread id
Thread->>Mem: new thread used for reads/writes
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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
🧹 Nitpick comments (1)
nowledge-mem-openclaw-plugin/tests/capture-thread-identity.test.mjs (1)
10-82: Consider adding test isolation utilities.The tests modify module-level state (
_conversationRootsBySessionId,_activeConversationRootsBySessionKey) that persists across test runs. While the current tests use unique identifiers to avoid interference, future test additions might benefit from an exported_resetConversationRoots()helper for explicit cleanup between tests.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-openclaw-plugin/tests/capture-thread-identity.test.mjs` around lines 10 - 82, Tests mutate module-level maps _conversationRootsBySessionId and _activeConversationRootsBySessionKey and need explicit reset to avoid cross-test leakage; add an exported helper function named _resetConversationRoots() in the module that defines registerSessionEndConversation/registerSessionStartConversation/buildStableThreadId which clears those maps (and any related module-level state), then call _resetConversationRoots() in these tests (e.g., in a beforeEach/afterEach or at the top of each test) to ensure isolation.
🤖 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-openclaw-plugin/src/tools/status.js`:
- Around line 72-73: The bug is that ceSlot is initialized from
details.contextEngineSlot (which is uninitialized) instead of the local
contextEngineSlot variable; change the initialization to read from the local
contextEngineSlot (e.g., const ceSlot = contextEngineSlot) and then assign it
back to details.contextEngineSlot as needed so subsequent checks using ceSlot
(and later references to details.contextEngineSlot) use the correct value.
---
Nitpick comments:
In `@nowledge-mem-openclaw-plugin/tests/capture-thread-identity.test.mjs`:
- Around line 10-82: Tests mutate module-level maps
_conversationRootsBySessionId and _activeConversationRootsBySessionKey and need
explicit reset to avoid cross-test leakage; add an exported helper function
named _resetConversationRoots() in the module that defines
registerSessionEndConversation/registerSessionStartConversation/buildStableThreadId
which clears those maps (and any related module-level state), then call
_resetConversationRoots() in these tests (e.g., in a beforeEach/afterEach or at
the top of each test) to ensure isolation.
🪄 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: 5d02af57-eb94-4a54-a46d-61897ff880f7
⛔ Files ignored due to path filters (1)
nowledge-mem-openclaw-plugin/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
README.mddocs/PLUGIN_DEVELOPMENT_GUIDE.mdintegrations.jsonnowledge-mem-openclaw-plugin/CHANGELOG.mdnowledge-mem-openclaw-plugin/README.mdnowledge-mem-openclaw-plugin/openclaw.plugin.jsonnowledge-mem-openclaw-plugin/package.jsonnowledge-mem-openclaw-plugin/src/hooks/capture.jsnowledge-mem-openclaw-plugin/src/index.jsnowledge-mem-openclaw-plugin/src/tools/status.jsnowledge-mem-openclaw-plugin/tests/capture-thread-identity.test.mjsshared/behavioral-guidance.md
Member
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nowledge-mem-openclaw-plugin/src/client.js (1)
75-91:⚠️ Potential issue | 🟠 MajorPreserve an explicit “Default” choice instead of falling back to inherited env.
This
||chain treatsspace: ""as “missing”, so a profile that intentionally stays onDefaultstill inheritsNMEM_SPACE/NMEM_SPACE_ID. That defeats the config precedence already computed inparseConfig()and can send memory/thread traffic to the wrong lane.Suggested fix
- this._spaceRef = ( - credentials.space || - credentials.spaceId || - process.env.NMEM_SPACE || - process.env.NMEM_SPACE_ID || - "" - ).trim(); + const resolvedSpace = + credentials.space ?? + credentials.spaceId ?? + process.env.NMEM_SPACE ?? + process.env.NMEM_SPACE_ID ?? + ""; + this._spaceRef = String(resolvedSpace).trim();Based on learnings, "Profile- or provider-owned space config should be the primary lane control when the host supports it (ambient space rule)."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-openclaw-plugin/src/client.js` around lines 75 - 91, The space selection currently uses an || chain so an explicitly provided empty string in credentials (e.g. credentials.space = "") is treated as "missing" and falls back to NMEM_SPACE / NMEM_SPACE_ID; change the logic in the constructor that sets this._spaceRef to prefer an explicitly provided credential value (even if empty) over environment variables by testing presence rather than truthiness — e.g. check for the existence of credentials.space or credentials.spaceId (using hasOwnProperty or 'in') and use their trimmed value if present, otherwise fall back to process.env.NMEM_SPACE / process.env.NMEM_SPACE_ID and finally default to "" so parseConfig()’s explicit profile/provider choice is preserved.
♻️ Duplicate comments (1)
nowledge-mem-openclaw-plugin/src/tools/status.js (1)
72-73:⚠️ Potential issue | 🔴 CriticalInitialize
ceSlotfromruntimeInfo.contextEngineSlot, notdetails.
details.contextEngineSlothas not been assigned yet here, soceSlotbecomesundefinedand the later checks at Lines 104, 114, and 124 can misreport Context Engine state and capture routing.🐛 Proposed fix
- const ceSlot = details.contextEngineSlot; - details.contextEngineSlot = ceSlot; + const ceSlot = contextEngineSlot; + details.contextEngineSlot = ceSlot;Based on learnings:
nowledge_mem_statustool must display plugin trust status, memory slot status, effective config, backend connectivity, and version.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-openclaw-plugin/src/tools/status.js` around lines 72 - 73, The code initializes ceSlot from details.contextEngineSlot which is still unassigned, causing false undefined reads later; update the initialization to read from runtimeInfo.contextEngineSlot (i.e., set ceSlot = runtimeInfo.contextEngineSlot) and then assign details.contextEngineSlot = ceSlot so later checks in the status tool (the checks referencing ceSlot at/around the lines that report Context Engine state and capture routing) use the actual runtime value; ensure runtimeInfo is the source of truth for contextEngineSlot in the same function where ceSlot and details are used.
🤖 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-alma-plugin/main.js`:
- Around line 44-69: The function resolveAmbientSpace currently treats empty
strings from getSetting as "not set" and falls through to env; change
resolveAmbientSpace to treat an explicit setting (even the empty string) as
authoritative: call getSetting for "nowledgeMem.space" and if it returns any
string value (including ""), return { space: normalizeSpaceRef(value), source:
"settings" } so an explicit empty opt-out is preserved; similarly, for
"nowledgeMem.spaceTemplate", treat any returned string as authoritative, run
resolveSpaceTemplate on it and return { space: normalizeSpaceRef(resolved),
source: "settings:template" } even when that resolves to "", only falling back
to env/default when getSetting returns undefined / no key (or when settingsApi
indicates the key is not present).
- Around line 255-267: The catch currently swallows all errors from the
this._fetch("/agent/working-memory") call and returns available:false for
non-default spaces; change it to only treat a genuine "not found/unsupported"
response as a safe fallback and rethrow all other errors. Concretely, in the
catch after calling this._fetch, inspect the thrown error for an HTTP status
(e.g., error.response?.status === 404 or a specific 'unsupported'/code from the
backend) or parse error.message, and only return { available:false, content:"" }
when the error indicates 404/unsupported; for authentication failures, 5xx,
network errors, etc., rethrow the error so callers can surface failures. Use the
existing this._fetch and this._spaceRef references to locate and update the
handler.
In `@nowledge-mem-hermes/provider.py`:
- Line 245: The timeout parsing at the top-level uses int(config.get("timeout",
30) or 30) which can raise ValueError on invalid user input (e.g., "abc"); wrap
this conversion in a try/except (catch ValueError and TypeError), default to 30
(or the desired safe default) on error, and emit a warning via the module logger
(or the provider's logger) so startup continues safely; locate the conversion
around the timeout variable in provider.py and replace it with a guarded parse
that falls back to the default and logs the bad value.
In `@nowledge-mem-hermes/README.md`:
- Around line 134-136: The README shows only the NMEM_SPACE variable assignment
which isn't runnable; update the example to include a runnable form by either
showing how to export the environment variable (e.g., use NMEM_SPACE in an
export command) or presenting an inline launch command that sets NMEM_SPACE for
a single command; reference the NMEM_SPACE variable in the README example and
replace the bare assignment with one clear runnable variant (export
NMEM_SPACE=... or NMEM_SPACE="..." <command>).
In `@nowledge-mem-openclaw-plugin/src/client.js`:
- Around line 109-128: The helpers _withSpaceQuery and _withSpaceBody should be
applied centrally in apiJson so every request (including HTTP fallback paths
like searchTemporal -> /memories/search) is automatically scoped to
this._spaceRef; update the apiJson function to pass its path through
this._withSpaceQuery(...) and its JSON body through this._withSpaceBody(...)
before building/fetching the request (ensure both the primary fetch flow and any
HTTP/fallback branch use these transformed values), leaving searchTemporal and
other callers unchanged.
In `@nowledge-mem-openclaw-plugin/src/config.js`:
- Around line 381-390: The current spaceChoice construction eagerly calls
resolveEnvTemplate on fst/pst which can throw even when a higher-priority source
(fs or ps) should win; change to lazy evaluation so resolveEnvTemplate is only
invoked for the chosen branch: update the spaceChoice entries for the template
sources to provide thunks (e.g., value as a function/closure) or otherwise defer
evaluation (use a getter or change firstDefined to accept supplier functions),
and ensure firstDefined (or its call site) invokes the supplier only for the
selected entry; refer to spaceChoice, firstDefined, resolveEnvTemplate, and the
fst/pst fs/ps variables when making this change.
In `@nowledge-mem-opencode-plugin/README.md`:
- Around line 123-125: The README snippet sets the NMEM_SPACE example but isn't
runnable; update the example in README.md to show either exporting the variable
(export NMEM_SPACE="Research Agent") followed by the OpenCode launch command, or
show the env var prefixed on the command invocation so users can copy/paste and
actually launch OpenCode in that lane; reference the NMEM_SPACE symbol and the
repository's OpenCode CLI/launch command used elsewhere to demonstrate the full,
runnable sequence.
---
Outside diff comments:
In `@nowledge-mem-openclaw-plugin/src/client.js`:
- Around line 75-91: The space selection currently uses an || chain so an
explicitly provided empty string in credentials (e.g. credentials.space = "") is
treated as "missing" and falls back to NMEM_SPACE / NMEM_SPACE_ID; change the
logic in the constructor that sets this._spaceRef to prefer an explicitly
provided credential value (even if empty) over environment variables by testing
presence rather than truthiness — e.g. check for the existence of
credentials.space or credentials.spaceId (using hasOwnProperty or 'in') and use
their trimmed value if present, otherwise fall back to process.env.NMEM_SPACE /
process.env.NMEM_SPACE_ID and finally default to "" so parseConfig()’s explicit
profile/provider choice is preserved.
---
Duplicate comments:
In `@nowledge-mem-openclaw-plugin/src/tools/status.js`:
- Around line 72-73: The code initializes ceSlot from details.contextEngineSlot
which is still unassigned, causing false undefined reads later; update the
initialization to read from runtimeInfo.contextEngineSlot (i.e., set ceSlot =
runtimeInfo.contextEngineSlot) and then assign details.contextEngineSlot =
ceSlot so later checks in the status tool (the checks referencing ceSlot
at/around the lines that report Context Engine state and capture routing) use
the actual runtime value; ensure runtimeInfo is the source of truth for
contextEngineSlot in the same function where ceSlot and details are used.
🪄 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: 62eb64e5-0869-48d3-a9c6-78fdddd47a28
⛔ Files ignored due to path filters (1)
nowledge-mem-openclaw-plugin/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (65)
CLAUDE.mdREADME.mddocs/PLUGIN_DEVELOPMENT_GUIDE.mdexamples/AGENTS.mdintegrations.jsonnowledge-mem-alma-plugin/CHANGELOG.mdnowledge-mem-alma-plugin/README.mdnowledge-mem-alma-plugin/main.jsnowledge-mem-alma-plugin/manifest.jsonnowledge-mem-alma-plugin/package.jsonnowledge-mem-alma-plugin/tests/space-resolution.test.mjsnowledge-mem-bub-plugin/README.mdnowledge-mem-claude-code-plugin/README.mdnowledge-mem-claude-code-plugin/skills/read-working-memory/SKILL.mdnowledge-mem-claude-code-plugin/skills/search-memory/SKILL.mdnowledge-mem-codex-plugin/AGENTS.mdnowledge-mem-codex-plugin/README.mdnowledge-mem-codex-plugin/skills/search-memory/SKILL.mdnowledge-mem-codex-plugin/skills/working-memory/SKILL.mdnowledge-mem-codex-prompts/AGENTS.mdnowledge-mem-codex-prompts/read_working_memory.mdnowledge-mem-codex-prompts/search_memory.mdnowledge-mem-cursor-plugin/README.mdnowledge-mem-droid-plugin/README.mdnowledge-mem-droid-plugin/commands/nowledge-read-working-memory.mdnowledge-mem-droid-plugin/skills/read-working-memory/SKILL.mdnowledge-mem-gemini-clinowledge-mem-hermes/AGENTS.mdnowledge-mem-hermes/CHANGELOG.mdnowledge-mem-hermes/README.mdnowledge-mem-hermes/client.pynowledge-mem-hermes/plugin.yamlnowledge-mem-hermes/provider.pynowledge-mem-hermes/tests/test_space_resolution.pynowledge-mem-npx-skills/README.mdnowledge-mem-npx-skills/skills/read-working-memory/SKILL.mdnowledge-mem-npx-skills/skills/search-memory/SKILL.mdnowledge-mem-openclaw-plugin/CHANGELOG.mdnowledge-mem-openclaw-plugin/CLAUDE.mdnowledge-mem-openclaw-plugin/README.mdnowledge-mem-openclaw-plugin/SKILL.mdnowledge-mem-openclaw-plugin/openclaw.plugin.jsonnowledge-mem-openclaw-plugin/package.jsonnowledge-mem-openclaw-plugin/scripts/validate-plugin.mjsnowledge-mem-openclaw-plugin/src/client.jsnowledge-mem-openclaw-plugin/src/config.jsnowledge-mem-openclaw-plugin/src/index.jsnowledge-mem-openclaw-plugin/src/spawn-env.jsnowledge-mem-openclaw-plugin/src/tools/status.jsnowledge-mem-openclaw-plugin/tests/space-config.test.mjsnowledge-mem-opencode-plugin/AGENTS.mdnowledge-mem-opencode-plugin/CHANGELOG.mdnowledge-mem-opencode-plugin/README.mdnowledge-mem-opencode-plugin/package.jsonnowledge-mem-opencode-plugin/src/index.tsnowledge-mem-pi-package/AGENTS.mdnowledge-mem-pi-package/skills/read-working-memory/SKILL.mdnowledge-mem-pi-package/skills/search-memory/SKILL.mdnowledge-mem-raycast/CHANGELOG.mdnowledge-mem-raycast/README.mdnowledge-mem-raycast/package.jsonnowledge-mem-raycast/src/api.tsnowledge-mem-raycast/src/edit-working-memory.tsxnowledge-mem-raycast/src/working-memory.tsxshared/behavioral-guidance.md
✅ Files skipped from review due to trivial changes (34)
- nowledge-mem-claude-code-plugin/skills/search-memory/SKILL.md
- nowledge-mem-claude-code-plugin/skills/read-working-memory/SKILL.md
- nowledge-mem-codex-plugin/skills/working-memory/SKILL.md
- nowledge-mem-codex-prompts/read_working_memory.md
- nowledge-mem-droid-plugin/commands/nowledge-read-working-memory.md
- nowledge-mem-hermes/plugin.yaml
- nowledge-mem-droid-plugin/README.md
- nowledge-mem-npx-skills/skills/search-memory/SKILL.md
- nowledge-mem-opencode-plugin/package.json
- nowledge-mem-pi-package/skills/search-memory/SKILL.md
- nowledge-mem-codex-prompts/search_memory.md
- nowledge-mem-npx-skills/README.md
- examples/AGENTS.md
- nowledge-mem-alma-plugin/package.json
- nowledge-mem-codex-plugin/AGENTS.md
- nowledge-mem-codex-plugin/README.md
- nowledge-mem-droid-plugin/skills/read-working-memory/SKILL.md
- nowledge-mem-hermes/AGENTS.md
- nowledge-mem-pi-package/skills/read-working-memory/SKILL.md
- nowledge-mem-cursor-plugin/README.md
- nowledge-mem-codex-prompts/AGENTS.md
- nowledge-mem-opencode-plugin/CHANGELOG.md
- nowledge-mem-bub-plugin/README.md
- nowledge-mem-claude-code-plugin/README.md
- nowledge-mem-opencode-plugin/AGENTS.md
- nowledge-mem-raycast/package.json
- nowledge-mem-pi-package/AGENTS.md
- nowledge-mem-hermes/CHANGELOG.md
- nowledge-mem-raycast/CHANGELOG.md
- nowledge-mem-alma-plugin/CHANGELOG.md
- nowledge-mem-openclaw-plugin/README.md
- CLAUDE.md
- docs/PLUGIN_DEVELOPMENT_GUIDE.md
- nowledge-mem-openclaw-plugin/CLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (4)
- nowledge-mem-openclaw-plugin/package.json
- nowledge-mem-openclaw-plugin/openclaw.plugin.json
- integrations.json
- nowledge-mem-openclaw-plugin/src/index.js
Member
Author
|
@codex plz review |
Member
Author
|
bugbot run |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a0d7d748f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
@codex reivew |
Member
Author
|
bugbot run |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 384f90f233
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
nowledge-mem-codex-plugin/README.md (1)
135-139:⚠️ Potential issue | 🟡 MinorMake the
NMEM_SPACEexample runnable.Line 138 is only a variable assignment, so users copying the block verbatim will not start Codex inside that lane. Show either an
export+ launch flow or an inline launch command.Suggested doc fix
```bash -NMEM_SPACE="Research Agent" +export NMEM_SPACE="Research Agent" +codex</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@nowledge-mem-codex-plugin/README.mdaround lines 135 - 139, The README
example only shows an environment variable assignment (NMEM_SPACE) which won't
launch Codex; update the example to be runnable by showing either an
export-plus-launch or an inline launch invocation (e.g., use export
NMEM_SPACE="Research Agent" followed by running the codex command, or use
NMEM_SPACE="Research Agent" codex). Edit the README snippet that contains
NMEM_SPACE to replace the single-variable line with one of these runnable
patterns so users start Codex in the intended space.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>nowledge-mem-openclaw-plugin/tests/status-tool.test.mjs (1)</summary><blockquote> `51-54`: **Add explicit assertions for ambient-space status fields.** `cfg.space` and `_sources.space` are set (Line 29 and Line 40), but the test never verifies the corresponding status output. Please assert `details.space` / `details.spaceSource` (or at least presence), plus a text-line check for ambient space reporting. <details> <summary>✅ Suggested test extension</summary> ```diff assert.equal(result.details.contextEngineSlot, "nowledge-mem"); assert.equal(result.details.captureMode, "context-engine+hooks"); assert.match(result.content[0].text, /Context Engine slot: nowledge-mem \(active\)/); + assert.ok(Object.prototype.hasOwnProperty.call(result.details, "space")); + assert.ok(Object.prototype.hasOwnProperty.call(result.details, "spaceSource")); + assert.match(result.content[0].text, /Ambient space:/); });Based on learnings:
nowledge_mem_statusshould expose effective config/status details, so tests should cover those surfaced fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nowledge-mem-openclaw-plugin/tests/status-tool.test.mjs` around lines 51 - 54, Add assertions to verify the ambient-space fields exposed by the status output: assert that result.details.space and result.details.spaceSource are set (or equal to the expected values configured via cfg.space and _sources.space), and add a text-line assertion on result.content to match the ambient space reporting (e.g., a regex matching "Ambient space" or the configured space name). Locate the test that builds result in status-tool.test.mjs (references to cfg.space and _sources.space) and extend it to include these checks so nowledge_mem_status's surfaced space fields are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integrations.json`:
- Around line 492-502: The autonomy config for the OpenCode integration
currently claims automatic thread capture ("threads": "automatic-capture") which
contradicts autoCapture: false and the presence of explicit save tools
(nowledge_mem_save_thread / nowledge_mem_save_handoff); update the autonomy.
Change the "threads" value from "automatic-capture" to a guided/manual mode
(e.g., "guided") to reflect explicit invocation, and ensure any
documentation/comments reference using nowledge_mem_save_thread to capture full
message history at natural stopping points or when requested.
In `@nowledge-mem-alma-plugin/main.js`:
- Line 1043: The Working Memory description string overstates fallback behavior:
update the message shown where the description is defined so it does not claim
~/ai-now/memory.md is used as a fallback for non-default spaces (or
alternatively make the report conditional). Specifically, either change the
static description text to indicate that ~/ai-now/memory.md is only relevant for
the Default space, or adjust the UI logic that displays wm.path to only show
that path when readWorkingMemory() confirms availability (i.e., when wm.path is
present and the current space is Default); reference readWorkingMemory(),
wm.path and the Working Memory description string in main.js to locate the code
to modify.
- Around line 575-588: The code only assigns ambientSpace = nextAmbientSpace
when nextAmbientSpace.space differs, leaving ambientSpace.source stale; change
the update logic around resolveAmbientSpace so ambientSpace.source (or
ambientSpace) is refreshed whenever resolveAmbientSpace returns a new object —
e.g., compare both nextAmbientSpace.space and nextAmbientSpace.source or simply
always set ambientSpace = nextAmbientSpace before constructing the
NowledgeMemClient (symbols: resolveAmbientSpace, nextAmbientSpace, ambientSpace,
apiUrl, apiKey, NowledgeMemClient) so nowledge_mem_status reflects the correct
source.
In `@nowledge-mem-hermes/provider.py`:
- Around line 491-493: The current logic uses configured_space =
str(config.get("space") or "").strip() and treats an explicit empty string the
same as "no config", allowing NMEM_SPACE to override; change the branching so
that presence of the "space" key in config (even if empty string) wins: check
for the existence of the "space" key on the config and return its value
(allowing "" to mean Default) before falling back to environment NMEM_SPACE;
update the same pattern used around the NMEM_SPACE fallback (lines handling
NMEM_SPACE) so provider/profile-owned space takes precedence over the env var.
- Around line 495-512: The code currently forces agent_identity to "default"
(identity = str(kwargs.get("agent_identity", "default") or "default")), which
allows space_template/space_by_identity to synthesize lanes when the host never
provided an identity; change the logic in the function that computes the lane so
you first capture the raw value (e.g., raw_agent_identity =
kwargs.get("agent_identity")), treat a missing or empty raw_agent_identity as
"no identity" and do NOT apply identity_map or space_template in that case, and
only run the existing mapping/template code (the identity_map lookup and the
template replace that produce mapped/resolved) when raw_agent_identity is a
non-empty string provided by the host; ensure when no real identity is present
the function falls through to the fixed default lane behavior (so caller's
fallback/default is used) rather than synthesizing one.
In `@nowledge-mem-openclaw-plugin/src/config.js`:
- Around line 31-32: The current strict validation only checks pluginConfig but
ignores the top-level ~/.nowledge-mem/openclaw.json, so unknown keys there
(e.g., "space", "spaceTemplate") are silently accepted; update the config
loading flow in config.js to run the same strict parser/validator (the function
used to validate pluginConfig—e.g., validateConfig/parseConfig) against the
file-backed config read from openclaw.json as well, and if unknown keys are
present reject/throw an error (or return a validation failure) so unknown keys
in the file are not accepted; ensure the code path that merges/overrides
pluginConfig with the file still enforces strict validation on the file input
before merging.
- Around line 377-399: The current selection logic uses pickStr (which turns ""
into undefined) so an explicit space: "" in resolvedFile or resolvedPlugin is
treated as missing and falls through to envStr; update the selection so explicit
empty strings are preserved: when building spaceChoice (and before using pickStr
values fs/ps), detect whether resolvedFile or resolvedPlugin actually have the
"space" property (e.g., Object.prototype.hasOwnProperty.call(resolvedFile,
"space") / resolvedPlugin) and if present use their raw value (including "") as
the chosen source; keep resolveEnvTemplate(fst/pst) and
envStr("NMEM_SPACE"/"NMEM_SPACE_ID") only as fallbacks so NMEM_SPACE is used
only when no profile/provider space is explicitly configured.
---
Duplicate comments:
In `@nowledge-mem-codex-plugin/README.md`:
- Around line 135-139: The README example only shows an environment variable
assignment (NMEM_SPACE) which won't launch Codex; update the example to be
runnable by showing either an export-plus-launch or an inline launch invocation
(e.g., use export NMEM_SPACE="Research Agent" followed by running the codex
command, or use NMEM_SPACE="Research Agent" codex). Edit the README snippet that
contains NMEM_SPACE to replace the single-variable line with one of these
runnable patterns so users start Codex in the intended space.
---
Nitpick comments:
In `@nowledge-mem-openclaw-plugin/tests/status-tool.test.mjs`:
- Around line 51-54: Add assertions to verify the ambient-space fields exposed
by the status output: assert that result.details.space and
result.details.spaceSource are set (or equal to the expected values configured
via cfg.space and _sources.space), and add a text-line assertion on
result.content to match the ambient space reporting (e.g., a regex matching
"Ambient space" or the configured space name). Locate the test that builds
result in status-tool.test.mjs (references to cfg.space and _sources.space) and
extend it to include these checks so nowledge_mem_status's surfaced space fields
are covered.
🪄 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: 05b71bb6-270a-4073-8f6d-a40508eb9e06
📒 Files selected for processing (43)
README.mdintegrations.jsonnowledge-mem-alma-plugin/CHANGELOG.mdnowledge-mem-alma-plugin/main.jsnowledge-mem-alma-plugin/tests/space-resolution.test.mjsnowledge-mem-codex-plugin/.codex-plugin/plugin.jsonnowledge-mem-codex-plugin/AGENTS.mdnowledge-mem-codex-plugin/CHANGELOG.mdnowledge-mem-codex-plugin/README.mdnowledge-mem-codex-plugin/skills/distill-memory/SKILL.mdnowledge-mem-codex-plugin/skills/search-memory/SKILL.mdnowledge-mem-codex-plugin/skills/working-memory/SKILL.mdnowledge-mem-codex-prompts/AGENTS.mdnowledge-mem-codex-prompts/README.mdnowledge-mem-cursor-plugin/rules/nowledge-mem.mdcnowledge-mem-cursor-plugin/skills/distill-memory/SKILL.mdnowledge-mem-cursor-plugin/skills/read-working-memory/SKILL.mdnowledge-mem-cursor-plugin/skills/search-memory/SKILL.mdnowledge-mem-droid-plugin/README.mdnowledge-mem-droid-plugin/skills/distill-memory/SKILL.mdnowledge-mem-droid-plugin/skills/read-working-memory/SKILL.mdnowledge-mem-droid-plugin/skills/search-memory/SKILL.mdnowledge-mem-hermes/CHANGELOG.mdnowledge-mem-hermes/README.mdnowledge-mem-hermes/provider.pynowledge-mem-hermes/tests/test_space_resolution.pynowledge-mem-npx-skills/skills/check-integration/SKILL.mdnowledge-mem-npx-skills/skills/distill-memory/SKILL.mdnowledge-mem-npx-skills/skills/read-working-memory/SKILL.mdnowledge-mem-npx-skills/skills/search-memory/SKILL.mdnowledge-mem-openclaw-plugin/CHANGELOG.mdnowledge-mem-openclaw-plugin/src/client.jsnowledge-mem-openclaw-plugin/src/config.jsnowledge-mem-openclaw-plugin/src/hooks/capture.jsnowledge-mem-openclaw-plugin/src/tools/status.jsnowledge-mem-openclaw-plugin/tests/capture-thread-identity.test.mjsnowledge-mem-openclaw-plugin/tests/space-config.test.mjsnowledge-mem-openclaw-plugin/tests/status-tool.test.mjsnowledge-mem-opencode-plugin/README.mdnowledge-mem-pi-package/skills/distill-memory/SKILL.mdnowledge-mem-pi-package/skills/read-working-memory/SKILL.mdnowledge-mem-pi-package/skills/search-memory/SKILL.mdshared/behavioral-guidance.md
✅ Files skipped from review due to trivial changes (19)
- nowledge-mem-codex-plugin/skills/distill-memory/SKILL.md
- nowledge-mem-droid-plugin/skills/distill-memory/SKILL.md
- nowledge-mem-cursor-plugin/skills/read-working-memory/SKILL.md
- nowledge-mem-npx-skills/skills/distill-memory/SKILL.md
- nowledge-mem-cursor-plugin/skills/distill-memory/SKILL.md
- nowledge-mem-pi-package/skills/distill-memory/SKILL.md
- nowledge-mem-droid-plugin/skills/search-memory/SKILL.md
- nowledge-mem-codex-plugin/.codex-plugin/plugin.json
- nowledge-mem-pi-package/skills/search-memory/SKILL.md
- nowledge-mem-cursor-plugin/skills/search-memory/SKILL.md
- nowledge-mem-pi-package/skills/read-working-memory/SKILL.md
- nowledge-mem-codex-plugin/skills/working-memory/SKILL.md
- nowledge-mem-codex-plugin/skills/search-memory/SKILL.md
- nowledge-mem-opencode-plugin/README.md
- nowledge-mem-codex-plugin/CHANGELOG.md
- nowledge-mem-droid-plugin/skills/read-working-memory/SKILL.md
- nowledge-mem-hermes/CHANGELOG.md
- nowledge-mem-openclaw-plugin/tests/capture-thread-identity.test.mjs
- nowledge-mem-openclaw-plugin/tests/space-config.test.mjs
🚧 Files skipped from review as they are similar to previous changes (6)
- nowledge-mem-npx-skills/skills/search-memory/SKILL.md
- nowledge-mem-droid-plugin/README.md
- nowledge-mem-codex-prompts/AGENTS.md
- nowledge-mem-hermes/tests/test_space_resolution.py
- nowledge-mem-openclaw-plugin/src/tools/status.js
- nowledge-mem-openclaw-plugin/src/client.js
Member
Author
|
bugbot run |
Member
Author
|
@codex review plz |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8885c6490
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
bugbot run |
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6559092397
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
bugbot run |
Member
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bacf47c8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Member
Author
|
@codex review |
Member
Author
|
bugbot run |
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 7a0982c. Configure here.
Member
Author
|
bugbot run |
Member
Author
|
@codex review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 09d5419. Configure here.
|
Codex Review: Didn't find any major issues. Another round soon, please! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
3 tasks
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.
Summary by CodeRabbit
New Features
Behavior
Documentation
Tests
Note
Medium Risk
Medium risk because it extends the
integrations.jsonschema with newautonomyfields consumed by multiple runtimes, and changes Alma/Hermes runtime behavior to inject/overridespace_id/NMEM_SPACEwhich could affect where data is read/written if misresolved.Overview
Adds a cross-integration autonomy contract and documents a unified ambient space (lane) routing rule, clarifying when integrations are
automaticvsguidedvsmanualand howspaceshould be chosen (config → template/map → env →Default).Extends
integrations.jsonwith a newautonomyblock per integration (and bumps multiple integration versions/metadata), plus updates guidance in top-level docs/READMEs and skills/prompts to consistently use--space/NMEM_SPACEand treat~/ai-now/memory.mdas Default-space fallback only.Implements space-aware execution in the Alma plugin (settings-based
space/spaceTemplateresolution, automaticspace_idinjection into HTTP requests, improved Working Memory error handling and status reporting) and in the Hermes provider (configurable space routing with identity mapping/template support, explicit-empty semantics, safer timeout parsing, and subprocess env control), with new unit tests for both.Reviewed by Cursor Bugbot for commit 09d5419. Bugbot is set up for automated code reviews on this repo. Configure here.