Skip to content

refactor(ci): consolidate config precedence helpers #574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wesm merged 1 commit into from
Mar 24, 2026
Merged

refactor(ci): consolidate config precedence helpers #574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
124 changes: 6 additions & 118 deletions cmd/roborev/ci.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,15 +162,15 @@ func runCIReview(ctx context.Context, opts ciReviewOpts) error {
}

// Resolve agents
agents := resolveAgentList(
agents := config.ResolveCIAgents(
opts.agents, repoCfg, globalCfg)
if len(agents) == 0 {
return fmt.Errorf("no agents configured " +
"(check --agent flag or config)")
}

// Resolve review types
reviewTypes := resolveReviewTypes(
reviewTypes := config.ResolveCIReviewTypes(
opts.reviewTypes, repoCfg, globalCfg)
if len(reviewTypes) == 0 {
return fmt.Errorf("no review types configured " +
Expand All @@ -183,21 +183,21 @@ func runCIReview(ctx context.Context, opts ciReviewOpts) error {
}

// Resolve reasoning
reasoningLevel, err := resolveCIReasoning(
reasoningLevel, err := config.ResolveCIReasoning(
opts.reasoning, repoCfg, globalCfg)
if err != nil {
return err
}

// Resolve min severity
minSev, err := resolveCIMinSeverity(
minSev, err := config.ResolveCIMinSeverity(
opts.minSeverity, repoCfg, globalCfg)
if err != nil {
return err
}

// Resolve synthesis agent
synthAgent := resolveCISynthesisAgent(
synthAgent := config.ResolveCISynthesisAgent(
opts.synthesisAgent, repoCfg, globalCfg)

log.Printf(
Expand Down Expand Up @@ -264,7 +264,7 @@ func runCIReview(ctx context.Context, opts ciReviewOpts) error {
prNumber = detected
}

upsert := resolveCIUpsertComments(
upsert := config.ResolveCIUpsertComments(
repoCfg, globalCfg)
if err := postCIComment(
ctx, ghRepo, prNumber, comment, upsert,
Expand All @@ -285,118 +285,6 @@ func runCIReview(ctx context.Context, opts ciReviewOpts) error {
return nil
}

func resolveAgentList(
flag string,
repoCfg *config.RepoConfig,
globalCfg *config.Config,
) []string {
if flag != "" {
return splitTrimmed(flag)
}
if repoCfg != nil && len(repoCfg.CI.Agents) > 0 {
return repoCfg.CI.Agents
}
if globalCfg != nil && len(globalCfg.CI.Agents) > 0 {
return globalCfg.CI.Agents
}
// Default: empty string = auto-detect
return []string{""}
}

func resolveReviewTypes(
flag string,
repoCfg *config.RepoConfig,
globalCfg *config.Config,
) []string {
if flag != "" {
return splitTrimmed(flag)
}
if repoCfg != nil && len(repoCfg.CI.ReviewTypes) > 0 {
return repoCfg.CI.ReviewTypes
}
if globalCfg != nil && len(globalCfg.CI.ReviewTypes) > 0 {
return globalCfg.CI.ReviewTypes
}
return []string{config.ReviewTypeSecurity}
}

func resolveCIReasoning(
flag string,
repoCfg *config.RepoConfig,
globalCfg *config.Config,
) (string, error) {
if flag != "" {
n, err := config.NormalizeReasoning(flag)
if err != nil {
return "", err
}
return n, nil
}
if repoCfg != nil && repoCfg.CI.Reasoning != "" {
if n, err := config.NormalizeReasoning(
repoCfg.CI.Reasoning); err == nil {
return n, nil
}
}
return "thorough", nil
}

func resolveCIMinSeverity(
flag string,
repoCfg *config.RepoConfig,
globalCfg *config.Config,
) (string, error) {
if flag != "" {
n, err := config.NormalizeMinSeverity(flag)
if err != nil {
return "", err
}
return n, nil
}
if repoCfg != nil && repoCfg.CI.MinSeverity != "" {
if n, err := config.NormalizeMinSeverity(
repoCfg.CI.MinSeverity); err == nil {
return n, nil
}
}
if globalCfg != nil && globalCfg.CI.MinSeverity != "" {
if n, err := config.NormalizeMinSeverity(
globalCfg.CI.MinSeverity); err == nil {
return n, nil
}
}
return "", nil
}

func resolveCISynthesisAgent(
flag string,
repoCfg *config.RepoConfig,
globalCfg *config.Config,
) string {
if flag != "" {
return flag
}
if globalCfg != nil && globalCfg.CI.SynthesisAgent != "" {
return globalCfg.CI.SynthesisAgent
}
return ""
}

// resolveCIUpsertComments determines whether to upsert PR comments.
// Priority: repo config > global config > false.
func resolveCIUpsertComments(
repoCfg *config.RepoConfig,
globalCfg *config.Config,
) bool {
if repoCfg != nil && repoCfg.CI.UpsertComments != nil {
return *repoCfg.CI.UpsertComments
}
if globalCfg != nil {
return globalCfg.CI.UpsertComments
}
return false
}

func splitTrimmed(s string) []string {
parts := strings.Split(s, ",")
out := make([]string, 0, len(parts))
Expand Down
78 changes: 71 additions & 7 deletions cmd/roborev/ci_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,41 +159,41 @@ func TestExtractHeadSHA(t *testing.T) {

func TestResolveAgentList(t *testing.T) {
t.Run("flag", func(t *testing.T) {
agents := resolveAgentList(
agents := config.ResolveCIAgents(
"codex,gemini", nil, nil)
assert.False(t, len(agents) != 2 ||
agents[0] != "codex" ||
agents[1] != "gemini")
})

t.Run("default", func(t *testing.T) {
agents := resolveAgentList("", nil, nil)
agents := config.ResolveCIAgents("", nil, nil)
assert.False(t, len(agents) != 1 || agents[0] != "")
})
}

func TestResolveReviewTypes(t *testing.T) {
t.Run("flag", func(t *testing.T) {
types := resolveReviewTypes(
types := config.ResolveCIReviewTypes(
"security,design", nil, nil)
assert.Len(t, types, 2)
})

t.Run("default", func(t *testing.T) {
types := resolveReviewTypes("", nil, nil)
types := config.ResolveCIReviewTypes("", nil, nil)
assert.False(t, len(types) != 1 || types[0] != "security")
})
}

func TestResolveAgentList_EmptyFlag(t *testing.T) {
// Comma-only flag should resolve to empty list.
agents := resolveAgentList(",", nil, nil)
agents := config.ResolveCIAgents(",", nil, nil)
assert.Empty(t, agents)
}

func TestResolveReviewTypes_EmptyFlag(t *testing.T) {
// Whitespace-comma flag should resolve to empty list.
types := resolveReviewTypes(" , ", nil, nil)
types := config.ResolveCIReviewTypes(" , ", nil, nil)
assert.Empty(t, types)
}

Expand Down Expand Up @@ -249,12 +249,76 @@ func TestResolveCIUpsertComments(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := resolveCIUpsertComments(tt.repo, tt.global)
got := config.ResolveCIUpsertComments(tt.repo, tt.global)
assert.Equal(t, tt.want, got)
})
}
}

func TestResolveCIReasoning(t *testing.T) {
t.Run("explicit flag wins", func(t *testing.T) {
got, err := config.ResolveCIReasoning("high", nil, nil)
require.NoError(t, err)
assert.Equal(t, "thorough", got)
})

t.Run("repo override wins", func(t *testing.T) {
got, err := config.ResolveCIReasoning("", &config.RepoConfig{
CI: config.RepoCIConfig{Reasoning: "medium"},
}, &config.Config{})
require.NoError(t, err)
assert.Equal(t, "medium", got)
})

t.Run("invalid repo config falls back to default", func(t *testing.T) {
got, err := config.ResolveCIReasoning("", &config.RepoConfig{
CI: config.RepoCIConfig{Reasoning: "nope"},
}, nil)
require.NoError(t, err)
assert.Equal(t, "thorough", got)
})
}

func TestResolveCIMinSeverity(t *testing.T) {
t.Run("explicit flag wins", func(t *testing.T) {
got, err := config.ResolveCIMinSeverity("HIGH", nil, nil)
require.NoError(t, err)
assert.Equal(t, "high", got)
})

t.Run("repo override beats global", func(t *testing.T) {
got, err := config.ResolveCIMinSeverity("", &config.RepoConfig{
CI: config.RepoCIConfig{MinSeverity: "medium"},
}, &config.Config{
CI: config.CIConfig{MinSeverity: "high"},
})
require.NoError(t, err)
assert.Equal(t, "medium", got)
})

t.Run("invalid repo config falls through to valid global", func(t *testing.T) {
got, err := config.ResolveCIMinSeverity("", &config.RepoConfig{
CI: config.RepoCIConfig{MinSeverity: "bogus"},
}, &config.Config{
CI: config.CIConfig{MinSeverity: "critical"},
})
require.NoError(t, err)
assert.Equal(t, "critical", got)
})
}

func TestResolveCISynthesisAgent(t *testing.T) {
got := config.ResolveCISynthesisAgent("", &config.RepoConfig{}, &config.Config{
CI: config.CIConfig{SynthesisAgent: "gemini"},
})
assert.Equal(t, "gemini", got)

got = config.ResolveCISynthesisAgent("codex", nil, &config.Config{
CI: config.CIConfig{SynthesisAgent: "gemini"},
})
assert.Equal(t, "codex", got)
}

func TestSplitTrimmed(t *testing.T) {
tests := []struct {
in string
Expand Down
19 changes: 2 additions & 17 deletions cmd/roborev/ghaction.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,23 +119,8 @@ func resolveWorkflowConfig(

globalCfg, _ := config.LoadGlobal()
repoCfg, _ := config.LoadRepoConfig(repoRoot)

// Resolve agents: flag > repo [ci] agents > repo agent
// > global [ci] agents > global default_agent > default
if agentFlag != "" {
cfg.Agents = splitTrimmed(agentFlag)
} else if repoCfg != nil &&
len(repoCfg.CI.Agents) > 0 {
cfg.Agents = repoCfg.CI.Agents
} else if repoCfg != nil && repoCfg.Agent != "" {
cfg.Agents = []string{repoCfg.Agent}
} else if globalCfg != nil &&
len(globalCfg.CI.Agents) > 0 {
cfg.Agents = globalCfg.CI.Agents
} else if globalCfg != nil &&
globalCfg.DefaultAgent != "" {
cfg.Agents = []string{globalCfg.DefaultAgent}
}
cfg.Agents = config.ResolveCIWorkflowAgents(
agentFlag, repoCfg, globalCfg)

if roborevVersion != "" {
cfg.RoborevVersion = roborevVersion
Expand Down
Loading
Loading