Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 67 additions & 1 deletion internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type Config struct {
ReviewContextCount int `toml:"review_context_count"`
DefaultAgent string `toml:"default_agent"`
DefaultModel string `toml:"default_model"` // Default model for agents (format varies by agent)
DefaultBackupAgent string `toml:"default_backup_agent"`
JobTimeoutMinutes int `toml:"job_timeout_minutes"`

// Workflow-specific agent/model configuration
Expand Down Expand Up @@ -92,7 +93,15 @@ type Config struct {
DesignModelFast string `toml:"design_model_fast"`
DesignModelStandard string `toml:"design_model_standard"`
DesignModelThorough string `toml:"design_model_thorough"`
AllowUnsafeAgents *bool `toml:"allow_unsafe_agents"` // nil = not set, allows commands to choose their own default

// Backup agents for failover
ReviewBackupAgent string `toml:"review_backup_agent"`
RefineBackupAgent string `toml:"refine_backup_agent"`
FixBackupAgent string `toml:"fix_backup_agent"`
SecurityBackupAgent string `toml:"security_backup_agent"`
DesignBackupAgent string `toml:"design_backup_agent"`

AllowUnsafeAgents *bool `toml:"allow_unsafe_agents"` // nil = not set, allows commands to choose their own default

// Agent commands
CodexCmd string `toml:"codex_cmd"`
Expand Down Expand Up @@ -348,6 +357,7 @@ type RepoCIConfig struct {
type RepoConfig struct {
Agent string `toml:"agent"`
Model string `toml:"model"` // Model for agents (format varies by agent)
BackupAgent string `toml:"backup_agent"`
ReviewContextCount int `toml:"review_context_count"`
ReviewGuidelines string `toml:"review_guidelines"`
JobTimeoutMinutes int `toml:"job_timeout_minutes"`
Expand Down Expand Up @@ -402,6 +412,13 @@ type RepoConfig struct {
DesignModelStandard string `toml:"design_model_standard"`
DesignModelThorough string `toml:"design_model_thorough"`

// Backup agents for failover
ReviewBackupAgent string `toml:"review_backup_agent"`
RefineBackupAgent string `toml:"refine_backup_agent"`
FixBackupAgent string `toml:"fix_backup_agent"`
SecurityBackupAgent string `toml:"security_backup_agent"`
DesignBackupAgent string `toml:"design_backup_agent"`

// Hooks configuration (per-repo)
Hooks []HookConfig `toml:"hooks"`

Expand Down Expand Up @@ -729,6 +746,55 @@ func ResolveModelForWorkflow(cli, repoPath string, globalCfg *Config, workflow,
return getWorkflowValue(repoCfg, globalCfg, workflow, level, false)
}

// ResolveBackupAgentForWorkflow returns the backup agent for a workflow,
// or empty string if none is configured.
// Priority:
// 1. Repo {workflow}_backup_agent
// 2. Repo backup_agent (generic)
// 3. Global {workflow}_backup_agent
// 4. Global default_backup_agent
// 5. "" (no backup)
func ResolveBackupAgentForWorkflow(repoPath string, globalCfg *Config, workflow string) string {
repoCfg, _ := LoadRepoConfig(repoPath)

// Repo layer: workflow-specific > generic
if repoCfg != nil {
if s := lookupFieldByTag(reflect.ValueOf(*repoCfg), workflow+"_backup_agent"); s != "" {
return s
}
if s := strings.TrimSpace(repoCfg.BackupAgent); s != "" {
return s
}
}

// Global layer: workflow-specific > default
if globalCfg != nil {
if s := lookupFieldByTag(reflect.ValueOf(*globalCfg), workflow+"_backup_agent"); s != "" {
return s
}
if s := strings.TrimSpace(globalCfg.DefaultBackupAgent); s != "" {
return s
}
}

return ""
}

// lookupFieldByTag finds a struct field by its TOML tag and returns its trimmed value.
func lookupFieldByTag(v reflect.Value, key string) string {
t := v.Type()
for i := 0; i < t.NumField(); i++ {
tag := t.Field(i).Tag.Get("toml")
if tag == "" {
continue
}
if strings.Split(tag, ",")[0] == key {
return strings.TrimSpace(v.Field(i).String())
}
}
return ""
}

// getWorkflowValue looks up agent or model config following Option A priority.
func getWorkflowValue(repo *RepoConfig, global *Config, workflow, level string, isAgent bool) string {
// Repo layer: level-specific > workflow-specific > generic
Expand Down
96 changes: 96 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"os"
"os/exec"
"path/filepath"
"reflect"
"strings"
"testing"

Expand Down Expand Up @@ -1227,6 +1228,101 @@ func TestResolveModelForWorkflow(t *testing.T) {
}
}

func TestResolveBackupAgentForWorkflow(t *testing.T) {
tests := []struct {
name string
repo map[string]string
global map[string]string
workflow string
expect string
}{
// No backup configured
{"empty config", nil, nil, "review", ""},
{"only primary agent configured", M{"review_agent": "claude"}, nil, "review", ""},

// Global backup agent
{"global backup only", nil, M{"review_backup_agent": "test"}, "review", "test"},
{"global backup for refine", nil, M{"refine_backup_agent": "claude"}, "refine", "claude"},
{"global backup for fix", nil, M{"fix_backup_agent": "codex"}, "fix", "codex"},
{"global backup for security", nil, M{"security_backup_agent": "gemini"}, "security", "gemini"},
{"global backup for design", nil, M{"design_backup_agent": "droid"}, "design", "droid"},

// Repo backup agent overrides global
{"repo overrides global", M{"review_backup_agent": "repo-test"}, M{"review_backup_agent": "global-test"}, "review", "repo-test"},
{"repo backup only", M{"review_backup_agent": "test"}, nil, "review", "test"},

// Different workflows resolve independently
{"review backup doesn't affect refine", M{"review_backup_agent": "claude"}, nil, "refine", ""},
{"each workflow has own backup", M{"review_backup_agent": "claude", "refine_backup_agent": "codex"}, nil, "review", "claude"},
{"each workflow has own backup - refine", M{"review_backup_agent": "claude", "refine_backup_agent": "codex"}, nil, "refine", "codex"},

// Unknown workflow returns empty
{"unknown workflow", M{"review_backup_agent": "test"}, nil, "unknown", ""},

// No reasoning level support for backup agents
{"no level variants recognized", M{"review_backup_agent_fast": "claude"}, nil, "review", ""},
{"backup agent doesn't use levels", M{"review_backup_agent": "claude"}, nil, "review", "claude"},

// Default/generic backup agent fallback
{"global default_backup_agent", nil, M{"default_backup_agent": "test"}, "review", "test"},
{"global default_backup_agent for any workflow", nil, M{"default_backup_agent": "test"}, "fix", "test"},
{"global workflow-specific overrides default", nil, M{"default_backup_agent": "test", "review_backup_agent": "claude"}, "review", "claude"},
{"global default used when workflow not set", nil, M{"default_backup_agent": "test", "review_backup_agent": "claude"}, "fix", "test"},
{"repo backup_agent generic", M{"backup_agent": "repo-fallback"}, nil, "review", "repo-fallback"},
{"repo backup_agent generic for any workflow", M{"backup_agent": "repo-fallback"}, nil, "refine", "repo-fallback"},
{"repo workflow-specific overrides repo generic", M{"backup_agent": "generic", "review_backup_agent": "specific"}, nil, "review", "specific"},
{"repo generic overrides global workflow-specific", M{"backup_agent": "repo"}, M{"review_backup_agent": "global"}, "review", "repo"},
{"repo generic overrides global default", M{"backup_agent": "repo"}, M{"default_backup_agent": "global"}, "review", "repo"},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Create temp dir for repo config
repoDir := t.TempDir()

// Write repo config if provided
if tt.repo != nil {
writeRepoConfig(t, repoDir, tt.repo)
}

// Create global config if provided
var globalCfg *Config
if tt.global != nil {
globalCfg = &Config{}
populateConfigFromMap(globalCfg, tt.global)
}

// Test the function
result := ResolveBackupAgentForWorkflow(repoDir, globalCfg, tt.workflow)

if result != tt.expect {
t.Errorf("ResolveBackupAgentForWorkflow(%q, global, %q) = %q, want %q",
repoDir, tt.workflow, result, tt.expect)
}
})
}
}

// populateConfigFromMap is a helper to set config fields from a map
func populateConfigFromMap(cfg *Config, m map[string]string) {
v := reflect.ValueOf(cfg).Elem()
t := v.Type()

for i := 0; i < t.NumField(); i++ {
field := t.Field(i)
tag := field.Tag.Get("toml")
if tag == "" {
continue
}
tagName := strings.Split(tag, ",")[0]
if val, ok := m[tagName]; ok {
if field.Type.Kind() == reflect.String {
v.Field(i).SetString(val)
}
}
}
}

// M is a shorthand type for map[string]string to keep test tables compact
type M = map[string]string

Expand Down
2 changes: 1 addition & 1 deletion internal/daemon/ci_poller.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ func (p *CIPoller) processPR(ctx context.Context, ghRepo string, pr ghPR, cfg *c
resolvedAgent = resolved.Name()
}

// Resolve model through workflow config when not explicitly set
// Resolve model through workflow config
resolvedModel := config.ResolveModelForWorkflow(cfg.CI.Model, repo.RootPath, cfg, workflow, reasoning)

job, err := p.db.EnqueueJob(storage.EnqueueOpts{
Expand Down
33 changes: 23 additions & 10 deletions internal/daemon/ci_poller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1278,7 +1278,9 @@ func initGitRepoWithOrigin(t *testing.T) (dir string, runGit func(args ...string
runGit("init", "-b", "main")
runGit("config", "user.email", "test@test.com")
runGit("config", "user.name", "Test")
os.WriteFile(filepath.Join(dir, "README.md"), []byte("init"), 0644)
if err := os.WriteFile(filepath.Join(dir, "README.md"), []byte("init"), 0644); err != nil {
t.Fatalf("write README.md: %v", err)
}
runGit("add", "-A")
runGit("commit", "-m", "initial")
runGit("remote", "add", "origin", dir)
Expand All @@ -1291,8 +1293,10 @@ func TestLoadCIRepoConfig_LoadsFromDefaultBranch(t *testing.T) {
dir, runGit := initGitRepoWithOrigin(t)

// Commit .roborev.toml on main with CI agents override
os.WriteFile(filepath.Join(dir, ".roborev.toml"),
[]byte("[ci]\nagents = [\"claude\"]\n"), 0644)
if err := os.WriteFile(filepath.Join(dir, ".roborev.toml"),
[]byte("[ci]\nagents = [\"claude\"]\n"), 0644); err != nil {
t.Fatalf("write .roborev.toml: %v", err)
}
runGit("add", ".roborev.toml")
runGit("commit", "-m", "add config")
runGit("fetch", "origin")
Expand All @@ -1313,8 +1317,10 @@ func TestLoadCIRepoConfig_FallsBackWhenNoConfigOnDefaultBranch(t *testing.T) {
dir, _ := initGitRepoWithOrigin(t)

// No .roborev.toml on origin/main, but put one in the working tree
os.WriteFile(filepath.Join(dir, ".roborev.toml"),
[]byte("[ci]\nagents = [\"codex\"]\n"), 0644)
if err := os.WriteFile(filepath.Join(dir, ".roborev.toml"),
[]byte("[ci]\nagents = [\"codex\"]\n"), 0644); err != nil {
t.Fatalf("write .roborev.toml: %v", err)
}

cfg, err := loadCIRepoConfig(dir)
if err != nil {
Expand All @@ -1332,15 +1338,19 @@ func TestLoadCIRepoConfig_PropagatesParseError(t *testing.T) {
dir, runGit := initGitRepoWithOrigin(t)

// Commit invalid TOML on main
os.WriteFile(filepath.Join(dir, ".roborev.toml"),
[]byte("this is not valid toml [[["), 0644)
if err := os.WriteFile(filepath.Join(dir, ".roborev.toml"),
[]byte("this is not valid toml [[["), 0644); err != nil {
t.Fatalf("write .roborev.toml: %v", err)
}
runGit("add", ".roborev.toml")
runGit("commit", "-m", "add bad config")
runGit("fetch", "origin")

// Also put valid config in working tree — should NOT be used
os.WriteFile(filepath.Join(dir, ".roborev.toml"),
[]byte("[ci]\nagents = [\"codex\"]\n"), 0644)
// Also put valid config in working tree -- should NOT be used
if err := os.WriteFile(filepath.Join(dir, ".roborev.toml"),
[]byte("[ci]\nagents = [\"codex\"]\n"), 0644); err != nil {
t.Fatalf("write .roborev.toml: %v", err)
}

cfg, err := loadCIRepoConfig(dir)
if err == nil {
Expand Down Expand Up @@ -1388,6 +1398,9 @@ func TestCIPollerProcessPR_SetsPendingCommitStatus(t *testing.T) {
if sc.state != "pending" {
t.Errorf("state=%q, want pending", sc.state)
}
if sc.desc != "Review in progress" {
t.Errorf("desc=%q, want %q", sc.desc, "Review in progress")
}
}

func TestCIPollerPostBatchResults_SetsSuccessStatus(t *testing.T) {
Expand Down
2 changes: 1 addition & 1 deletion internal/daemon/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1591,7 +1591,7 @@ func TestHandleRerunJob(t *testing.T) {
commit, _ := db.GetOrCreateCommit(repo.ID, "rerun-failed", "Author", "Subject", time.Now())
job, _ := db.EnqueueJob(storage.EnqueueOpts{RepoID: repo.ID, CommitID: commit.ID, GitRef: "rerun-failed", Agent: "test"})
db.ClaimJob("worker-1")
db.FailJob(job.ID, "some error")
db.FailJob(job.ID, "", "some error")

req := testutil.MakeJSONRequest(t, http.MethodPost, "/api/job/rerun", RerunJobRequest{JobID: job.ID})
w := httptest.NewRecorder()
Expand Down
Loading
Loading