Let review profiles carry each reviewer's own agent config

cmd/entire/cli/agent/claudecode/review_config.go

@@ -0,0 +1,129 @@
+package claudecode
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"path/filepath"
+	"strings"
+
+	"github.com/entireio/cli/cmd/entire/cli/jsonutil"
+	"github.com/entireio/cli/cmd/entire/cli/review"
+	reviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types"
+)
+
+// reviewPluginName namespaces the checkout's skills and commands, which are
+// loaded as a plugin when a profile config replaces the checkout's settings.
+const reviewPluginName = "project"
+
+// prepareReviewAgentConfig applies a review profile's agent config: the
+// checkout's settings and MCP servers are not loaded (--setting-sources user,
+// --strict-mcp-config); the profile's settings, with Entire's own hooks added
+// so the session is still tracked, and MCP servers are used instead. The
+// checkout's skills and commands are loaded as the "project" plugin, copied
+// from its committed tree.
+func prepareReviewAgentConfig(ctx context.Context, cfg reviewtypes.RunConfig) (reviewtypes.RunConfig, func(), error) {
+	if cfg.AgentConfig == nil {
+		return cfg, nil, nil
+	}
+	run, err := review.NewAgentConfigRun(ctx)
+	if err != nil {
+		return cfg, nil, err //nolint:wrapcheck // already names the step
+	}
+	fail := func(err error) (reviewtypes.RunConfig, func(), error) {
+		run.Cleanup()
+		return cfg, nil, err
+	}
+	if err := review.ValidateAgentConfig("claude-code", cfg.AgentConfig, run.ForbiddenRoots); err != nil {
+		return fail(fmt.Errorf("review profile config: %w", err))
+	}
+	settingsJSON, err := reviewProfileSettings(cfg.AgentConfig.Settings)
+	if err != nil {
+		return fail(err)
+	}
+	settingsPath, err := run.WriteFile("claude/settings.json", settingsJSON)
+	if err != nil {
+		return fail(err)
+	}
+	servers := cfg.AgentConfig.MCPServers
+	if servers == nil {
+		servers = map[string]json.RawMessage{}
+	}
+	mcpJSON, err := jsonutil.MarshalWithNoHTMLEscape(map[string]any{"mcpServers": servers})
+	if err != nil {
+		return fail(fmt.Errorf("encode MCP config: %w", err))
+	}
+	mcpPath, err := run.WriteFile("claude/mcp.json", mcpJSON)
+	if err != nil {
+		return fail(err)
+	}
+	cfg.ExtraArgs = append(cfg.ExtraArgs, flagSettingSources, "user", "--settings", settingsPath, "--strict-mcp-config", "--mcp-config", mcpPath)
+
+	copied, err := run.CopyCheckoutTree(ctx, map[string]string{
+		".claude/skills":   "claude/plugin/skills",
+		".claude/commands": "claude/plugin/commands",
+	})
+	if err != nil {
+		return fail(fmt.Errorf("load the checkout's skills: %w", err))
+	}
+	if copied > 0 {
+		manifest := fmt.Sprintf(`{"name":%q,"version":"0.0.0","description":"Skills and commands from the reviewed checkout"}`, reviewPluginName)
+		manifestPath, err := run.WriteFile("claude/plugin/.claude-plugin/plugin.json", []byte(manifest))
+		if err != nil {
+			return fail(err)
+		}
+		cfg.ExtraArgs = append(cfg.ExtraArgs, "--plugin-dir", filepath.Dir(filepath.Dir(manifestPath)))
+		cfg.Skills = namespaceProjectSkills(ctx, run, cfg.Skills)
+	}
+	return cfg, run.Cleanup, nil
+}
+
+// reviewProfileSettings returns the profile's settings object with Entire's
+// own hooks added, so the review session is tracked even though the
+// checkout's settings (where Entire's hooks normally live) are not loaded.
+func reviewProfileSettings(profile json.RawMessage) ([]byte, error) {
+	obj := map[string]json.RawMessage{}
+	if len(profile) > 0 {
+		if err := json.Unmarshal(profile, &obj); err != nil {
+			return nil, fmt.Errorf("review profile settings: %w", err)
+		}
+	}
+	rawHooks := map[string]json.RawMessage{}
+	if existing, ok := obj["hooks"]; ok {
+		if err := json.Unmarshal(existing, &rawHooks); err != nil {
+			return nil, fmt.Errorf("review profile settings hooks: %w", err)
+		}
+	}
+	installHookEntries(rawHooks, false)
+	hooks, err := jsonutil.MarshalWithNoHTMLEscape(rawHooks)
+	if err != nil {
+		return nil, fmt.Errorf("encode hooks: %w", err)
+	}
+	obj["hooks"] = hooks
+	out, err := jsonutil.MarshalWithNoHTMLEscape(obj)
+	if err != nil {
+		return nil, fmt.Errorf("encode settings: %w", err)
+	}
+	return out, nil
+}
+
+// namespaceProjectSkills rewrites "/name" to "/project:name" for skills and
+// commands that came from the checkout, since plugin skills are invoked with
+// the plugin's prefix.
+func namespaceProjectSkills(ctx context.Context, run *review.AgentConfigRun, skills []string) []string {
+	names := map[string]bool{}
+	for _, dir := range []string{".claude/skills", ".claude/commands"} {
+		for _, name := range run.TreeEntryNames(ctx, dir) {
+			names[strings.TrimSuffix(name, ".md")] = true
+		}
+	}
+	out := make([]string, 0, len(skills))
+	for _, skill := range skills {
+		if rest, ok := strings.CutPrefix(skill, "/"); ok && names[rest] {
+			out = append(out, "/"+reviewPluginName+":"+rest)
+			continue
+		}
+		out = append(out, skill)
+	}
+	return out
+}

