fix: address review feedback — remove unrelated umask, fix fetchFromGitHub naming, simplify checks

- Remove accidentally introduced umask/migration changes (unrelated to plugins)
- Add pluginName helper, fix fetchFromGitHub producing name='source'
- Show name= in extraPlugins example docs
- Simplify checks.nix: use hermes-agent.override instead of re-callPackage
- Fix fragile grep shell logic in checks
This commit is contained in:
alt-glitch
2026-04-26 14:07:14 +05:30
parent 4875ee5c04
commit 8d8d7029db
2 changed files with 39 additions and 52 deletions

View File

@@ -7,9 +7,7 @@
perSystem = { pkgs, system, lib, inputs', ... }: perSystem = { pkgs, system, lib, inputs', ... }:
let let
hermes-agent = inputs.self.packages.${system}.default; hermes-agent = inputs.self.packages.${system}.default;
hermesVenv = pkgs.callPackage ./python.nix { hermesVenv = hermes-agent.hermesVenv;
inherit (inputs) uv2nix pyproject-nix pyproject-build-systems;
};
configMergeScript = pkgs.callPackage ./configMergeScript.nix { }; configMergeScript = pkgs.callPackage ./configMergeScript.nix { };
@@ -196,9 +194,7 @@ json.dump(sorted(leaf_paths(DEFAULT_CONFIG)), sys.stdout, indent=2)
# Verify extraPythonPackages PYTHONPATH injection # Verify extraPythonPackages PYTHONPATH injection
extra-python-packages = let extra-python-packages = let
testPkg = pkgs.python312Packages.pyfiglet; testPkg = pkgs.python312Packages.pyfiglet;
hermesWithExtra = pkgs.callPackage ./hermes-agent.nix { hermesWithExtra = hermes-agent.override {
inherit (inputs) uv2nix pyproject-nix pyproject-build-systems;
npm-lockfile-fix = inputs'.npm-lockfile-fix.packages.default;
extraPythonPackages = [ testPkg ]; extraPythonPackages = [ testPkg ];
}; };
in pkgs.runCommand "hermes-extra-python-packages" { } '' in pkgs.runCommand "hermes-extra-python-packages" { } ''
@@ -214,8 +210,9 @@ json.dump(sorted(leaf_paths(DEFAULT_CONFIG)), sys.stdout, indent=2)
echo "PASS: test package path found in wrapper" echo "PASS: test package path found in wrapper"
echo "=== Checking base package has no PYTHONPATH ===" echo "=== Checking base package has no PYTHONPATH ==="
grep -q "PYTHONPATH" ${hermes-agent}/bin/hermes && \ if grep -q "PYTHONPATH" ${hermes-agent}/bin/hermes; then
(echo "FAIL: base package should not have PYTHONPATH"; exit 1) || true echo "FAIL: base package should not have PYTHONPATH"; exit 1
fi
echo "PASS: base package clean" echo "PASS: base package clean"
echo "=== All extraPythonPackages checks passed ===" echo "=== All extraPythonPackages checks passed ==="

View File

