Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 78 additions & 3 deletions agents/hermes/plugin/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,15 @@
"""
NemoClaw plugin for Hermes Agent.

Provides sandbox status tools and a startup banner when Hermes runs inside
an OpenShell sandbox managed by NemoClaw.
Provides sandbox status tools, skill hot-reload, and a startup banner when
Hermes runs inside an OpenShell sandbox managed by NemoClaw.

Skill hot-reload: Hermes caches its skill slash-command registry in a
module-global dict on first scan. New skills dropped on disk are invisible
until the cache is cleared. This plugin provides a nemoclaw_reload_skills
tool that clears the cache and re-scans, letting the agent pick up new
skills without a gateway restart. The on_session_start hook also refreshes
skills automatically at session boundaries.
"""

import json
Expand Down Expand Up @@ -104,6 +111,49 @@ def _handle_info(tool_input, context):
return json.dumps(_get_sandbox_info(), indent=2)


def _reload_skills():
"""Clear the Hermes skill slash-command cache and re-scan skill directories.

Hermes's ``agent.skill_commands`` module caches discovered skills in a
module-global dict (``_skill_commands``). ``get_skill_commands()`` only
scans on first call, so skills installed after gateway startup are
invisible. We clear the dict and call ``scan_skill_commands()`` to force
a fresh scan.

Returns the dict of discovered skills, or None on failure.
"""
try:
import agent.skill_commands as sc

sc._skill_commands.clear()
return sc.scan_skill_commands()
except ImportError:
return None
except Exception:
return None
Comment on lines +125 to +133

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

fd -i 'skill_commands.py$' | while read -r f; do
  echo "== $f =="
  rg -n -C2 --type=py '_skill_commands|def scan_skill_commands|def get_skill_commands|def reset' "$f"
done

Repository: NVIDIA/NemoClaw

Length of output: 41


🏁 Script executed:

# Check if the file exists and read the relevant lines
cat -n agents/hermes/plugin/__init__.py 2>/dev/null | sed -n '120,140p'

Repository: NVIDIA/NemoClaw

Length of output: 765


🏁 Script executed:

# Search for skill_commands module - try different patterns
fd -i skill_commands || echo "--- No exact match, searching for 'skill_commands' in code ---"
rg -l 'skill_commands' --type=py 2>/dev/null | head -20

Repository: NVIDIA/NemoClaw

Length of output: 91


🏁 Script executed:

# Search for _skill_commands references
rg '_skill_commands' --type=py -n 2>/dev/null

Repository: NVIDIA/NemoClaw

Length of output: 436


🏁 Script executed:

# Find the agent/skill_commands module
fd -i 'skill_commands' . || echo "No match"
find . -name '*skill_commands*' -type f 2>/dev/null

Repository: NVIDIA/NemoClaw

Length of output: 41


🏁 Script executed:

# Search for scan_skill_commands definition
rg 'def scan_skill_commands|def get_skill_commands|_skill_commands\s*=' --type=py -B2 -A5 2>/dev/null

Repository: NVIDIA/NemoClaw

Length of output: 41


🏁 Script executed:

# Check if agent package exists and its structure
ls -la agent/ 2>/dev/null || echo "agent directory not found in root"
find . -path '*/agent/*' -name '*.py' | grep -i skill | head -20

Repository: NVIDIA/NemoClaw

Length of output: 92


🏁 Script executed:

# Read more context around the _reload_skills function
cat -n agents/hermes/plugin/__init__.py | sed -n '115,135p'

Repository: NVIDIA/NemoClaw

Length of output: 893


🏁 Script executed:

# Check for setup.py, pyproject.toml, requirements.txt mentioning 'agent' package
find . -name 'setup.py' -o -name 'pyproject.toml' -o -name 'requirements*.txt' | xargs grep -l 'agent' 2>/dev/null | head -10

Repository: NVIDIA/NemoClaw

Length of output: 41


🏁 Script executed:

# Check for any external dependencies or imports of 'agent'
rg 'from agent|import agent' --type=py 2>/dev/null | head -20

Repository: NVIDIA/NemoClaw

Length of output: 133


🏁 Script executed:

# Look at the function signature and docstring
cat -n agents/hermes/plugin/__init__.py | sed -n '110,135p'

Repository: NVIDIA/NemoClaw

Length of output: 1082


Harden reload error handling and private-cache access.

Line 128 directly accesses the private sc._skill_commands attribute without checking if it exists or is a dict. Line 132 swallows all exceptions, making hot-reload failures silent and hard to diagnose. Even though agent.skill_commands is an optional external dependency, once imported, the code should not assume a specific API shape without defensive checks.

