mars(mnemosyne): fix review blockers — relative plugin symlink, wipe-and-rebuild venv, PyYAML config cue, drop root-executes-luna-tree

hosts/mars/hermes-agent.nix

@@ -76,15 +76,24 @@ let
 
   # Mnemosyne memory provider (local SQLite, third-party plugin — not bundled
   # with Hermes). Requirements pins live in ./mnemosyne/requirements.txt; see
-  # the provisioning unit near the bottom of this file for the layout mapped
+  # the provisioning unit near the bottom of this file. The venv lives at
-  # into hermesHome. The symlink target below must be the SAME path the
+  # ${hermesHome}/mnemosyne-venv and the plugin discovery symlink
-  # side venv was built with (hermesHome/mnemosyne-venv) — Hermes resolves
+  # (${hermesHome}/plugins/mnemosyne) is made RELATIVE — ../mnemosyne-venv/…
-  # plugin modules through it, so relative traversal after the bind mount
+  # — so it resolves identically on the host and inside the container, where
-  # still resolves inside the container identically.
+  # /opt/data is a bind mount of hermesHome and hosts/containers see
+  # different mountpoint prefixes for the same tree.
   mnemosyneReqs = pkgs.writeText "mnemosyne-requirements.txt" (
     builtins.readFile ./mnemosyne/requirements.txt
   );
 
+  # config.yaml provider-cue setter (PyYAML, preserves all other keys) — run
+  # host-side by the provisioning unit against the just-built, still
+  # root-owned venv's interpreter, BEFORE that venv is chowned to the
+  # container uid (root never executes from a luna-writable tree).
+  mnemosyneSetProvider = pkgs.writeText "mnemosyne-set-provider.py" (
+    builtins.readFile ./mnemosyne/set-provider.py
+  );
+
   # Wire event names (X-GitHub-Event) each route accepts — NOT the
   # subscription names the gitea hooks in services/dev/gitea.nix use. The two
   # namespaces collide; see the long comment on the route unit below.
@@ -435,110 +444,141 @@ in
   # bundled with the official image. Two pieces must exist before the gateway
   # starts for memory.provider = mnemosyne to activate:
   #
-  #   1. ${hermesHome}/plugins/mnemosyne — a symlink to the plugin package
+  #   1. ${hermesHome}/plugins/mnemosyne — a RELATIVE symlink to the plugin
-  #      inside the side venv. Hermes discovers providers by scanning
+  #      package inside the side venv (../mnemosyne-venv/lib/…/site-packages/
-  #      $HERMES_HOME/plugins (see its plugins/memory discovery code), reads
+  #      hermes_memory_provider). Hermes discovers providers by scanning
-  #      __init__.py/.  For it to IMPORT cleanly the plugin's sibling
+  #      $HERMES_HOME/plugins; resolve() keeps the traversal inside the
-  #      `mnemosyne` core package must be importable too — which is exactly
+  #      bind-mounted tree, so it lands on the same files whether read
-  #      why the plugin code lives inside the side venv's site-packages
+  #      host-side (/var/lib/hermes/…) or container-side (/opt/data/…).
-  #      rather than as a bare writable copy.
   #
-  #   2. The side venv itself (${hermesHome}/mnemosyne-venv), built with the
+  #   2. The side venv (${hermesHome}/mnemosyne-venv), built with the pinned
-  #      pinned pins in ./mnemosyne/requirements.txt. Inside hermesHome so
+  #      pins in ./mnemosyne/requirements.txt. Inside hermesHome so it lives
-  #      it lands inside HERMES_WRITE_SAFE_ROOT=/opt/data (visible to the
+  #      under the container's HERMES_WRITE_SAFE_ROOT and survives image
-  #      container at /opt/data/mnemosyne-venv) and survives image rebuilds.
+  #      rebuilds.
   #
-  # The venv's absolute paths embed ${hermesHome}: uv venv records the
+  # Import mechanics (verified against the plugin, not assumed): Hermes's own
-  # creation prefix, which is by construction identical inside and outside
+  # interpreter imports hermes_memory_provider OFF THE SYMLINK; that module's
-  # the container thanks to /opt/data being a bind mount of hermesHome.
+  # __init__.py itself inserts Path(__file__).resolve().parent.parent — the
+  # venv's site-packages — into sys.path before importing `mnemosyne.*`. So
+  # nothing needs to EXECUTE the venv's interpreter inside the container: the
+  # interpreter is only used host-side, by this unit, at provisioning time.
   #
   # Idempotent: marked done by a stamp file keyed by the hash of the
-  # requirements text, so a changed pin re-provisions. Never deletes —
+  # requirements text; the console script is verified before the stamp is
-  # removing memory.provider from config is what retires it.
+  # written, so a half-install (venv present but install died) re-provisions
+  # rather than exiting on a stale stamp. A pin change also re-provisions.
+  #
+  # Re-provisioning always wipes and recreates the venv (`uv venv --clear`,
+  # plus an explicit rm -f for a leftover non-directory): `uv venv` refuses
+  # to reuse an existing dir, and a wipe-and-rebuild is precisely what a
+  # changed stamp is supposed to mean. Working under root against a
+  # luna-writable ${hermesHome} means Python must never be made to IMPORT
+  # from a tree she has written to — that's why siteDir is composed here
+  # (python3.13 is pinned in the uv venv path) instead of executing
+  # $venv/bin/python to ask it, and why the rm -f/ln -sfn pair cannot leave
+  # stale venv content behind. Deleting first is what guarantees the new
+  # venv is hermetic to root, not incremental.
   #
   # Ordering: before podman-hermes-agent (the gateway needs the plugin at
