Skip to content

Commit 1b35541

Browse files
authored
[fix] Route diagnostic warnings to stderr; surface IsDirty errors (#189)
[fix] Route diagnostic warnings to stderr; surface IsDirty errors (#170) - Route all Warning: … lines to cmd.ErrOrStderr() (was cmd.OutOrStdout()) in cleanFetchMergeRef, printWarnings, syncCmd.RunE fetch path, syncWorktree non-dirty skip path, and listRenderTable ghWarning — stdout is now clean for pipeline and JSON consumers. - filterDirtyWorktrees (CLI) and filterDirty (MCP) now treat an IsDirty error as dirty (include the worktree) and emit a Warning: … line to stderr so the error is visible; previously the error was silently swallowed and the worktree was excluded.
1 parent a0c2ddf commit 1b35541

13 files changed

Lines changed: 251 additions & 25 deletions

File tree

cmd/clean.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ func cleanFetchMergeRef(cmd *cobra.Command, r git.Runner, s *spinner.Spinner, ma
235235
s.Start("Fetching from origin...")
236236
if err := git.Fetch(r, "origin"); err != nil {
237237
s.Stop()
238-
fmt.Fprintf(cmd.OutOrStdout(), "Warning: fetch failed (no remote?): continuing with local state\n")
238+
fmt.Fprintf(cmd.ErrOrStderr(), "Warning: fetch failed (no remote?): continuing with local state\n")
239239
return mainBranch
240240
}
241241
return "origin/" + mainBranch
@@ -288,7 +288,7 @@ func confirmRemoval(cmd *cobra.Command, count int, label string) bool {
288288

289289
func printWarnings(cmd *cobra.Command, warnings []string) {
290290
for _, w := range warnings {
291-
fmt.Fprintf(cmd.OutOrStdout(), "Warning: %s\n", w)
291+
fmt.Fprintf(cmd.ErrOrStderr(), "Warning: %s\n", w)
292292
}
293293
}
294294

cmd/clean_test.go

Lines changed: 53 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,15 @@ import (
1111

1212
"github.com/lugassawan/rimba/internal/config"
1313
"github.com/lugassawan/rimba/internal/operations"
14+
"github.com/lugassawan/rimba/internal/spinner"
1415
"github.com/spf13/cobra"
1516
)
1617

1718
func TestCleanMergedFetchFails(t *testing.T) {
1819
worktreeOut := cleanMergedWorktreeOut()
19-
cmd, buf := newCleanMergedCmd()
20+
cmd, outBuf := newCleanMergedCmd()
21+
var errBuf bytes.Buffer
22+
cmd.SetErr(&errBuf) // separate stderr so we can verify warning routing
2023
_ = cmd.Flags().Set(flagForce, "true")
2124

2225
var mergedRef string
@@ -47,9 +50,11 @@ func TestCleanMergedFetchFails(t *testing.T) {
4750
if err != nil {
4851
t.Fatalf(fatalCleanMerged, err)
4952
}
50-
out := buf.String()
51-
if !strings.Contains(out, "Warning: fetch failed") {
52-
t.Errorf("expected fetch warning, got %q", out)
53+
if strings.Contains(outBuf.String(), "Warning: fetch failed") {
54+
t.Errorf("warning leaked to stdout: %q", outBuf.String())
55+
}
56+
if !strings.Contains(errBuf.String(), "Warning: fetch failed") {
57+
t.Errorf("expected fetch warning on stderr, got %q", errBuf.String())
5358
}
5459
if mergedRef != branchMain {
5560
t.Errorf("merged ref = %q, want %q (local fallback)", mergedRef, branchMain)
@@ -569,18 +574,54 @@ func TestCleanStaleForce(t *testing.T) {
569574

570575
func TestPrintWarnings(t *testing.T) {
571576
cmd := &cobra.Command{}
572-
var buf bytes.Buffer
573-
cmd.SetOut(&buf)
577+
var outBuf, errBuf bytes.Buffer
578+
cmd.SetOut(&outBuf)
579+
cmd.SetErr(&errBuf)
574580
printWarnings(cmd, []string{"first", "second"})
575-
out := buf.String()
576-
if !strings.Contains(out, "Warning: first") || !strings.Contains(out, "Warning: second") {
577-
t.Errorf("got %q", out)
581+
582+
if strings.Contains(outBuf.String(), "Warning") {
583+
t.Errorf("warnings leaked to stdout: %q", outBuf.String())
584+
}
585+
if !strings.Contains(errBuf.String(), "Warning: first") || !strings.Contains(errBuf.String(), "Warning: second") {
586+
t.Errorf("stderr = %q, want both warnings", errBuf.String())
578587
}
579588

580-
buf.Reset()
589+
outBuf.Reset()
590+
errBuf.Reset()
581591
printWarnings(cmd, nil)
582-
if buf.Len() != 0 {
583-
t.Errorf("expected no output for empty warnings, got %q", buf.String())
592+
if outBuf.Len() != 0 || errBuf.Len() != 0 {
593+
t.Errorf("expected no output for empty warnings")
594+
}
595+
}
596+
597+
func TestCleanFetchMergeRefWarningOnStderr(t *testing.T) {
598+
cmd := &cobra.Command{}
599+
cmd.Flags().Bool(flagNoColor, true, "")
600+
cmd.Flags().Bool(flagJSON, false, "")
601+
var outBuf, errBuf bytes.Buffer
602+
cmd.SetOut(&outBuf)
603+
cmd.SetErr(&errBuf)
604+
605+
r := &mockRunner{
606+
run: func(args ...string) (string, error) {
607+
if len(args) > 0 && args[0] == cmdFetch {
608+
return "", errors.New("no remote")
609+
}
610+
return "", nil
611+
},
612+
}
613+
s := spinner.New(spinnerOpts(cmd))
614+
defer s.Stop()
615+
616+
ref := cleanFetchMergeRef(cmd, r, s, branchMain)
617+
if ref != branchMain {
618+
t.Errorf("ref = %q, want %q (local fallback)", ref, branchMain)
619+
}
620+
if strings.Contains(outBuf.String(), "Warning") {
621+
t.Errorf("warning leaked to stdout: %q", outBuf.String())
622+
}
623+
if !strings.Contains(errBuf.String(), "Warning: fetch failed") {
624+
t.Errorf("stderr = %q, want 'Warning: fetch failed'", errBuf.String())
584625
}
585626
}
586627

cmd/exec.go

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ func execSelectWorktrees(cmd *cobra.Command, r git.Runner, s *spinner.Spinner, o
147147
}
148148

149149
if opts.dirty {
150-
filtered = filterDirtyWorktrees(r, s, filtered)
150+
filtered = filterDirtyWorktrees(cmd, r, s, filtered)
151151
}
152152
return filtered, nil
153153
}
@@ -217,8 +217,11 @@ func init() {
217217
}
218218

219219
// filterDirtyWorktrees filters worktrees to only those with uncommitted changes.
220-
func filterDirtyWorktrees(r git.Runner, s *spinner.Spinner, worktrees []resolver.WorktreeInfo) []resolver.WorktreeInfo {
220+
// If IsDirty returns an error for a worktree, it is treated as dirty (included)
221+
// and a warning is emitted to cmd.ErrOrStderr() so the error is visible.
222+
func filterDirtyWorktrees(cmd *cobra.Command, r git.Runner, s *spinner.Spinner, worktrees []resolver.WorktreeInfo) []resolver.WorktreeInfo {
221223
isDirty := make([]bool, len(worktrees))
224+
warnings := make([]string, len(worktrees))
222225
var wg sync.WaitGroup
223226
sem := make(chan struct{}, 8)
224227

@@ -231,13 +234,24 @@ func filterDirtyWorktrees(r git.Runner, s *spinner.Spinner, worktrees []resolver
231234
defer func() { <-sem }()
232235

233236
dirty, err := git.IsDirty(r, path)
234-
if err == nil && dirty {
237+
if err != nil {
238+
warnings[idx] = fmt.Sprintf("Warning: cannot check dirty status for %s: %v", path, err)
239+
isDirty[idx] = true
240+
return
241+
}
242+
if dirty {
235243
isDirty[idx] = true
236244
}
237245
}(i, wt.Path)
238246
}
239247
wg.Wait()
240248

249+
for _, w := range warnings {
250+
if w != "" {
251+
fmt.Fprintln(cmd.ErrOrStderr(), w)
252+
}
253+
}
254+
241255
var out []resolver.WorktreeInfo
242256
for i, wt := range worktrees {
243257
if isDirty[i] {

cmd/exec_test.go

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cmd
22

33
import (
4+
"bytes"
45
"context"
56
"encoding/json"
67
"errors"
@@ -233,7 +234,7 @@ func TestFilterDirtyWorktrees(t *testing.T) {
233234
s := testExecSpinner(cmd)
234235
defer s.Stop()
235236

236-
result := filterDirtyWorktrees(r, s, worktrees)
237+
result := filterDirtyWorktrees(cmd, r, s, worktrees)
237238
if len(result) != 1 {
238239
t.Fatalf("expected 1 dirty worktree, got %d", len(result))
239240
}
@@ -242,6 +243,41 @@ func TestFilterDirtyWorktrees(t *testing.T) {
242243
}
243244
}
244245

246+
func TestFilterDirtyWorktreesIsDirtyErrorIncludedAndWarned(t *testing.T) {
247+
worktrees := []resolver.WorktreeInfo{
248+
{Path: "/wt/error", Branch: "feature/a"},
249+
}
250+
r := &mockRunner{
251+
run: func(_ ...string) (string, error) { return "", nil },
252+
runInDir: func(_ string, _ ...string) (string, error) {
253+
return "", errors.New("permission denied")
254+
},
255+
}
256+
257+
cmd, _ := newTestCmd()
258+
var outBuf, errBuf bytes.Buffer
259+
cmd.SetOut(&outBuf)
260+
cmd.SetErr(&errBuf)
261+
262+
s := testExecSpinner(cmd)
263+
defer s.Stop()
264+
265+
result := filterDirtyWorktrees(cmd, r, s, worktrees)
266+
267+
if len(result) != 1 {
268+
t.Fatalf("expected erroring worktree to be included (treated as dirty), got %d", len(result))
269+
}
270+
if result[0].Path != "/wt/error" {
271+
t.Errorf("path = %q, want /wt/error", result[0].Path)
272+
}
273+
if strings.Contains(outBuf.String(), "Warning") {
274+
t.Errorf("warning leaked to stdout: %q", outBuf.String())
275+
}
276+
if !strings.Contains(errBuf.String(), "Warning: cannot check dirty status") {
277+
t.Errorf("stderr = %q, want warning", errBuf.String())
278+
}
279+
}
280+
245281
func TestExecTypeFlagCompletion(t *testing.T) {
246282
fn, ok := execCmd.GetFlagCompletionFunc(flagType)
247283
if !ok {

cmd/list_render.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func listRenderTable(cmd *cobra.Command, rows []resolver.WorktreeDetail, full bo
5252
p := termcolor.NewPainter(noColor)
5353

5454
if ghWarning != "" {
55-
fmt.Fprintln(cmd.OutOrStdout(), p.Paint(ghWarning, termcolor.Yellow))
55+
fmt.Fprintln(cmd.ErrOrStderr(), p.Paint(ghWarning, termcolor.Yellow))
5656
}
5757

5858
tbl := termcolor.NewTable(2)

cmd/list_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -834,6 +834,25 @@ func TestListRenderTableFullWithPRInfo(t *testing.T) {
834834
}
835835
}
836836

837+
func TestListRenderTableGhWarningOnStderr(t *testing.T) {
838+
cmd, _ := newListTestCmd()
839+
var outBuf, errBuf bytes.Buffer
840+
cmd.SetOut(&outBuf)
841+
cmd.SetErr(&errBuf)
842+
rows := []resolver.WorktreeDetail{
843+
{Task: "a", Branch: "feature/a", Type: "feature", Path: "/wt/a"},
844+
}
845+
846+
listRenderTable(cmd, rows, false, nil, "gh unavailable; PR/CI columns blank")
847+
848+
if strings.Contains(outBuf.String(), "gh unavailable") {
849+
t.Errorf("warning leaked to stdout: %q", outBuf.String())
850+
}
851+
if !strings.Contains(errBuf.String(), "gh unavailable") {
852+
t.Errorf("stderr = %q, want gh warning", errBuf.String())
853+
}
854+
}
855+
837856
func TestListRenderJSONPopulatesPRInfo(t *testing.T) {
838857
cmd, buf := newListTestCmd()
839858
rows := []resolver.WorktreeDetail{

cmd/sync.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ var syncCmd = &cobra.Command{
8484
s.Start("Fetching from origin...")
8585
if err := git.Fetch(r, "origin"); err != nil {
8686
s.Stop()
87-
fmt.Fprintf(cmd.OutOrStdout(), "Warning: fetch failed (no remote?): continuing with local state\n")
87+
fmt.Fprintf(cmd.ErrOrStderr(), "Warning: fetch failed (no remote?): continuing with local state\n")
8888
}
8989

9090
repoRoot, err := git.MainRepoRoot(r)
@@ -211,7 +211,7 @@ func syncWorktree(sc *syncContext, mainBranch string, wt resolver.WorktreeInfo,
211211
if sr.SkipReason == "dirty" {
212212
fmt.Fprintf(sc.cmd.OutOrStdout(), "Skipping %s (dirty)\n", sr.Branch)
213213
} else {
214-
fmt.Fprintf(sc.cmd.OutOrStdout(), "Warning: %s: %s\n", sr.Branch, sr.SkipReason)
214+
fmt.Fprintf(sc.cmd.ErrOrStderr(), "Warning: %s: %s\n", sr.Branch, sr.SkipReason)
215215
}
216216
case sr.Failed:
217217
sc.res.failed++

cmd/sync_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package cmd
22

33
import (
4+
"bytes"
45
"errors"
56
"strings"
67
"testing"
@@ -513,3 +514,30 @@ func TestPrintSyncSummary(t *testing.T) {
513514
}
514515
})
515516
}
517+
518+
func TestSyncWorktreeSkipWarningOnStderr(t *testing.T) {
519+
var outBuf, errBuf bytes.Buffer
520+
cmd := &cobra.Command{}
521+
cmd.Flags().Bool(flagNoColor, true, "")
522+
cmd.Flags().Bool(flagJSON, false, "")
523+
cmd.SetOut(&outBuf)
524+
cmd.SetErr(&errBuf)
525+
526+
r := &mockRunner{
527+
run: func(_ ...string) (string, error) { return "", nil },
528+
runInDir: func(_ string, _ ...string) (string, error) {
529+
return "", errGitFailed
530+
},
531+
}
532+
533+
sc := &syncContext{cmd: cmd, r: r, res: &syncResult{}}
534+
wt := resolver.WorktreeInfo{Branch: branchFeature, Path: pathWtFeatureLogin}
535+
syncWorktree(sc, branchMain, wt, false, false)
536+
537+
if strings.Contains(outBuf.String(), "Warning") {
538+
t.Errorf("warning leaked to stdout: %q", outBuf.String())
539+
}
540+
if !strings.Contains(errBuf.String(), "Warning") {
541+
t.Errorf("stderr = %q, want warning", errBuf.String())
542+
}
543+
}

internal/mcp/tool_exec.go

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package mcp
33
import (
44
"context"
55
"fmt"
6+
"os"
67
"sync"
78

89
"github.com/lugassawan/rimba/internal/config"
@@ -163,8 +164,11 @@ func buildExecData(command string, results []executor.Result) execData {
163164
}
164165

165166
// filterDirty filters worktrees to only those with uncommitted changes.
167+
// If IsDirty returns an error, the worktree is treated as dirty (included)
168+
// and a warning is written to os.Stderr so the operator can investigate.
166169
func filterDirty(r git.Runner, worktrees []resolver.WorktreeInfo) []resolver.WorktreeInfo {
167170
isDirtyFlags := make([]bool, len(worktrees))
171+
warnings := make([]string, len(worktrees))
168172
var wg sync.WaitGroup
169173
sem := make(chan struct{}, 8)
170174

@@ -176,13 +180,24 @@ func filterDirty(r git.Runner, worktrees []resolver.WorktreeInfo) []resolver.Wor
176180
defer func() { <-sem }()
177181

178182
d, err := git.IsDirty(r, path)
179-
if err == nil && d {
183+
if err != nil {
184+
warnings[idx] = fmt.Sprintf("Warning: cannot check dirty status for %s: %v", path, err)
185+
isDirtyFlags[idx] = true
186+
return
187+
}
188+
if d {
180189
isDirtyFlags[idx] = true
181190
}
182191
}(i, wt.Path)
183192
}
184193
wg.Wait()
185194

195+
for _, w := range warnings {
196+
if w != "" {
197+
fmt.Fprintln(os.Stderr, w)
198+
}
199+
}
200+
186201
var out []resolver.WorktreeInfo
187202
for i, wt := range worktrees {
188203
if isDirtyFlags[i] {

0 commit comments

Comments
 (0)