diff --git a/server/internal/daemon/daemon.go b/server/internal/daemon/daemon.go index 44c1fa8b3a..99e09fddac 100644 --- a/server/internal/daemon/daemon.go +++ b/server/internal/daemon/daemon.go @@ -2065,8 +2065,18 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i // Try to reuse the workdir from a previous task on the same (agent, issue) pair. var env *execenv.Environment codexVersion := d.agentVersion("codex") + openclawBin := "" + if provider == "openclaw" { + openclawBin = entry.Path + } if task.PriorWorkDir != "" { - env = execenv.Reuse(task.PriorWorkDir, provider, codexVersion, taskCtx, d.logger) + env = execenv.Reuse(execenv.ReuseParams{ + WorkDir: task.PriorWorkDir, + Provider: provider, + CodexVersion: codexVersion, + OpenclawBin: openclawBin, + Task: taskCtx, + }, d.logger) } if env == nil { var err error @@ -2077,6 +2087,7 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i AgentName: agentName, Provider: provider, CodexVersion: codexVersion, + OpenclawBin: openclawBin, Task: taskCtx, }, d.logger) if err != nil { @@ -2142,6 +2153,22 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i if env.CodexHome != "" { agentEnv["CODEX_HOME"] = env.CodexHome } + // Point OpenClaw at the per-task synthesized config. The config pins + // agents.defaults.workspace (and any agents.list[].workspace) to the + // task workdir, so the CLI's native skill scanner picks up the per-task + // skills written under {workDir}/skills/. Falls back silently when the + // preparer didn't run (non-openclaw provider, or write failure). + if env.OpenclawConfigPath != "" { + agentEnv["OPENCLAW_CONFIG_PATH"] = env.OpenclawConfigPath + } + // Grant the wrapper config permission to $include the user's active + // config across directories. OpenClaw's $include defaults to confining + // resolution to the wrapper's own directory; without this, the + // wrapper-out-of-envRoot $include into ~/.openclaw/openclaw.json is + // rejected and the run boots with no user-registered agents. + if rootsValue, ok := composeOpenclawIncludeRoots(env.OpenclawIncludeRoot, os.Getenv("OPENCLAW_INCLUDE_ROOTS")); ok { + agentEnv["OPENCLAW_INCLUDE_ROOTS"] = rootsValue + } // Inject user-configured custom environment variables (e.g. ANTHROPIC_API_KEY, // ANTHROPIC_BASE_URL for router/proxy mode, or CLAUDE_CODE_USE_BEDROCK for // Bedrock). These are set per-agent via the agent settings UI. @@ -2213,8 +2240,12 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i } // Some providers do not reliably load the per-task runtime config files we // write into the task workdir: - // - openclaw loads bootstrap files (AGENTS.md, SOUL.md, ...) from its own - // workspace dir rather than the task workdir. + // - openclaw is pinned to the task workdir via the per-task config we + // synthesize (see prepareOpenclawConfig), so AGENTS.md / .agent_context/ + // in the workdir ARE picked up by the CLI. Inline injection is retained + // as a belt-and-suspenders for older openclaw releases until that load + // path stabilises in production; remove this once a release tracks the + // workdir bootstrap reliably end-to-end. // - kiro and kimi are wrapped through their own CLIs whose cwd handling // is opaque enough that we can't trust the file-based path either. // Pass the full runtime brief inline (CLI catalog + workflow steps + agent @@ -2714,6 +2745,40 @@ func convertSkillsForEnv(skills []SkillData) []execenv.SkillContextForEnv { return result } +// composeOpenclawIncludeRoots returns the value the daemon should set for +// OPENCLAW_INCLUDE_ROOTS on the child openclaw process so its `$include` +// loader will follow the wrapper's reference out of envRoot into the +// user's active config directory. +// +// addRoot is the directory we must grant (typically dirname of the user's +// active openclaw.json). userValue is whatever the daemon's own +// environment already has under OPENCLAW_INCLUDE_ROOTS — the user's own +// cross-directory layout. We prepend addRoot, dedupe by string equality, +// drop empty path segments, and return ok=false when there's nothing to +// grant (addRoot is empty — fresh install case), so callers can leave the +// env var alone in that case. +// +// Path separator is the OS-native list separator (`:` on Unix, `;` on +// Windows) to match how OpenClaw splits the env var. +func composeOpenclawIncludeRoots(addRoot, userValue string) (string, bool) { + if addRoot == "" { + return "", false + } + parts := []string{addRoot} + seen := map[string]struct{}{addRoot: {}} + for _, p := range strings.Split(userValue, string(os.PathListSeparator)) { + if p == "" { + continue + } + if _, dup := seen[p]; dup { + continue + } + seen[p] = struct{}{} + parts = append(parts, p) + } + return strings.Join(parts, string(os.PathListSeparator)), true +} + // isBlockedEnvKey returns true if the key must not be overridden by user- // configured custom_env. This prevents accidental or malicious override of // daemon-internal variables and critical system paths. @@ -2723,7 +2788,7 @@ func isBlockedEnvKey(key string) bool { return true } switch upper { - case "HOME", "PATH", "USER", "SHELL", "TERM", "CODEX_HOME": + case "HOME", "PATH", "USER", "SHELL", "TERM", "CODEX_HOME", "OPENCLAW_CONFIG_PATH", "OPENCLAW_INCLUDE_ROOTS": return true } return false diff --git a/server/internal/daemon/daemon_test.go b/server/internal/daemon/daemon_test.go index 025ee49b5d..3b02c6a434 100644 --- a/server/internal/daemon/daemon_test.go +++ b/server/internal/daemon/daemon_test.go @@ -205,6 +205,80 @@ func TestProviderNeedsInlineSystemPrompt(t *testing.T) { } } +// TestComposeOpenclawIncludeRoots — the Elon must-fix regression: the +// daemon must grant OpenClaw permission to follow the wrapper's $include +// link from envRoot into the user's active config dir, while preserving +// any roots the user already configured in their shell env so their own +// cross-directory layouts keep working. +func TestComposeOpenclawIncludeRoots(t *testing.T) { + t.Parallel() + + sep := string(os.PathListSeparator) + cases := []struct { + name string + add string + user string + want string + wantSet bool + }{ + { + // Fresh install — preparer emits no $include, so daemon + // shouldn't touch OPENCLAW_INCLUDE_ROOTS at all. + name: "fresh_install_no_root_to_grant", + add: "", + user: "/some/user/dir", + wantSet: false, + }, + { + // User has no existing value — output is just the granted dir. + name: "no_user_value", + add: "/home/alice/.openclaw", + user: "", + want: "/home/alice/.openclaw", + wantSet: true, + }, + { + // User has their own include roots — daemon must prepend + // granted dir AND preserve user's entries verbatim. + name: "preserves_user_value", + add: "/home/alice/.openclaw", + user: "/etc/openclaw" + sep + "/opt/openclaw/shared", + want: "/home/alice/.openclaw" + sep + "/etc/openclaw" + sep + "/opt/openclaw/shared", + wantSet: true, + }, + { + // User's value already contains the granted dir — daemon + // must dedupe rather than emit a redundant entry that would + // trip OpenClaw confused-deputy heuristics. + name: "dedupes_when_user_already_grants_same_dir", + add: "/home/alice/.openclaw", + user: "/home/alice/.openclaw" + sep + "/etc/openclaw", + want: "/home/alice/.openclaw" + sep + "/etc/openclaw", + wantSet: true, + }, + { + // Stray empty segments from a malformed user env are skipped. + name: "skips_empty_segments_in_user_value", + add: "/home/alice/.openclaw", + user: "" + sep + "/etc/openclaw" + sep + "", + want: "/home/alice/.openclaw" + sep + "/etc/openclaw", + wantSet: true, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + got, ok := composeOpenclawIncludeRoots(tc.add, tc.user) + if ok != tc.wantSet { + t.Fatalf("ok = %v, want %v (got = %q)", ok, tc.wantSet, got) + } + if got != tc.want { + t.Errorf("got = %q, want %q", got, tc.want) + } + }) + } +} + func TestBuildPromptContainsIssueID(t *testing.T) { t.Parallel() diff --git a/server/internal/daemon/execenv/context.go b/server/internal/daemon/execenv/context.go index 39efbad4fc..870190ce55 100644 --- a/server/internal/daemon/execenv/context.go +++ b/server/internal/daemon/execenv/context.go @@ -16,7 +16,7 @@ import ( // Codex: skills → handled separately in Prepare via codex-home // Copilot: skills → {workDir}/.github/skills/{name}/SKILL.md (native project-level discovery) // OpenCode: skills → {workDir}/.opencode/skills/{name}/SKILL.md (native discovery) -// OpenClaw: skills → {workDir}/.agent_context/skills/{name}/SKILL.md (NO native auto-discovery — see note in resolveSkillsDir) +// OpenClaw: skills → {workDir}/skills/{name}/SKILL.md (native discovery — paired with a per-task synthesized openclaw-config.json that pins agents.defaults.workspace to workDir; see openclaw_config.go) // Pi: skills → {workDir}/.pi/skills/{name}/SKILL.md (native discovery) // Cursor: skills → {workDir}/.cursor/skills/{name}/SKILL.md (native discovery) // Kimi: skills → {workDir}/.kimi/skills/{name}/SKILL.md (native discovery) @@ -134,6 +134,14 @@ func resolveSkillsDir(workDir, provider string) (string, error) { case "opencode": // OpenCode natively discovers skills from .opencode/skills/ in the workdir. skillsDir = filepath.Join(workDir, ".opencode", "skills") + case "openclaw": + // OpenClaw's native skill scanner reads /skills/. The + // daemon pairs this with a per-task synthesized openclaw-config.json + // (see openclaw_config.go) that pins agents.defaults.workspace to + // workDir, so writing here is what the CLI actually scans. Before + // MUL-2219 this used to fall back to .agent_context/skills/, which + // no openclaw scan path ever inspected. + skillsDir = filepath.Join(workDir, "skills") case "pi": // Pi natively discovers skills from .pi/skills/ in the workdir. skillsDir = filepath.Join(workDir, ".pi", "skills") diff --git a/server/internal/daemon/execenv/execenv.go b/server/internal/daemon/execenv/execenv.go index 1e9df41ba1..ff83f108f8 100644 --- a/server/internal/daemon/execenv/execenv.go +++ b/server/internal/daemon/execenv/execenv.go @@ -37,6 +37,7 @@ type PrepareParams struct { AgentName string // for git branch naming only Provider string // agent provider (determines runtime config and skill injection paths) CodexVersion string // detected Codex CLI version (only used when Provider == "codex") + OpenclawBin string // resolved openclaw CLI path (only used when Provider == "openclaw"); empty = look up on PATH Task TaskContextForEnv // context data for writing files } @@ -84,6 +85,19 @@ type Environment struct { WorkDir string // CodexHome is the path to the per-task CODEX_HOME directory (set only for codex provider). CodexHome string + // OpenclawConfigPath is the path to the per-task synthesized OpenClaw + // config (set only for openclaw provider). The daemon exports this as + // OPENCLAW_CONFIG_PATH on the openclaw subprocess so its native skill + // scanner pins workspaceDir to WorkDir. + OpenclawConfigPath string + // OpenclawIncludeRoot is the directory of the user's active OpenClaw + // config (set only for openclaw provider with an on-disk user config). + // The daemon must prepend it to OPENCLAW_INCLUDE_ROOTS so OpenClaw is + // allowed to follow the wrapper's `$include` link out of envRoot into + // the user's config — by default OpenClaw confines `$include` to the + // directory holding the wrapper file. Empty when no $include is + // emitted (fresh install). + OpenclawIncludeRoot string logger *slog.Logger // for cleanup logging } @@ -152,48 +166,86 @@ func Prepare(params PrepareParams, logger *slog.Logger) (*Environment, error) { env.CodexHome = codexHome } + // For OpenClaw, synthesize a per-task config that pins workspace to + // workDir. The skill scanner then reads {workDir}/skills/ (written by + // writeContextFiles above). Fail closed on errors: a malformed user + // config that the openclaw CLI can't read is a real problem and + // silently degrading to a minimal config would mask it by booting + // OpenClaw without the agents / providers / API keys it expects. + if params.Provider == "openclaw" { + result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{OpenclawBin: params.OpenclawBin}) + if err != nil { + return nil, fmt.Errorf("execenv: prepare openclaw config: %w", err) + } + env.OpenclawConfigPath = result.ConfigPath + env.OpenclawIncludeRoot = result.IncludeRoot + } + logger.Info("execenv: prepared env", "root", envRoot, "repos_available", len(params.Task.Repos)) return env, nil } +// ReuseParams describes the inputs to Reuse. It mirrors PrepareParams for +// the per-provider knobs (CodexVersion, OpenclawBin) so callers can pass +// the same resolved binary path on both first-run and reuse paths. +type ReuseParams struct { + WorkDir string + Provider string + CodexVersion string // only used when Provider == "codex" + OpenclawBin string // only used when Provider == "openclaw"; empty = PATH lookup + Task TaskContextForEnv // refreshed context files / skills +} + // Reuse wraps an existing workdir into an Environment and refreshes context files. // Returns nil if the workdir does not exist (caller should fall back to Prepare). -// -// codexVersion is the detected Codex CLI version, used (only when provider is -// "codex") to pick the right sandbox policy for the per-task config.toml. -// Pass an empty string when the version is unknown. -func Reuse(workDir, provider, codexVersion string, task TaskContextForEnv, logger *slog.Logger) *Environment { - if _, err := os.Stat(workDir); err != nil { +func Reuse(params ReuseParams, logger *slog.Logger) *Environment { + if _, err := os.Stat(params.WorkDir); err != nil { return nil } env := &Environment{ - RootDir: filepath.Dir(workDir), - WorkDir: workDir, + RootDir: filepath.Dir(params.WorkDir), + WorkDir: params.WorkDir, logger: logger, } // Refresh context files (issue_context.md, skills). - if err := writeContextFiles(workDir, provider, task); err != nil { + if err := writeContextFiles(params.WorkDir, params.Provider, params.Task); err != nil { logger.Warn("execenv: refresh context files failed", "error", err) } // Restore CodexHome for Codex provider — the per-task codex-home directory // lives alongside the workdir. Re-run prepareCodexHomeWithOpts to ensure // config (especially sandbox/network access) is up to date. - if provider == "codex" { + if params.Provider == "codex" { codexHome := filepath.Join(env.RootDir, "codex-home") - if err := prepareCodexHomeWithOpts(codexHome, CodexHomeOptions{CodexVersion: codexVersion}, logger); err != nil { + if err := prepareCodexHomeWithOpts(codexHome, CodexHomeOptions{CodexVersion: params.CodexVersion}, logger); err != nil { logger.Warn("execenv: refresh codex-home failed", "error", err) } else { env.CodexHome = codexHome - if err := hydrateCodexSkills(codexHome, task.AgentSkills, logger); err != nil { + if err := hydrateCodexSkills(codexHome, params.Task.AgentSkills, logger); err != nil { logger.Warn("execenv: refresh codex skills failed", "error", err) } } } - logger.Info("execenv: reusing env", "workdir", workDir) + // Refresh the per-task OpenClaw config on reuse — the user may have + // added/removed agents or rotated providers since the prior task ran, + // and the workspace override always re-targets the current workDir. + // Fail closed: a user config that can no longer be parsed should block + // reuse rather than degrade to a minimal config that boots OpenClaw + // without the registered agents. + if params.Provider == "openclaw" { + result, err := prepareOpenclawConfig(env.RootDir, params.WorkDir, OpenclawConfigPrep{OpenclawBin: params.OpenclawBin}) + if err != nil { + logger.Warn("execenv: refresh openclaw config failed", "error", err) + return nil + } + env.OpenclawConfigPath = result.ConfigPath + env.OpenclawIncludeRoot = result.IncludeRoot + } + + logger.Info("execenv: reusing env", "workdir", params.WorkDir) return env } diff --git a/server/internal/daemon/execenv/execenv_test.go b/server/internal/daemon/execenv/execenv_test.go index 9a2e11c1f1..a00aeed09d 100644 --- a/server/internal/daemon/execenv/execenv_test.go +++ b/server/internal/daemon/execenv/execenv_test.go @@ -802,13 +802,13 @@ func TestWriteContextFilesOpencodeNativeSkills(t *testing.T) { } } -// OpenClaw scans its own workspaceDir (resolved from the openclaw config, -// not the task workdir) and never reads {workDir}/.openclaw/skills/. Until -// per-task workspace integration lands, openclaw skills fall back to -// .agent_context/skills/ — the meta AGENTS.md content references that path -// explicitly. This test fails closed if someone re-adds a dead-drop case to -// resolveSkillsDir. -func TestWriteContextFilesOpenclawFallsBackToAgentContext(t *testing.T) { +// OpenClaw's native skill scanner reads {workspaceDir}/skills/. The daemon +// pairs writeContextFiles with a per-task synthesized openclaw-config.json +// (see openclaw_config.go) that pins agents.defaults.workspace to workDir, +// so writing skills to {workDir}/skills/ is what the CLI actually scans. +// This test pins the post-MUL-2219 write path; the previous fallback into +// .agent_context/skills/ was a dead drop the openclaw scanner never read. +func TestWriteContextFilesOpenclawNativeSkills(t *testing.T) { t.Parallel() dir := t.TempDir() @@ -829,15 +829,15 @@ func TestWriteContextFilesOpenclawFallsBackToAgentContext(t *testing.T) { t.Fatalf("writeContextFiles failed: %v", err) } - skillMd, err := os.ReadFile(filepath.Join(dir, ".agent_context", "skills", "go-conventions", "SKILL.md")) + skillMd, err := os.ReadFile(filepath.Join(dir, "skills", "go-conventions", "SKILL.md")) if err != nil { - t.Fatalf("failed to read .agent_context/skills/go-conventions/SKILL.md: %v", err) + t.Fatalf("failed to read skills/go-conventions/SKILL.md: %v", err) } if !strings.Contains(string(skillMd), "Follow Go conventions.") { t.Error("SKILL.md missing content") } - supportFile, err := os.ReadFile(filepath.Join(dir, ".agent_context", "skills", "go-conventions", "templates", "example.go")) + supportFile, err := os.ReadFile(filepath.Join(dir, "skills", "go-conventions", "templates", "example.go")) if err != nil { t.Fatalf("failed to read supporting file: %v", err) } @@ -845,6 +845,10 @@ func TestWriteContextFilesOpenclawFallsBackToAgentContext(t *testing.T) { t.Errorf("supporting file content = %q, want %q", string(supportFile), "package main") } + // The pre-MUL-2219 fallback path must NOT be written: openclaw never scans it. + if _, err := os.Stat(filepath.Join(dir, ".agent_context", "skills")); !os.IsNotExist(err) { + t.Error(".agent_context/skills/ MUST NOT be written for openclaw — the scanner does not read that path") + } if _, err := os.Stat(filepath.Join(dir, ".openclaw", "skills")); !os.IsNotExist(err) { t.Error(".openclaw/skills/ MUST NOT be written — openclaw never scans that path; writing there is a dead drop") } @@ -1914,7 +1918,7 @@ func TestReuseRestoresCodexHome(t *testing.T) { } // Reuse should restore CodexHome. - reused := Reuse(env.WorkDir, "codex", "", TaskContextForEnv{IssueID: "reuse-test"}, testLogger()) + reused := Reuse(ReuseParams{WorkDir: env.WorkDir, Provider: "codex", Task: TaskContextForEnv{IssueID: "reuse-test"}}, testLogger()) if reused == nil { t.Fatal("Reuse returned nil") } @@ -1964,7 +1968,7 @@ func TestReuseRestoresCodexPluginCache(t *testing.T) { t.Fatalf("remove codex plugins dir: %v", err) } - reused := Reuse(env.WorkDir, "codex", "", TaskContextForEnv{IssueID: "reuse-plugin-test"}, testLogger()) + reused := Reuse(ReuseParams{WorkDir: env.WorkDir, Provider: "codex", Task: TaskContextForEnv{IssueID: "reuse-plugin-test"}}, testLogger()) if reused == nil { t.Fatal("Reuse returned nil") } @@ -2002,7 +2006,7 @@ func TestReuseWritesMissingCodexWorkspaceSkills(t *testing.T) { t.Fatalf("remove codex skills dir: %v", err) } - reused := Reuse(env.WorkDir, "codex", "", TaskContextForEnv{ + reused := Reuse(ReuseParams{WorkDir: env.WorkDir, Provider: "codex", Task: TaskContextForEnv{ IssueID: "reuse-skill-test", AgentSkills: []SkillContextForEnv{ { @@ -2011,7 +2015,7 @@ func TestReuseWritesMissingCodexWorkspaceSkills(t *testing.T) { Files: []SkillFileContextForEnv{{Path: "examples/example.md", Content: "Example"}}, }, }, - }, testLogger()) + }}, testLogger()) if reused == nil { t.Fatal("Reuse returned nil") } @@ -2061,7 +2065,7 @@ func TestReuseUpdatesCodexWorkspaceSkills(t *testing.T) { } defer env.Cleanup(true) - reused := Reuse(env.WorkDir, "codex", "", TaskContextForEnv{ + reused := Reuse(ReuseParams{WorkDir: env.WorkDir, Provider: "codex", Task: TaskContextForEnv{ IssueID: "reuse-skill-update-test", AgentSkills: []SkillContextForEnv{ { @@ -2070,7 +2074,7 @@ func TestReuseUpdatesCodexWorkspaceSkills(t *testing.T) { Files: []SkillFileContextForEnv{{Path: "examples/example.md", Content: "Updated example"}}, }, }, - }, testLogger()) + }}, testLogger()) if reused == nil { t.Fatal("Reuse returned nil") } @@ -2328,9 +2332,9 @@ func TestReuseSeedsUserSkillUpdates(t *testing.T) { t.Fatalf("update user SKILL.md: %v", err) } - reused := Reuse(env.WorkDir, "codex", "", TaskContextForEnv{ + reused := Reuse(ReuseParams{WorkDir: env.WorkDir, Provider: "codex", Task: TaskContextForEnv{ IssueID: "user-skill-reuse-test", - }, testLogger()) + }}, testLogger()) if reused == nil { t.Fatal("Reuse returned nil") } @@ -2383,12 +2387,12 @@ func TestReuseClearsUserSkillResidueOnWorkspaceConflict(t *testing.T) { t.Fatalf("user support file should be seeded in round 1: %v", err) } - reused := Reuse(env.WorkDir, "codex", "", TaskContextForEnv{ + reused := Reuse(ReuseParams{WorkDir: env.WorkDir, Provider: "codex", Task: TaskContextForEnv{ IssueID: "reuse-conflict-test", AgentSkills: []SkillContextForEnv{ {Name: "Writing", Content: "workspace writing"}, }, - }, testLogger()) + }}, testLogger()) if reused == nil { t.Fatal("Reuse returned nil") } @@ -2445,9 +2449,9 @@ func TestReuseClearsRemovedUserSkill(t *testing.T) { t.Fatalf("remove user skill: %v", err) } - reused := Reuse(env.WorkDir, "codex", "", TaskContextForEnv{ + reused := Reuse(ReuseParams{WorkDir: env.WorkDir, Provider: "codex", Task: TaskContextForEnv{ IssueID: "reuse-remove-test", - }, testLogger()) + }}, testLogger()) if reused == nil { t.Fatal("Reuse returned nil") } diff --git a/server/internal/daemon/execenv/openclaw_config.go b/server/internal/daemon/execenv/openclaw_config.go new file mode 100644 index 0000000000..f5288d08f5 --- /dev/null +++ b/server/internal/daemon/execenv/openclaw_config.go @@ -0,0 +1,332 @@ +package execenv + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "time" +) + +// openclawConfigFile is the per-task synthesized OpenClaw config the daemon +// points the openclaw CLI at via OPENCLAW_CONFIG_PATH. It sits in the env +// root (alongside workdir/, output/, logs/) so the GC reaper sweeps it with +// the rest of the task env. +const openclawConfigFile = "openclaw-config.json" + +// openclawCLITimeout caps each `openclaw config ...` invocation during task +// setup. The CLI is fast (<200ms normal); 5s leaves headroom for a cold +// node start without letting a hung CLI stall task dispatch indefinitely. +const openclawCLITimeout = 5 * time.Second + +// OpenclawConfigPrep is the input to prepareOpenclawConfig. Only OpenclawBin +// is meaningful in production — Timeout is here for tests that need a tight +// cap to assert error paths. +type OpenclawConfigPrep struct { + // OpenclawBin is the openclaw CLI binary to invoke for config introspection. + // Empty means resolve "openclaw" from PATH at exec time. + OpenclawBin string + // Timeout caps each CLI invocation. Zero falls back to openclawCLITimeout. + Timeout time.Duration +} + +// OpenclawConfigResult is what prepareOpenclawConfig returns to its callers +// in execenv.go. ConfigPath is the wrapper file the daemon points +// OPENCLAW_CONFIG_PATH at. IncludeRoot is the directory the daemon must add +// to OPENCLAW_INCLUDE_ROOTS so OpenClaw will follow the $include link out +// of envRoot into the user's active config; it is empty when no $include +// is emitted (fresh install). +type OpenclawConfigResult struct { + ConfigPath string + IncludeRoot string +} + +// prepareOpenclawConfig writes a per-task OpenClaw config to envRoot and +// returns its absolute path along with the include root the daemon must +// grant. The daemon sets OPENCLAW_CONFIG_PATH to the path on the spawned +// openclaw subprocess so the CLI resolves its `agents.defaults.workspace` +// (and every `agents.list[].workspace`) to the task workdir — which is +// what makes OpenClaw's native skill scanner pick up the per-task skills +// we write under `/skills/`. +// +// Strategy: delegate JSON5 / $include / env-substitution / state-dir +// resolution to the openclaw CLI itself rather than re-implementing the +// spec. We: +// +// 1. Run `openclaw config file` to find the user's active config path +// (handles OPENCLAW_CONFIG_PATH, OPENCLAW_STATE_DIR, OPENCLAW_HOME, and +// the default location). +// 2. Run `openclaw config get agents.list --json` to enumerate every +// registered agent ID with its resolved fields. The CLI parses JSON5, +// follows $include, and substitutes ${VAR} for us. +// 3. Write a wrapper config to envRoot/openclaw-config.json that +// `$include`s the active path and overrides +// `agents.defaults.workspace` plus every `agents.list[].workspace` to +// workDir. The original config bytes are not mutated — they are loaded +// by openclaw's own loader through the $include link, which preserves +// comments, secrets, and nested $include chains verbatim. +// +// **Cross-directory $include confinement.** OpenClaw confines `$include` +// resolution to the directory containing the wrapper file unless the +// target's parent is listed in `OPENCLAW_INCLUDE_ROOTS`. Our wrapper lives +// in envRoot but $includes the user's active config (typically +// `~/.openclaw/openclaw.json`) — a cross-directory hop. We surface +// `filepath.Dir(activePath)` as IncludeRoot so the daemon can prepend it +// to whatever the user already has in OPENCLAW_INCLUDE_ROOTS; without +// this, OpenClaw refuses to follow the link and the wrapper boots with no +// user config. Fresh install emits no $include, so IncludeRoot is "". +// +// **Intentional task isolation.** The override of every per-agent workspace +// is deliberate. OpenClaw's resolution order is +// `agents.list[id].workspace → agents.defaults.workspace → ~/.openclaw/ +// workspace`. Pinning only the default would let a per-agent workspace the +// user configured at host scope silently re-route the scanner back to the +// shared workspace, defeating the per-task skill discovery this whole flow +// exists for. The cost is that any per-agent SOUL.md / MEMORY.md / standing +// orders the user laid in `/` are NOT visible to the +// in-task openclaw run — task isolation wins over host carry-over. The +// user's on-disk config is untouched; this only affects the wrapper used +// for this single task. +// +// **Fail closed.** Missing openclaw binary, CLI errors, malformed CLI +// output, or any IO error during write surfaces as an error to the caller +// rather than degrading to a minimal config. An earlier version silently +// synthesized a minimal config on parse failure; that masked broken user +// configs by starting OpenClaw without the registered agents / model +// providers / API keys it expects, which led to tasks routing to the wrong +// agent or failing to authenticate. The only "synthesize minimal" case +// kept is a fresh install where the CLI reports a path but no file exists +// — there is no user data to lose in that case. +func prepareOpenclawConfig(envRoot, workDir string, opts OpenclawConfigPrep) (OpenclawConfigResult, error) { + bin := opts.OpenclawBin + if bin == "" { + bin = "openclaw" + } + timeout := opts.Timeout + if timeout <= 0 { + timeout = openclawCLITimeout + } + + activePath, exists, err := openclawActiveConfigPath(bin, timeout) + if err != nil { + return OpenclawConfigResult{}, fmt.Errorf("locate openclaw active config: %w", err) + } + + var resolvedList []any + if exists { + resolvedList, err = openclawResolvedAgentsList(bin, timeout) + if err != nil { + return OpenclawConfigResult{}, fmt.Errorf("read openclaw agents.list: %w", err) + } + } + + cfg := buildPerTaskOpenclawConfig(activePath, exists, resolvedList, workDir) + + data, err := json.MarshalIndent(cfg, "", " ") + if err != nil { + return OpenclawConfigResult{}, fmt.Errorf("marshal openclaw config: %w", err) + } + outPath := filepath.Join(envRoot, openclawConfigFile) + // 0o600 — defense in depth. The wrapper itself carries no secrets (the + // $include link is just a filesystem path), but the file lives next to + // task scratch and we keep the same posture as ~/.openclaw/openclaw.json. + if err := os.WriteFile(outPath, data, 0o600); err != nil { + return OpenclawConfigResult{}, fmt.Errorf("write openclaw config: %w", err) + } + result := OpenclawConfigResult{ConfigPath: outPath} + if exists { + // Only emit an include root when we actually emit a $include line + // (i.e. the user has an on-disk config). On fresh install the + // wrapper is self-contained and OpenClaw never needs to step out + // of envRoot, so no extra root is required. + result.IncludeRoot = filepath.Dir(activePath) + } + return result, nil +} + +// buildPerTaskOpenclawConfig assembles the wrapper map that goes on disk. +// +// Exists=true: emit a $include link to the user's active config plus the +// workspace overrides as siblings. OpenClaw deep-merges sibling object keys +// after includes, so agents.defaults.workspace lands correctly. The +// agents.list override is emitted as a full replacement carrying every +// field of every resolved entry (id, model, prompts, tools, …) verbatim +// with only `workspace` rewritten — this is robust regardless of whether +// the runtime merges the sibling array or replaces it, because either way +// the resulting list is shape-equivalent to the user's minus workspace. +// +// Exists=false: a fresh install with no on-disk config. Emit a minimal +// config containing only the workspace override. There is no user data to +// $include here, so this is not the silent-fallback case the reviewer +// flagged. +func buildPerTaskOpenclawConfig(activePath string, exists bool, resolvedList []any, workDir string) map[string]any { + agents := map[string]any{ + "defaults": map[string]any{"workspace": workDir}, + } + if rewritten := rewriteAgentsListWorkspaces(resolvedList, workDir); rewritten != nil { + agents["list"] = rewritten + } + cfg := map[string]any{ + "agents": agents, + } + if exists { + // Array form (not single-file form) so OpenClaw deep-merges the + // included object with our sibling keys rather than letting the + // include replace the whole containing object. + cfg["$include"] = []any{activePath} + } + return cfg +} + +// rewriteAgentsListWorkspaces copies every entry of the resolved agents.list +// and pins its `workspace` field to workDir. Returns nil when the input is +// nil or empty so buildPerTaskOpenclawConfig can omit the key entirely +// (avoiding an empty `agents.list: []` that would replace whatever the +// include carries). +func rewriteAgentsListWorkspaces(list []any, workDir string) []any { + if len(list) == 0 { + return nil + } + out := make([]any, 0, len(list)) + for _, item := range list { + entry, ok := item.(map[string]any) + if !ok { + // Shape we don't recognize — skip rather than guess. Worst case + // the user loses native skill discovery on that one agent; we + // still won't crash the wrapper. + continue + } + copyEntry := make(map[string]any, len(entry)+1) + for k, v := range entry { + copyEntry[k] = v + } + copyEntry["workspace"] = workDir + out = append(out, copyEntry) + } + if len(out) == 0 { + return nil + } + return out +} + +// openclawActiveConfigPath runs `openclaw config file` to discover the path +// the openclaw CLI considers active. Returns (absolutePath, exists, error). +// +// The CLI handles the full resolution chain — OPENCLAW_CONFIG_PATH, the +// state directory (OPENCLAW_STATE_DIR / OPENCLAW_HOME / default), legacy +// migration, and `~` expansion — so we don't re-implement it here. +// +// The reported path uses `~` shorthand for the user's home; we expand it +// so the $include reference we write is unambiguous absolute. +func openclawActiveConfigPath(bin string, timeout time.Duration) (string, bool, error) { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + out, err := openclawExec(ctx, bin, "config", "file") + if err != nil { + return "", false, err + } + path := strings.TrimSpace(out) + if path == "" { + return "", false, fmt.Errorf("`openclaw config file` returned empty output") + } + if path == "~" || strings.HasPrefix(path, "~/") { + home, herr := os.UserHomeDir() + if herr != nil { + return "", false, fmt.Errorf("expand `~` in openclaw config path: %w", herr) + } + if path == "~" { + path = home + } else { + path = filepath.Join(home, strings.TrimPrefix(path, "~/")) + } + } + if !filepath.IsAbs(path) { + return "", false, fmt.Errorf("openclaw reported non-absolute config path %q", path) + } + info, err := os.Stat(path) + if errors.Is(err, os.ErrNotExist) { + return path, false, nil + } + if err != nil { + return "", false, fmt.Errorf("stat openclaw config %s: %w", path, err) + } + if info.IsDir() { + return "", false, fmt.Errorf("openclaw config path %s is a directory, not a file", path) + } + return path, true, nil +} + +// openclawResolvedAgentsList fetches the user's resolved agents.list via +// `openclaw config get agents.list --json`. The CLI returns the post- +// include, post-env-substitution view of the array, which is exactly the +// shape we need to rewrite each entry's workspace. +// +// Returns nil (not an error) when agents.list is unset. +func openclawResolvedAgentsList(bin string, timeout time.Duration) ([]any, error) { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + out, err := openclawExec(ctx, bin, "config", "get", "agents.list", "--json") + if err != nil { + if isOpenclawKeyMissing(err) { + return nil, nil + } + return nil, err + } + trimmed := strings.TrimSpace(out) + if trimmed == "" || trimmed == "null" { + return nil, nil + } + var list []any + if err := json.Unmarshal([]byte(trimmed), &list); err != nil { + return nil, fmt.Errorf("parse `openclaw config get agents.list --json` output: %w", err) + } + return list, nil +} + +// openclawExec is the runtime hook prepareOpenclawConfig uses to invoke the +// openclaw CLI. Production points at execOpenclawCLI; tests swap in a stub +// to avoid spawning a real binary. Production code never reassigns it. +var openclawExec = execOpenclawCLI + +// execOpenclawCLI executes an openclaw subcommand and returns its stdout. +// The daemon's environment is inherited so OPENCLAW_CONFIG_PATH / +// OPENCLAW_STATE_DIR / OPENCLAW_HOME / OPENCLAW_INCLUDE_ROOTS pass through. +// +// stderr is captured separately and appended to error messages — failures +// here surface up to the daemon log, and a `openclaw doctor` hint there is +// more useful than just an exit code. +func execOpenclawCLI(ctx context.Context, bin string, args ...string) (string, error) { + cmd := exec.CommandContext(ctx, bin, args...) + cmd.Env = os.Environ() + var stderr strings.Builder + cmd.Stderr = &stderr + raw, err := cmd.Output() + if err != nil { + stderrMsg := strings.TrimSpace(stderr.String()) + if stderrMsg != "" { + return "", fmt.Errorf("openclaw %s: %w (stderr: %s)", strings.Join(args, " "), err, stderrMsg) + } + return "", fmt.Errorf("openclaw %s: %w", strings.Join(args, " "), err) + } + return string(raw), nil +} + +// isOpenclawKeyMissing returns true when the CLI error indicates the asked- +// for path simply isn't set, as opposed to a real failure (bad config, +// CLI bug, missing binary). The CLI's "key not found" exit text has varied +// across versions, so we match on a handful of substrings rather than the +// exit code alone. +func isOpenclawKeyMissing(err error) bool { + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "No value at ") || + strings.Contains(msg, "not set") || + strings.Contains(msg, "missing key") || + strings.Contains(msg, "Path not found") +} diff --git a/server/internal/daemon/execenv/openclaw_config_test.go b/server/internal/daemon/execenv/openclaw_config_test.go new file mode 100644 index 0000000000..3b0caa9b1b --- /dev/null +++ b/server/internal/daemon/execenv/openclaw_config_test.go @@ -0,0 +1,625 @@ +package execenv + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "log/slog" + "os" + "path/filepath" + "strings" + "testing" +) + +// openclawCLIStub captures one or more (subcommand, response) pairs and +// installs itself into the package-level openclawExec hook for the duration +// of a test. Each call records the args it saw so assertions can verify the +// preparer hit `config file` and `config get agents.list --json`. +type openclawCLIStub struct { + t *testing.T + bin string + responses map[string]openclawResponse + calls []openclawCall +} + +type openclawCall struct { + bin string + args []string +} + +type openclawResponse struct { + stdout string + err error +} + +func installOpenclawStub(t *testing.T, responses map[string]openclawResponse) *openclawCLIStub { + t.Helper() + stub := &openclawCLIStub{ + t: t, + bin: "/test/stub/openclaw", + responses: responses, + } + prev := openclawExec + openclawExec = stub.exec + t.Cleanup(func() { openclawExec = prev }) + return stub +} + +func (s *openclawCLIStub) exec(_ context.Context, bin string, args ...string) (string, error) { + s.calls = append(s.calls, openclawCall{bin: bin, args: append([]string(nil), args...)}) + key := strings.Join(args, " ") + resp, ok := s.responses[key] + if !ok { + return "", fmt.Errorf("openclawCLIStub: unexpected args %q", key) + } + return resp.stdout, resp.err +} + +func mustReadJSON(t *testing.T, path string) map[string]any { + t.Helper() + raw, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read synthesized cfg: %v", err) + } + var got map[string]any + if err := json.Unmarshal(raw, &got); err != nil { + t.Fatalf("parse synthesized cfg: %v", err) + } + return got +} + +// TestPrepareOpenclawConfigDelegatesParsingToCLI is the headline assertion +// for the Elon must-fix: instead of re-parsing the user's openclaw.json +// with encoding/json (which can't read JSON5 / $include / env-var +// substitution), we delegate the read to the openclaw CLI. The wrapper +// $includes the user's active path so OpenClaw's own loader handles the +// JSON5 / $include resolution; we only emit workspace overrides. +func TestPrepareOpenclawConfigDelegatesParsingToCLI(t *testing.T) { + envRoot := t.TempDir() + workDir := filepath.Join(envRoot, "workdir") + if err := os.MkdirAll(workDir, 0o755); err != nil { + t.Fatalf("mkdir workdir: %v", err) + } + + // JSON5 user config — comments and trailing commas would break the old + // encoding/json reader. The stub doesn't actually parse this; it just + // proves the wrapper points the $include at the right file regardless + // of its on-disk syntax. + userConfigDir := t.TempDir() + userConfigPath := filepath.Join(userConfigDir, "openclaw.json") + json5Body := `// User config with JSON5 features the old parser couldn't read +{ + agents: { + defaults: { + workspace: "/Users/alice/.openclaw/workspace", + model: { primary: "anthropic/claude-sonnet-4-6" }, + }, + list: [ + { id: "scout", workspace: "/Users/alice/projects/scout", }, + { id: "coder", model: "openai/gpt-5", }, + ], + }, + gateway: { port: 18789 }, // trailing comma +} +` + if err := os.WriteFile(userConfigPath, []byte(json5Body), 0o600); err != nil { + t.Fatalf("write user cfg: %v", err) + } + + stub := installOpenclawStub(t, map[string]openclawResponse{ + "config file": {stdout: userConfigPath + "\n"}, + "config get agents.list --json": {stdout: `[ + { "id": "scout", "workspace": "/Users/alice/projects/scout" }, + { "id": "coder", "model": "openai/gpt-5" } + ]`}, + }) + + result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{OpenclawBin: stub.bin}) + if err != nil { + t.Fatalf("prepareOpenclawConfig: %v", err) + } + cfgPath := result.ConfigPath + if cfgPath != filepath.Join(envRoot, openclawConfigFile) { + t.Errorf("cfgPath = %q, want %q", cfgPath, filepath.Join(envRoot, openclawConfigFile)) + } + + got := mustReadJSON(t, cfgPath) + + // $include must reference the user's active config so OpenClaw's own + // loader does the JSON5 / $include / env-substitution work. + include, ok := got["$include"].([]any) + if !ok || len(include) != 1 || include[0] != userConfigPath { + t.Errorf("$include = %v, want [%q]", got["$include"], userConfigPath) + } + + // The wrapper $includes a path that lives outside envRoot. OpenClaw + // confines $include resolution to the wrapper file's own directory + // unless OPENCLAW_INCLUDE_ROOTS lists the target. Surface the user + // config's dirname so the daemon can grant it. + if result.IncludeRoot != userConfigDir { + t.Errorf("IncludeRoot = %q, want %q (dirname of active config so wrapper can $include across dirs)", result.IncludeRoot, userConfigDir) + } + + agents := got["agents"].(map[string]any) + defaults := agents["defaults"].(map[string]any) + if defaults["workspace"] != workDir { + t.Errorf("agents.defaults.workspace = %v, want %q", defaults["workspace"], workDir) + } + + // Per-agent workspaces must be rewritten so a host-scope agents.list[]. + // workspace cannot silently win over our defaults override. This is + // intentional per-task isolation (see prepareOpenclawConfig doc). + list := agents["list"].([]any) + if len(list) != 2 { + t.Fatalf("agents.list length = %d, want 2", len(list)) + } + for i, item := range list { + entry := item.(map[string]any) + if entry["workspace"] != workDir { + t.Errorf("agents.list[%d].workspace = %v, want %q (per-agent overrides must be rewritten so they don't beat defaults)", i, entry["workspace"], workDir) + } + } + // Non-workspace fields per entry are carried over so a sibling-replace + // merge in OpenClaw's $include semantics doesn't silently lose them. + if list[0].(map[string]any)["id"] != "scout" { + t.Errorf("agents.list[0].id lost in carryover: %v", list[0]) + } + if list[1].(map[string]any)["model"] != "openai/gpt-5" { + t.Errorf("agents.list[1].model lost in carryover: %v", list[1]) + } +} + +// TestPrepareOpenclawConfigFailsClosedOnCLIError — the headline regression +// for Elon's review. When the openclaw CLI fails (broken config, missing +// binary, etc.), prepareOpenclawConfig MUST surface the error rather than +// silently synthesize a minimal config that would mask the user's broken +// state and boot OpenClaw without their registered agents. +func TestPrepareOpenclawConfigFailsClosedOnCLIError(t *testing.T) { + envRoot := t.TempDir() + workDir := filepath.Join(envRoot, "workdir") + if err := os.MkdirAll(workDir, 0o755); err != nil { + t.Fatalf("mkdir workdir: %v", err) + } + + stub := installOpenclawStub(t, map[string]openclawResponse{ + "config file": {err: errors.New("exec: openclaw: no such file or directory")}, + }) + + _, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{OpenclawBin: stub.bin}) + if err == nil { + t.Fatal("prepareOpenclawConfig succeeded on CLI failure; expected fail closed") + } + if !strings.Contains(err.Error(), "locate openclaw active config") { + t.Errorf("error message %q does not name the failed step", err.Error()) + } + + // No stale wrapper left behind. + if _, err := os.Stat(filepath.Join(envRoot, openclawConfigFile)); !os.IsNotExist(err) { + t.Errorf("wrapper config should not exist after fail-closed; got err = %v", err) + } +} + +// TestPrepareOpenclawConfigFailsClosedOnMalformedAgentsList — the second +// fail-closed surface. When `openclaw config get agents.list --json` +// returns junk we can't parse, we fail rather than guess. +func TestPrepareOpenclawConfigFailsClosedOnMalformedAgentsList(t *testing.T) { + envRoot := t.TempDir() + workDir := filepath.Join(envRoot, "workdir") + if err := os.MkdirAll(workDir, 0o755); err != nil { + t.Fatalf("mkdir workdir: %v", err) + } + + userConfigPath := filepath.Join(t.TempDir(), "openclaw.json") + if err := os.WriteFile(userConfigPath, []byte(`{}`), 0o600); err != nil { + t.Fatalf("write user cfg: %v", err) + } + + stub := installOpenclawStub(t, map[string]openclawResponse{ + "config file": {stdout: userConfigPath}, + "config get agents.list --json": {stdout: "<<>>"}, + }) + + _, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{OpenclawBin: stub.bin}) + if err == nil { + t.Fatal("prepareOpenclawConfig succeeded on malformed agents.list output; expected fail closed") + } + if !strings.Contains(err.Error(), "agents.list") { + t.Errorf("error message %q does not name the failed step", err.Error()) + } +} + +// TestPrepareOpenclawConfigKeyMissingTreatedAsEmpty — `config get` exits +// non-zero when a path is unset. That is not a failure; the user simply has +// no agents.list. We must produce a valid wrapper with just the defaults +// override. +func TestPrepareOpenclawConfigKeyMissingTreatedAsEmpty(t *testing.T) { + envRoot := t.TempDir() + workDir := filepath.Join(envRoot, "workdir") + if err := os.MkdirAll(workDir, 0o755); err != nil { + t.Fatalf("mkdir workdir: %v", err) + } + + userConfigPath := filepath.Join(t.TempDir(), "openclaw.json") + if err := os.WriteFile(userConfigPath, []byte(`{}`), 0o600); err != nil { + t.Fatalf("write user cfg: %v", err) + } + + stub := installOpenclawStub(t, map[string]openclawResponse{ + "config file": {stdout: userConfigPath}, + "config get agents.list --json": {err: errors.New("openclaw: No value at agents.list")}, + }) + + result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{OpenclawBin: stub.bin}) + if err != nil { + t.Fatalf("prepareOpenclawConfig: %v", err) + } + cfgPath := result.ConfigPath + got := mustReadJSON(t, cfgPath) + if _, present := got["agents"].(map[string]any)["list"]; present { + t.Errorf("agents.list should be omitted when user has none, got %v", got["agents"]) + } + if got["agents"].(map[string]any)["defaults"].(map[string]any)["workspace"] != workDir { + t.Errorf("defaults.workspace not set when agents.list missing") + } +} + +// TestPrepareOpenclawConfigFreshInstallNoOnDiskConfig — the only legitimate +// "synthesize minimal" case. `openclaw config file` reports a path (the +// default) but the file does not exist yet. We emit a wrapper with the +// workspace override and NO $include (there is nothing to include). +func TestPrepareOpenclawConfigFreshInstallNoOnDiskConfig(t *testing.T) { + envRoot := t.TempDir() + workDir := filepath.Join(envRoot, "workdir") + if err := os.MkdirAll(workDir, 0o755); err != nil { + t.Fatalf("mkdir workdir: %v", err) + } + + // CLI reports a default path that doesn't exist (fresh install). + missingPath := filepath.Join(t.TempDir(), "openclaw.json") + + stub := installOpenclawStub(t, map[string]openclawResponse{ + "config file": {stdout: missingPath}, + // `config get` should not be called when the file does not exist; + // the stub will fail "unexpected args" if it is. + }) + + result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{OpenclawBin: stub.bin}) + if err != nil { + t.Fatalf("prepareOpenclawConfig: %v", err) + } + cfgPath := result.ConfigPath + got := mustReadJSON(t, cfgPath) + if _, present := got["$include"]; present { + t.Errorf("$include should be absent for fresh install, got %v", got["$include"]) + } + if got["agents"].(map[string]any)["defaults"].(map[string]any)["workspace"] != workDir { + t.Errorf("defaults.workspace not set on fresh-install wrapper") + } + // Fresh install emits no $include, so no extra include root is needed + // — the wrapper never steps outside envRoot. Daemon should leave the + // user's OPENCLAW_INCLUDE_ROOTS alone. + if result.IncludeRoot != "" { + t.Errorf("IncludeRoot = %q on fresh install, want empty (no $include emitted)", result.IncludeRoot) + } +} + +// TestPrepareOpenclawConfigExpandsTilde — `openclaw config file` reports +// paths with `~` shortened. The $include in our wrapper must be absolute so +// the loader resolves it unambiguously. +func TestPrepareOpenclawConfigExpandsTilde(t *testing.T) { + envRoot := t.TempDir() + workDir := filepath.Join(envRoot, "workdir") + if err := os.MkdirAll(workDir, 0o755); err != nil { + t.Fatalf("mkdir workdir: %v", err) + } + + fakeHome := t.TempDir() + t.Setenv("HOME", fakeHome) + if err := os.MkdirAll(filepath.Join(fakeHome, ".openclaw"), 0o755); err != nil { + t.Fatalf("mkdir home/.openclaw: %v", err) + } + realPath := filepath.Join(fakeHome, ".openclaw", "openclaw.json") + if err := os.WriteFile(realPath, []byte(`{}`), 0o600); err != nil { + t.Fatalf("write user cfg: %v", err) + } + + stub := installOpenclawStub(t, map[string]openclawResponse{ + "config file": {stdout: "~/.openclaw/openclaw.json\n"}, + "config get agents.list --json": {stdout: "null"}, + }) + + result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{OpenclawBin: stub.bin}) + if err != nil { + t.Fatalf("prepareOpenclawConfig: %v", err) + } + cfgPath := result.ConfigPath + got := mustReadJSON(t, cfgPath) + include := got["$include"].([]any) + if include[0] != realPath { + t.Errorf("$include[0] = %v, want %q (tilde must be expanded to absolute)", include[0], realPath) + } + // IncludeRoot must also use the expanded absolute dirname, otherwise + // the daemon would export a `~/.openclaw`-shaped root that OpenClaw + // would not match against the resolved absolute include target. + wantRoot := filepath.Join(fakeHome, ".openclaw") + if result.IncludeRoot != wantRoot { + t.Errorf("IncludeRoot = %q, want %q (must be expanded absolute dirname)", result.IncludeRoot, wantRoot) + } +} + +// TestPrepareOpenclawConfigWrapperLoadableUnderIncludeConfinement is the +// regression test for the Elon include-confinement blocker. OpenClaw +// resolves `$include` only inside the wrapper file's own directory unless +// the target's parent dir is granted via OPENCLAW_INCLUDE_ROOTS. The +// previous PR wrote a wrapper at envRoot that $included +// `~/.openclaw/openclaw.json` (cross-directory) but never surfaced the +// dirname; OpenClaw would have refused to follow the link at runtime. +// +// This test simulates the same confinement check OpenClaw performs: +// +// - For every `$include` target, assert filepath.Dir(target) is either +// the wrapper's own dir OR matches the IncludeRoot we surface for the +// daemon to grant. +// +// It does NOT shell out to a real openclaw binary — the spec is small and +// stable enough that mirroring it in-test is more reliable than depending +// on the CLI being installed in CI. If this assertion ever drifts from the +// real loader, the upstream docs are the source of truth: +// https://github.com/openclaw/openclaw/blob/main/docs/gateway/configuration.md +func TestPrepareOpenclawConfigWrapperLoadableUnderIncludeConfinement(t *testing.T) { + envRoot := t.TempDir() + workDir := filepath.Join(envRoot, "workdir") + if err := os.MkdirAll(workDir, 0o755); err != nil { + t.Fatalf("mkdir workdir: %v", err) + } + + // User's active config sits in its own dir, not envRoot. This is the + // realistic shape (~/.openclaw/openclaw.json is never inside the task + // workspace) and is the exact case the bug paper-trail flagged. + userConfigDir := t.TempDir() + userConfigPath := filepath.Join(userConfigDir, "openclaw.json") + if err := os.WriteFile(userConfigPath, []byte(`{}`), 0o600); err != nil { + t.Fatalf("write user cfg: %v", err) + } + + stub := installOpenclawStub(t, map[string]openclawResponse{ + "config file": {stdout: userConfigPath}, + "config get agents.list --json": {stdout: "null"}, + }) + + result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{OpenclawBin: stub.bin}) + if err != nil { + t.Fatalf("prepareOpenclawConfig: %v", err) + } + + got := mustReadJSON(t, result.ConfigPath) + rawIncludes, ok := got["$include"].([]any) + if !ok || len(rawIncludes) == 0 { + t.Fatalf("wrapper has no $include entries, but a user config is present: %v", got) + } + + // Mirror OpenClaw's confinement check: every cross-dir $include target + // must have its dirname covered by either the wrapper's own dir or the + // IncludeRoot we surface. + wrapperDir := filepath.Dir(result.ConfigPath) + granted := []string{wrapperDir} + if result.IncludeRoot != "" { + granted = append(granted, result.IncludeRoot) + } + for _, raw := range rawIncludes { + target, ok := raw.(string) + if !ok { + t.Fatalf("$include entry is not a string: %T %v", raw, raw) + } + targetDir := filepath.Dir(target) + allowed := false + for _, g := range granted { + if targetDir == g { + allowed = true + break + } + } + if !allowed { + t.Errorf("$include target %q has dirname %q which is not in granted include roots %v — OpenClaw would refuse to load it", + target, targetDir, granted) + } + } +} + +// TestPrepareOpenclawSkillWriteMatchesScanPath is the regression test the +// MUL-2219 DoD calls out: the directory Multica writes skills into MUST be +// the same directory the OpenClaw scanner reads from. We assert this by +// resolving the workspaceDir the way OpenClaw does (agents.defaults.workspace +// from the synthesized config) and proving {workspaceDir}/skills/ holds the +// skill we wrote. Previous fixes asserted "we wrote a file" without checking +// the scanner would ever see it; that is why MUL-2213 / #2621 needed a +// follow-up. +func TestPrepareOpenclawSkillWriteMatchesScanPath(t *testing.T) { + envRoot := t.TempDir() + workDir := filepath.Join(envRoot, "workdir") + for _, sub := range []string{workDir, filepath.Join(envRoot, "output"), filepath.Join(envRoot, "logs")} { + if err := os.MkdirAll(sub, 0o755); err != nil { + t.Fatalf("mkdir %s: %v", sub, err) + } + } + + stub := installOpenclawStub(t, map[string]openclawResponse{ + // Fresh install — no user config on disk. Wrapper carries only the + // workspace override, which is what the scanner reads. + "config file": {stdout: filepath.Join(t.TempDir(), "absent-openclaw.json")}, + }) + + skills := []SkillContextForEnv{ + {Name: "Issue Review", Content: "Review issues thoroughly."}, + {Name: "Local Dev", Content: "Spin up the local dev env."}, + } + + result, err := prepareOpenclawConfig(envRoot, workDir, OpenclawConfigPrep{OpenclawBin: stub.bin}) + if err != nil { + t.Fatalf("prepareOpenclawConfig: %v", err) + } + cfgPath := result.ConfigPath + if err := writeContextFiles(workDir, "openclaw", TaskContextForEnv{ + IssueID: "issue-1", + AgentSkills: skills, + }); err != nil { + t.Fatalf("writeContextFiles: %v", err) + } + + cfg := mustReadJSON(t, cfgPath) + wsDir := cfg["agents"].(map[string]any)["defaults"].(map[string]any)["workspace"].(string) + for _, s := range skills { + want := filepath.Join(wsDir, "skills", sanitizeSkillName(s.Name), "SKILL.md") + if _, err := os.Stat(want); err != nil { + t.Errorf("openclaw scan target %s missing — Multica's write path and the openclaw scanner are out of sync: %v", want, err) + } + } +} + +// TestPrepareEnvironmentOpenclawWiresConfigPath — end-to-end: Prepare sets +// env.OpenclawConfigPath so the daemon can export OPENCLAW_CONFIG_PATH, and +// the path resolves to a file with the correct workspace override. With +// fail-closed semantics, Prepare itself errors when the CLI is unavailable; +// a stub here keeps the happy path observable. +func TestPrepareEnvironmentOpenclawWiresConfigPath(t *testing.T) { + wsRoot := t.TempDir() + + stub := installOpenclawStub(t, map[string]openclawResponse{ + "config file": {stdout: filepath.Join(t.TempDir(), "absent.json")}, + }) + + env, err := Prepare(PrepareParams{ + WorkspacesRoot: wsRoot, + WorkspaceID: "ws-1", + TaskID: "11111111-2222-3333-4444-555555555555", + AgentName: "scout", + Provider: "openclaw", + OpenclawBin: stub.bin, + Task: TaskContextForEnv{ + IssueID: "issue-1", + }, + }, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err != nil { + t.Fatalf("Prepare: %v", err) + } + if env.OpenclawConfigPath == "" { + t.Fatal("Prepare(openclaw) did not set OpenclawConfigPath") + } + got := mustReadJSON(t, env.OpenclawConfigPath) + workspace := got["agents"].(map[string]any)["defaults"].(map[string]any)["workspace"] + if workspace != env.WorkDir { + t.Errorf("agents.defaults.workspace = %v, want %q", workspace, env.WorkDir) + } + // Fresh install path emits no $include, so the Environment should + // leave OpenclawIncludeRoot empty — the daemon must NOT spuriously + // grant include roots when no cross-dir hop is being made. + if env.OpenclawIncludeRoot != "" { + t.Errorf("OpenclawIncludeRoot = %q on fresh install, want empty", env.OpenclawIncludeRoot) + } +} + +// TestPrepareEnvironmentOpenclawWiresIncludeRoot — when the user has an +// on-disk active config (the common non-fresh-install case), Prepare must +// surface the active config's dirname on the Environment so the daemon +// can export OPENCLAW_INCLUDE_ROOTS. Without this, the wrapper's +// $include into ~/.openclaw/openclaw.json is rejected at runtime. +func TestPrepareEnvironmentOpenclawWiresIncludeRoot(t *testing.T) { + wsRoot := t.TempDir() + + userCfgDir := t.TempDir() + userCfgPath := filepath.Join(userCfgDir, "openclaw.json") + if err := os.WriteFile(userCfgPath, []byte(`{}`), 0o600); err != nil { + t.Fatalf("write user cfg: %v", err) + } + stub := installOpenclawStub(t, map[string]openclawResponse{ + "config file": {stdout: userCfgPath}, + "config get agents.list --json": {stdout: "null"}, + }) + + env, err := Prepare(PrepareParams{ + WorkspacesRoot: wsRoot, + WorkspaceID: "ws-1", + TaskID: "33333333-2222-3333-4444-555555555555", + AgentName: "scout", + Provider: "openclaw", + OpenclawBin: stub.bin, + Task: TaskContextForEnv{IssueID: "issue-1"}, + }, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err != nil { + t.Fatalf("Prepare: %v", err) + } + if env.OpenclawIncludeRoot != userCfgDir { + t.Errorf("OpenclawIncludeRoot = %q, want %q (dirname of active config so daemon can grant OPENCLAW_INCLUDE_ROOTS)", env.OpenclawIncludeRoot, userCfgDir) + } +} + +// TestPrepareEnvironmentOpenclawFailsClosed — when the openclaw CLI errors +// during Prepare, the whole call must fail. Previously the preparer logged +// a warning and continued with no config; we have removed that path. +func TestPrepareEnvironmentOpenclawFailsClosed(t *testing.T) { + wsRoot := t.TempDir() + + stub := installOpenclawStub(t, map[string]openclawResponse{ + "config file": {err: errors.New("openclaw config validation failed")}, + }) + + _, err := Prepare(PrepareParams{ + WorkspacesRoot: wsRoot, + WorkspaceID: "ws-1", + TaskID: "22222222-2222-3333-4444-555555555555", + AgentName: "scout", + Provider: "openclaw", + OpenclawBin: stub.bin, + Task: TaskContextForEnv{IssueID: "issue-1"}, + }, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err == nil { + t.Fatal("Prepare(openclaw) succeeded when CLI errored; expected fail closed") + } + if !strings.Contains(err.Error(), "prepare openclaw config") { + t.Errorf("error message %q does not name the openclaw config step", err.Error()) + } +} + +// TestPrepareEnvironmentNonOpenclawSkipsConfig — non-openclaw providers +// must not get a synthesized openclaw config (it would be dead weight on +// disk and confuse the GC reaper's idea of what an env contains). They +// also must NOT shell out to the openclaw CLI, so the stub here records +// zero calls. +func TestPrepareEnvironmentNonOpenclawSkipsConfig(t *testing.T) { + wsRoot := t.TempDir() + + stub := installOpenclawStub(t, map[string]openclawResponse{}) + + taskIDs := map[string]string{ + "claude": "aaaaaaaa-1111-2222-3333-444444444444", + "opencode": "bbbbbbbb-1111-2222-3333-444444444444", + "hermes": "cccccccc-1111-2222-3333-444444444444", + "kiro": "dddddddd-1111-2222-3333-444444444444", + } + for provider, taskID := range taskIDs { + t.Run(provider, func(t *testing.T) { + env, err := Prepare(PrepareParams{ + WorkspacesRoot: wsRoot, + WorkspaceID: "ws-1", + TaskID: taskID, + AgentName: "scout", + Provider: provider, + Task: TaskContextForEnv{IssueID: "issue-1"}, + }, slog.New(slog.NewTextHandler(io.Discard, nil))) + if err != nil { + t.Fatalf("Prepare(%s): %v", provider, err) + } + if env.OpenclawConfigPath != "" { + t.Errorf("provider %s should not get an OpenclawConfigPath, got %q", provider, env.OpenclawConfigPath) + } + if _, err := os.Stat(filepath.Join(env.RootDir, openclawConfigFile)); !os.IsNotExist(err) { + t.Errorf("provider %s left a stray openclaw-config.json", provider) + } + }) + } + if len(stub.calls) != 0 { + t.Errorf("non-openclaw providers shelled out to openclaw CLI %d times: %+v", len(stub.calls), stub.calls) + } +} diff --git a/server/internal/daemon/execenv/runtime_config.go b/server/internal/daemon/execenv/runtime_config.go index 5c60e509b3..5365e392c4 100644 --- a/server/internal/daemon/execenv/runtime_config.go +++ b/server/internal/daemon/execenv/runtime_config.go @@ -56,7 +56,7 @@ func formatProjectResource(r ProjectResourceForEnv) string { // For Codex: writes {workDir}/AGENTS.md (skills discovered natively via CODEX_HOME) // For Copilot: writes {workDir}/AGENTS.md (skills discovered natively from .github/skills/) // For OpenCode: writes {workDir}/AGENTS.md (skills discovered natively from .opencode/skills/) -// For OpenClaw: writes {workDir}/AGENTS.md (skills fall back to .agent_context/skills/; OpenClaw does not auto-discover from workdir) +// For OpenClaw: writes {workDir}/AGENTS.md (skills discovered natively from {workDir}/skills/ via per-task openclaw-config.json that pins agents.defaults.workspace) // For Hermes: writes {workDir}/AGENTS.md (skills fall back to .agent_context/skills/; AGENTS.md points there) // For Gemini: writes {workDir}/GEMINI.md (discovered natively by the Gemini CLI) // For Pi: writes {workDir}/AGENTS.md (skills discovered natively from .pi/skills/) @@ -305,15 +305,17 @@ func buildMetaSkillContent(provider string, ctx TaskContextForEnv) string { case "claude": // Claude discovers skills natively from .claude/skills/ — just list names. b.WriteString("You have the following skills installed (discovered automatically):\n\n") - case "codex", "copilot", "opencode", "pi", "cursor", "kimi", "kiro": - // Codex, Copilot, OpenCode, Pi, Cursor, Kimi, and Kiro discover skills natively from their respective paths — just list names. + case "codex", "copilot", "opencode", "openclaw", "pi", "cursor", "kimi", "kiro": + // Codex, Copilot, OpenCode, OpenClaw, Pi, Cursor, Kimi, and Kiro discover skills + // natively from their respective paths. For OpenClaw, the daemon also writes a + // per-task openclaw-config.json (exported via OPENCLAW_CONFIG_PATH) that pins + // agents.defaults.workspace to the task workdir so the CLI's scanner picks up + // {workDir}/skills/. b.WriteString("You have the following skills installed (discovered automatically):\n\n") - case "gemini", "hermes", "openclaw": + case "gemini", "hermes": // Gemini reads GEMINI.md directly. Hermes has no native skills discovery - // path wired up in resolveSkillsDir. OpenClaw scans its own workspaceDir - // (resolved from the openclaw config, not the task workdir) so per-task - // skills written by Multica are not visible to its native loader; we - // fall back to .agent_context/skills/ and reference the files explicitly. + // path wired up in resolveSkillsDir; both fall back to referencing the + // files explicitly under .agent_context/skills/. b.WriteString("Detailed skill instructions are in `.agent_context/skills/`. Each subdirectory contains a `SKILL.md`.\n\n") default: b.WriteString("Detailed skill instructions are in `.agent_context/skills/`. Each subdirectory contains a `SKILL.md`.\n\n")