-  # import time), after network (uv may fetch wheels on first provision),
+  # import time), after network (uv fetches wheels on first provision,
-  # with a bounded timeout so a broken proxy cannot hang boot.
+  # ~largest payload is onnxruntime), with a bounded timeout so a broken
+  # proxy cannot hang boot.
+  #
+  # Deliberately NOT Required= / requiredBy: a failed provision leaves the
+  # container running as before, without mnemosyne (RETAINED on purpose —
+  # hermes-webhook-routes and the gateway keep working, and config.yaml
+  # stays untouched, so a plain retry after fixing the network/mirror is
+  # enough). If mnemosyne activation itself should hard-fail boot, that
+  # needs an explicit decision from darman — the default here errs toward
+  # "don't take memory down along with everything else".
+  #
+  # No RequiresMountsFor: this unit only touches ${stateDir}, which is on
+  # the local filesystem (not a mount) on mars.
   systemd.services.hermes-agent-mnemosyne-provision = {
     description = "Provision Mnemosyne memory provider (side venv + plugin symlink)";
     before = [ "podman-hermes-agent.service" ];
     wantedBy = [ "podman-hermes-agent.service" ];
     wants = [ "network-online.target" ];
     after = [ "network-online.target" ];
-    unitConfig.RequiresMountsFor = [ "/mnt/jupiter" ];
+    path = [ pkgs.uv pkgs.coreutils ];
-    path = [ pkgs.python3 pkgs.uv pkgs.coreutils ];
     serviceConfig = {
       Type = "oneshot";
       TimeoutStartSec = 600;
     };
     environment = {
-      UV_PYTHON_INSTALL_DIR = "${hermesHome}/mnemosyne-uv/python";
       UV_CACHE_DIR = "${hermesHome}/mnemosyne-uv/cache";
       UV_COMPILE_BYTECODE = "1";
     };
     script = ''
       set -euo pipefail
       venv=${hermesHome}/mnemosyne-venv
-      pluginDir=${hermesHome}/plugins/mnemosyne
+      pluginsDir=${hermesHome}/plugins
+      pluginDir=$pluginsDir/mnemosyne
       stampFile=${hermesHome}/mnemosyne-provision.stamp
       reqHash=$(sha256sum ${mnemosyneReqs} | cut -d" " -f1)
 
-      if [ -x "$venv/bin/python" ] && [ -f "$stampFile" ] \
+      if [ -x "$venv/bin/mnemosyne-hermes" ] && [ -f "$stampFile" ] \
          && [ "$(cat "$stampFile")" = "$reqHash" ] \
-         && [ -e "$pluginDir" ] \
+         && [ -e "$pluginDir" ]; then
-         && [ -x "$venv/bin/mnemosyne-hermes" ]; then
         exit 0
       fi
 
-      mkdir -p ${hermesHome}/plugins ${hermesHome}/mnemosyne
+      mkdir -p "$pluginsDir"
-      uv venv "$venv" --python ${pkgs.python313}/bin/python3 --quiet
+      # Recreate from scratch so she cannot place anything into it that
-      UV_VENV="$venv" uv pip install \
+      # provisioning would then execute or trust (root/luna trust boundary:
+      # root only ever IMPORTS from a venv IT just built).
+      rm -rf "$venv"
+      uv venv "$venv" --python ${pkgs.python313}/bin/python3 --quiet --clear
+
+      uv pip install \
         --python "$venv/bin/python" \
         --requirement ${mnemosyneReqs} --quiet
 
-      # The plugin wrapper lands as hermes_memory_provider inside site-packages;
+      # Console script EXISTENCE is the success marker — verified before the
-      # uv installs the exact entry point scripts shown below. Symlink the
+      # stamp, so a half-install cannot be trusted on the next run.
-      # discovered package dir (never a fixed guess — find it by marker).
+      [ -x "$venv/bin/mnemosyne-hermes" ] || {
-      siteDir=$("$venv/bin/python" -c 'import site; print(site.getsitepackages()[0])')
+        echo "mnemosyne-hermes console script missing after install" >&2; exit 1;
-      target="$siteDir/hermes_memory_provider"
+      }
-      [ -d "$target" ] || { echo "mnemosyne plugin package not found in venv" >&2; exit 1; }
 
-      install -d -m 0755 -o ${hermesUid} -g ${hermesGid} \
+      # The plugin package dir name hermes_memory_provider is fixed by the
-        "$(dirname "$pluginDir")"
+      # upstream wheel; catching a rename here costs one ls per provision
+      # and is cheaper than importing from a mid-name drift.
+      siteDir="$venv/lib/python3.13/site-packages"
+      target="hermes_memory_provider"
+      [ -d "$siteDir/$target" ] || {
+        echo "mnemosyne plugin package not found in venv" >&2; exit 1;
+      }
+
+      # RELATIVE symlink: unambiguous across the bind mount (host prefix
+      # /var/lib/hermes vs container prefix /opt/data point at the same
+      # tree; build the traversal from plugins/mnemosyne, not from any
+      # absolute path baked in either direction).
       rm -f "$pluginDir"
-      ln -s "$target" "$pluginDir"
+      ln -sfn "../mnemosyne-venv/lib/python3.13/site-packages/$target" "$pluginDir"
+
+      chmod 0755 "$(dirname "$pluginDir")" "$pluginsDir"
+      chown ${hermesUid}:${hermesGid} "$(dirname "$pluginDir")"
+
+      # Config cue FIRST — the venv is still root-owned here, so root is
+      # executing its own freshly built interpreter, not a container-uid
+      # tree (the chown below hands that tree over; nothing executes from
+      # it after that point).
+      cfg=${hermesHome}/config.yaml
+      # A pre-existing wrong `provider:` value, `memory: null` / `memory: {}`,
+      # a missing memory section and a missing config.yaml are all handled
+      # inside the helper (see its header) — never a bare sed on YAML.
+      if [ -f "$cfg" ]; then
+        "$venv/bin/python" ${mnemosyneSetProvider} "$cfg"
+        chown ${hermesUid}:${hermesGid} "$cfg"
+      else
+        # Keep the cue out of first-run's way; just logged, not fatal.
+        echo "WARNING: $cfg not found; skipping provider cue (first-run will seed it)" >&2
+      fi
 
       printf '%s' "$reqHash" > "$stampFile"
       chown -R ${hermesUid}:${hermesGid} \
-        "$venv" "$(dirname "$pluginDir")" "$stampFile" \
+        "$venv" "$pluginsDir" "$stampFile" \
         ${hermesHome}/mnemosyne-uv
-
-      # Mirror the "active provider" cue into config.yaml — equivalent to
-      # `hermes config set memory.provider mnemosyne`, but idempotent and
-      # non-interactive. Only touches the one key, never rewrites the file.
-      cfg=${hermesHome}/config.yaml
-      if [ -f "$cfg" ]; then
-        if ! grep -q '^  provider: mnemosyne' "$cfg"; then
-          if grep -q '^memory:' "$cfg"; then
-            sed -i 's/^memory:$/memory:\n  provider: mnemosyne/' "$cfg"
-          else
-            printf '\nmemory:\n  provider: mnemosyne\n' >> "$cfg"
-          fi
-          chown ${hermesUid}:${hermesGid} "$cfg"
-        fi
-      else
-        printf 'memory:\n  provider: mnemosyne\n' > "$cfg"
-        chown ${hermesUid}:${hermesGid} "$cfg"
-      fi
     '';
   };
