fix(openclaw-memory-plugin): improve port management and preserve existing config

[qin-ctx]

Description

Improve the openclaw-memory-plugin's local mode startup logic and installation scripts to be more robust and non-destructive.

Related Issue

close #512

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update
• Refactoring (no functional changes)
• Performance improvement
• Test update

Changes Made

• Replace fail-on-port-occupied with smart prepareLocalPort: kills stale OpenViking processes or auto-finds free ports instead of erroring out
• Simplify agent ID default from random per-session generation to static "default"
• Change install scripts (install.sh, cli.js) to merge into existing plugins.allow and plugins.load.paths instead of overwriting them
• Reduce default recall limit from 20 to 6
• Update SKILL.md documentation to reflect new agentId default

Testing

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• I have tested this on the following platforms:
  • Linux
  • macOS
  • Windows

Checklist

• My code follows the project's coding style
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• Any dependent changes have been merged and published

Additional Notes

🤖 Generated with Claude Code

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Behavioral Regression The previous logic to reuse an existing healthy OpenViking server was removed. Now prepareLocalPort kills any OpenViking process on the configured port, even if it's already healthy, leading to unnecessary restarts. // Prepare port: kill stale OpenViking, or auto-find free port if occupied by others const actualPort = await prepareLocalPort(cfg.port, api.logger); const baseUrl = `http://127.0.0.1:${actualPort}`; Configuration Overwrite In the else block of configureOpenclawViaCli, plugins.load.paths is still overwritten with a new array containing only the current plugin path, instead of merging into the existing list as intended. await runNoShell("openclaw", ["config", "set", "plugins.load.paths", JSON.stringify([pluginPath])], { silent: true }).catch(() => {}); Fragile JSON Manipulation The sed-based JSON array merging for plugins.allow and plugins.load.paths is fragile and may break with non-trivial JSON structures (e.g., elements with commas in strings). if ! echo "$existing_allow" | grep -q '"memory-openviking"'; then existing_allow=$(echo "$existing_allow" | sed 's/]$//' | sed 's/$/,"memory-openviking"]/' | sed 's/\[,/[/') fi "${oc_env[@]}" openclaw config set plugins.allow "$existing_allow" --json "${oc_env[@]}" openclaw config set gateway.mode local "${oc_env[@]}" openclaw config set plugins.slots.memory memory-openviking # Merge into existing load paths instead of overwriting local existing_paths existing_paths=$("${oc_env[@]}" openclaw config get plugins.load.paths --json 2>/dev/null || echo '[]') if ! echo "$existing_paths" | grep -q "\"${PLUGIN_DEST}\""; then existing_paths=$(echo "$existing_paths" | sed 's/]$//' | sed "s/$/,\"${PLUGIN_DEST}\"]/" | sed 's/\[,/[/') fi

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Merge load paths instead of overwriting The current code overwrites plugins.load.paths instead of merging, which could erase existing paths. Instead, retrieve the current paths, add the plugin path if missing, then set the merged list. examples/openclaw-memory-plugin/setup-helper/cli.js [531] - await runNoShell("openclaw", ["config", "set", "plugins.load.paths", JSON.stringify([pluginPath])], { silent: true }).catch(() => {}); + let currentPaths = []; + try { + const raw = await run("openclaw", ["config", "get", "plugins.load.paths", "--json"], { ...extraEnv, silent: true }); + currentPaths = JSON.parse(raw.stdout || "[]"); + } catch {} + if (!Array.isArray(currentPaths)) currentPaths = []; + if (!currentPaths.includes(pluginPath)) currentPaths.push(pluginPath); + await runNoShell("openclaw", ["config", "set", "plugins.load.paths", JSON.stringify(currentPaths), "--json"], { silent: true }).catch(() => {}); Suggestion importance[1-10]: 7 __ Why: The PR fixes overwriting for plugins.allow in other places but still overwrites plugins.load.paths here, which could erase existing user-configured paths. Merging maintains consistency and preserves user settings.	Medium
General	Verify port is free after killing process After killing the stale process, verify the port is actually free before returning it, to avoid race conditions where the OS hasn't released the port yet. examples/openclaw-memory-plugin/process-manager.ts [152-157] if (isOpenViking) { logger.info?.(`memory-openviking: killing stale OpenViking on port ${port}`); await killProcessOnPort(port, logger); + // Wait up to 2 seconds for port to be released + for (let i = 0; i < 4; i++) { + const stillOccupied = await quickTcpProbe("127.0.0.1", port, 500); + if (!stillOccupied) break; + await new Promise(r => setTimeout(r, 500)); + } return port; } Suggestion importance[1-10]: 6 __ Why: There's a potential race condition where the OS hasn't released the port after killing the process. Adding a wait/verify step improves reliability, though it's not a guaranteed bug in all scenarios.	Low
	Use Python for safe JSON manipulation Using grep and sed to manipulate JSON is fragile. Instead, use a JSON-aware tool like jq (if available) or fall back to Python's json module, which is widely available, to safely parse and modify the JSON. examples/openclaw-memory-plugin/install.sh [612-618] # Merge into existing allow list instead of overwriting local existing_allow existing_allow=$("${oc_env[@]}" openclaw config get plugins.allow --json 2>/dev/null || echo '[]') - if ! echo "$existing_allow" | grep -q '"memory-openviking"'; then - existing_allow=$(echo "$existing_allow" | sed 's/]$//' | sed 's/$/,"memory-openviking"]/' | sed 's/\[,/[/') + if command -v python3 &>/dev/null; then + existing_allow=$(python3 -c " + import json, sys + data = json.loads(sys.stdin.read()) + if 'memory-openviking' not in data: + data.append('memory-openviking') + print(json.dumps(data)) + " <<< "$existing_allow") + elif command -v python &>/dev/null; then + existing_allow=$(python -c " + import json, sys + data = json.loads(sys.stdin.read()) + if 'memory-openviking' not in data: + data.append('memory-openviking') + print(json.dumps(data)) + " <<< "$existing_allow") + else + # Fallback to sed/grep if Python not available + if ! echo "$existing_allow" | grep -q '"memory-openviking"'; then + existing_allow=$(echo "$existing_allow" | sed 's/]$//' | sed 's/$/,"memory-openviking"]/' | sed 's/\[,/[/') + fi fi "${oc_env[@]}" openclaw config set plugins.allow "$existing_allow" --json Suggestion importance[1-10]: 5 __ Why: Using grep/sed for JSON is fragile (e.g., fails with whitespace variations). The suggestion improves robustness with Python (with fallback), but it's a non-critical maintainability improvement.	Low

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}