cmd/entire/cli/agent/claudecode/review_config_test.go

@@ -0,0 +1,132 @@
+package claudecode
+
+import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"slices"
+	"strings"
+	"testing"
+
+	"github.com/entireio/cli/cmd/entire/cli/paths"
+	reviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types"
+	"github.com/entireio/cli/cmd/entire/cli/testutil"
+)
+
+func newReviewConfigRepo(t *testing.T, files map[string]string) string {
+	t.Helper()
+	dir := t.TempDir()
+	testutil.InitRepo(t, dir)
+	names := []string{}
+	for name, content := range files {
+		testutil.WriteFile(t, dir, name, content)
+		names = append(names, name)
+	}
+	testutil.WriteFile(t, dir, "README.md", "x")
+	testutil.GitAdd(t, dir, append(names, "README.md")...)
+	testutil.GitCommit(t, dir, "init")
+	t.Chdir(dir)
+	paths.ClearWorktreeRootCache()
+	t.Cleanup(paths.ClearWorktreeRootCache)
+	return dir
+}
+
+func argAfter(args []string, flag string) string {
+	if i := slices.Index(args, flag); i >= 0 && i+1 < len(args) {
+		return args[i+1]
+	}
+	return ""
+}
+
+// A profile config replaces the checkout's settings and MCP servers, keeps
+// Entire's tracking hooks, and loads the checkout's skills as a plugin.
+func TestPrepareReviewAgentConfig(t *testing.T) {
+	newReviewConfigRepo(t, map[string]string{
+		".claude/skills/review/SKILL.md": "---\nname: review\ndescription: d\n---\nReview.",
+		".claude/commands/check.md":      "Check.",
+	})
+	cfg := reviewtypes.RunConfig{
+		Skills: []string{"/review", "/other"},
+		AgentConfig: &reviewtypes.AgentConfig{
+			Settings:   json.RawMessage(`{"hooks":{"Stop":[{"hooks":[{"type":"command","command":"/usr/bin/true"}]}]}}`),
+			MCPServers: map[string]json.RawMessage{"docs": json.RawMessage(`{"url":"https://mcp.example"}`)},
+		},
+	}
+	got, cleanup, err := prepareReviewAgentConfig(t.Context(), cfg)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, flag := range []string{"--strict-mcp-config", flagSettingSources} {
+		if !slices.Contains(got.ExtraArgs, flag) {
+			t.Errorf("ExtraArgs %v missing %s", got.ExtraArgs, flag)
+		}
+	}
+	if argAfter(got.ExtraArgs, flagSettingSources) != "user" {
+		t.Errorf("--setting-sources = %q, want user", argAfter(got.ExtraArgs, flagSettingSources))
+	}
+	settingsData, err := os.ReadFile(argAfter(got.ExtraArgs, "--settings"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, want := range []string{"/usr/bin/true", "entire hooks claude-code stop"} {
+		if !strings.Contains(string(settingsData), want) {
+			t.Errorf("settings missing %q: %s", want, settingsData)
+		}
+	}
+	mcpData, err := os.ReadFile(argAfter(got.ExtraArgs, "--mcp-config"))
+	if err != nil || !strings.Contains(string(mcpData), "mcp.example") {
+		t.Errorf("mcp config = %s (%v)", mcpData, err)
+	}
+	pluginDir := argAfter(got.ExtraArgs, "--plugin-dir")
+	for _, rel := range []string{".claude-plugin/plugin.json", "skills/review/SKILL.md", "commands/check.md"} {
+		if _, err := os.Stat(filepath.Join(pluginDir, rel)); err != nil {
+			t.Errorf("plugin missing %s: %v", rel, err)
+		}
+	}
+	if !slices.Equal(got.Skills, []string{"/project:review", "/other"}) {
+		t.Errorf("Skills = %v, want the checkout's skill namespaced", got.Skills)
+	}
+	cleanup()
+	if _, err := os.Stat(pluginDir); !os.IsNotExist(err) {
+		t.Errorf("cleanup left %s behind", pluginDir)
+	}
+}
+
+// Without a profile config nothing changes.
+func TestPrepareReviewAgentConfigNoConfig(t *testing.T) {
+	t.Parallel()
+	cfg := reviewtypes.RunConfig{Skills: []string{"/review"}}
+	got, cleanup, err := prepareReviewAgentConfig(t.Context(), cfg)
+	if err != nil || cleanup != nil || len(got.ExtraArgs) != 0 {
+		t.Fatalf("got %+v, cleanup %v, err %v", got, cleanup != nil, err)
+	}
+}
+
+// A symlinked skill is refused rather than followed.
+func TestPrepareReviewAgentConfigRefusesSymlinkedSkill(t *testing.T) {
+	dir := newReviewConfigRepo(t, map[string]string{"elsewhere.md": "x"})
+	if err := os.MkdirAll(filepath.Join(dir, ".claude", "skills"), 0o750); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink("../../elsewhere.md", filepath.Join(dir, ".claude", "skills", "link.md")); err != nil {
+		t.Fatal(err)
+	}
+	testutil.GitAdd(t, dir, ".claude/skills/link.md")
+	testutil.GitCommit(t, dir, "symlink")
+
+	_, _, err := prepareReviewAgentConfig(t.Context(), reviewtypes.RunConfig{AgentConfig: &reviewtypes.AgentConfig{}})
+	if err == nil || !strings.Contains(err.Error(), "symlink") {
+		t.Fatalf("err = %v, want a symlink refusal", err)
+	}
+}
+
+// A hook that runs code from the checkout is refused at run time.
+func TestPrepareReviewAgentConfigRefusesCheckoutCommand(t *testing.T) {
+	newReviewConfigRepo(t, nil)
+	cfg := reviewtypes.RunConfig{AgentConfig: &reviewtypes.AgentConfig{
+		Settings: json.RawMessage(`{"hooks":{"Stop":[{"hooks":[{"type":"command","command":"./hook.sh"}]}]}}`),
+	}}
+	if _, _, err := prepareReviewAgentConfig(t.Context(), cfg); err == nil || !strings.Contains(err.Error(), "relative path") {
+		t.Fatalf("err = %v, want a relative-path refusal", err)
+	}
+}

cmd/entire/cli/agent/claudecode/reviewer.go

@@ -31,6 +31,7 @@ func NewReviewer() *reviewtypes.ReviewerTemplate {
 		AgentName: "claude-code",
 		BuildCmd:  buildReviewCmd,
 		Parser:    parseClaudeOutput,
+		Prepare:   prepareReviewAgentConfig,
 	}
 }
 
@@ -39,6 +40,7 @@ func NewReviewer() *reviewtypes.ReviewerTemplate {
 func buildReviewCmd(ctx context.Context, cfg reviewtypes.RunConfig) *exec.Cmd {
 	prompt := review.ComposeReviewPrompt(cfg)
 	args := []string{"-p", prompt, flagOutputFormat, "stream-json", "--verbose", "--append-system-prompt", review.ReviewerGuardrail}
+	args = append(args, cfg.ExtraArgs...)
 	args = review.AppendModelFlag(args, cfg.Model)
 	cmd := exec.CommandContext(ctx, "claude", args...)
 	cmd.Env = review.AppendReviewEnv(os.Environ(), "claude-code", cfg, prompt)

cmd/entire/cli/agent/codex/review_config.go

@@ -0,0 +1,109 @@
+package codex
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"slices"
+	"strings"
+
+	"github.com/entireio/cli/cmd/entire/cli/review"
+	reviewtypes "github.com/entireio/cli/cmd/entire/cli/review/types"
+)
+
+// prepareCodexReviewConfig applies a review profile's agent config: the
+// reviewed checkout, and the main repository a linked worktree shares trust
+// with, are marked untrusted for this run, which drops their project config,
+// hooks and rules; the profile's MCP servers are passed with -c.
+func prepareCodexReviewConfig(ctx context.Context, cfg reviewtypes.RunConfig) (reviewtypes.RunConfig, func(), error) {
+	if cfg.AgentConfig == nil {
+		return cfg, nil, nil
+	}
+	run, err := review.NewAgentConfigRun(ctx)
+	if err != nil {
+		return cfg, nil, err //nolint:wrapcheck // already names the step
+	}
+	// Nothing is written for Codex; the run only resolves paths.
+	defer run.Cleanup()
+	if err := review.ValidateAgentConfig("codex", cfg.AgentConfig, run.ForbiddenRoots); err != nil {
+		return cfg, nil, fmt.Errorf("review profile config: %w", err)
+	}
+	mainRoot, err := run.MainRepoRoot(ctx)
+	if err != nil {
+		return cfg, nil, err //nolint:wrapcheck // already names the step
+	}
+	roots := []string{run.CheckoutRoot}
+	if mainRoot != run.CheckoutRoot {
+		roots = append(roots, mainRoot)
+	}
+	cfg.ExtraArgs = append(cfg.ExtraArgs, "-c", untrustedProjectOverride(roots))
+	servers, err := codexMCPOverrides(cfg.AgentConfig.MCPServers)
+	if err != nil {
+		return cfg, nil, err
+	}
+	cfg.ExtraArgs = append(cfg.ExtraArgs, servers...)
+	cfg.WorkDir = run.CheckoutRoot
+	return cfg, nil, nil
+}
+
+// untrustedProjectOverride marks roots untrusted for one codex run. It is one
+// override: each -c projects=... replaces the whole table, so a second would
+// drop the first.
+func untrustedProjectOverride(roots []string) string {
+	entries := make([]string, 0, len(roots))
+	for _, root := range roots {
+		quoted, err := json.Marshal(root)
+		if err != nil {
+			quoted = []byte(`""`)
+		}
+		entries = append(entries, string(quoted)+`={trust_level="untrusted"}`)
+	}
+	return "projects={" + strings.Join(entries, ",") + "}"
+}
+
+// codexMCPOverrides turns profile MCP servers into -c overrides. Values are
+// JSON-encoded, which is valid TOML. Literal env values are refused: they
+// would be visible in the process list.
+func codexMCPOverrides(servers map[string]json.RawMessage) ([]string, error) {
+	names := make([]string, 0, len(servers))
+	for name := range servers {
+		names = append(names, name)
+	}
+	slices.Sort(names)
+	var args []string
+	for _, name := range names {
+		var server struct {
+			Command string            `json:"command"`
+			Args    []string          `json:"args"`
+			URL     string            `json:"url"`
+			Env     map[string]string `json:"env"`
+		}
+		if err := json.Unmarshal(servers[name], &server); err != nil {
+			return nil, fmt.Errorf("MCP server %q: %w", name, err)
+		}
+		if len(server.Env) > 0 {
+			return nil, fmt.Errorf("MCP server %q: env values aren't supported for Codex in a review profile yet", name)
+		}
+		set := func(key string, value any) error {
+			encoded, err := json.Marshal(value)
+			if err != nil {
+				return fmt.Errorf("MCP server %q: %w", name, err)
+			}
+			args = append(args, "-c", "mcp_servers."+name+"."+key+"="+string(encoded))
+			return nil
+		}
+		var err error
+		if server.Command != "" {
+			err = set("command", server.Command)
+			if err == nil && len(server.Args) > 0 {
+				err = set("args", server.Args)
+			}
+		} else {
+			err = set("url", server.URL)
+		}
+		if err != nil {
+			return nil, err
+		}
+	}
+	return args, nil
+}