-
-  # Also assert mnemosyne as the active provider so the container's own
-  # config.yaml says the same thing statelessly — mirrored from the docs'
-  # `hermes config set memory.provider mnemosyne`. Done here (not a separate
-  # unit) so venv and config-cue stay in lockstep; never edits anything else
-  # in the file. Runs at the tail of the provisioning oneshot, after a
-  # successful venv, so a half-provision never flips the provider on.
-  # (system.activationScripts is NOT used — the file must exist first, and
-  # activation would run before hermesHome's own cont-init has created it.)
 }

hosts/mars/mnemosyne/set-provider.py

@@ -0,0 +1,45 @@
+# Set memory.provider = mnemosyne in Hermes's config.yaml, preserving every
+# other key, comment-free but value-faithful. Written as a separate file so
+# the provisioning unit runs it from the nix store (never inline) and ALWAYS
+# before the venv is chowned to the container uid — root must not execute an
+# interpreter inside a tree luna can write to.
+#
+# Behaviour per config.yaml state:
+#   existing `memory:` mapping (incl. an old `provider:` value)  → merge/replace
+#   `memory: null` or `memory: {}` or key missing                → create mapping
+#   top-level not a mapping                                      → abort loudly
+#   file missing                                                 → SKIP: Hermes's
+#     first-run seeding must create it; a one-key stub would stop that.
+import sys
+
+import yaml
+
+
+def set_provider(path: str) -> int:
+    try:
+        with open(path) as f:
+            data = yaml.safe_load(f) or {}
+    except FileNotFoundError:
+        print(
+            f"WARNING: {path} not found; skipping provider cue (first-run will seed it)",
+            file=sys.stderr,
+        )
+        return 0
+    if not isinstance(data, dict):
+        print(
+            f"ERROR: {path} is a {type(data).__name__}, not a mapping; not touched",
+            file=sys.stderr,
+        )
+        return 1
+    mem = data.get("memory")
+    if isinstance(mem, dict):
+        mem["provider"] = "mnemosyne"
+    else:
+        data["memory"] = {"provider": "mnemosyne"}
+    with open(path, "w") as f:
+        yaml.safe_dump(data, f, sort_keys=False)
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(set_provider(sys.argv[1]))