Proposed fix
 def _reload_skills():
     """Clear the Hermes skill slash-command cache and re-scan skill directories.
 
     Hermes's ``agent.skill_commands`` module caches discovered skills in a
     module-global dict (``_skill_commands``).  ``get_skill_commands()`` only
     scans on first call, so skills installed after gateway startup are
     invisible.  We clear the dict and call ``scan_skill_commands()`` to force
     a fresh scan.
 
     Returns the dict of discovered skills, or None on failure.
     """
     try:
         import agent.skill_commands as sc
 
-        sc._skill_commands.clear()
+        cache = getattr(sc, "_skill_commands", None)
+        if isinstance(cache, dict):
+            cache.clear()
         return sc.scan_skill_commands()
     except ImportError:
         return None
-    except Exception:
+    except (AttributeError, TypeError):
+        return None
+    except Exception as exc:
+        # Preserve debuggability while keeping current API behavior.
+        print(f"_reload_skills: unexpected reload failure: {exc}")
         return None
📝 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
try:
import agent.skill_commands as sc
sc._skill_commands.clear()
return sc.scan_skill_commands()
except ImportError:
return None
except Exception:
return None
try:
import agent.skill_commands as sc
cache = getattr(sc, "_skill_commands", None)
if isinstance(cache, dict):
cache.clear()
return sc.scan_skill_commands()
except ImportError:
return None
except (AttributeError, TypeError):
return None
except Exception as exc:
# Preserve debuggability while keeping current API behavior.
print(f"_reload_skills: unexpected reload failure: {exc}")
return None
🧰 Tools
🪛 Ruff (0.15.9)

[warning] 132-132: Do not catch blind exception: Exception

