Skip to content
8 changes: 7 additions & 1 deletion cmd/roborev/analyze.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,11 +751,17 @@ func runFixAgent(cmd *cobra.Command, repoPath, agentName, model, reasoning, prom
if reasonErr != nil {
return fmt.Errorf("resolve fix reasoning: %w", reasonErr)
}
if err := config.ValidateRepoConfig(repoPath); err != nil {
return fmt.Errorf("resolve workflow config: %w", err)
}

// Resolve agent and model via fix workflow config.
resolution := agent.ResolveWorkflowConfig(
resolution, err := agent.ResolveWorkflowConfig(
agentName, repoPath, cfg, "fix", reasoning,
)
if err != nil {
return fmt.Errorf("resolve workflow config: %w", err)
}
agentName = resolution.PreferredAgent

a, err := agent.GetAvailableWithConfig(
Expand Down
3 changes: 3 additions & 0 deletions cmd/roborev/compact.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,9 @@ func enqueueConsolidation(ctx context.Context, cmd *cobra.Command, repoRoot stri
if err != nil {
return 0, fmt.Errorf("resolve reasoning: %w", err)
}
if err := config.ValidateRepoConfig(repoRoot); err != nil {
return 0, fmt.Errorf("resolve agent/model: %w", err)
}
agentName := config.ResolveAgentForWorkflow(
opts.agentName, repoRoot, cfg, "fix", reasoning,
)
Expand Down
20 changes: 16 additions & 4 deletions cmd/roborev/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,13 +303,19 @@ func fixJobDirect(ctx context.Context, params fixJobParams, prompt string) (*fix
func resolveFixModel(
selectedAgent, cliModel, repoPath string,
cfg *config.Config, reasoning string,
) string {
resolution := agent.ResolveWorkflowConfig(
) (string, error) {
if err := config.ValidateRepoConfig(repoPath); err != nil {
return "", err
}
resolution, err := agent.ResolveWorkflowConfig(
"", repoPath, cfg, "fix", reasoning,
)
if err != nil {
return "", err
}
return resolution.ModelForSelectedAgent(
selectedAgent, cliModel,
)
), nil
}

// resolveFixAgent resolves and configures the agent for fix operations.
Expand All @@ -323,10 +329,16 @@ func resolveFixAgent(repoPath string, opts fixOptions) (agent.Agent, error) {
if err != nil {
return nil, fmt.Errorf("resolve fix reasoning: %w", err)
}
if err := config.ValidateRepoConfig(repoPath); err != nil {
return nil, fmt.Errorf("resolve workflow config: %w", err)
}

resolution := agent.ResolveWorkflowConfig(
resolution, err := agent.ResolveWorkflowConfig(
opts.agentName, repoPath, cfg, "fix", reasoning,
)
if err != nil {
return nil, fmt.Errorf("resolve workflow config: %w", err)
}

a, err := agent.GetAvailableWithConfig(
resolution.PreferredAgent, cfg, resolution.BackupAgent,
Expand Down
11 changes: 8 additions & 3 deletions cmd/roborev/fix_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2481,9 +2481,10 @@ default_model = "gpt-5.4"
cfg, err := config.LoadGlobal()
require.NoError(t, err, "LoadGlobal: %v")

modelStr := resolveFixModel(
modelStr, err := resolveFixModel(
tt.agentName, tt.model, tmpDir, cfg, "fast",
)
require.NoError(t, err)

if tt.wantModel && modelStr == "" {
assert.False(t, tt.wantModel && modelStr == "", "expected model to be set, got empty")
Expand Down Expand Up @@ -2550,7 +2551,8 @@ fix_agent = "claude"
cfg, err := config.LoadGlobal()
require.NoError(t, err, "LoadGlobal: %v")

modelStr := resolveFixModel("claude-code", "", tmpDir, cfg, "standard")
modelStr, err := resolveFixModel("claude-code", "", tmpDir, cfg, "standard")
require.NoError(t, err)
assert.Empty(t, modelStr)
}

Expand Down Expand Up @@ -2676,7 +2678,10 @@ func TestResolveFixAgentSameAsDefault(t *testing.T) {
require.NoError(t, err, "LoadGlobal: %v")

// Call the production resolveFixModel function directly
modelStr := resolveFixModel(tt.cliAgent, "", tmpDir, cfg, "fast")
modelStr, err := resolveFixModel(
tt.cliAgent, "", tmpDir, cfg, "fast",
)
require.NoError(t, err)

assert.Equal(t, tt.wantModel, modelStr)
})
Expand Down
40 changes: 40 additions & 0 deletions cmd/roborev/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"io"
"net/http"
"os"
"os/exec"
"path/filepath"
"strings"
"sync/atomic"
Expand Down Expand Up @@ -248,6 +249,45 @@ func TestRunRefineFailsOnInvalidGlobalConfigBeforeDaemonStartup(t *testing.T) {
assert.Contains(t, err.Error(), "load config:")
}

func TestRunRefineFailsOnInvalidRepoConfigBeforeDaemonStartup(t *testing.T) {
repoDir, _ := setupRefineRepo(t)

err := os.WriteFile(
filepath.Join(repoDir, ".roborev.toml"),
[]byte("refine_model = ["),
0o644,
)
require.NoError(t, err, "write invalid repo config: %v", err)
gitAdd := exec.Command("git", "add", ".roborev.toml")
gitAdd.Dir = repoDir
out, err := gitAdd.CombinedOutput()
require.NoError(t, err, "git add failed: %s", out)
gitCommit := exec.Command("git", "commit", "-m", "add invalid repo config")
gitCommit.Dir = repoDir
out, err = gitCommit.CombinedOutput()
require.NoError(t, err, "git commit failed: %s", out)

origGetAnyRunningDaemon := getAnyRunningDaemon
getAnyRunningDaemon = func() (*daemon.RuntimeInfo, error) {
require.FailNow(t, "ensureDaemon should not run when repo config is invalid")
return nil, ErrDaemonNotRunning
}
t.Cleanup(func() {
getAnyRunningDaemon = origGetAnyRunningDaemon
})

ctx := newFastRunContext(repoDir)

err = runRefine(ctx, refineOptions{
agentName: "test",
reasoning: "fast",
maxIterations: 1,
quiet: true,
})
require.Error(t, err, "expected repo config error")
assert.Contains(t, err.Error(), "resolve workflow config:")
}

func TestRunRefineQuietNonTTYTimerOutput(t *testing.T) {
repoDir, headSHA := setupRefineRepo(t)

Expand Down
30 changes: 19 additions & 11 deletions cmd/roborev/refine.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,25 @@ func runRefine(ctx RunContext, opts refineOptions) error {
return fmt.Errorf("load config: %w", err)
}

// Resolve config-driven workflow settings before touching the daemon so
// malformed repo config fails during validation, not after startup.
resolvedReasoning, err := config.ResolveRefineReasoning(
opts.reasoning, repoPath, cfg,
)
if err != nil {
return err
}
reasoningLevel := agent.ParseReasoningLevel(resolvedReasoning)
if err := config.ValidateRepoConfig(repoPath); err != nil {
return fmt.Errorf("resolve workflow config: %w", err)
}
resolution, err := agent.ResolveWorkflowConfig(
opts.agentName, repoPath, cfg, "refine", resolvedReasoning,
)
if err != nil {
return fmt.Errorf("resolve workflow config: %w", err)
}

// 2. Connect to daemon (only after all validation passes)
if err := ensureDaemon(); err != nil {
return fmt.Errorf("daemon not running: %w", err)
Expand All @@ -352,17 +371,6 @@ func runRefine(ctx RunContext, opts refineOptions) error {
fmt.Printf("Refining branch %q (diverged from %s at %s)\n", currentBranch, defaultBranch, git.ShortSHA(mergeBase))
}

// Resolve reasoning level from CLI or config (default: standard for refine)
resolvedReasoning, err := config.ResolveRefineReasoning(opts.reasoning, repoPath, cfg)
if err != nil {
return err
}
reasoningLevel := agent.ParseReasoningLevel(resolvedReasoning)

// Resolve agent/model preferences for refine at this reasoning level.
resolution := agent.ResolveWorkflowConfig(
opts.agentName, repoPath, cfg, "refine", resolvedReasoning,
)
allowUnsafe := resolveAllowUnsafeAgents(opts.allowUnsafeAgents, opts.unsafeFlagChanged, cfg)
agent.SetAllowUnsafeAgents(allowUnsafe)
if cfg != nil {
Expand Down
8 changes: 7 additions & 1 deletion cmd/roborev/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,11 +383,17 @@ func runLocalReview(cmd *cobra.Command, repoPath, gitRef, diffContent, agentName
if !config.IsDefaultReviewType(reviewType) {
workflow = reviewType
}
if err := config.ValidateRepoConfig(repoPath); err != nil {
return fmt.Errorf("resolve workflow config: %w", err)
}

// Resolve agent/model preferences (matches daemon behavior).
resolution := agent.ResolveWorkflowConfig(
resolution, err := agent.ResolveWorkflowConfig(
agentName, repoPath, cfg, workflow, reasoning,
)
if err != nil {
return fmt.Errorf("resolve workflow config: %w", err)
}

// Get the agent (try backup before hardcoded chain)
a, err := agent.GetAvailableWithConfig(
Expand Down
8 changes: 6 additions & 2 deletions internal/agent/model_resolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,23 @@ type WorkflowConfig struct {
// ResolveWorkflowConfig resolves the preferred and backup agents for a
// workflow while retaining the workflow and reasoning context needed to
// resolve the final model after an agent has been selected.
//
// This helper intentionally does not validate repo config. Callers that
// must fail fast on malformed .roborev.toml should call
// config.ValidateRepoConfig before invoking it.
func ResolveWorkflowConfig(
cliAgent, repoPath string,
globalCfg *config.Config,
workflow, reasoning string,
) WorkflowConfig {
) (WorkflowConfig, error) {
return WorkflowConfig{
RepoPath: repoPath,
GlobalConfig: globalCfg,
Workflow: workflow,
Reasoning: reasoning,
PreferredAgent: config.ResolveAgentForWorkflow(cliAgent, repoPath, globalCfg, workflow, reasoning),
BackupAgent: config.ResolveBackupAgentForWorkflow(repoPath, globalCfg, workflow),
}
}, nil
}

// AgentMatches reports whether two agent names refer to the same logical
Expand Down
28 changes: 26 additions & 2 deletions internal/agent/model_resolution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,10 @@ func TestResolveWorkflowConfigModelForSelectedAgent_UsesBackupModelForAliasMatch
ReviewBackupModel: "claude-sonnet",
}

resolution := ResolveWorkflowConfig("", t.TempDir(), cfg, "review", "standard")
resolution, err := ResolveWorkflowConfig(
"", t.TempDir(), cfg, "review", "standard",
)
require.NoError(t, err)

require.Equal(t, "gemini", resolution.PreferredAgent)
require.Equal(t, "claude", resolution.BackupAgent)
Expand All @@ -299,7 +302,28 @@ func TestResolveWorkflowConfigModelForSelectedAgent_BackupWithoutModelKeepsDefau
ReviewBackupAgent: "claude",
}

resolution := ResolveWorkflowConfig("", t.TempDir(), cfg, "review", "standard")
resolution, err := ResolveWorkflowConfig(
"", t.TempDir(), cfg, "review", "standard",
)
require.NoError(t, err)

require.Empty(t, resolution.ModelForSelectedAgent("claude-code", ""))
}

func TestResolveWorkflowConfigIgnoresMalformedRepoConfig(t *testing.T) {
t.Parallel()

repoPath := t.TempDir()
err := os.WriteFile(
filepath.Join(repoPath, ".roborev.toml"),
[]byte("review_model = ["),
0o644,
)
require.NoError(t, err)

resolution, err := ResolveWorkflowConfig("", repoPath, &config.Config{
ReviewAgent: "gemini",
}, "review", "fast")
require.NoError(t, err)
require.Equal(t, "gemini", resolution.PreferredAgent)
}
16 changes: 15 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ func (e *ConfigParseError) Unwrap() error { return e.Err }
// is a ConfigParseError.
func IsConfigParseError(err error) bool {
var pe *ConfigParseError
return errors.As(err, &pe)
if errors.As(err, &pe) {
return true
}
var syntaxErr toml.ParseError
return errors.As(err, &syntaxErr)
}

// HookConfig defines a hook that runs on review events
Expand Down Expand Up @@ -831,6 +835,16 @@ func LoadRepoConfig(repoPath string) (*RepoConfig, error) {
return &cfg, nil
}

// ValidateRepoConfig returns any repo-config load or parse error for repoPath.
// Missing repo config is treated as valid.
func ValidateRepoConfig(repoPath string) error {
if strings.TrimSpace(repoPath) == "" {
return nil
}
_, err := LoadRepoConfig(repoPath)
return err
}

// ResolvePostCommitReview returns the post-commit review mode for a repo.
// Returns "branch" when configured, otherwise "commit" (the default).
func ResolvePostCommitReview(repoPath string) string {
Expand Down
1 change: 1 addition & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,7 @@ func TestResolveReasoning(t *testing.T) {
{"repo config overrides global config", "", fmt.Sprintf(`%s = "%s"`, configKey, repoVal), fmt.Sprintf(`%s = "%s"`, configKey, defaultVal), repoVal, false},
{"explicit overrides repo config", "fast", fmt.Sprintf(`%s = "%s"`, configKey, repoVal), fmt.Sprintf(`%s = "%s"`, configKey, defaultVal), "fast", false},
{"explicit normalization", "FAST", "", "", "fast", false},
{"explicit bypasses malformed repo config", "fast", fmt.Sprintf(`%s = [`, configKey), "", "fast", false},
{"invalid explicit", "unknown", "", "", "", true},
{"invalid repo config", "", fmt.Sprintf(`%s = "invalid"`, configKey), "", "", true},
{"malformed repo config does not fall back to global", "", fmt.Sprintf(`%s = [`, configKey), fmt.Sprintf(`%s = "%s"`, configKey, repoVal), "", true},
Expand Down
19 changes: 17 additions & 2 deletions internal/daemon/ci_poller.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ type CIPoller struct {
gitFetchPRHeadFn func(context.Context, string, int, []string) error
gitCloneFn func(ctx context.Context, ghRepo, targetPath string, env []string) error
mergeBaseFn func(string, string, string) (string, error)
loadRepoConfigFn func(string) (*config.RepoConfig, error)
buildReviewPromptFn func(string, string, int64, int, string, string, string, *config.Config) (string, error)
postPRCommentFn func(string, int, string) error
setCommitStatusFn func(ghRepo, sha, state, description string) error
Expand Down Expand Up @@ -100,6 +101,7 @@ func NewCIPoller(db *storage.DB, cfgGetter ConfigGetter, broadcaster Broadcaster
p.gitFetchFn = gitFetchCtx
p.gitFetchPRHeadFn = gitFetchPRHead
p.mergeBaseFn = gitpkg.GetMergeBase
p.loadRepoConfigFn = loadCIRepoConfig
p.buildReviewPromptFn = func(repoPath, gitRef string, repoID int64, contextCount int, agentName, reviewType, additionalContext string, cfg *config.Config) (string, error) {
builder := prompt.NewBuilderWithConfig(p.db, cfg)
return builder.BuildWithAdditionalContext(repoPath, gitRef, repoID, contextCount, agentName, reviewType, additionalContext)
Expand Down Expand Up @@ -406,8 +408,17 @@ func (p *CIPoller) processPR(ctx context.Context, ghRepo string, pr ghPR, cfg *c
matrix := cfg.CI.ResolvedReviewMatrix()
reasoning := "thorough"

repoCfg, repoCfgErr := loadCIRepoConfig(repo.RootPath)
loadRepoConfig := p.loadRepoConfigFn
if loadRepoConfig == nil {
loadRepoConfig = loadCIRepoConfig
}
repoCfg, repoCfgErr := loadRepoConfig(repo.RootPath)
if repoCfgErr != nil {
if !config.IsConfigParseError(repoCfgErr) {
return fmt.Errorf("load repo config: %w", repoCfgErr)
}
// CI review intentionally falls back to global/default settings so a
// broken repo override does not disable PR review entirely.
log.Printf("CI poller: warning: failed to load repo config for %s: %v", ghRepo, repoCfgErr)
}
if repoCfg != nil {
Expand Down Expand Up @@ -586,9 +597,13 @@ func (p *CIPoller) processPR(ctx context.Context, ghRepo string, pr ghPR, cfg *c
}

// Resolve agent through workflow config when not explicitly set.
resolution := agent.ResolveWorkflowConfig(
resolution, err := agent.ResolveWorkflowConfig(
ag, repo.RootPath, cfg, workflow, reasoning,
)
if err != nil {
rollback("Review enqueue failed")
return fmt.Errorf("resolve workflow config: %w", err)
}
resolvedAgent := resolution.PreferredAgent
if p.agentResolverFn != nil {
name, err := p.agentResolverFn(resolvedAgent)
Expand Down
Loading
Loading