openclaw: patch plugins.allow at install time + drop blocking auto-recall#124
Open
kaghni wants to merge 1 commit into
Open
openclaw: patch plugins.allow at install time + drop blocking auto-recall#124kaghni wants to merge 1 commit into
kaghni wants to merge 1 commit into
Conversation
…call Two related fixes for openclaw users (issue #121): 1. plugins.allow gating: the installer drops files into ~/.openclaw/extensions/hivemind/ but never touches the gateway's openclaw.json. If plugins.allow is an explicit array missing "hivemind", openclaw refuses to load the plugin — auto-capture silently does nothing because agent_end never fires, and /hivemind_setup is unreachable from inside the agent (chicken-and-egg: the slash command needs the plugin to load). src/cli/install-openclaw.ts now calls the same ensureHivemindAllowlisted helper that /hivemind_setup uses, so the installer patches plugins.allow + tools.alsoAllow at install time (when each is an explicit array). Mirrors openclaw's own ensurePluginAllowlisted semantics: never creates the array if absent, never touches it if empty — default-allow stays default-allow so we don't flip the user into restrictive mode and break other plugins. Adds a restart hint + "capture starts on the next turn — no backfill" caveat to installer output so users aren't confused when older turns don't appear. 2. blocking auto-recall removed: before_agent_start used to run a searchDeeplakeTables query across memory + sessions on every user turn. With Deeplake's sessions-table latency varying from 200ms to 10s+ (measured 11.7s/10.8s/11.2s across three sequential probes), every openclaw turn was paying up to 10s of blocking recall before the agent could even read the message. Other agents (claude-code/codex/cursor/hermes/pi) don't do this — they intercept the agent's own Grep tool calls and let the agent decide when to search. openclaw now matches that pattern: recall flows exclusively through the registered hivemind_search/_read/_index tools, with the SKILL.md body in the system prompt directing the agent to call them. Side effect: the recursive <recalled-memories> capture (the old recall context was getting captured as part of the user's prompt and stored) is gone too. The before_agent_start hook still handles the login nudge and the post-auth welcome banner — those don't touch Deeplake. Tests: - openclaw/tests/setup-command.test.ts: 9 new cases covering plugins.allow patching, idempotency, default-allow semantics, both-arrays patched, detectAllowlistMissing extension - openclaw/tests/install-openclaw.test.ts (new): 12 cases driving the real installer against tmpdir fixtures — file copy, both patches, idempotency, default-allow preservation, malformed config tolerance, restart hint, no-duplicate - openclaw/tests/auto-recall.test.ts: rewritten — asserts no Deeplake call on a normal turn, no <recalled-memories> injection, hivemind_search/_read/_index tools still registered, login nudge path preserved Real-world E2E on a live openclaw gateway: - Bundle installed → "Hivemind plugin registered" in gateway log - Idempotency: re-running installer logs "allowlist already covers hivemind", no extra backup - Zero "Auto-recall failed" log lines after the restart with the new bundle (previous bundle emitted them on every turn that hit Deeplake slowness) Caveat: capture (agent_end → INSERT into sessions) still hits the same Deeplake-side timeout intermittently. That's a separate performance issue and runs after the agent responds, so it doesn't block the user — but it does drop some turns from memory until Deeplake's sessions queries are faster. Confidence: high, because the unit tests cover every edge case of the new config-patching logic, the local fixture E2E drove the real installer end-to-end against multiple fixture shapes, and the live gateway confirms the bundle loads + the recall hook no longer fires. Untested: openclaw versions older than 2026.4.21 (where the auto-promote-via-tools.alsoAllow heuristic doesn't exist) — the manual fix from the original reporter implies they would have been the broken-state case; the installer's plugins.allow patch is the right fix for them, but we couldn't reproduce that exact host state on the 2026.4.21 gateway available for testing. Fixes #121
📝 WalkthroughWalkthroughThis PR implements issue ChangesOpenClaw allowlist patching and plugin hook simplification
Node imports and path infrastructure refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 1 file changed
Generated for commit 8fb1fa1. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openclaw/src/index.ts (1)
1009-1032:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
before_agent_startshould no longer be gated byautoRecall(Line 1009).Now that proactive recall was removed, this gate unintentionally disables the login nudge + post-auth welcome paths for users who already have
autoRecall: false.Suggested fix
- if (config.autoRecall !== false) { - hook("before_agent_start", async (event: { prompt?: string }) => { + hook("before_agent_start", async (event: { prompt?: string }) => { if (!event.prompt || event.prompt.length < 5) return; try { const dl = await getApi(); @@ } catch (err) { logger.error(`before_agent_start failed: ${err instanceof Error ? err.message : String(err)}`); } }); - }🤖 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 `@openclaw/src/index.ts` around lines 1009 - 1032, The before_agent_start hook is incorrectly wrapped by the config.autoRecall check which prevents the login nudge and post-auth welcome for users with autoRecall: false; remove this gating so the hook registration always runs (i.e., register the hook unconditionally instead of inside the if (config.autoRecall !== false) block) while keeping the existing logic that checks getApi(), authUrl, and justAuthenticated handling inside the hook callback (referencing the before_agent_start hook, getApi(), justAuthenticated, loadCredentials(), and authUrl).
🤖 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 `@bundle/cli.js`:
- Around line 456-458: The current coercion of tools.alsoAllow to [] forces
patching even when the field is absent; change the logic so you preserve
"missing" vs "explicit empty" by not defaulting to [], e.g. keep alsoAllowRaw as
undefined when tools.alsoAllow is not an array and compute
toolsAlsoAllowNeedsPatch only when an actual array was provided (e.g.
toolsAlsoAllowNeedsPatch = Array.isArray(alsoAllowRaw) ?
!isAllowlistCoveringHivemind(alsoAllowRaw) : false); apply the same pattern to
the analogous block around lines 470-474 so patching only occurs for an
explicitly supplied non-empty allowlist.
- Line 3313: The command string built for Hermes is not quoting the executable
path which will break on paths with spaces; update the assignment that creates
command (currently using `node ${join8(BUNDLE_DIR, bundleFile)}`) to wrap the
resolved path in quotes (e.g. `node "..."`) so the `join8(BUNDLE_DIR,
bundleFile)` result is quoted, ensuring paths with spaces are handled correctly;
locate the `command` property assignment in bundle/cli.js and modify it to
include the quotes around the template substitution.
- Around line 516-529: The installer currently ignores result.status === "error"
from ensureHivemindAllowlisted(), so surface failures by handling that branch:
when ensureHivemindAllowlisted() returns result.status === "error", log a clear
error including result.error, result.configPath and result.backupPath (like the
other branches), and ensure the installer returns non-zero/aborts or otherwise
indicates failure (same behavior as on other fatal install errors). Update the
code path around ensureHivemindAllowlisted() to check result.status for "error"
and emit a descriptive processLogger/log error message using those result
fields.
- Around line 453-456: The code dereferences parsed.plugins and parsed.tools
without validating parsed's shape; update the top of the parsing logic to verify
parsed is a non-null object and throw a clear, structured error if not (e.g.,
"Invalid config: expected JSON object"); then guard access to parsed.plugins and
parsed.tools by treating them as objects (e.g., if parsed.plugins is not an
object default to {} before reading plugins.allow) and ensure tools.alsoAllow is
read only after confirming tools is an object (or default to {}), keeping the
existing assignments to pluginsAllowRaw and alsoAllowRaw but using these
validated/defaulted objects.
In `@openclaw/src/index.ts`:
- Line 663: The manual-fix string returned when updating the allowlist (the
returned object that uses result.error and result.configPath) is too
prescriptive; change it to recommend adding "hivemind" to tools' alsoAllow and
only recommend adding it to plugins.allow if an explicit plugin allowlist
already exists (i.e., plugins.allow is present and non-empty). Update the
message built around result.error/result.configPath to conditionally instruct:
if plugins.allow exists suggest adding to both, otherwise suggest adding to
tools.alsoAllow and warn not to create a plugins.allow (to avoid switching to
restrictive explicit-allow mode).
In `@openclaw/tests/auto-recall.test.ts`:
- Line 157: The test calls expect(before({ prompt: "anything that triggered the
path before" })) but uses the wrong matcher; replace .resolves.not.toThrow()
with .resolves.toBeUndefined() so the promise resolution is asserted correctly
(i.e., expect(before(...)).resolves.toBeUndefined()).
---
Outside diff comments:
In `@openclaw/src/index.ts`:
- Around line 1009-1032: The before_agent_start hook is incorrectly wrapped by
the config.autoRecall check which prevents the login nudge and post-auth welcome
for users with autoRecall: false; remove this gating so the hook registration
always runs (i.e., register the hook unconditionally instead of inside the if
(config.autoRecall !== false) block) while keeping the existing logic that
checks getApi(), authUrl, and justAuthenticated handling inside the hook
callback (referencing the before_agent_start hook, getApi(), justAuthenticated,
loadCredentials(), and authUrl).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 609726ec-8d39-46ec-ae3f-586579147896
📒 Files selected for processing (7)
bundle/cli.jsopenclaw/src/index.tsopenclaw/src/setup-config.tsopenclaw/tests/auto-recall.test.tsopenclaw/tests/install-openclaw.test.tsopenclaw/tests/setup-command.test.tssrc/cli/install-openclaw.ts
Comment on lines
+453
to
+456
| const plugins = parsed.plugins ?? {}; | ||
| const pluginsAllowRaw = plugins.allow; | ||
| const tools = parsed.tools ?? {}; | ||
| const alsoAllowRaw = Array.isArray(tools.alsoAllow) ? tools.alsoAllow : []; |
There was a problem hiding this comment.
Validate parsed config shape before property access.
JSON.parse can return null, and parsed.plugins/parsed.tools then throws at runtime. Return a structured error before dereferencing.
Suggested fix
let parsed;
try {
const raw = readFileSync4(configPath, "utf-8");
parsed = JSON.parse(raw);
} catch (e) {
return { status: "error", configPath, error: `could not read/parse config: ${e instanceof Error ? e.message : String(e)}` };
}
+ if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
+ return { status: "error", configPath, error: "config must be a JSON object" };
+ }
const plugins = parsed.plugins ?? {};🤖 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 `@bundle/cli.js` around lines 453 - 456, The code dereferences parsed.plugins
and parsed.tools without validating parsed's shape; update the top of the
parsing logic to verify parsed is a non-null object and throw a clear,
structured error if not (e.g., "Invalid config: expected JSON object"); then
guard access to parsed.plugins and parsed.tools by treating them as objects
(e.g., if parsed.plugins is not an object default to {} before reading
plugins.allow) and ensure tools.alsoAllow is read only after confirming tools is
an object (or default to {}), keeping the existing assignments to
pluginsAllowRaw and alsoAllowRaw but using these validated/defaulted objects.
Comment on lines
+456
to
+458
| const alsoAllowRaw = Array.isArray(tools.alsoAllow) ? tools.alsoAllow : []; | ||
| const pluginsAllowNeedsPatch = isPluginsAllowMissingHivemind(pluginsAllowRaw); | ||
| const toolsAlsoAllowNeedsPatch = !isAllowlistCoveringHivemind(alsoAllowRaw); |
There was a problem hiding this comment.
tools.alsoAllow patching currently breaks the “explicit non-empty only” contract.
At Line 456, absent/non-array values are coerced to [], and Line 458 then always flags patching. This creates tools.alsoAllow even when missing/empty, which changes default-allow behavior.
Suggested fix
- const alsoAllowRaw = Array.isArray(tools.alsoAllow) ? tools.alsoAllow : [];
+ const alsoAllowRaw = tools.alsoAllow;
const pluginsAllowNeedsPatch = isPluginsAllowMissingHivemind(pluginsAllowRaw);
- const toolsAlsoAllowNeedsPatch = !isAllowlistCoveringHivemind(alsoAllowRaw);
+ const toolsAlsoAllowNeedsPatch =
+ Array.isArray(alsoAllowRaw) &&
+ alsoAllowRaw.length > 0 &&
+ !isAllowlistCoveringHivemind(alsoAllowRaw);
...
if (toolsAlsoAllowNeedsPatch) {
updated.tools = {
...tools,
alsoAllow: [...alsoAllowRaw, "hivemind"]
};
}Also applies to: 470-474
🤖 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 `@bundle/cli.js` around lines 456 - 458, The current coercion of
tools.alsoAllow to [] forces patching even when the field is absent; change the
logic so you preserve "missing" vs "explicit empty" by not defaulting to [],
e.g. keep alsoAllowRaw as undefined when tools.alsoAllow is not an array and
compute toolsAlsoAllowNeedsPatch only when an actual array was provided (e.g.
toolsAlsoAllowNeedsPatch = Array.isArray(alsoAllowRaw) ?
!isAllowlistCoveringHivemind(alsoAllowRaw) : false); apply the same pattern to
the analogous block around lines 470-474 so patching only occurs for an
explicitly supplied non-empty allowlist.
Comment on lines
+516
to
+529
| const result = ensureHivemindAllowlisted(); | ||
| if (result.status === "added") { | ||
| const touched = []; | ||
| if (result.delta.pluginsAllow) | ||
| touched.push("plugins.allow"); | ||
| if (result.delta.toolsAlsoAllow) | ||
| touched.push("tools.alsoAllow"); | ||
| log(` OpenClaw patched ${touched.join(" + ")} in ${result.configPath}`); | ||
| log(` OpenClaw backup: ${result.backupPath}`); | ||
| log(` OpenClaw restart the gateway to activate: systemctl --user restart openclaw-gateway.service`); | ||
| log(` OpenClaw capture starts on the NEXT turn \u2014 earlier turns are NOT backfilled`); | ||
| } else if (result.status === "already-set") { | ||
| log(` OpenClaw allowlist already covers hivemind in ${result.configPath}`); | ||
| } |
There was a problem hiding this comment.
Handle and surface allowlist patch failures in installer output.
installOpenclaw() ignores result.status === "error", so failed patch attempts are silent and users won’t know why the plugin still doesn’t load.
Suggested fix
} else if (result.status === "already-set") {
log(` OpenClaw allowlist already covers hivemind in ${result.configPath}`);
+ } else if (result.status === "error") {
+ warn(` OpenClaw could not patch allowlist in ${result.configPath}: ${result.error}`);
}📝 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.
Suggested change
| const result = ensureHivemindAllowlisted(); | |
| if (result.status === "added") { | |
| const touched = []; | |
| if (result.delta.pluginsAllow) | |
| touched.push("plugins.allow"); | |
| if (result.delta.toolsAlsoAllow) | |
| touched.push("tools.alsoAllow"); | |
| log(` OpenClaw patched ${touched.join(" + ")} in ${result.configPath}`); | |
| log(` OpenClaw backup: ${result.backupPath}`); | |
| log(` OpenClaw restart the gateway to activate: systemctl --user restart openclaw-gateway.service`); | |
| log(` OpenClaw capture starts on the NEXT turn \u2014 earlier turns are NOT backfilled`); | |
| } else if (result.status === "already-set") { | |
| log(` OpenClaw allowlist already covers hivemind in ${result.configPath}`); | |
| } | |
| const result = ensureHivemindAllowlisted(); | |
| if (result.status === "added") { | |
| const touched = []; | |
| if (result.delta.pluginsAllow) | |
| touched.push("plugins.allow"); | |
| if (result.delta.toolsAlsoAllow) | |
| touched.push("tools.alsoAllow"); | |
| log(` OpenClaw patched ${touched.join(" + ")} in ${result.configPath}`); | |
| log(` OpenClaw backup: ${result.backupPath}`); | |
| log(` OpenClaw restart the gateway to activate: systemctl --user restart openclaw-gateway.service`); | |
| log(` OpenClaw capture starts on the NEXT turn \u2014 earlier turns are NOT backfilled`); | |
| } else if (result.status === "already-set") { | |
| log(` OpenClaw allowlist already covers hivemind in ${result.configPath}`); | |
| } else if (result.status === "error") { | |
| warn(` OpenClaw could not patch allowlist in ${result.configPath}: ${result.error}`); | |
| } |
🤖 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 `@bundle/cli.js` around lines 516 - 529, The installer currently ignores
result.status === "error" from ensureHivemindAllowlisted(), so surface failures
by handling that branch: when ensureHivemindAllowlisted() returns result.status
=== "error", log a clear error including result.error, result.configPath and
result.backupPath (like the other branches), and ensure the installer returns
non-zero/aborts or otherwise indicates failure (same behavior as on other fatal
install errors). Update the code path around ensureHivemindAllowlisted() to
check result.status for "error" and emit a descriptive processLogger/log error
message using those result fields.
| function buildHookEntry(bundleFile, timeout, matcher) { | ||
| const entry = { | ||
| command: `node ${join7(BUNDLE_DIR, bundleFile)}`, | ||
| command: `node ${join8(BUNDLE_DIR, bundleFile)}`, |
There was a problem hiding this comment.
Quote Hermes hook command path.
The command path is unquoted; homes/project paths with spaces will break execution.
Suggested fix
- command: `node ${join8(BUNDLE_DIR, bundleFile)}`,
+ command: `node "${join8(BUNDLE_DIR, bundleFile)}"`,📝 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.
Suggested change
| command: `node ${join8(BUNDLE_DIR, bundleFile)}`, | |
| command: `node "${join8(BUNDLE_DIR, bundleFile)}"`, |
🤖 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 `@bundle/cli.js` at line 3313, The command string built for Hermes is not
quoting the executable path which will break on paths with spaces; update the
assignment that creates command (currently using `node ${join8(BUNDLE_DIR,
bundleFile)}`) to wrap the resolved path in quotes (e.g. `node "..."`) so the
`join8(BUNDLE_DIR, bundleFile)` result is quoted, ensuring paths with spaces are
handled correctly; locate the `command` property assignment in bundle/cli.js and
modify it to include the quotes around the template substitution.
| return { text: `✅ Added:\n • ${touched.join("\n • ")}\n\nOpenclaw will detect the config change and restart. On the next turn, the agent will have access to hivemind_search, hivemind_read, and hivemind_index. **Capture starts on the next turn — earlier turns are NOT backfilled.**\n\nBackup of previous config: ${result.backupPath}${skillifyHint}` }; | ||
| } | ||
| return { text: `⚠️ Could not update allowlist: ${result.error}\n\nManual fix: open ${result.configPath} and add "hivemind" to the "alsoAllow" array under "tools".` }; | ||
| return { text: `⚠️ Could not update allowlist: ${result.error}\n\nManual fix: open ${result.configPath} and add "hivemind" to BOTH the "allow" array under "plugins" AND the "alsoAllow" array under "tools".` }; |
There was a problem hiding this comment.
Manual-fix text is overly strict and can break default-allow setups (Line 663).
The current message says to add "hivemind" to BOTH arrays unconditionally; for absent/empty plugins.allow, that can accidentally switch users into restrictive explicit-allowlist mode.
Suggested wording adjustment
- return { text: `⚠️ Could not update allowlist: ${result.error}\n\nManual fix: open ${result.configPath} and add "hivemind" to BOTH the "allow" array under "plugins" AND the "alsoAllow" array under "tools".` };
+ return { text: `⚠️ Could not update allowlist: ${result.error}\n\nManual fix: open ${result.configPath} and add "hivemind" to "tools.alsoAllow". If "plugins.allow" already exists as a non-empty explicit allowlist, add "hivemind" there too.` };📝 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.
Suggested change
| return { text: `⚠️ Could not update allowlist: ${result.error}\n\nManual fix: open ${result.configPath} and add "hivemind" to BOTH the "allow" array under "plugins" AND the "alsoAllow" array under "tools".` }; | |
| return { text: `⚠️ Could not update allowlist: ${result.error}\n\nManual fix: open ${result.configPath} and add "hivemind" to "tools.alsoAllow". If "plugins.allow" already exists as a non-empty explicit allowlist, add "hivemind" there 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 `@openclaw/src/index.ts` at line 663, The manual-fix string returned when
updating the allowlist (the returned object that uses result.error and
result.configPath) is too prescriptive; change it to recommend adding "hivemind"
to tools' alsoAllow and only recommend adding it to plugins.allow if an explicit
plugin allowlist already exists (i.e., plugins.allow is present and non-empty).
Update the message built around result.error/result.configPath to conditionally
instruct: if plugins.allow exists suggest adding to both, otherwise suggest
adding to tools.alsoAllow and warn not to create a plugins.allow (to avoid
switching to restrictive explicit-allow mode).
| // assertion here is conservative: the hook must NOT call query, and | ||
| // must NOT throw, when no creds exist. | ||
| const before = hooks.get("before_agent_start")!; | ||
| await expect(before({ prompt: "anything that triggered the path before" })).resolves.not.toThrow(); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Vitest v4, is await expect(promise).resolves.not.toThrow()valid when the promise resolves toundefined (not a function)? What matcher is recommended instead?
💡 Result:
No. In Vitest v4, await expect(promise).resolves.not.toThrow() is not a valid pattern for the case where the promise resolves to undefined (i.e., “not a function”). The root issue is that Vitest’s .resolves unwrapping expects to apply the matcher to the resolved value, but toThrow-style matchers are for asserting that a function throws; they don’t apply meaningfully to non-functions like undefined in the way your expression suggests. Related reports show .resolves is not compatible with Jest’s “function form” behavior for toThrow (the .resolves path doesn’t support that compatibility, unlike .rejects). [1] Recommended matcher instead: assert the resolved value directly, e.g. await expect(promise).resolves.toBeUndefined() (or toEqual(undefined)), rather than using not.toThrow. Vitest’s docs describe .resolves as unwrapping the promise and then applying a normal matcher to the resolved value; if the promise rejects the assertion fails. [2][3] Example: - await expect(promise).resolves.toBeUndefined() [2][3]
Citations:
- 1: "You must provide a Promise to expect()" inconsistent between
resolvesandrejectsvitest-dev/vitest#6643 - 2: https://vitest.dev/guide/learn/async
- 3: https://vitest.dev/api/expect.html
Use resolves.toBeUndefined() instead of resolves.not.toThrow() on line 157.
The .toThrow() matcher applies only to functions, not to resolved promise values. Since before() resolves to undefined, use a concrete value assertion instead.
Fix
- await expect(before({ prompt: "anything that triggered the path before" })).resolves.not.toThrow();
+ await expect(before({ prompt: "anything that triggered the path before" })).resolves.toBeUndefined();📝 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.
Suggested change
| await expect(before({ prompt: "anything that triggered the path before" })).resolves.not.toThrow(); | |
| await expect(before({ prompt: "anything that triggered the path before" })).resolves.toBeUndefined(); |
🤖 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 `@openclaw/tests/auto-recall.test.ts` at line 157, The test calls
expect(before({ prompt: "anything that triggered the path before" })) but uses
the wrong matcher; replace .resolves.not.toThrow() with
.resolves.toBeUndefined() so the promise resolution is asserted correctly (i.e.,
expect(before(...)).resolves.toBeUndefined()).
Collaborator
Author
User-flow E2E: drove the real
|
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.
Fixes #121.
Why
Two related bugs surfaced in a user report (the original symptom: "openclaw plugin installed but auto-capture silently does nothing"):
plugins.allowgating — the installer copies files into~/.openclaw/extensions/hivemind/but never patches~/.openclaw/openclaw.json. If the gateway'splugins.allowis an explicit array missing"hivemind", the plugin never registers,agent_endnever fires, and/hivemind_setupis unreachable from inside the agent (chicken-and-egg: the slash command needs the plugin to load first).before_agent_startran a Deeplakesessions-table search on every user turn. Sessions queries currently take 11.7s / 10.8s / 11.2s (three sequential probes) against a 10s SDK timeout, so every turn paid the full timeout before the agent could read the message. Other agents (claude-code/codex/cursor/hermes/pi) don't do this — they intercept the agent'sGreptool and let the agent decide when to search.What changed
plugins.allowpatch in the installersrc/cli/install-openclaw.tsnow calls the sameensureHivemindAllowlistedhelper that/hivemind_setupusesensurePluginAllowlistedsemantics (ext/openclaw/src/config/plugins-allowlist.ts:7): only patchesplugins.allowwhen it's an explicit non-empty array. Absent / empty → left alone (default-allow stays default-allow; we don't flip the user into restrictive mode and break their other plugins)tools.alsoAllowBlocking auto-recall removed
openclaw/src/index.ts:before_agent_startno longer runssearchDeeplakeTableshivemind_search/hivemind_read/hivemind_indextools (already registered), with theSKILL.mdbody in the system prompt directing the agent to use them<recalled-memories>capture bug (old recall context was getting captured as part of the user's prompt and stored) is gone tooextractKeywords+RECALL_STOPWORDSremoved as dead codeTests
openclaw/tests/setup-command.test.ts— +9 cases forplugins.allowpatching: idempotency, default-allow semantics, both-arrays patched,detectAllowlistMissingextensionopenclaw/tests/install-openclaw.test.ts(new, 12 cases) — drives the real installer against tmpdir fixtures: file copy, both patches, idempotency, default-allow preservation, malformed config tolerance, restart hint, no-duplicateopenclaw/tests/auto-recall.test.ts— rewritten: asserts no Deeplake call on a normal turn, no<recalled-memories>injection, tools still registered, login-nudge path preservedAll 2214 hivemind tests pass.
Real-world E2E on a live openclaw gateway (2026.4.21)
Hivemind plugin registeredin gateway logallowlist already covers hivemind, no extra backupAuto-recall failedlog lines after the restart with the new bundle (previous bundle emitted them on every turn that hit Deeplake slowness)hivemind_search/hivemind_read/hivemind_indextools confirmed registered inbefore_prompt_buildCaveats
Auto-capture failed: Query timeout after 10000mslines appear in the gateway log.agent_endruns after the agent responds, so this doesn't block the user, but it does drop some turns from memory until Deeplake'ssessions-table query latency improves. Separate issue, not addressed here."hivemind"toplugins.allowwas needed even though tools were allowlisted) can't be reproduced on openclaw 2026.4.21 — that version has an "auto-promote plugins when their tools are inalsoAllow" feature that masks the gating. Older openclaw versions without that feature are the broken-state case the installer'splugins.allowpatch is for.Confidence
Confidence: high, because the unit tests cover every edge case of the new config-patching logic, the local fixture E2E drove the real installer end-to-end against multiple fixture shapes, and the live gateway confirms the bundle loads + the recall hook no longer fires.Untested: openclaw versions older than 2026.4.21(no host available; the installer's plugins.allow patch is the right fix for them but unverified end-to-end on those versions).Test plan
openclaw/src/setup-config.tssemantics (esp.isPluginsAllowMissingHivemindonly-patch-explicit-arrays rule)openclaw/tests/install-openclaw.test.tsand confirms test coverage matches the design intent in the descriptionSummary by CodeRabbit
New Features
Changes