(BLE001)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@agents/hermes/plugin/__init__.py` around lines 125 - 133, The code assumes
agent.skill_commands provides a private dict _skill_commands and currently
swallows all errors; change to defensively access and clear the cache using
something like cache = getattr(sc, "_skill_commands", None) and only call
cache.clear() if cache is not None and has a clear() method (or is a dict), then
call sc.scan_skill_commands(); replace the broad except Exception: return None
with an except Exception as e: that logs the exception (using a module logger or
logging.exception) then returns None so hot-reload failures are visible; keep
ImportError handling as before and reference the module agent.skill_commands,
attribute _skill_commands, and function scan_skill_commands in your changes.



def _handle_reload_skills(tool_input, context):
"""Handle the nemoclaw_reload_skills tool call."""
commands = _reload_skills()
if commands is None:
return (
"Failed to reload skills. The agent.skill_commands module may "
"not be available in this Hermes version."
)

if not commands:
return "Skill reload complete. No skills found in skill directories."

names = sorted(commands.keys())
lines = [f"Skill reload complete. {len(names)} skill(s) discovered:", ""]
for name in names:
info = commands[name]
desc = info.get("description", "no description")
lines.append(f" {name}: {desc}")
return "\n".join(lines)


def register(ctx):
"""Register NemoClaw tools and hooks with Hermes."""

Expand Down Expand Up @@ -142,8 +192,32 @@ def register(ctx):
description="NemoClaw sandbox info (JSON)",
)

# Register skill reload tool
ctx.register_tool(
name="nemoclaw_reload_skills",
toolset="nemoclaw",
schema={
"type": "function",
"function": {
"name": "nemoclaw_reload_skills",
"description": (
"Reload and re-discover skills from the skill directories. "
"Call this after new skills have been installed to make them "
"available as slash commands without restarting the gateway."
),
"parameters": {"type": "object", "properties": {}},
},
},
handler=_handle_reload_skills,
description="Reload skills from disk without gateway restart",
)

# Startup banner on session start
def _on_session_start(**kwargs):
# Refresh skill cache so skills installed since last session are
# immediately available as slash commands.
_reload_skills()

info = _get_sandbox_info()
banner = (
"\n"
Expand All @@ -153,7 +227,8 @@ def _on_session_start(**kwargs):
f" \u2502 Model: {info['model']:<40}\u2502\n"
f" \u2502 Provider: {info['provider']:<40}\u2502\n"
f" \u2502 Gateway: {info['gateway']:<40}\u2502\n"
" \u2502 Tools: nemoclaw_status, nemoclaw_info \u2502\n"
" \u2502 Tools: nemoclaw_status, nemoclaw_info, \u2502\n"
" \u2502 nemoclaw_reload_skills \u2502\n"
" \u2514\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2500\u2518\n"
)
try:
Expand Down
3 changes: 2 additions & 1 deletion agents/hermes/plugin/plugin.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
# SPDX-License-Identifier: Apache-2.0

name: nemoclaw
version: "0.0.11"
version: "0.0.12"
description: "NemoClaw sandbox management for Hermes running inside OpenShell"
author: "NVIDIA Corporation"
manifest_version: 1

provides_tools:
- nemoclaw_status
- nemoclaw_info
- nemoclaw_reload_skills

provides_hooks:
- on_session_start
233 changes: 233 additions & 0 deletions src/lib/skill-install.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,233 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import { describe, it, expect } from "vitest";
import { mkdtempSync, writeFileSync, mkdirSync, rmSync } from "node:fs";
import { join } from "node:path";
import { tmpdir } from "node:os";
// Import from compiled dist/ so coverage is attributed correctly.
import {
parseFrontmatter,
resolveSkillPaths,
collectFiles,
validateRelativePath,
shellQuote,
} from "../../dist/lib/skill-install";

describe("parseFrontmatter", () => {
it("extracts name from valid frontmatter", () => {
const result = parseFrontmatter("---\nname: my-skill\ndescription: test\n---\n# Body");
expect(result).toEqual({ name: "my-skill" });
});

it("handles quoted name values", () => {
expect(parseFrontmatter('---\nname: "my-tool"\n---\n').name).toBe("my-tool");
expect(parseFrontmatter("---\nname: 'demo.tool'\n---\n").name).toBe("demo.tool");
});

it("handles name with dots, hyphens, and underscores", () => {
expect(parseFrontmatter("---\nname: my_skill.v2-beta\n---\n").name).toBe("my_skill.v2-beta");
});

it("parses complex YAML metadata beyond name", () => {
const fm = parseFrontmatter(
'---\nname: rich-skill\ndescription: "A skill"\nmetadata: { "openclaw": { "emoji": "🔧" } }\n---\n',
);
expect(fm.name).toBe("rich-skill");
});

it("rejects malformed YAML", () => {
expect(() =>
parseFrontmatter("---\nname: ok\ndescription: [broken\n---\n"),
).toThrow("not valid YAML");
});

it("rejects non-mapping frontmatter", () => {
expect(() => parseFrontmatter("---\n- just\n- a list\n---\n")).toThrow("must be a YAML mapping");
});

it("throws when frontmatter is missing entirely", () => {
expect(() => parseFrontmatter("# Just markdown\nNo frontmatter")).toThrow(
"missing YAML frontmatter",
);
});

it("throws when closing delimiter is missing", () => {
expect(() => parseFrontmatter("---\nname: broken\n# No closing")).toThrow(
"missing closing --- frontmatter delimiter",
);
});

it("throws when name field is absent", () => {
expect(() => parseFrontmatter("---\ndescription: no name here\n---\n")).toThrow(
"missing required 'name' field",
);
});

it("throws when name field is empty or null", () => {
expect(() => parseFrontmatter("---\nname:\n---\n")).toThrow("missing required 'name' field");
expect(() => parseFrontmatter('---\nname: ""\n---\n')).toThrow("missing required 'name' field");
});

it("rejects names with invalid characters", () => {
expect(() => parseFrontmatter("---\nname: my skill\n---\n")).toThrow("invalid characters");
expect(() => parseFrontmatter("---\nname: ../escape\n---\n")).toThrow("invalid characters");
expect(() => parseFrontmatter("---\nname: a/b\n---\n")).toThrow("invalid characters");
});
});

describe("validateRelativePath", () => {
it("accepts safe paths", () => {
expect(validateRelativePath("SKILL.md")).toBe(true);
expect(validateRelativePath("scripts/helper.js")).toBe(true);
expect(validateRelativePath("data/config-v2.yaml")).toBe(true);
});

it("rejects shell metacharacters", () => {
expect(validateRelativePath("$(touch /tmp/pwn).js")).toBe(false);
expect(validateRelativePath("a'b.txt")).toBe(false);
expect(validateRelativePath('a"b.txt')).toBe(false);
expect(validateRelativePath("a`b`.txt")).toBe(false);
expect(validateRelativePath("file name.txt")).toBe(false);
expect(validateRelativePath("a;rm -rf.txt")).toBe(false);
});

it("rejects directory traversal", () => {
expect(validateRelativePath("../escape")).toBe(false);
expect(validateRelativePath("foo/../../etc/passwd")).toBe(false);
expect(validateRelativePath("./current")).toBe(false);
});

it("rejects empty and degenerate paths", () => {
expect(validateRelativePath("")).toBe(false);
expect(validateRelativePath("/absolute")).toBe(false);
expect(validateRelativePath("foo//bar")).toBe(false);
});
});

describe("shellQuote", () => {
it("wraps simple strings in single quotes", () => {
expect(shellQuote("hello")).toBe("'hello'");
});

it("escapes embedded single quotes", () => {
expect(shellQuote("it's")).toBe("'it'\\''s'");
});
});

describe("collectFiles", () => {
let tmpDir: string;

function setup(files: Record<string, string>) {
tmpDir = mkdtempSync(join(tmpdir(), "skill-test-"));
for (const [rel, content] of Object.entries(files)) {
const full = join(tmpDir, rel);
mkdirSync(join(full, ".."), { recursive: true });
writeFileSync(full, content);
}
}

function cleanup() {
if (tmpDir) rmSync(tmpDir, { recursive: true, force: true });
}

it("collects a single SKILL.md", () => {
setup({ "SKILL.md": "---\nname: solo\n---\n" });
try {
const { files, skippedDotfiles, unsafePaths } = collectFiles(tmpDir);
expect(files).toEqual(["SKILL.md"]);
expect(skippedDotfiles).toEqual([]);
expect(unsafePaths).toEqual([]);
} finally {
cleanup();
}
});

it("collects SKILL.md plus nested scripts, skips dotfiles", () => {
setup({
"SKILL.md": "---\nname: rich\n---\n",
"scripts/helper.js": "console.log('hi')",
".env": "KEY=val",
});
try {
const { files, skippedDotfiles } = collectFiles(tmpDir);
expect(files.sort()).toEqual(["SKILL.md", "scripts/helper.js"]);
expect(skippedDotfiles).toEqual([".env"]);
} finally {
cleanup();
}
});

it("flags files with unsafe characters", () => {
setup({
"SKILL.md": "---\nname: bad\n---\n",
"has space.txt": "content",
});
try {
const { files, unsafePaths } = collectFiles(tmpDir);
expect(files).toEqual(["SKILL.md"]);
expect(unsafePaths).toEqual(["has space.txt"]);
} finally {
cleanup();
}
});
});

describe("resolveSkillPaths", () => {
it("returns OpenClaw defaults when agent is null", () => {
const paths = resolveSkillPaths(null, "weather");
expect(paths.uploadDir).toBe("/sandbox/.openclaw/skills/weather");
expect(paths.mirrorDir).toBe("$HOME/.openclaw/skills/weather");
expect(paths.sessionFile).toBe(
"/sandbox/.openclaw-data/agents/main/sessions/sessions.json",
);
expect(paths.isOpenClaw).toBe(true);
});

it("returns OpenClaw paths when agent.name is 'openclaw'", () => {
const agent = {
name: "openclaw",
configPaths: {
immutableDir: "/sandbox/.openclaw",
writableDir: "/sandbox/.openclaw-data",
},
};
const paths = resolveSkillPaths(agent, "my-skill");
expect(paths.uploadDir).toBe("/sandbox/.openclaw/skills/my-skill");
expect(paths.mirrorDir).toBe("$HOME/.openclaw/skills/my-skill");
expect(paths.sessionFile).toBe(
"/sandbox/.openclaw-data/agents/main/sessions/sessions.json",
);
expect(paths.isOpenClaw).toBe(true);
});

it("returns Hermes paths without mirror or session refresh", () => {
const agent = {
name: "hermes",
configPaths: {
immutableDir: "/sandbox/.hermes",
writableDir: "/sandbox/.hermes-data",
},
};
const paths = resolveSkillPaths(agent, "demo-skill");
expect(paths.uploadDir).toBe("/sandbox/.hermes/skills/demo-skill");
expect(paths.mirrorDir).toBeNull();
expect(paths.sessionFile).toBeNull();
expect(paths.isOpenClaw).toBe(false);
});

it("returns generic paths for a hypothetical future agent", () => {
const agent = {
name: "future-agent",
configPaths: {
immutableDir: "/sandbox/.future",
writableDir: "/sandbox/.future-data",
},
};
const paths = resolveSkillPaths(agent, "test-skill");
expect(paths.uploadDir).toBe("/sandbox/.future/skills/test-skill");
expect(paths.mirrorDir).toBeNull();
expect(paths.sessionFile).toBeNull();
expect(paths.isOpenClaw).toBe(false);
});
});
Loading
Loading