mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-23 18:17:44 +02:00
* fix(execenv): overlay per-task HERMES_HOME so Hermes discovers bound skills Hermes has no workspace-relative skill discovery — it scans <HERMES_HOME>/skills first, then skills.external_dirs from config.yaml (verified against the bundled agent/skill_utils.py). The daemon wrote assigned skills to the generic .agent_context/skills/ fallback, which Hermes never reads, so they silently never took effect (#5242). When (and only when) an agent has skills bound, redirect HERMES_HOME to a minimal per-task compatibility overlay; a skill-less Hermes task keeps its real home and original behavior: - mirror every top-level entry of the shared home via symlink except the overlay-owned ones (denylist), reconciling entries deleted from the shared home; - derive a task-local config.yaml whose skills.external_dirs references the shared skills dir plus the user's existing external_dirs, expanded against the sanitized effective child env (unknown vars preserved, blocklisted keys resolved to the process value) and normalized to absolute paths; - write only the bound skills into the task-local skills/ dir (home skills scanned first, so they win); global skills are referenced, not copied; - keep memories/ overlay-owned (fresh per-task dir) AND disable the external memory.provider, so neither on-disk memory nor a shared backend crosses tasks; - keep active_profile/profiles out of the overlay so Hermes can't follow a sticky profile and redirect past it at startup. Profile handling mirrors hermes_cli.profiles: the daemon reads -p/--profile with agent.HermesProfileFromArgs and seeds the overlay from that profile's home via ResolveHermesSourceHome (default/invalid -> base, valid name -> <base>/profiles/ <name>, validated; a missing named profile fails closed). The profile flags are stripped from the acp argv ONLY when the overlay is active (hermesLaunchArgs), so a skill-less task's profile passes through unchanged. HERMES_HOME is no longer custom_env-blocklisted: no skills -> user value passes through; skills -> overlay overrides after layering. Fail closed — Prepare errors, Reuse returns nil. Task home 0700, derived config 0600 via atomic replace. Platform-native default home (%LOCALAPPDATA%\hermes, incl. the LOCALAPPDATA-missing fallback, on Windows). Tests span execenv/daemon/agent: no-skill no-op, child-env layering + env sanitization, profile parse/unquote + conditional strip + final args/env per scenario, custom/profile/default/invalid/missing/Windows source home, sticky- profile not mirrored, memory dir isolation + external provider disable, mirror reconciliation, external_dirs rebasing + sanitized/unknown-var expansion, local-precedence slug, perms, fail-closed, resume teardown. Docs (en + ja/ko/zh). Fixes #5242 * fix(hermes): make profile selection one resolver contract matching Hermes Round 5 review: the profile chain approximated Hermes' semantics in three separate places (argv parsing, source-home selection, arg filtering), so it diverged from native Hermes in several merge-blocking cases. Collapse it into one authoritative resolution: - agent.ParseHermesProfileArgs replaces HermesProfileFromArgs/ FilterHermesProfileArgs. It reproduces _apply_profile_override step 1/1b (first occurrence, value-flag skipping, `--` and `mcp add --args` boundaries, space-form profile-id guard) and returns the exact argv occurrence to consume; StripHermesProfileArgs removes only that occurrence. - execenv.ResolveHermesProfile replaces ResolveHermesSourceHome. It derives the Hermes root exactly like get_default_hermes_root (an already-profile-scoped HERMES_HOME roots at its grandparent), selects an explicit profile first, otherwise trusts a profile-scoped home (step 1.5) and only then the sticky <root>/active_profile (step 2), and validates via normalize/validate_profile_name (reserved hermes/test/tmp/root/sudo and empty inline `--profile=` are hard errors). Profiles always resolve under the root, so `-p default` re-roots and `-p <sibling>` is a sibling, never nested. - The daemon runs one parse + resolve, fails the task closed on a reserved/ invalid selection (matching Hermes' sys.exit(1)), and exports the selected source home as the effective env's HERMES_HOME so ${HERMES_HOME} in a profile's skills.external_dirs expands against the selected profile home (as native Hermes does before loading config.yaml), not the root or the overlay. Regressions added: root + sticky named profile selection; already-profile-scoped home with no flag; that home with -p default and -p <sibling>; reserved and empty inline profile values; and a selected profile whose external_dirs contains ${HERMES_HOME}. * fix(hermes): overlay-owned derived .env + symlink-resolved root Round 6 review, two remaining overlay-bypass paths: 1. A source `.env` could redirect HERMES_HOME after profile resolution. Hermes runs `_apply_profile_override()` then `load_hermes_dotenv()`, which loads `<HERMES_HOME>/.env` with override=True — so a mirrored source `.env` carrying an out-of-band `HERMES_HOME=` overwrote the overlay's home, repointing skill discovery and memory back at the source. `.env` is now overlay-owned and DERIVED (writeDerivedHermesEnv): it preserves the source's credentials/settings but strips any `HERMES_HOME` assignment and pins `HERMES_HOME` to the overlay last (single-quoted, literal), written 0600 via atomic replace. It is written even when the source has none, so Hermes' project-`.env` fallback (override=True only when no user `.env` loaded) can't relocate the home either. 2. Root derivation was lexical-only, diverging from `get_default_hermes_root`, which compares `env_path.resolve()` with `native_home.resolve()`. A HERMES_HOME symlinked into `<native>/profiles/<x>` was treated as its own root, so `-p default`/`-p <sibling>` resolved wrong. `hermesRootFromHomeFor` now resolves symlinks (Path.resolve(strict=False)-style best effort) for the containment decision while keeping the returned root unresolved, matching Hermes. Regressions: source `.env` with HERMES_HOME replayed through the override=True dotenv order (bound skill + task memory stay on the overlay; creds preserved); minimal overlay `.env` created when the source has none; and a symlinked profile home resolving `-p default`/`-p <sibling>` to the native root.
734 lines
30 KiB
Go
734 lines
30 KiB
Go
package execenv
|
|
|
|
import (
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
|
|
"gopkg.in/yaml.v3"
|
|
)
|
|
|
|
// hermesExternalDirs parses skills.external_dirs out of a derived config.yaml.
|
|
func hermesExternalDirs(t *testing.T, configPath string) []string {
|
|
t.Helper()
|
|
data, err := os.ReadFile(configPath)
|
|
if err != nil {
|
|
t.Fatalf("read derived config: %v", err)
|
|
}
|
|
var parsed struct {
|
|
Skills struct {
|
|
ExternalDirs []string `yaml:"external_dirs"`
|
|
} `yaml:"skills"`
|
|
}
|
|
if err := yaml.Unmarshal(data, &parsed); err != nil {
|
|
t.Fatalf("parse derived config: %v\n%s", err, data)
|
|
}
|
|
return parsed.Skills.ExternalDirs
|
|
}
|
|
|
|
// hermesMemoryProvider returns the memory.provider value in a derived config,
|
|
// and whether the key is present.
|
|
func hermesMemoryProvider(t *testing.T, configPath string) (string, bool) {
|
|
t.Helper()
|
|
data, err := os.ReadFile(configPath)
|
|
if err != nil {
|
|
t.Fatalf("read derived config: %v", err)
|
|
}
|
|
var parsed struct {
|
|
Memory struct {
|
|
Provider *string `yaml:"provider"`
|
|
} `yaml:"memory"`
|
|
}
|
|
if err := yaml.Unmarshal(data, &parsed); err != nil {
|
|
t.Fatalf("parse derived config: %v", err)
|
|
}
|
|
if parsed.Memory.Provider == nil {
|
|
return "", false
|
|
}
|
|
return *parsed.Memory.Provider, true
|
|
}
|
|
|
|
// TestPrepareHermesHomeOverlay verifies the compatibility overlay: shared state
|
|
// is mirrored via symlink, the derived config references the user's real skills
|
|
// as an external root, and only the bound skill lands in the task-local skills/
|
|
// dir (the user's global skills are referenced, not copied).
|
|
func TestPrepareHermesHomeOverlay(t *testing.T) {
|
|
t.Parallel()
|
|
sharedHome := t.TempDir()
|
|
mustWrite(t, filepath.Join(sharedHome, "auth.json"), `{"token":"secret"}`)
|
|
mustWrite(t, filepath.Join(sharedHome, ".env"), "API_KEY=abc")
|
|
mustWrite(t, filepath.Join(sharedHome, "config.yaml"), "model: hermes-4\n")
|
|
mustWrite(t, filepath.Join(sharedHome, "plugins", "custom-provider", "plugin.py"), "# provider plugin")
|
|
mustWrite(t, filepath.Join(sharedHome, "oauth_state.json"), `{"nous":"tok"}`)
|
|
mustWrite(t, filepath.Join(sharedHome, "skills", "personal-notes", "SKILL.md"), "My personal notes skill.")
|
|
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "Help review code."}}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
|
|
for _, name := range []string{"auth.json", "plugins", "oauth_state.json"} {
|
|
fi, err := os.Lstat(filepath.Join(hermesHome, name))
|
|
if err != nil {
|
|
t.Fatalf("%s not mirrored into overlay: %v", name, err)
|
|
}
|
|
if fi.Mode()&os.ModeSymlink == 0 {
|
|
t.Errorf("%s should be a symlink into the shared home", name)
|
|
}
|
|
}
|
|
|
|
// .env is overlay-owned/derived (not symlinked): it preserves the source's
|
|
// credentials but pins HERMES_HOME to the overlay so Hermes' override=True
|
|
// dotenv load can't relocate the home past it.
|
|
envPath := filepath.Join(hermesHome, ".env")
|
|
if fi, err := os.Lstat(envPath); err != nil {
|
|
t.Fatalf(".env missing from overlay: %v", err)
|
|
} else if fi.Mode()&os.ModeSymlink != 0 {
|
|
t.Error(".env should be a derived copy, not a symlink into the shared home")
|
|
}
|
|
if data, _ := os.ReadFile(envPath); !strings.Contains(string(data), "API_KEY=abc") {
|
|
t.Error("derived .env dropped the source credentials")
|
|
} else if !strings.Contains(string(data), "HERMES_HOME='"+hermesHome+"'") {
|
|
t.Errorf("derived .env must pin HERMES_HOME to the overlay, got:\n%s", data)
|
|
}
|
|
|
|
cfgPath := filepath.Join(hermesHome, "config.yaml")
|
|
if fi, err := os.Lstat(cfgPath); err != nil {
|
|
t.Fatalf("config.yaml missing: %v", err)
|
|
} else if fi.Mode()&os.ModeSymlink != 0 {
|
|
t.Error("config.yaml should be a derived copy, not a symlink")
|
|
}
|
|
if data, _ := os.ReadFile(cfgPath); !strings.Contains(string(data), "hermes-4") {
|
|
t.Error("derived config dropped the user's model setting")
|
|
}
|
|
wantExternal := filepath.Join(sharedHome, "skills")
|
|
if got := hermesExternalDirs(t, cfgPath); len(got) != 1 || got[0] != wantExternal {
|
|
t.Errorf("external_dirs = %v, want [%s]", got, wantExternal)
|
|
}
|
|
|
|
if body, err := os.ReadFile(filepath.Join(hermesHome, "skills", "review-helper", "SKILL.md")); err != nil {
|
|
t.Fatalf("bound skill not written: %v", err)
|
|
} else if !strings.Contains(string(body), "Help review code.") {
|
|
t.Error("bound SKILL.md missing content")
|
|
}
|
|
if _, err := os.Stat(filepath.Join(hermesHome, "skills", "personal-notes")); !os.IsNotExist(err) {
|
|
t.Error("user global skill should be referenced via external_dirs, not copied into the task-local skills/")
|
|
}
|
|
}
|
|
|
|
// TestHermesDisablesExternalMemoryProvider is the regression for the shared
|
|
// memory-backend blocker: a host-configured memory.provider must be neutralized
|
|
// in the derived config so managed tasks don't share an external memory bank.
|
|
func TestHermesDisablesExternalMemoryProvider(t *testing.T) {
|
|
t.Parallel()
|
|
sharedHome := t.TempDir()
|
|
mustWrite(t, filepath.Join(sharedHome, "config.yaml"),
|
|
"memory:\n provider: supermemory\n memory_enabled: true\n")
|
|
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
|
|
got, ok := hermesMemoryProvider(t, filepath.Join(hermesHome, "config.yaml"))
|
|
if !ok {
|
|
t.Fatal("memory.provider should be present and explicitly disabled")
|
|
}
|
|
if got != "" {
|
|
t.Errorf("memory.provider = %q, want \"\" (external backend disabled)", got)
|
|
}
|
|
if data, _ := os.ReadFile(filepath.Join(hermesHome, "config.yaml")); !strings.Contains(string(data), "memory_enabled: true") {
|
|
t.Error("built-in memory settings should be preserved")
|
|
}
|
|
}
|
|
|
|
// TestHermesDerivedConfigRebasesRelativeExternalDirs is the regression for the
|
|
// silent-repoint bug: relative external_dirs must be rewritten to absolute paths
|
|
// anchored at the shared home, absolute entries left intact, and the real skills
|
|
// dir appended.
|
|
func TestHermesDerivedConfigRebasesRelativeExternalDirs(t *testing.T) {
|
|
t.Parallel()
|
|
sharedHome := t.TempDir()
|
|
mustWrite(t, filepath.Join(sharedHome, "config.yaml"),
|
|
"model: hermes-4\nskills:\n external_dirs:\n - team-skills\n - /opt/shared/skills\n")
|
|
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
|
|
got := hermesExternalDirs(t, filepath.Join(hermesHome, "config.yaml"))
|
|
want := []string{
|
|
filepath.Join(sharedHome, "team-skills"),
|
|
"/opt/shared/skills",
|
|
filepath.Join(sharedHome, "skills"),
|
|
}
|
|
if strings.Join(got, "\n") != strings.Join(want, "\n") {
|
|
t.Errorf("external_dirs =\n%v\nwant\n%v", got, want)
|
|
}
|
|
}
|
|
|
|
// TestHermesExternalDirsExpandsSanitizedEnv verifies a ${VAR} present in the
|
|
// sanitized effective env expands, while an UNKNOWN var is preserved verbatim
|
|
// (Hermes/Python expandvars semantics) instead of collapsing to empty and being
|
|
// silently rewritten to an absolute path.
|
|
func TestHermesExternalDirsExpandsSanitizedEnv(t *testing.T) {
|
|
t.Parallel()
|
|
sharedHome := t.TempDir()
|
|
mustWrite(t, filepath.Join(sharedHome, "config.yaml"),
|
|
"skills:\n external_dirs:\n - ${TEAM_SKILLS}/reviews\n - ${MYSTERY_VAR}/x\n")
|
|
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
env := map[string]string{"TEAM_SKILLS": "/srv/team"} // MYSTERY_VAR set nowhere
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, env, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
|
|
got := hermesExternalDirs(t, filepath.Join(hermesHome, "config.yaml"))
|
|
want := []string{
|
|
"/srv/team/reviews", // known var expanded
|
|
"${MYSTERY_VAR}/x", // unknown var preserved verbatim, NOT absolutized
|
|
filepath.Join(sharedHome, "skills"),
|
|
}
|
|
if strings.Join(got, "\n") != strings.Join(want, "\n") {
|
|
t.Errorf("external_dirs =\n%v\nwant\n%v", got, want)
|
|
}
|
|
}
|
|
|
|
// TestHermesBoundSkillKeepsNaturalSlug asserts a bound skill sharing a name with
|
|
// a user global skill keeps its natural slug (so Hermes resolves the bound
|
|
// version, home skills first) and leaves the user's shared copy untouched.
|
|
func TestHermesBoundSkillKeepsNaturalSlug(t *testing.T) {
|
|
t.Parallel()
|
|
sharedHome := t.TempDir()
|
|
userSkill := filepath.Join(sharedHome, "skills", "review-helper", "SKILL.md")
|
|
mustWrite(t, userSkill, "USER VERSION")
|
|
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "WORKSPACE VERSION"}}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
|
|
body, err := os.ReadFile(filepath.Join(hermesHome, "skills", "review-helper", "SKILL.md"))
|
|
if err != nil {
|
|
t.Fatalf("read bound skill: %v", err)
|
|
}
|
|
if strings.Contains(string(body), "USER VERSION") || !strings.Contains(string(body), "WORKSPACE VERSION") {
|
|
t.Errorf("bound skill should keep natural slug with its own content, got: %q", body)
|
|
}
|
|
if data, _ := os.ReadFile(userSkill); string(data) != "USER VERSION" {
|
|
t.Errorf("user's shared skill was modified: %q", data)
|
|
}
|
|
}
|
|
|
|
// TestHermesOverlayIsolatesMemories asserts the host memory dir isn't reachable
|
|
// from the task, task writes don't touch the host, and the task-local dir
|
|
// survives reuse.
|
|
func TestHermesOverlayIsolatesMemories(t *testing.T) {
|
|
t.Parallel()
|
|
sharedHome := t.TempDir()
|
|
mustWrite(t, filepath.Join(sharedHome, "config.yaml"), "model: hermes-4\n")
|
|
mustWrite(t, filepath.Join(sharedHome, "memories", "MEMORY.md"), "HOST MEMORY — must not leak")
|
|
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
|
|
memDir := filepath.Join(hermesHome, "memories")
|
|
if fi, err := os.Lstat(memDir); err != nil {
|
|
t.Fatalf("task memories dir missing: %v", err)
|
|
} else if fi.Mode()&os.ModeSymlink != 0 {
|
|
t.Fatal("task memories/ must be a real dir, not a symlink into the host home")
|
|
}
|
|
if _, err := os.Stat(filepath.Join(memDir, "MEMORY.md")); !os.IsNotExist(err) {
|
|
t.Error("host MEMORY.md must not be visible in the task")
|
|
}
|
|
|
|
mustWrite(t, filepath.Join(memDir, "MEMORY.md"), "TASK MEMORY")
|
|
if data, _ := os.ReadFile(filepath.Join(sharedHome, "memories", "MEMORY.md")); string(data) != "HOST MEMORY — must not leak" {
|
|
t.Errorf("host memory was modified through the overlay: %q", data)
|
|
}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome (reuse) failed: %v", err)
|
|
}
|
|
if data, _ := os.ReadFile(filepath.Join(memDir, "MEMORY.md")); string(data) != "TASK MEMORY" {
|
|
t.Errorf("reuse should preserve task-local memory, got: %q", data)
|
|
}
|
|
}
|
|
|
|
// TestHermesOverlayPermissions asserts the task home is 0700 and the derived
|
|
// config (which can hold inline api_key secrets) is 0600.
|
|
func TestHermesOverlayPermissions(t *testing.T) {
|
|
t.Parallel()
|
|
sharedHome := t.TempDir()
|
|
mustWrite(t, filepath.Join(sharedHome, "config.yaml"), "model: hermes-4\napi_key: sk-secret\n")
|
|
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
|
|
if fi, err := os.Stat(hermesHome); err != nil {
|
|
t.Fatalf("stat home: %v", err)
|
|
} else if fi.Mode().Perm() != 0o700 {
|
|
t.Errorf("task home perms = %o, want 0700", fi.Mode().Perm())
|
|
}
|
|
if fi, err := os.Stat(filepath.Join(hermesHome, "config.yaml")); err != nil {
|
|
t.Fatalf("stat config: %v", err)
|
|
} else if fi.Mode().Perm() != 0o600 {
|
|
t.Errorf("derived config perms = %o, want 0600", fi.Mode().Perm())
|
|
}
|
|
}
|
|
|
|
// TestHermesOverlayReconcilesDeletedSharedEntry asserts a top-level entry
|
|
// removed from the shared home is dropped from the overlay on rebuild.
|
|
func TestHermesOverlayReconcilesDeletedSharedEntry(t *testing.T) {
|
|
t.Parallel()
|
|
sharedHome := t.TempDir()
|
|
mustWrite(t, filepath.Join(sharedHome, "config.yaml"), "model: hermes-4\n")
|
|
mustWrite(t, filepath.Join(sharedHome, "plugins", "p", "plugin.py"), "# plugin")
|
|
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
if _, err := os.Lstat(filepath.Join(hermesHome, "plugins")); err != nil {
|
|
t.Fatalf("plugins not mirrored: %v", err)
|
|
}
|
|
if err := os.RemoveAll(filepath.Join(sharedHome, "plugins")); err != nil {
|
|
t.Fatalf("remove shared plugins: %v", err)
|
|
}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome (rebuild) failed: %v", err)
|
|
}
|
|
if _, err := os.Lstat(filepath.Join(hermesHome, "plugins")); !os.IsNotExist(err) {
|
|
t.Error("stale mirrored plugins should be reconciled away after deletion in the shared home")
|
|
}
|
|
}
|
|
|
|
// TestPrepareHermesHomeFailsClosed asserts prepareHermesHome returns an error
|
|
// when required overlay state can't be built (here an unreadable shared config).
|
|
func TestPrepareHermesHomeFailsClosed(t *testing.T) {
|
|
t.Parallel()
|
|
sharedHome := t.TempDir()
|
|
if err := os.MkdirAll(filepath.Join(sharedHome, "config.yaml"), 0o755); err != nil {
|
|
t.Fatalf("mkdir config.yaml dir: %v", err)
|
|
}
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, nil, testLogger()); err == nil {
|
|
t.Fatal("expected prepareHermesHome to fail closed on an unreadable shared config")
|
|
}
|
|
}
|
|
|
|
// TestResolveHermesProfile exercises the one resolver contract against the
|
|
// behaviors the review requires it to match Hermes on: sticky selection, an
|
|
// already-profile-scoped home, `-p default`/`-p <sibling>` re-rooting, and native
|
|
// failure on reserved/invalid/empty names. Temp dirs stand in for custom Hermes
|
|
// roots (they are never under the host's platform-default home, so root
|
|
// derivation takes the deterministic custom/profile branch).
|
|
func TestResolveHermesProfile(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
t.Run("no profile resolves to the base home", func(t *testing.T) {
|
|
t.Parallel()
|
|
base := t.TempDir()
|
|
res := ResolveHermesProfile(base, "", false, false)
|
|
if res.Err != nil || res.SourceHome != base || res.MustExist {
|
|
t.Fatalf("got %+v, want SourceHome=%q MustExist=false Err=nil", res, base)
|
|
}
|
|
})
|
|
|
|
// The review's blocker 1: a sticky active_profile must be SELECTED as the
|
|
// overlay source, not merely blocked from bypassing.
|
|
t.Run("root + sticky named profile selects the profile as source", func(t *testing.T) {
|
|
t.Parallel()
|
|
root := t.TempDir()
|
|
mustWrite(t, filepath.Join(root, "active_profile"), "coder\n")
|
|
res := ResolveHermesProfile(root, "", false, false)
|
|
want := filepath.Join(root, "profiles", "coder")
|
|
if res.Err != nil || res.SourceHome != want || !res.MustExist {
|
|
t.Fatalf("sticky: got %+v, want SourceHome=%q MustExist=true", res, want)
|
|
}
|
|
})
|
|
|
|
t.Run("sticky default is ignored", func(t *testing.T) {
|
|
t.Parallel()
|
|
root := t.TempDir()
|
|
mustWrite(t, filepath.Join(root, "active_profile"), "default\n")
|
|
res := ResolveHermesProfile(root, "", false, false)
|
|
if res.Err != nil || res.SourceHome != root || res.MustExist {
|
|
t.Fatalf("sticky default: got %+v, want SourceHome=%q MustExist=false", res, root)
|
|
}
|
|
})
|
|
|
|
t.Run("already-profile-scoped home with no flag is trusted", func(t *testing.T) {
|
|
t.Parallel()
|
|
root := t.TempDir()
|
|
scoped := filepath.Join(root, "profiles", "coder")
|
|
// A sticky at the root must NOT override an explicit profile-scoped home.
|
|
mustWrite(t, filepath.Join(root, "active_profile"), "research\n")
|
|
res := ResolveHermesProfile(scoped, "", false, false)
|
|
if res.Err != nil || res.SourceHome != scoped || !res.MustExist {
|
|
t.Fatalf("profile-scoped: got %+v, want SourceHome=%q MustExist=true", res, scoped)
|
|
}
|
|
})
|
|
|
|
t.Run("-p default from a profile-scoped home re-roots to the root", func(t *testing.T) {
|
|
t.Parallel()
|
|
root := t.TempDir()
|
|
scoped := filepath.Join(root, "profiles", "coder")
|
|
res := ResolveHermesProfile(scoped, "default", true, false)
|
|
if res.Err != nil || res.SourceHome != root || res.MustExist {
|
|
t.Fatalf("-p default: got %+v, want SourceHome=%q MustExist=false", res, root)
|
|
}
|
|
})
|
|
|
|
t.Run("-p sibling from a profile-scoped home is a sibling, not nested", func(t *testing.T) {
|
|
t.Parallel()
|
|
root := t.TempDir()
|
|
scoped := filepath.Join(root, "profiles", "coder")
|
|
res := ResolveHermesProfile(scoped, "research", true, false)
|
|
want := filepath.Join(root, "profiles", "research")
|
|
if res.Err != nil || res.SourceHome != want || !res.MustExist {
|
|
t.Fatalf("-p sibling: got %+v, want SourceHome=%q MustExist=true", res, want)
|
|
}
|
|
})
|
|
|
|
t.Run("explicit named profile resolves under the root", func(t *testing.T) {
|
|
t.Parallel()
|
|
root := t.TempDir()
|
|
res := ResolveHermesProfile(root, "research", true, false)
|
|
want := filepath.Join(root, "profiles", "research")
|
|
if res.Err != nil || res.SourceHome != want || !res.MustExist {
|
|
t.Fatalf("named: got %+v, want SourceHome=%q MustExist=true", res, want)
|
|
}
|
|
})
|
|
|
|
t.Run("reserved names fail closed", func(t *testing.T) {
|
|
t.Parallel()
|
|
root := t.TempDir()
|
|
for _, name := range []string{"hermes", "test", "tmp", "root", "sudo"} {
|
|
if res := ResolveHermesProfile(root, name, true, false); res.Err == nil {
|
|
t.Errorf("reserved %q: expected Err, got SourceHome=%q", name, res.SourceHome)
|
|
}
|
|
}
|
|
})
|
|
|
|
t.Run("empty inline value fails closed", func(t *testing.T) {
|
|
t.Parallel()
|
|
root := t.TempDir()
|
|
if res := ResolveHermesProfile(root, "", true, true); res.Err == nil {
|
|
t.Error("empty inline --profile= must fail closed, not fall back to default")
|
|
}
|
|
})
|
|
|
|
t.Run("empty inputs resolve to a platform default", func(t *testing.T) {
|
|
t.Parallel()
|
|
if res := ResolveHermesProfile("", "", false, false); res.SourceHome == "" {
|
|
t.Error("empty inputs should resolve to a platform default, not empty")
|
|
}
|
|
})
|
|
}
|
|
|
|
// TestHermesExternalDirsExpandsAgainstSelectedProfileHome is the review's blocker
|
|
// 3: a ${HERMES_HOME} in a selected profile's skills.external_dirs must expand
|
|
// against the SELECTED profile home (what native Hermes sees after applying the
|
|
// profile override before loading config.yaml), not the pre-resolution/root home
|
|
// or the task overlay. The daemon sets the effective env's HERMES_HOME to the
|
|
// resolved source home for exactly this reason.
|
|
func TestHermesExternalDirsExpandsAgainstSelectedProfileHome(t *testing.T) {
|
|
t.Parallel()
|
|
profileHome := t.TempDir() // stands in for <root>/profiles/coder
|
|
mustWrite(t, filepath.Join(profileHome, "config.yaml"),
|
|
"skills:\n external_dirs:\n - ${HERMES_HOME}/profile-skills\n")
|
|
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
env := map[string]string{"HERMES_HOME": profileHome} // as the daemon sets it
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, profileHome, true, skills, env, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
|
|
got := hermesExternalDirs(t, filepath.Join(hermesHome, "config.yaml"))
|
|
want := []string{
|
|
filepath.Join(profileHome, "profile-skills"),
|
|
filepath.Join(profileHome, "skills"),
|
|
}
|
|
if strings.Join(got, "\n") != strings.Join(want, "\n") {
|
|
t.Errorf("external_dirs =\n%v\nwant\n%v", got, want)
|
|
}
|
|
}
|
|
|
|
// TestHermesOverlayEnvPinsHomeAfterDotenvOverride is the review's blocker 1: a
|
|
// source .env that sets HERMES_HOME must not relocate the home past the overlay.
|
|
// Hermes loads <HERMES_HOME>/.env with override=True right after profile
|
|
// resolution; we replay that last-wins/override order over the derived overlay
|
|
// .env and prove the effective HERMES_HOME stays the overlay — so bound-skill
|
|
// discovery and task-local memory keep using it — while source creds survive.
|
|
func TestHermesOverlayEnvPinsHomeAfterDotenvOverride(t *testing.T) {
|
|
t.Parallel()
|
|
sourceHome := t.TempDir()
|
|
mustWrite(t, filepath.Join(sourceHome, ".env"),
|
|
"ANTHROPIC_API_KEY=sk-source\nexport HERMES_HOME=/home/u/.hermes/profiles/coder\n")
|
|
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, sourceHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
|
|
envPath := filepath.Join(hermesHome, ".env")
|
|
if fi, err := os.Stat(envPath); err != nil {
|
|
t.Fatalf(".env missing: %v", err)
|
|
} else if perm := fi.Mode().Perm(); perm != 0o600 {
|
|
t.Errorf(".env perms = %o, want 600 (holds credentials)", perm)
|
|
}
|
|
|
|
env := applyDotenvOverride(t, envPath)
|
|
if env["HERMES_HOME"] != hermesHome {
|
|
t.Errorf("after override=True dotenv load HERMES_HOME = %q, want the overlay %q", env["HERMES_HOME"], hermesHome)
|
|
}
|
|
if env["ANTHROPIC_API_KEY"] != "sk-source" {
|
|
t.Errorf("source credential dropped: ANTHROPIC_API_KEY = %q", env["ANTHROPIC_API_KEY"])
|
|
}
|
|
// HERMES_HOME pinned to the overlay ⇒ skill discovery and memory use the
|
|
// overlay's own dirs.
|
|
if _, err := os.Stat(filepath.Join(hermesHome, "skills", "review-helper", "SKILL.md")); err != nil {
|
|
t.Errorf("bound skill not in overlay skills dir: %v", err)
|
|
}
|
|
if fi, err := os.Stat(filepath.Join(hermesHome, "memories")); err != nil || !fi.IsDir() {
|
|
t.Errorf("overlay memories dir missing: %v", err)
|
|
}
|
|
}
|
|
|
|
// TestHermesOverlayEnvCreatedWhenSourceHasNone ensures a minimal overlay .env is
|
|
// written even when the source has none, so Hermes' project-.env fallback (loaded
|
|
// with override=True only when no user .env loaded) can't relocate the home.
|
|
func TestHermesOverlayEnvCreatedWhenSourceHasNone(t *testing.T) {
|
|
t.Parallel()
|
|
sourceHome := t.TempDir() // no .env present
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, sourceHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
env := applyDotenvOverride(t, filepath.Join(hermesHome, ".env"))
|
|
if env["HERMES_HOME"] != hermesHome {
|
|
t.Errorf("overlay .env must exist and pin HERMES_HOME even with no source .env; got %q", env["HERMES_HOME"])
|
|
}
|
|
}
|
|
|
|
// applyDotenvOverride replays python-dotenv's single-file override=True load over
|
|
// a .env: KEY=VALUE lines in order, last assignment wins, surrounding quotes and
|
|
// a leading `export` stripped. It mirrors how Hermes loads <HERMES_HOME>/.env.
|
|
func applyDotenvOverride(t *testing.T, path string) map[string]string {
|
|
t.Helper()
|
|
data, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read overlay .env: %v", err)
|
|
}
|
|
env := map[string]string{}
|
|
for _, line := range strings.Split(string(data), "\n") {
|
|
s := strings.TrimSpace(line)
|
|
if s == "" || strings.HasPrefix(s, "#") {
|
|
continue
|
|
}
|
|
if rest := strings.TrimPrefix(s, "export"); rest != s && rest != "" && (rest[0] == ' ' || rest[0] == '\t') {
|
|
s = strings.TrimSpace(rest)
|
|
}
|
|
eq := strings.IndexByte(s, '=')
|
|
if eq <= 0 {
|
|
continue
|
|
}
|
|
key := strings.TrimSpace(s[:eq])
|
|
val := strings.TrimSpace(s[eq+1:])
|
|
if len(val) >= 2 && (val[0] == '\'' || val[0] == '"') && val[len(val)-1] == val[0] {
|
|
val = val[1 : len(val)-1]
|
|
}
|
|
env[key] = val // last wins (override=True)
|
|
}
|
|
return env
|
|
}
|
|
|
|
// TestHermesRootFromHomeResolvesSymlinks is the review's blocker 2: a HERMES_HOME
|
|
// symlinked into <native>/profiles/<x> must root at native (so -p default
|
|
// re-roots to native and -p <sibling> is a native sibling), which requires
|
|
// resolving symlinks for the containment decision — lexical containment alone
|
|
// would treat the symlink path as its own root.
|
|
func TestHermesRootFromHomeResolvesSymlinks(t *testing.T) {
|
|
t.Parallel()
|
|
native := t.TempDir()
|
|
coder := filepath.Join(native, "profiles", "coder")
|
|
if err := os.MkdirAll(coder, 0o755); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
link := filepath.Join(t.TempDir(), "coder-home")
|
|
if err := os.Symlink(coder, link); err != nil {
|
|
t.Skipf("symlink unsupported here: %v", err)
|
|
}
|
|
|
|
root := hermesRootFromHomeFor(link, native)
|
|
if root != native {
|
|
t.Fatalf("symlinked profile home: root = %q, want native %q", root, native)
|
|
}
|
|
if home, _, err := hermesProfileDir(root, "default"); err != nil || home != native {
|
|
t.Fatalf("-p default: home=%q err=%v, want %q", home, err, native)
|
|
}
|
|
sibling := filepath.Join(native, "profiles", "research")
|
|
if home, _, err := hermesProfileDir(root, "research"); err != nil || home != sibling {
|
|
t.Fatalf("-p sibling: home=%q err=%v, want %q", home, err, sibling)
|
|
}
|
|
}
|
|
|
|
// TestPlatformDefaultHermesHome covers the Windows branch off a Windows host,
|
|
// including the LOCALAPPDATA-missing fallback to %USERPROFILE%\AppData\Local.
|
|
func TestPlatformDefaultHermesHome(t *testing.T) {
|
|
t.Parallel()
|
|
la := filepath.Join(string(filepath.Separator)+"Users", "me", "AppData", "Local")
|
|
home := filepath.Join(string(filepath.Separator)+"home", "me")
|
|
|
|
if got, want := platformDefaultHermesHomeFor("windows", la, home), filepath.Join(la, "hermes"); got != want {
|
|
t.Errorf("windows: got %q, want %q", got, want)
|
|
}
|
|
// No LOCALAPPDATA → %USERPROFILE%\AppData\Local\hermes, matching Hermes.
|
|
if got, want := platformDefaultHermesHomeFor("windows", "", home), filepath.Join(home, "AppData", "Local", "hermes"); got != want {
|
|
t.Errorf("windows no LOCALAPPDATA: got %q, want %q", got, want)
|
|
}
|
|
if got, want := platformDefaultHermesHomeFor("linux", la, home), filepath.Join(home, ".hermes"); got != want {
|
|
t.Errorf("linux: got %q, want %q", got, want)
|
|
}
|
|
}
|
|
|
|
// TestHermesOverlayDoesNotMirrorStickyProfile is the regression for the sticky
|
|
// active_profile bypass: active_profile and profiles/ from the shared home must
|
|
// NOT be mirrored into the overlay, or Hermes would follow the sticky profile at
|
|
// startup and redirect HERMES_HOME past the overlay.
|
|
func TestHermesOverlayDoesNotMirrorStickyProfile(t *testing.T) {
|
|
t.Parallel()
|
|
sharedHome := t.TempDir()
|
|
mustWrite(t, filepath.Join(sharedHome, "config.yaml"), "model: hermes-4\n")
|
|
mustWrite(t, filepath.Join(sharedHome, "active_profile"), "coder")
|
|
mustWrite(t, filepath.Join(sharedHome, "profiles", "coder", "config.yaml"), "model: coder\n")
|
|
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, sharedHome, false, skills, nil, testLogger()); err != nil {
|
|
t.Fatalf("prepareHermesHome failed: %v", err)
|
|
}
|
|
for _, name := range []string{"active_profile", "profiles"} {
|
|
if _, err := os.Lstat(filepath.Join(hermesHome, name)); !os.IsNotExist(err) {
|
|
t.Errorf("%s must not be mirrored into the overlay (sticky-profile bypass)", name)
|
|
}
|
|
}
|
|
}
|
|
|
|
// TestPrepareHermesHomeFailsOnMissingNamedProfile asserts an explicitly named
|
|
// profile whose home doesn't exist fails closed rather than silently seeding
|
|
// from an empty dir (which would drop the user's auth/config), matching Hermes'
|
|
// own sys.exit(1).
|
|
func TestPrepareHermesHomeFailsOnMissingNamedProfile(t *testing.T) {
|
|
t.Parallel()
|
|
base := t.TempDir()
|
|
missingProfileHome := filepath.Join(base, "profiles", "does-not-exist")
|
|
hermesHome := filepath.Join(t.TempDir(), "hermes-home")
|
|
skills := []SkillContextForEnv{{Name: "Review Helper", Content: "x"}}
|
|
if err := prepareHermesHome(hermesHome, missingProfileHome, true, skills, nil, testLogger()); err == nil {
|
|
t.Fatal("expected a missing named profile home to fail closed")
|
|
}
|
|
}
|
|
|
|
// TestPrepareHermesNoSkillsLeavesHomeUnset is the regression for the review's
|
|
// top blocker: a Hermes task with no bound skills must NOT get a redirected
|
|
// HERMES_HOME.
|
|
func TestPrepareHermesNoSkillsLeavesHomeUnset(t *testing.T) {
|
|
t.Parallel()
|
|
env, err := Prepare(PrepareParams{
|
|
WorkspacesRoot: t.TempDir(),
|
|
WorkspaceID: "ws-hermes-noskill",
|
|
TaskID: "aaaa1111-2222-3333-4444-555566667777",
|
|
Provider: "hermes",
|
|
Task: TaskContextForEnv{IssueID: "no-skill"},
|
|
}, testLogger())
|
|
if err != nil {
|
|
t.Fatalf("Prepare failed: %v", err)
|
|
}
|
|
defer env.Cleanup(true)
|
|
|
|
if env.HermesHome != "" {
|
|
t.Errorf("skill-less Hermes task must not redirect HERMES_HOME, got %q", env.HermesHome)
|
|
}
|
|
if _, err := os.Stat(filepath.Join(env.RootDir, "hermes-home")); !os.IsNotExist(err) {
|
|
t.Error("no hermes-home overlay should be created for a skill-less task")
|
|
}
|
|
}
|
|
|
|
// TestReuseHermesTearsDownWhenSkillsRemoved covers the resume path: a task that
|
|
// had a skill and lost its last one must drop the redirect and remove the
|
|
// overlay.
|
|
func TestReuseHermesTearsDownWhenSkillsRemoved(t *testing.T) {
|
|
t.Parallel()
|
|
sharedHome := t.TempDir()
|
|
mustWrite(t, filepath.Join(sharedHome, "config.yaml"), "model: hermes-4\n")
|
|
|
|
withSkill := TaskContextForEnv{
|
|
IssueID: "hermes-resume",
|
|
AgentSkills: []SkillContextForEnv{{Name: "Review Helper", Content: "Help review."}},
|
|
}
|
|
env, err := Prepare(PrepareParams{
|
|
WorkspacesRoot: t.TempDir(),
|
|
WorkspaceID: "ws-hermes-resume",
|
|
TaskID: "bbbb1111-2222-3333-4444-555566667777",
|
|
Provider: "hermes",
|
|
HermesSourceHome: sharedHome,
|
|
Task: withSkill,
|
|
}, testLogger())
|
|
if err != nil {
|
|
t.Fatalf("Prepare failed: %v", err)
|
|
}
|
|
defer env.Cleanup(true)
|
|
|
|
if env.HermesHome == "" {
|
|
t.Fatal("expected HERMES_HOME redirect for a task with a bound skill")
|
|
}
|
|
overlayDir := filepath.Join(env.RootDir, "hermes-home")
|
|
|
|
if reused := Reuse(ReuseParams{WorkDir: env.WorkDir, Provider: "hermes", HermesSourceHome: sharedHome, Task: withSkill}, testLogger()); reused == nil {
|
|
t.Fatal("Reuse with skill returned nil")
|
|
} else if reused.HermesHome == "" {
|
|
t.Error("resume with a bound skill should keep the redirect")
|
|
}
|
|
|
|
noSkill := TaskContextForEnv{IssueID: "hermes-resume"}
|
|
reused := Reuse(ReuseParams{WorkDir: env.WorkDir, Provider: "hermes", HermesSourceHome: sharedHome, Task: noSkill}, testLogger())
|
|
if reused == nil {
|
|
t.Fatal("Reuse without skill returned nil")
|
|
}
|
|
if reused.HermesHome != "" {
|
|
t.Errorf("removing the last skill should clear HERMES_HOME, got %q", reused.HermesHome)
|
|
}
|
|
if _, err := os.Stat(overlayDir); !os.IsNotExist(err) {
|
|
t.Error("stale hermes-home overlay should be removed on teardown")
|
|
}
|
|
}
|
|
|
|
func mustWrite(t *testing.T, path, content string) {
|
|
t.Helper()
|
|
if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
|
|
t.Fatalf("mkdir for %s: %v", path, err)
|
|
}
|
|
if err := os.WriteFile(path, []byte(content), 0o644); err != nil {
|
|
t.Fatalf("write %s: %v", path, err)
|
|
}
|
|
}
|