Skip to content

Commit 6c136e1

Browse files
authored
Use actual agent for workflow model fallback (#470)
## Summary - stop applying generic `default_model` based on the configured workflow agent when the actual runnable agent differs - use the actual resolved agent for fix, review, refine, security, and design model fallback across CLI, daemon, CI, and batch paths - add regressions for workflow-agent fallback and unavailable-agent fallback cases ## Testing - go fmt ./... - go test ./... - go vet ./...
1 parent a3aaded commit 6c136e1

12 files changed

Lines changed: 552 additions & 45 deletions

File tree

cmd/roborev/analyze.go

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -800,11 +800,6 @@ func runFixAgent(cmd *cobra.Command, repoPath, agentName, model, reasoning, prom
800800
}
801801

802802
// Resolve agent and model via fix workflow config.
803-
// When the agent is explicitly overridden but the model is not,
804-
// skip generic default_model (it's paired with default_agent).
805-
configAgent := config.ResolveAgentForWorkflow("", repoPath, cfg, "fix", reasoning)
806-
cliAgentChanged := agentName != "" &&
807-
agent.CanonicalName(agentName) != agent.CanonicalName(configAgent)
808803
agentName = config.ResolveAgentForWorkflow(agentName, repoPath, cfg, "fix", reasoning)
809804
backupAgent := config.ResolveBackupAgentForWorkflow(repoPath, cfg, "fix")
810805

@@ -821,10 +816,8 @@ func runFixAgent(cmd *cobra.Command, repoPath, agentName, model, reasoning, prom
821816
agent.CanonicalName(a.Name()) != agent.CanonicalName(preferredForAnalyze)
822817
if usingBackup && model == "" {
823818
model = config.ResolveBackupModelForWorkflow(repoPath, cfg, "fix")
824-
} else if cliAgentChanged && model == "" {
825-
model = config.ResolveWorkflowModel(repoPath, cfg, "fix", reasoning)
826819
} else {
827-
model = config.ResolveModelForWorkflow(model, repoPath, cfg, "fix", reasoning)
820+
model = resolveFixModel(a.Name(), model, repoPath, cfg, reasoning)
828821
}
829822

830823
// Configure agent: agentic mode, with model and reasoning

cmd/roborev/fix.go

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -308,21 +308,16 @@ func fixJobDirect(ctx context.Context, params fixJobParams, prompt string) (*fix
308308
}
309309

310310
// resolveFixModel determines the model for a fix operation, skipping
311-
// generic default_model when the CLI agent differs from config default.
311+
// generic default_model when the actual fix agent that will run differs
312+
// from the generic default agent. In that case, an empty result lets
313+
// the fix agent keep its own built-in default model unless a
314+
// fix-specific model override is configured.
312315
func resolveFixModel(
313-
cliAgent, cliModel, repoPath string,
316+
selectedAgent, cliModel, repoPath string,
314317
cfg *config.Config, reasoning string,
315318
) string {
316-
configAgent := config.ResolveAgentForWorkflow(
317-
"", repoPath, cfg, "fix", reasoning,
318-
)
319-
cliAgentChanged := cliAgent != "" &&
320-
agent.CanonicalName(cliAgent) != agent.CanonicalName(configAgent)
321-
if cliAgentChanged && cliModel == "" {
322-
return config.ResolveWorkflowModel(repoPath, cfg, "fix", reasoning)
323-
}
324-
return config.ResolveModelForWorkflow(
325-
cliModel, repoPath, cfg, "fix", reasoning,
319+
return agent.ResolveWorkflowModelForAgent(
320+
selectedAgent, cliModel, repoPath, cfg, "fix", reasoning,
326321
)
327322
}
328323

@@ -356,7 +351,7 @@ func resolveFixAgent(repoPath string, opts fixOptions) (agent.Agent, error) {
356351
if usingBackup && opts.model == "" {
357352
modelStr = config.ResolveBackupModelForWorkflow(repoPath, cfg, "fix")
358353
} else {
359-
modelStr = resolveFixModel(opts.agentName, opts.model, repoPath, cfg, reasoning)
354+
modelStr = resolveFixModel(a.Name(), opts.model, repoPath, cfg, reasoning)
360355
}
361356

362357
reasoningLevel := agent.ParseReasoningLevel(reasoning)

cmd/roborev/fix_test.go

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/roborev-dev/roborev/internal/agent"
2727
"github.com/roborev-dev/roborev/internal/config"
2828
"github.com/roborev-dev/roborev/internal/storage"
29+
"github.com/roborev-dev/roborev/internal/testutil"
2930
)
3031

3132
func patchFixDaemonRetryForTest(t *testing.T, ensure func() error) {
@@ -2597,6 +2598,62 @@ fix_model = "gemini-2.5-pro"
25972598
}
25982599
}
25992600

2601+
func TestResolveFixAgentSkipsDefaultModelForConfiguredFixAgent(t *testing.T) {
2602+
// When fix_agent differs from default_agent and no fix_model is set,
2603+
// the fix agent should keep its own built-in default model.
2604+
2605+
tmpDir := t.TempDir()
2606+
t.Setenv("ROBOREV_DATA_DIR", tmpDir)
2607+
2608+
cfgPath := filepath.Join(tmpDir, "config.toml")
2609+
if err := os.WriteFile(cfgPath, []byte(`
2610+
default_agent = "codex"
2611+
default_model = "gpt-5.4"
2612+
fix_agent = "claude"
2613+
`), 0644); err != nil {
2614+
t.Fatal(err)
2615+
}
2616+
2617+
cfg, err := config.LoadGlobal()
2618+
if err != nil {
2619+
t.Fatalf("LoadGlobal: %v", err)
2620+
}
2621+
2622+
modelStr := resolveFixModel("claude-code", "", tmpDir, cfg, "standard")
2623+
if modelStr != "" {
2624+
t.Fatalf("expected empty model so claude keeps its default, got %q", modelStr)
2625+
}
2626+
}
2627+
2628+
func TestResolveFixAgentFallbackUsesDefaultModelForActualAgent(t *testing.T) {
2629+
t.Cleanup(testutil.MockExecutableIsolated(t, "codex", 0))
2630+
2631+
tmpDir := t.TempDir()
2632+
t.Setenv("ROBOREV_DATA_DIR", tmpDir)
2633+
2634+
cfgPath := filepath.Join(tmpDir, "config.toml")
2635+
if err := os.WriteFile(cfgPath, []byte(`
2636+
default_agent = "codex"
2637+
default_model = "gpt-5.4"
2638+
fix_agent = "claude"
2639+
`), 0644); err != nil {
2640+
t.Fatal(err)
2641+
}
2642+
2643+
selected, err := resolveFixAgent(tmpDir, fixOptions{})
2644+
if err != nil {
2645+
t.Fatalf("resolveFixAgent: %v", err)
2646+
}
2647+
2648+
codexAgent, ok := selected.(*agent.CodexAgent)
2649+
if !ok {
2650+
t.Fatalf("expected codex fallback agent, got %T", selected)
2651+
}
2652+
if codexAgent.Model != "gpt-5.4" {
2653+
t.Fatalf("expected fallback codex agent to use default_model, got %q", codexAgent.Model)
2654+
}
2655+
}
2656+
26002657
func TestResolveFixAgentUsesRepoWorkflowModel(t *testing.T) {
26012658
// Repo-level workflow-specific models should be used even when
26022659
// --agent is overridden on CLI.
@@ -2703,9 +2760,7 @@ func TestResolveFixAgentSameAsDefault(t *testing.T) {
27032760
}
27042761

27052762
// Call the production resolveFixModel function directly
2706-
modelStr := resolveFixModel(
2707-
tt.cliAgent, "", tmpDir, cfg, "fast",
2708-
)
2763+
modelStr := resolveFixModel(tt.cliAgent, "", tmpDir, cfg, "fast")
27092764

27102765
if modelStr != tt.wantModel {
27112766
t.Errorf("model = %q, want %q", modelStr, tt.wantModel)

cmd/roborev/refine.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,8 +1189,8 @@ func applyModelForAgent(
11891189
repoPath, cfg, workflow,
11901190
)
11911191
} else {
1192-
model = config.ResolveModelForWorkflow(
1193-
cliModel, repoPath, cfg, workflow, reasoning,
1192+
model = agent.ResolveWorkflowModelForAgent(
1193+
a.Name(), cliModel, repoPath, cfg, workflow, reasoning,
11941194
)
11951195
}
11961196

cmd/roborev/refine_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,6 +1087,41 @@ func TestApplyModelForAgent_SameAgentPrimaryAndBackup(t *testing.T) {
10871087
}
10881088
}
10891089

1090+
func TestApplyModelForAgentFallbackUsesDefaultModelForActualAgent(t *testing.T) {
1091+
a, err := agent.Get("codex")
1092+
if err != nil {
1093+
t.Fatalf("agent.Get: %v", err)
1094+
}
1095+
1096+
cfg := &config.Config{
1097+
DefaultAgent: "codex",
1098+
DefaultModel: "gpt-5.4",
1099+
ReviewAgent: "claude",
1100+
}
1101+
1102+
result, model := applyModelForAgent(
1103+
a,
1104+
"claude", // preferred but unavailable
1105+
"", // no backup
1106+
"", // no CLI model
1107+
"", // no repo path
1108+
cfg,
1109+
"review",
1110+
"standard",
1111+
)
1112+
1113+
codexAgent, ok := result.(*agent.CodexAgent)
1114+
if !ok {
1115+
t.Fatalf("expected *CodexAgent, got %T", result)
1116+
}
1117+
if model != "gpt-5.4" {
1118+
t.Fatalf("expected default model for actual fallback agent, got %q", model)
1119+
}
1120+
if codexAgent.Model != "gpt-5.4" {
1121+
t.Fatalf("expected codex fallback agent to use default_model, got %q", codexAgent.Model)
1122+
}
1123+
}
1124+
10901125
func TestRefineFlagValidation(t *testing.T) {
10911126
tests := []struct {
10921127
name string

internal/agent/model_resolution.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package agent
2+
3+
import (
4+
"strings"
5+
6+
"github.com/roborev-dev/roborev/internal/config"
7+
)
8+
9+
// ResolveWorkflowModelForAgent resolves a workflow model for the actual
10+
// agent that will run. If that agent differs from the generic default
11+
// agent and no explicit model was provided, generic default_model is
12+
// skipped so the selected agent can keep its own built-in default unless
13+
// a workflow-specific model override exists.
14+
func ResolveWorkflowModelForAgent(
15+
selectedAgent, cliModel, repoPath string,
16+
globalCfg *config.Config,
17+
workflow, level string,
18+
) string {
19+
if s := strings.TrimSpace(cliModel); s != "" {
20+
return config.ResolveModelForWorkflow(
21+
s, repoPath, globalCfg, workflow, level,
22+
)
23+
}
24+
25+
selectedAgent = strings.TrimSpace(selectedAgent)
26+
if selectedAgent == "" {
27+
return config.ResolveModelForWorkflow(
28+
"", repoPath, globalCfg, workflow, level,
29+
)
30+
}
31+
32+
defaultAgent := config.ResolveAgent("", repoPath, globalCfg)
33+
if workflowModelComparableAgentName(selectedAgent, globalCfg) !=
34+
workflowModelComparableAgentName(defaultAgent, globalCfg) {
35+
return config.ResolveWorkflowModel(
36+
repoPath, globalCfg, workflow, level,
37+
)
38+
}
39+
40+
return config.ResolveModelForWorkflow(
41+
"", repoPath, globalCfg, workflow, level,
42+
)
43+
}
44+
45+
func workflowModelComparableAgentName(name string, cfg *config.Config) string {
46+
name = strings.TrimSpace(name)
47+
if isConfiguredACPAgentName(name, cfg) {
48+
return defaultACPName
49+
}
50+
return CanonicalName(name)
51+
}

0 commit comments

Comments
 (0)