fix(openclaw): align paths with upstream openclaw/openclaw conventions#117
fix(openclaw): align paths with upstream openclaw/openclaw conventions#117
Conversation
- skillsDir: 'skills' -> '.openclaw/skills' (match project-scoped convention) - configFile: 'CLAUDE.md' -> 'AGENTS.md' (upstream uses AGENTS.md/SOUL.md) - globalSkillsDir: '~/.openclaw/skills' -> '~/.openclaw/workspace/skills' - altSkillsDirs: keep 'skills' for back-compat, add workspace variant - isDetected: also check project .openclaw/ and global workspace/ - MCP AGENT_DIR_MAP: .openclaw/skills for runtime discovery - README: reflect real paths Refs NousResearch upstream layout; pairs with #114 Hermes support.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughProject-local OpenClaw skills moved to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
| const globalWorkspace = join(homedir(), '.openclaw', 'workspace'); | ||
| const openclawConfig = join(process.cwd(), 'openclaw.json'); | ||
|
|
||
| return existsSync(globalOpenClaw) || existsSync(openclawConfig); | ||
| return existsSync(projectOpenClaw) || existsSync(globalOpenClaw) || | ||
| existsSync(globalWorkspace) || existsSync(openclawConfig); |
There was a problem hiding this comment.
🟡 Redundant existsSync(globalWorkspace) check — always shadowed by parent directory check
globalWorkspace is join(homedir(), '.openclaw', 'workspace') (i.e. ~/.openclaw/workspace), while globalOpenClaw is join(homedir(), '.openclaw') (i.e. ~/.openclaw). Since ~/.openclaw/workspace is a subdirectory of ~/.openclaw, it's impossible for existsSync(globalWorkspace) to be true when existsSync(globalOpenClaw) is false — the parent directory must exist for the child to exist. The globalWorkspace variable and its check on line 22-23 are dead code. If the intent was to detect specifically the workspace subdirectory (not just any .openclaw presence), the globalOpenClaw check should be removed or reordered.
| const globalWorkspace = join(homedir(), '.openclaw', 'workspace'); | |
| const openclawConfig = join(process.cwd(), 'openclaw.json'); | |
| return existsSync(globalOpenClaw) || existsSync(openclawConfig); | |
| return existsSync(projectOpenClaw) || existsSync(globalOpenClaw) || | |
| existsSync(globalWorkspace) || existsSync(openclawConfig); | |
| const globalWorkspace = join(homedir(), '.openclaw', 'workspace'); | |
| const openclawConfig = join(process.cwd(), 'openclaw.json'); | |
| return existsSync(projectOpenClaw) || existsSync(globalWorkspace) || | |
| existsSync(openclawConfig); |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/agent-config.ts`:
- Line 127: getSearchDirs() is still deriving the home path from
adapter.skillsDir so setting globalSkillsDir alone doesn't change the actual
search locations; update getSearchDirs() (and any code that reads
adapter.skillsDir) to include or prefer the new globalSkillsDir value when
constructing search paths (e.g., treat globalSkillsDir as the canonical
home/workspace path for global skills), and ensure adapter.skillsDir falls back
to or is composed from globalSkillsDir so that "~/.openclaw/workspace/skills" is
used instead of "~/.openclaw/skills".
- Line 125: ContextSync's detection currently only uses config.configFile and
changing it to 'AGENTS.md' hides projects that only have 'openclaw.json' even
though OpenClawAdapter.isDetected() still checks it; update ContextSync's
detection logic (where it reads config.configFile) to also consult a new
config.altConfigFiles array (or similar) so it checks both config.configFile and
each entry in config.altConfigFiles when determining detection; ensure
OpenClawAdapter.isDetected() remains compatible and add/rename the config
property references (config.configFile and config.altConfigFiles) used by the
detection code to cover both markers.
In `@packages/mcp/src/tools.ts`:
- Line 188: AGENT_DIR_MAP currently lists '.openclaw/skills' for the 'openclaw'
agent but omits the global workspace path, causing discovery to miss
'~/.openclaw/workspace/skills'; update AGENT_DIR_MAP['openclaw'] to include
'~/.openclaw/workspace/skills' (and mirror the same addition where AGENT_DIR_MAP
is defined around the similar block referenced) and ensure SKILL_DISCOVERY_PATHS
(or the discovery routine that checks agent-specific paths under home) will
expand and check tilde/home paths for agent entries so that both project-root
'.openclaw/skills' and the global '~/.openclaw/workspace/skills' are discovered
by skillkit_catalog/load flows.
🪄 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: 254b9a3d-2592-4f91-9e87-c65d80b71448
📒 Files selected for processing (4)
packages/agents/README.mdpackages/agents/src/openclaw.tspackages/core/src/agent-config.tspackages/mcp/src/tools.ts
| altSkillsDirs: ['~/.openclaw/skills'], | ||
| globalSkillsDir: '~/.openclaw/skills', | ||
| skillsDir: '.openclaw/skills', | ||
| configFile: 'AGENTS.md', |
There was a problem hiding this comment.
Keep openclaw.json as a core detection marker.
ContextSync checks only config.configFile, so changing this to AGENTS.md makes projects with only upstream openclaw.json invisible to core agent detection, even though OpenClawAdapter.isDetected() still checks it.
🐛 Proposed direction
export interface AgentDirectoryConfig {
/** Config file that references skills */
configFile: string;
+ /** Additional config files that identify the agent */
+ altConfigFiles?: string[]; openclaw: {
skillsDir: '.openclaw/skills',
configFile: 'AGENTS.md',
+ altConfigFiles: ['openclaw.json'],
altSkillsDirs: ['skills', '~/.openclaw/workspace/skills'],Then update core detection to check config.configFile plus config.altConfigFiles.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/agent-config.ts` at line 125, ContextSync's detection
currently only uses config.configFile and changing it to 'AGENTS.md' hides
projects that only have 'openclaw.json' even though OpenClawAdapter.isDetected()
still checks it; update ContextSync's detection logic (where it reads
config.configFile) to also consult a new config.altConfigFiles array (or
similar) so it checks both config.configFile and each entry in
config.altConfigFiles when determining detection; ensure
OpenClawAdapter.isDetected() remains compatible and add/rename the config
property references (config.configFile and config.altConfigFiles) used by the
detection code to cover both markers.
| skillsDir: '.openclaw/skills', | ||
| configFile: 'AGENTS.md', | ||
| altSkillsDirs: ['skills', '~/.openclaw/workspace/skills'], | ||
| globalSkillsDir: '~/.openclaw/workspace/skills', |
There was a problem hiding this comment.
Wire globalSkillsDir into skill search paths.
The new value is not enough by itself: getSearchDirs() currently derives the home path from adapter.skillsDir, which points OpenClaw at ~/.openclaw/skills instead of ~/.openclaw/workspace/skills.
🐛 Proposed direction
export function getSearchDirs(adapter: AgentAdapterInfo): string[] {
const dirs: string[] = [];
dirs.push(join(process.cwd(), adapter.skillsDir));
dirs.push(join(process.cwd(), '.agent', 'skills'));
- dirs.push(join(homedir(), adapter.skillsDir));
+ if (adapter.globalSkillsDir) {
+ dirs.push(adapter.globalSkillsDir.replace(/^~(?=\/)/, homedir()));
+ } else {
+ dirs.push(join(homedir(), adapter.skillsDir));
+ }
dirs.push(join(homedir(), '.agent', 'skills'));
return dirs;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/agent-config.ts` at line 127, getSearchDirs() is still
deriving the home path from adapter.skillsDir so setting globalSkillsDir alone
doesn't change the actual search locations; update getSearchDirs() (and any code
that reads adapter.skillsDir) to include or prefer the new globalSkillsDir value
when constructing search paths (e.g., treat globalSkillsDir as the canonical
home/workspace path for global skills), and ensure adapter.skillsDir falls back
to or is composed from globalSkillsDir so that "~/.openclaw/workspace/skills" is
used instead of "~/.openclaw/skills".
- skills.ts: add '.openclaw/skills' to SKILL_DISCOVERY_PATHS (alphabetical) - config.ts: getSearchDirs prefers globalSkillsDir for home path (was deriving from skillsDir, producing ~/.openclaw/skills instead of ~/.openclaw/workspace/skills) - agent-config.ts: add altConfigFiles to AgentDirectoryConfig schema, set openclaw altConfigFiles: ['openclaw.json'] - context/sync.ts: detectAgents also iterates altConfigFiles so projects with only openclaw.json still detected - mcp/tools.ts: AGENT_DIR_MAP includes '.openclaw/workspace/skills' for home-rooted global discovery
There was a problem hiding this comment.
🔴 getInstallDir global path inconsistent with getSearchDirs for openclaw
The PR updated getSearchDirs (line 104-112) to resolve the global skills directory from AGENT_CONFIG[adapter.type]?.globalSkillsDir, which for openclaw resolves to ~/.openclaw/workspace/skills. However, the companion function getInstallDir (packages/core/src/config.ts:118-124) was not updated and still uses join(homedir(), adapter.skillsDir), which for openclaw yields ~/.openclaw/skills. This means skills installed globally via skillkit install --global go to ~/.openclaw/skills, but getSearchDirs looks in ~/.openclaw/workspace/skills — so globally installed skills won't be found during search or sync. The same function is called by packages/cli/src/helpers.ts:36 and packages/tui/src/utils/helpers.ts:48 and used in install/quick commands.
(Refers to lines 119-120)
Prompt for agents
The getInstallDir function at packages/core/src/config.ts:118-124 uses join(homedir(), adapter.skillsDir) for the global install path. This was fine before because getSearchDirs also used the same expression. However, getSearchDirs was updated (lines 104-112) to resolve the actual globalSkillsDir from AGENT_CONFIG, which for openclaw is ~/.openclaw/workspace/skills (not ~/.openclaw/skills which adapter.skillsDir would produce).
The fix should make getInstallDir also use AGENT_CONFIG[adapter.type]?.globalSkillsDir when available, with the same tilde-resolution logic. Something like:
if (global) {
const globalDir = AGENT_CONFIG[adapter.type]?.globalSkillsDir;
if (globalDir) {
return globalDir.startsWith('~/')
? join(homedir(), globalDir.slice(2))
: globalDir;
}
return join(homedir(), adapter.skillsDir);
}
This ensures getInstallDir and getSearchDirs agree on the global directory location. The AGENT_CONFIG import is already present in this file (line 7).
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/config.ts (1)
98-121:⚠️ Potential issue | 🟠 MajorKeep OpenClaw search, alternate paths, and global install paths in sync.
getSearchDirs()now searches OpenClaw globals in~/.openclaw/workspace/skills, butgetInstallDir(adapter, true)still installs to~/.openclaw/skills. Also,altSkillsDirssuch as the back-compatskillspath are not added here, so older OpenClaw project layouts can still be missed by core search flows.🐛 Proposed fix
+function resolveConfiguredDir(dir: string, root: string): string { + if (dir.startsWith('~/')) { + return join(homedir(), dir.slice(2)); + } + return join(root, dir); +} + +function getGlobalSkillsDir(adapter: AgentAdapterInfo): string { + const globalDir = AGENT_CONFIG[adapter.type]?.globalSkillsDir; + return globalDir + ? resolveConfiguredDir(globalDir, homedir()) + : join(homedir(), adapter.skillsDir); +} + +function pushUnique(dirs: string[], dir: string): void { + if (!dirs.includes(dir)) { + dirs.push(dir); + } +} + export function getSearchDirs(adapter: AgentAdapterInfo): string[] { const dirs: string[] = []; - dirs.push(join(process.cwd(), adapter.skillsDir)); - dirs.push(join(process.cwd(), '.agent', 'skills')); + pushUnique(dirs, join(process.cwd(), adapter.skillsDir)); + pushUnique(dirs, join(process.cwd(), '.agent', 'skills')); - const globalDir = AGENT_CONFIG[adapter.type]?.globalSkillsDir; - if (globalDir) { - const resolved = globalDir.startsWith('~/') - ? join(homedir(), globalDir.slice(2)) - : globalDir; - dirs.push(resolved); - } else { - dirs.push(join(homedir(), adapter.skillsDir)); + for (const altDir of AGENT_CONFIG[adapter.type]?.altSkillsDirs ?? []) { + pushUnique(dirs, resolveConfiguredDir(altDir, process.cwd())); } - dirs.push(join(homedir(), '.agent', 'skills')); + + pushUnique(dirs, getGlobalSkillsDir(adapter)); + pushUnique(dirs, join(homedir(), '.agent', 'skills')); return dirs; } export function getInstallDir(adapter: AgentAdapterInfo, global = false): string { if (global) { - return join(homedir(), adapter.skillsDir); + return getGlobalSkillsDir(adapter); } return join(process.cwd(), adapter.skillsDir); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/config.ts` around lines 98 - 121, getSearchDirs and getInstallDir are out of sync for OpenClaw globals and alternate/back-compat paths; update getInstallDir(adapter, true) to mirror getSearchDirs' logic: resolve AGENT_CONFIG[adapter.type]?.globalSkillsDir (handling '~/'), fall back to the same homedir workspace path used in getSearchDirs (e.g., '~/.openclaw/workspace/skills' instead of '~/.openclaw/skills'), and ensure any alternate/back-compat directories (the same altSkillsDirs/legacy 'skills' paths you add to getSearchDirs) are included or considered when returning the global install target so installs and searches use the same set of global locations.
♻️ Duplicate comments (1)
packages/mcp/src/tools.ts (1)
188-236:⚠️ Potential issue | 🟠 MajorMake the OpenClaw workspace path discoverable without an agent filter.
Line 188 adds
.openclaw/workspace/skillsonly throughAGENT_DIR_MAP, but the extra-dir pass at Lines 224-233 runs only whenagentFilteris set.skillkit_load,skillkit_resource, and unfiltered catalog calls still miss~/.openclaw/workspace/skills.🐛 Proposed fix
- if (agentFilter) { - const agentDirs = AGENT_DIR_MAP[agentFilter] ?? []; - for (const rel of agentDirs) { + const extraAgentDirs = agentFilter + ? (AGENT_DIR_MAP[agentFilter] ?? []) + : [...new Set(Object.values(AGENT_DIR_MAP).flat())]; + + for (const rel of extraAgentDirs) { if (!SKILL_DISCOVERY_PATHS.includes(rel)) { const full = join(home, rel); if (existsSync(full) && !dirs.includes(full)) { dirs.push(full); } } - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/mcp/src/tools.ts` around lines 188 - 236, The getLocalSkillDirs function only includes '.openclaw/workspace/skills' via AGENT_DIR_MAP and adds AGENT_DIR_MAP-only paths to the discovered dirs only when agentFilter is provided, so unfiltered calls miss that workspace; update getLocalSkillDirs to treat AGENT_DIR_MAP entries that are not in SKILL_DISCOVERY_PATHS as additional discovery paths for all runs (not only when agentFilter is set) — e.g., merge unique extra agentDirs like '.openclaw/workspace/skills' into the SKILL_DISCOVERY_PATHS loop or add a precomputed set of additionalRoots used in the first roots loop; ensure you reference AGENT_DIR_MAP, SKILL_DISCOVERY_PATHS and getLocalSkillDirs when changing the logic so the home-based '.openclaw/workspace/skills' gets checked even without an agentFilter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/core/src/config.ts`:
- Around line 98-121: getSearchDirs and getInstallDir are out of sync for
OpenClaw globals and alternate/back-compat paths; update getInstallDir(adapter,
true) to mirror getSearchDirs' logic: resolve
AGENT_CONFIG[adapter.type]?.globalSkillsDir (handling '~/'), fall back to the
same homedir workspace path used in getSearchDirs (e.g.,
'~/.openclaw/workspace/skills' instead of '~/.openclaw/skills'), and ensure any
alternate/back-compat directories (the same altSkillsDirs/legacy 'skills' paths
you add to getSearchDirs) are included or considered when returning the global
install target so installs and searches use the same set of global locations.
---
Duplicate comments:
In `@packages/mcp/src/tools.ts`:
- Around line 188-236: The getLocalSkillDirs function only includes
'.openclaw/workspace/skills' via AGENT_DIR_MAP and adds AGENT_DIR_MAP-only paths
to the discovered dirs only when agentFilter is provided, so unfiltered calls
miss that workspace; update getLocalSkillDirs to treat AGENT_DIR_MAP entries
that are not in SKILL_DISCOVERY_PATHS as additional discovery paths for all runs
(not only when agentFilter is set) — e.g., merge unique extra agentDirs like
'.openclaw/workspace/skills' into the SKILL_DISCOVERY_PATHS loop or add a
precomputed set of additionalRoots used in the first roots loop; ensure you
reference AGENT_DIR_MAP, SKILL_DISCOVERY_PATHS and getLocalSkillDirs when
changing the logic so the home-based '.openclaw/workspace/skills' gets checked
even without an agentFilter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6cdb9d4-3497-4f30-8e67-fb21118e1aae
📒 Files selected for processing (5)
packages/core/src/agent-config.tspackages/core/src/config.tspackages/core/src/context/sync.tspackages/core/src/skills.tspackages/mcp/src/tools.ts
✅ Files skipped from review due to trivial changes (1)
- packages/core/src/skills.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/agent-config.ts
Includes: - v1.24.0 across all publishable packages - Add packages/mcp + packages/api to bump-version.sh (were drifting) - Regenerate assets/tags/v1.24.0.svg, remove stale v1.23.0.svg - Bump agent count references from 45 to 46 (adds Hermes) - Update gen-tags.mjs (version + agent count) - Sync onboarding logo test expectation Release covers rohitg00#114 (Hermes native support) + rohitg00#117 (OpenClaw upstream paths).
Summary
While reviewing #114 (Hermes support), noticed OpenClaw config drifts from real openclaw/openclaw upstream layout. This PR aligns it.
Upstream (openclaw/openclaw)
.openclaw/skills/<name>/SKILL.md~/.openclaw/workspace/skills/<name>/SKILL.mdAGENTS.md(+ SOUL.md, TOOLS.md)openclaw.jsonCurrent vs upstream
skills.openclaw/skillsCLAUDE.mdAGENTS.md~/.openclaw/skills~/.openclaw/workspace/skills['~/.openclaw/skills']skillsback-compat + added workspace~/.openclaw+openclaw.json.openclaw/+ workspace['skills']['.openclaw/skills']Changes
packages/core/src/agent-config.ts: skillsDir, configFile, globalSkillsDir, altSkillsDirspackages/agents/src/openclaw.ts: isDetected adds.openclaw/andworkspace/packages/mcp/src/tools.ts: AGENT_DIR_MAP runtime discovery for.openclaw/skillspackages/agents/README.md: table reflects real pathsValidation
Back-compat
altSkillsDirs: ['skills', '~/.openclaw/workspace/skills']keeps old bareskills/dir detectable so projects installed before this fix keep working.Follow-up
After #114 lands, Hermes has 17 touchpoints. OpenClaw now has matching path coverage. Both match upstream conventions.
Summary by CodeRabbit
Documentation
Chores
Bug Fixes / Improvements