fix: resolve agent workspaces from openclaw config#261
fix: resolve agent workspaces from openclaw config#261DrJsPBs wants to merge 2 commits intodaggerhashimoto:masterfrom
Conversation
|
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)
📝 WalkthroughWalkthroughThis PR adds reading of OpenClaw's Changes
Sequence DiagramsequenceDiagram
participant Client as Browser Client
participant Server as Nerve Server
participant Config as openclaw.json
participant FileWatcher as File Watcher
participant FS as Filesystem
Note over Server: Startup / runtime
Server->>Config: resolveOpenClawConfigPath() & read (cached)
Config-->>Server: agents.list & agents.defaults
Server->>Server: getConfiguredAgentWorkspace(agentId)
alt configured workspace exists
Server-->>Server: return configured workspaceRoot
else fallback
Server-->>Server: buildDefaultAgentWorkspacePath(agentId)
end
Server->>FileWatcher: listConfiguredAgentWorkspaces()
FileWatcher->>FS: add watchers for configured workspace roots
Note over Client,Server: API & agent creation
Client->>Server: GET /api/server-info
Server-->>Client: defaultAgentWorkspaceRoot
Client->>Client: compute workspace = defaultAgentWorkspaceRoot/agentId
Client->>Server: agents.create RPC (workspace)
Server->>FS: resolveAgentWorkspace and probe MEMORY.md / memory/
FS-->>Server: existence results
Server-->>Client: agent created
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
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 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: 1
🧹 Nitpick comments (1)
src/contexts/SessionContext.tsx (1)
160-164: Store the trimmed workspace root value after validation.Line 161 checks
.trim()but Line 162 stores the original string; leading/trailing whitespace can still propagate into workspace path construction.♻️ Suggested refinement
setDefaultAgentWorkspaceRoot( - typeof data.defaultAgentWorkspaceRoot === 'string' && data.defaultAgentWorkspaceRoot.trim() - ? data.defaultAgentWorkspaceRoot + typeof data.defaultAgentWorkspaceRoot === 'string' && data.defaultAgentWorkspaceRoot.trim() + ? data.defaultAgentWorkspaceRoot.trim() : null, );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/contexts/SessionContext.tsx` around lines 160 - 164, The current validation uses data.defaultAgentWorkspaceRoot.trim() to check for non-empty but then stores the original string, which allows leading/trailing whitespace to persist; update the setDefaultAgentWorkspaceRoot call so it stores the trimmed value (or null) by computing a trimmed variable from data.defaultAgentWorkspaceRoot and passing that trimmed string when non-empty, otherwise null—refer to setDefaultAgentWorkspaceRoot and data.defaultAgentWorkspaceRoot to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/lib/file-watcher.ts`:
- Around line 213-217: The watcher currently checks for the hardcoded string
'openclaw.json' so custom config filenames set via OPENCLAW_CONFIG_PATH won't
trigger refreshWorkspaceWatchers(); compute the actual config basename once
(e.g., const configFileName = path.basename(resolveOpenClawConfigPath())) and
update the rootDirWatcher callback condition to check file === configFileName
(and keep the existing watchingConfigFile && !file branch) so
getWatchFilename(...) comparisons correctly detect custom-config changes and
call refreshWorkspaceWatchers(); update imports/use of path.basename where
needed and keep references to watchingConfigFile, resolveOpenClawConfigPath,
getWatchFilename, refreshWorkspaceWatchers, and WORKSPACE_PREFIX.
---
Nitpick comments:
In `@src/contexts/SessionContext.tsx`:
- Around line 160-164: The current validation uses
data.defaultAgentWorkspaceRoot.trim() to check for non-empty but then stores the
original string, which allows leading/trailing whitespace to persist; update the
setDefaultAgentWorkspaceRoot call so it stores the trimmed value (or null) by
computing a trimmed variable from data.defaultAgentWorkspaceRoot and passing
that trimmed string when non-empty, otherwise null—refer to
setDefaultAgentWorkspaceRoot and data.defaultAgentWorkspaceRoot to locate the
change.
🪄 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: ee09025c-3534-44bd-958a-947dda4211f0
📒 Files selected for processing (8)
server/lib/agent-workspace.test.tsserver/lib/agent-workspace.tsserver/lib/file-watcher.tsserver/lib/openclaw-config.tsserver/routes/server-info.test.tsserver/routes/server-info.tssrc/contexts/SessionContext.test.tsxsrc/contexts/SessionContext.tsx
Summary
workspace-<agentId>agents.defaults.workspacewhen choosing the workspace path for newly created root agentsworkspace-*Problem
Closes #260.
Nerve was assuming every non-main agent lived at
~/.openclaw/workspace-<agentId>. On setups where OpenClaw stores explicitagents.list[].workspacepaths, local agents were misclassified as remote/sandboxed, which disabled memory management and caused workspace watcher drift.Verification
Targeted checks:
npm test -- --run server/lib/agent-workspace.test.ts server/routes/server-info.test.ts src/contexts/SessionContext.test.tsxnpm test -- --run server/routes/workspace.test.ts server/routes/file-browser.test.ts server/routes/skills.test.tsIsolated contributor-style verification (clean copy of the PR branch, no copied repo
.env):rsync -a --delete --exclude '.git' --exclude '.env' --exclude 'node_modules' /home/drj/nerve/ /tmp/nerve-upstream-verify/ln -s /home/drj/nerve/node_modules /tmp/nerve-upstream-verify/node_modulesenv -i HOME="$HOME" PATH="$PATH" TERM="${TERM:-xterm}" CI=1 npm run lintenv -i HOME="$HOME" PATH="$PATH" TERM="${TERM:-xterm}" CI=1 npm run build && env -i HOME="$HOME" PATH="$PATH" TERM="${TERM:-xterm}" CI=1 npm run build:serverenv -i HOME="$HOME" PATH="$PATH" TERM="${TERM:-xterm}" CI=1 npm testnpm run lint✅ (0 errors, 6 existing warnings)npm run build && npm run build:server✅npm test✅122 passed (122)1639 passed (1639)This also confirms the earlier
server/lib/config.test.tsfailure was local-env contamination from a repo.env, not a branch regression.Summary by CodeRabbit
New Features
Tests