@@ -32,6 +32,12 @@
else cfg.package.override { inherit (cfg) extraPythonPackages; }; else cfg.package.override { inherit (cfg) extraPythonPackages; };
hermes-agent = inputs.self.packages.${pkgs.stdenv.hostPlatform.system}.default; hermes-agent = inputs.self.packages.${pkgs.stdenv.hostPlatform.system}.default;
# Derive a human-readable plugin name. fetchFromGitHub yields name="source",
# so we fall through to baseNameOf which gives the store hash prefix — not
# ideal but unique. Users can set name= on their fetchFromGitHub to override.
pluginName = plugin:
plugin.pname or plugin.name or (builtins.baseNameOf (toString plugin));
# Deep-merge config type (from 0xrsydn/nix-hermes-agent) # Deep-merge config type (from 0xrsydn/nix-hermes-agent)
deepConfigType = lib.types.mkOptionType { deepConfigType = lib.types.mkOptionType {
name = "hermes-config-attrs"; name = "hermes-config-attrs";
@@ -166,9 +172,6 @@
export PATH="$TARGET_HOME/.venv/bin:$PATH" export PATH="$TARGET_HOME/.venv/bin:$PATH"
fi fi
# Match the native systemd UMask so files are group-writable
umask 0007
if command -v setpriv >/dev/null 2>&1; then if command -v setpriv >/dev/null 2>&1; then
exec setpriv --reuid="$HERMES_UID" --regid="$HERMES_GID" --init-groups "$@" exec setpriv --reuid="$HERMES_UID" --regid="$HERMES_GID" --init-groups "$@"
elif command -v su >/dev/null 2>&1; then elif command -v su >/dev/null 2>&1; then
@@ -474,6 +477,7 @@
(pkgs.fetchFromGitHub { (pkgs.fetchFromGitHub {
owner = "stephenschoettler"; owner = "stephenschoettler";
repo = "hermes-lcm"; repo = "hermes-lcm";
name = "hermes-lcm";
rev = "v0.7.0"; rev = "v0.7.0";
hash = "sha256-..."; hash = "sha256-...";
}) })
@@ -482,29 +486,29 @@
}; };
extraPythonPackages = mkOption { extraPythonPackages = mkOption {
type = types.listOf types.package; type = types.listOf types.package;
default = [ ]; default = [ ];
description = '' description = ''
Python packages to add to PYTHONPATH for entry-point plugin discovery. Python packages to add to PYTHONPATH for entry-point plugin discovery.
These are pip-packaged plugins that register via the These are pip-packaged plugins that register via the
hermes_agent.plugins entry-point group. Each package must be built hermes_agent.plugins entry-point group. Each package must be built
with the same Python interpreter as hermes (python312). with the same Python interpreter as hermes (python312).
''; '';
example = literalExpression '' example = literalExpression ''
[ [
(pkgs.python312Packages.buildPythonPackage { (pkgs.python312Packages.buildPythonPackage {
pname = "rtk-hermes"; pname = "rtk-hermes";
version = "1.0.0"; version = "1.0.0";
src = pkgs.fetchFromGitHub { src = pkgs.fetchFromGitHub {
owner = "ogallotti"; owner = "ogallotti";
repo = "rtk-hermes"; repo = "rtk-hermes";
rev = "main"; rev = "main";
hash = "sha256-..."; hash = "sha256-...";
}; };
}) })
] ]
''; '';
}; };
restart = mkOption { restart = mkOption {
type = types.str; type = types.str;
@@ -634,12 +638,10 @@
# ── Assertions ───────────────────────────────────────────────────── # ── Assertions ─────────────────────────────────────────────────────
{ {
assertions = let assertions = let
pluginNames = map (plugin: names = map pluginName cfg.extraPlugins;
plugin.pname or plugin.name or (builtins.baseNameOf (toString plugin))
) cfg.extraPlugins;
in [{ in [{
assertion = (lib.length pluginNames) == (lib.length (lib.unique pluginNames)); assertion = (lib.length names) == (lib.length (lib.unique names));
message = "services.hermes-agent.extraPlugins: duplicate plugin names detected: ${toString pluginNames}"; message = "services.hermes-agent.extraPlugins: duplicate plugin names detected: ${toString names}. If using fetchFromGitHub, set name = \"plugin-name\" to disambiguate.";
}]; }];
} }
@@ -694,18 +696,6 @@
-exec chmod g+rw {} + 2>/dev/null || true -exec chmod g+rw {} + 2>/dev/null || true
done done
# One-time migration: fix files created by the container before the
# umask 0007 fix. They were 0644 (group read-only) instead of 0660.
_migration=${cfg.stateDir}/.hermes/.migration-group-write
if [ ! -f "$_migration" ]; then
find ${cfg.stateDir}/.hermes -type f ! -perm -g=w \
-exec chmod g+w {} + 2>/dev/null || true
find ${cfg.stateDir}/.hermes -type d ! -perm -g=w \
-exec chmod g+w {} + 2>/dev/null || true
touch "$_migration"
chown ${cfg.user}:${cfg.group} "$_migration"
fi
# Merge Nix settings into existing config.yaml. # Merge Nix settings into existing config.yaml.
# Preserves user-added keys (skills, streaming, etc.); Nix keys win. # Preserves user-added keys (skills, streaming, etc.); Nix keys win.
# If configFile is user-provided (not generated), overwrite instead of merge. # If configFile is user-provided (not generated), overwrite instead of merge.
@@ -820,7 +810,7 @@ HERMES_NIX_ENV_EOF
${lib.concatStringsSep "\n" (map (plugin: ${lib.concatStringsSep "\n" (map (plugin:
let let
name = plugin.pname or plugin.name or (builtins.baseNameOf (toString plugin)); name = pluginName plugin;
in '' in ''
if [ ! -f "${plugin}/plugin.yaml" ]; then if [ ! -f "${plugin}/plugin.yaml" ]; then
echo "ERROR: extraPlugins entry '${plugin}' has no plugin.yaml" >&2 echo "ERROR: extraPlugins entry '${plugin}' has no plugin.yaml" >&2