Skip to content

Commit 0b5f8dc

Browse files
authored
Fix workspace status aggregation to exclude default-branch and passive-default repos (#15)
1 parent 159ce93 commit 0b5f8dc

4 files changed

Lines changed: 150 additions & 10 deletions

File tree

internal/status/resolve.go

Lines changed: 39 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ func (r *Resolver) ResolveRepo(ctx context.Context, spec *Spec, repo RepoInfo, w
8282

8383
// ResolveWorkspace resolves the status for each repo concurrently and
8484
// returns the aggregated workspace result. The workspace status is the
85-
// least-advanced status (highest index in spec order) across all repos.
85+
// least-advanced status (highest index in spec order) across all repos,
86+
// excluding skipped repos and repos at the default status.
8687
func (r *Resolver) ResolveWorkspace(ctx context.Context, spec *Spec, repos []RepoInfo, wsID, wsName string) *WorkspaceResult {
8788
wsStart := time.Now()
8889
result := &WorkspaceResult{
@@ -91,18 +92,24 @@ func (r *Resolver) ResolveWorkspace(ctx context.Context, spec *Spec, repos []Rep
9192
Repos: make([]RepoResult, len(repos)),
9293
}
9394

95+
// Find the default status index.
96+
defaultIdx := -1
97+
for i, e := range spec.Spec.Statuses {
98+
if e.Default {
99+
defaultIdx = i
100+
break
101+
}
102+
}
103+
94104
if len(repos) == 0 {
95-
// No repos — use the default status.
96-
for _, e := range spec.Spec.Statuses {
97-
if e.Default {
98-
result.Status = e.Name
99-
break
100-
}
105+
if defaultIdx >= 0 {
106+
result.Status = spec.Spec.Statuses[defaultIdx].Name
101107
}
102108
result.Duration = time.Since(wsStart)
103109
return result
104110
}
105111

112+
skipped := make([]bool, len(repos))
106113
var mu sync.Mutex
107114
var wg sync.WaitGroup
108115

@@ -111,8 +118,17 @@ func (r *Resolver) ResolveWorkspace(ctx context.Context, spec *Spec, repos []Rep
111118
go func(idx int, rp RepoInfo) {
112119
defer wg.Done()
113120
repoStart := time.Now()
121+
env := buildEnv(rp, wsID, wsName)
122+
123+
// Run skip check if configured.
124+
skip := false
125+
if spec.Spec.Skip != "" {
126+
skip = r.Runner.RunCheck(ctx, spec.Spec.Skip, env)
127+
}
128+
114129
status := r.ResolveRepo(ctx, spec, rp, wsID, wsName)
115130
mu.Lock()
131+
skipped[idx] = skip
116132
result.Repos[idx] = RepoResult{
117133
URL: rp.URL,
118134
Branch: rp.Branch,
@@ -132,16 +148,29 @@ func (r *Resolver) ResolveWorkspace(ctx context.Context, spec *Spec, repos []Rep
132148
orderIndex[e.Name] = i
133149
}
134150

135-
// Workspace status = least advanced (highest index) across repos.
151+
// Workspace status = least advanced (highest index) across repos,
152+
// excluding skipped repos and repos at the default status.
136153
worstIdx := -1
137-
for _, rr := range result.Repos {
154+
for i, rr := range result.Repos {
155+
if skipped[i] {
156+
continue
157+
}
138158
idx, ok := orderIndex[rr.Status]
139-
if ok && idx > worstIdx {
159+
if !ok {
160+
continue
161+
}
162+
if idx == defaultIdx {
163+
continue
164+
}
165+
if idx > worstIdx {
140166
worstIdx = idx
141167
}
142168
}
169+
143170
if worstIdx >= 0 {
144171
result.Status = spec.Spec.Statuses[worstIdx].Name
172+
} else if defaultIdx >= 0 {
173+
result.Status = spec.Spec.Statuses[defaultIdx].Name
145174
}
146175

147176
return result

internal/status/resolve_test.go

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,114 @@ func TestRepoSlug(t *testing.T) {
194194
}
195195
}
196196

197+
func testSpecWithSkip() *Spec {
198+
s := testSpec()
199+
s.Spec.Skip = "check-skip"
200+
return s
201+
}
202+
203+
func TestResolveWorkspaceSkippedRepoExcluded(t *testing.T) {
204+
// repo-a is in-progress (feature branch), repo-b is open (main, skipped).
205+
// Without skip, workspace would be open. With skip, repo-b excluded → in-progress.
206+
perRepoMock := &perRepoRunner{
207+
results: map[string]map[string]bool{
208+
"github.com/org/repo-a": {"check-progress": true},
209+
"github.com/org/repo-b": {"check-skip": true},
210+
},
211+
}
212+
resolver := &Resolver{Runner: perRepoMock}
213+
214+
repos := []RepoInfo{
215+
{URL: "github.com/org/repo-a", Branch: "feat/x", Path: "./repo-a"},
216+
{URL: "github.com/org/repo-b", Branch: "main", Path: "./repo-b"},
217+
}
218+
result := resolver.ResolveWorkspace(context.Background(), testSpecWithSkip(), repos, "ws-1", "my-ws")
219+
220+
if result.Status != "in-progress" {
221+
t.Errorf("workspace status = %q, want in-progress (skipped repo excluded)", result.Status)
222+
}
223+
}
224+
225+
func TestResolveWorkspaceDefaultStatusPassive(t *testing.T) {
226+
// Two feature repos: one closed, one at default (open).
227+
// Default status is passive — should not drag workspace to open.
228+
perRepoMock := &perRepoRunner{
229+
results: map[string]map[string]bool{
230+
"github.com/org/repo-a": {"check-closed": true},
231+
"github.com/org/repo-b": {},
232+
},
233+
}
234+
resolver := &Resolver{Runner: perRepoMock}
235+
236+
repos := []RepoInfo{
237+
{URL: "github.com/org/repo-a", Branch: "feat/x", Path: "./repo-a"},
238+
{URL: "github.com/org/repo-b", Branch: "feat/y", Path: "./repo-b"},
239+
}
240+
result := resolver.ResolveWorkspace(context.Background(), testSpec(), repos, "ws-1", "my-ws")
241+
242+
if result.Status != "closed" {
243+
t.Errorf("workspace status = %q, want closed (default status passive)", result.Status)
244+
}
245+
}
246+
247+
func TestResolveWorkspaceAllSkippedFallback(t *testing.T) {
248+
// All repos skipped → fall back to default status.
249+
mock := &mockRunner{results: map[string]bool{
250+
"check-skip": true,
251+
}}
252+
resolver := &Resolver{Runner: mock}
253+
254+
repos := []RepoInfo{
255+
{URL: "github.com/org/repo-a", Branch: "main", Path: "./repo-a"},
256+
{URL: "github.com/org/repo-b", Branch: "main", Path: "./repo-b"},
257+
}
258+
result := resolver.ResolveWorkspace(context.Background(), testSpecWithSkip(), repos, "ws-1", "my-ws")
259+
260+
if result.Status != "open" {
261+
t.Errorf("workspace status = %q, want open (all skipped fallback)", result.Status)
262+
}
263+
}
264+
265+
func TestResolveWorkspaceMixedSkipAndDefault(t *testing.T) {
266+
// repo-a: closed (feat), repo-b: in-review (feat), repo-c: open (main, skipped).
267+
// Skip excludes repo-c, default excludes nothing extra. Least advanced = in-review.
268+
perRepoMock := &perRepoRunner{
269+
results: map[string]map[string]bool{
270+
"github.com/org/repo-a": {"check-closed": true},
271+
"github.com/org/repo-b": {"check-review": true},
272+
"github.com/org/repo-c": {"check-skip": true},
273+
},
274+
}
275+
resolver := &Resolver{Runner: perRepoMock}
276+
277+
repos := []RepoInfo{
278+
{URL: "github.com/org/repo-a", Branch: "feat/x", Path: "./repo-a"},
279+
{URL: "github.com/org/repo-b", Branch: "feat/y", Path: "./repo-b"},
280+
{URL: "github.com/org/repo-c", Branch: "main", Path: "./repo-c"},
281+
}
282+
result := resolver.ResolveWorkspace(context.Background(), testSpecWithSkip(), repos, "ws-1", "my-ws")
283+
284+
if result.Status != "in-review" {
285+
t.Errorf("workspace status = %q, want in-review (mixed skip and default)", result.Status)
286+
}
287+
}
288+
289+
func TestResolveWorkspaceAllFeatureAtDefault(t *testing.T) {
290+
// All feature-branch repos at default status → default (no active signal).
291+
mock := &mockRunner{results: map[string]bool{}}
292+
resolver := &Resolver{Runner: mock}
293+
294+
repos := []RepoInfo{
295+
{URL: "github.com/org/repo-a", Branch: "feat/x", Path: "./repo-a"},
296+
{URL: "github.com/org/repo-b", Branch: "feat/y", Path: "./repo-b"},
297+
}
298+
result := resolver.ResolveWorkspace(context.Background(), testSpec(), repos, "ws-1", "my-ws")
299+
300+
if result.Status != "open" {
301+
t.Errorf("workspace status = %q, want open (all at default)", result.Status)
302+
}
303+
}
304+
197305
// perRepoRunner distinguishes check results by repo URL (extracted from env).
198306
type perRepoRunner struct {
199307
results map[string]map[string]bool // repoURL -> command -> result

internal/status/spec.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,8 @@ func DefaultSpec() *Spec {
118118
APIVersion: "flow/v1",
119119
Kind: "Status",
120120
Spec: SpecBody{
121+
Skip: `_default=$(git -C "$FLOW_REPO_PATH" symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's|refs/remotes/origin/||')
122+
[ -n "$_default" ] && [ "$FLOW_REPO_BRANCH" = "$_default" ]`,
121123
Statuses: []Entry{
122124
{
123125
Name: "closed",

internal/status/types.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import "time"
55

66
// SpecBody holds the statuses list nested under spec.
77
type SpecBody struct {
8+
Skip string `yaml:"skip,omitempty"`
89
Statuses []Entry `yaml:"statuses,omitempty"`
910
}
1011

0 commit comments

Comments
 (0)