diff --git a/server/internal/daemon/execenv/codex_home.go b/server/internal/daemon/execenv/codex_home.go index 6df7591f7..46b695a4f 100644 --- a/server/internal/daemon/execenv/codex_home.go +++ b/server/internal/daemon/execenv/codex_home.go @@ -89,6 +89,16 @@ func prepareCodexHomeWithOpts(codexHome string, opts CodexHomeOptions, logger *s } } + // Drop `[[skills.config]]` entries inherited from the user's + // ~/.codex/config.toml. Codex Desktop writes plugin-backed skills with a + // `name` and no `path`, which the CLI's stricter TOML parser rejects with + // `missing field path` and bails out of `thread/start`. Multica writes the + // agent's active skills directly to `codex-home/skills/`, so the + // user-level registry is redundant here. See codex_skill_strip.go. + if err := sanitizeCopiedCodexConfig(filepath.Join(codexHome, "config.toml")); err != nil { + logger.Warn("execenv: codex-home sanitize config failed", "error", err) + } + if err := exposeSharedCodexPluginCache(codexHome, sharedHome); err != nil { logger.Warn("execenv: codex-home plugin cache exposure failed", "error", err) } diff --git a/server/internal/daemon/execenv/codex_skill_strip.go b/server/internal/daemon/execenv/codex_skill_strip.go new file mode 100644 index 000000000..6447a28fa --- /dev/null +++ b/server/internal/daemon/execenv/codex_skill_strip.go @@ -0,0 +1,87 @@ +package execenv + +import ( + "fmt" + "os" + "strings" +) + +// stripSkillsConfigEntries removes every `[[skills.config]]` array-of-tables +// block from the given config.toml content. +// +// Background: Codex Desktop writes one `[[skills.config]]` entry per skill it +// knows about — file-backed skills get a `path = "..."` field, while +// plugin-backed skills (e.g. `name = "superpowers:brainstorming"`) only get a +// `name`. Codex CLI 0.114's TOML deserializer treats `path` as a required +// field, so it rejects the plugin entries with `missing field path` and +// refuses to start. Multica copies the user's `~/.codex/config.toml` verbatim +// into each task's isolated codex-home, which propagates the broken entries +// into the per-task config and blocks `codex thread/start`. +// +// Stripping the whole `[[skills.config]]` array sidesteps the issue: Multica +// writes the agent's currently assigned skills directly to +// `codex-home/skills//SKILL.md`, and Codex auto-discovers them from +// that directory. The user-level skill registry is irrelevant to a per-task +// run, so dropping it is both safe and the right scope of isolation. +// +// Lines outside `[[skills.config]]` blocks are preserved untouched. +func stripSkillsConfigEntries(content string) string { + if !strings.Contains(content, "[[skills.config]]") { + return content + } + + lines := strings.Split(content, "\n") + out := make([]string, 0, len(lines)) + inSkillsConfig := false + for _, line := range lines { + trimmed := strings.TrimSpace(line) + + // A new TOML header always closes the current `[[skills.config]]` + // block, regardless of whether it's another entry of the same array + // or a different table. + if strings.HasPrefix(trimmed, "[") { + if trimmed == "[[skills.config]]" { + inSkillsConfig = true + continue + } + inSkillsConfig = false + out = append(out, line) + continue + } + + if inSkillsConfig { + continue + } + out = append(out, line) + } + + stripped := strings.Join(out, "\n") + // Collapse the trailing blank-line cluster that the removal can leave + // behind so repeated copies don't grow the file unboundedly. + stripped = strings.TrimRight(stripped, "\n") + "\n" + if strings.TrimSpace(stripped) == "" { + return "" + } + return stripped +} + +// sanitizeCopiedCodexConfig rewrites the per-task config.toml in place, +// dropping `[[skills.config]]` entries inherited from the shared +// `~/.codex/config.toml`. No-op if the file doesn't exist or doesn't change. +func sanitizeCopiedCodexConfig(configPath string) error { + data, err := os.ReadFile(configPath) + if err != nil { + if os.IsNotExist(err) { + return nil + } + return fmt.Errorf("read config.toml: %w", err) + } + stripped := stripSkillsConfigEntries(string(data)) + if stripped == string(data) { + return nil + } + if err := os.WriteFile(configPath, []byte(stripped), 0o644); err != nil { + return fmt.Errorf("write config.toml: %w", err) + } + return nil +} diff --git a/server/internal/daemon/execenv/codex_skill_strip_test.go b/server/internal/daemon/execenv/codex_skill_strip_test.go new file mode 100644 index 000000000..a1be1afbe --- /dev/null +++ b/server/internal/daemon/execenv/codex_skill_strip_test.go @@ -0,0 +1,199 @@ +package execenv + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestStripSkillsConfigEntries(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + in string + want string + }{ + { + name: "no skills config — returned unchanged", + in: "model = \"o3\"\n", + want: "model = \"o3\"\n", + }, + { + name: "drops well-formed file-backed entry", + in: `model = "o3" + +[[skills.config]] +path = "/Users/x/SKILL.md" +enabled = true +`, + want: `model = "o3" +`, + }, + { + name: "drops plugin entry that lacks path", + in: `[[skills.config]] +name = "superpowers:brainstorming" +enabled = false + +[profiles.default] +model = "o3" +`, + want: `[profiles.default] +model = "o3" +`, + }, + { + name: "drops a mix of consecutive entries and preserves surrounding tables", + in: `model = "o3" + +[[skills.config]] +path = "/Users/x/SKILL.md" +enabled = false + +[[skills.config]] +path = "/Users/y/SKILL.md" +enabled = false + +[[skills.config]] +name = "superpowers:brainstorming" +enabled = false + +[profiles.default] +model = "o3" + +[mcp_servers.foo] +command = "foo" +`, + want: `model = "o3" + +[profiles.default] +model = "o3" + +[mcp_servers.foo] +command = "foo" +`, + }, + { + name: "skills.config at EOF", + in: `model = "o3" + +[[skills.config]] +name = "superpowers:dispatching-parallel-agents" +enabled = false +`, + want: `model = "o3" +`, + }, + { + name: "preserves unrelated [skills] table (single brackets)", + in: `[skills] +discovery_path = "skills" +`, + want: `[skills] +discovery_path = "skills" +`, + }, + { + name: "fully empty after strip returns empty string", + in: `[[skills.config]] +name = "x" +enabled = false +`, + want: ``, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got := stripSkillsConfigEntries(tt.in) + if got != tt.want { + t.Errorf("stripSkillsConfigEntries result mismatch\n--- got ---\n%s\n--- want ---\n%s", got, tt.want) + } + }) + } +} + +func TestSanitizeCopiedCodexConfig(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, "config.toml") + original := `model = "o3" + +[[skills.config]] +name = "superpowers:brainstorming" +enabled = false + +[[skills.config]] +path = "/Users/x/SKILL.md" +enabled = true + +[profiles.default] +model = "o3" +` + if err := os.WriteFile(configPath, []byte(original), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + + if err := sanitizeCopiedCodexConfig(configPath); err != nil { + t.Fatalf("sanitizeCopiedCodexConfig failed: %v", err) + } + + data, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("read result: %v", err) + } + got := string(data) + if strings.Contains(got, "[[skills.config]]") { + t.Errorf("expected all [[skills.config]] entries to be removed, got:\n%s", got) + } + if !strings.Contains(got, `[profiles.default]`) { + t.Errorf("unrelated tables should be preserved, got:\n%s", got) + } + if !strings.Contains(got, `model = "o3"`) { + t.Errorf("top-level keys should be preserved, got:\n%s", got) + } +} + +func TestSanitizeCopiedCodexConfigNoop(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + configPath := filepath.Join(dir, "config.toml") + original := "model = \"o3\"\n" + if err := os.WriteFile(configPath, []byte(original), 0o644); err != nil { + t.Fatalf("write fixture: %v", err) + } + infoBefore, err := os.Stat(configPath) + if err != nil { + t.Fatalf("stat before: %v", err) + } + + if err := sanitizeCopiedCodexConfig(configPath); err != nil { + t.Fatalf("sanitizeCopiedCodexConfig failed: %v", err) + } + + infoAfter, err := os.Stat(configPath) + if err != nil { + t.Fatalf("stat after: %v", err) + } + if !infoAfter.ModTime().Equal(infoBefore.ModTime()) { + t.Errorf("file should not be rewritten when there is nothing to strip") + } + data, _ := os.ReadFile(configPath) + if string(data) != original { + t.Errorf("content drifted: got %q, want %q", data, original) + } +} + +func TestSanitizeCopiedCodexConfigMissingFile(t *testing.T) { + t.Parallel() + + missing := filepath.Join(t.TempDir(), "does-not-exist.toml") + if err := sanitizeCopiedCodexConfig(missing); err != nil { + t.Errorf("missing file should be a no-op, got error: %v", err) + } +} diff --git a/server/internal/daemon/execenv/execenv_test.go b/server/internal/daemon/execenv/execenv_test.go index e628be12b..5f7953739 100644 --- a/server/internal/daemon/execenv/execenv_test.go +++ b/server/internal/daemon/execenv/execenv_test.go @@ -1125,6 +1125,57 @@ func TestPrepareCodexHomeSeedsFromShared(t *testing.T) { } } +// Regression test for #1753 — Codex Desktop writes plugin-backed +// `[[skills.config]]` entries without a `path` field, and the CLI's TOML +// parser rejects them with `missing field path`. prepareCodexHome must drop +// every `[[skills.config]]` entry while copying the user's config.toml so +// the per-task home stays parseable. +func TestPrepareCodexHomeStripsSkillsConfigEntries(t *testing.T) { + // Cannot use t.Parallel() with t.Setenv. + + sharedHome := t.TempDir() + sharedConfig := `model = "o3" + +[[skills.config]] +path = "/Users/x/SKILL.md" +enabled = false + +[[skills.config]] +name = "superpowers:brainstorming" +enabled = false + +[profiles.default] +model = "o3" +` + if err := os.WriteFile(filepath.Join(sharedHome, "config.toml"), []byte(sharedConfig), 0o644); err != nil { + t.Fatalf("write shared config.toml: %v", err) + } + t.Setenv("CODEX_HOME", sharedHome) + + codexHome := filepath.Join(t.TempDir(), "codex-home") + if err := prepareCodexHome(codexHome, testLogger()); err != nil { + t.Fatalf("prepareCodexHome failed: %v", err) + } + + data, err := os.ReadFile(filepath.Join(codexHome, "config.toml")) + if err != nil { + t.Fatalf("read per-task config.toml: %v", err) + } + tomlStr := string(data) + if strings.Contains(tomlStr, "[[skills.config]]") { + t.Errorf("per-task config.toml should not inherit [[skills.config]] entries, got:\n%s", tomlStr) + } + if strings.Contains(tomlStr, "superpowers:brainstorming") { + t.Errorf("per-task config.toml should not retain plugin skill names, got:\n%s", tomlStr) + } + if !strings.Contains(tomlStr, `model = "o3"`) { + t.Errorf("top-level keys should be preserved, got:\n%s", tomlStr) + } + if !strings.Contains(tomlStr, "[profiles.default]") { + t.Errorf("unrelated tables should be preserved, got:\n%s", tomlStr) + } +} + func TestPrepareCodexHomeSkipsMissingFiles(t *testing.T) { // Cannot use t.Parallel() with t.Setenv.