fix(printer,syncer): address Copilot review feedback from PR #26

.github/copilot-instructions.md

@@ -0,0 +1 @@
+../AGENTS.md

AGENTS.md

@@ -0,0 +1,28 @@
+# Repository Guidelines
+
+## Project Structure & Module Organization
+`gih/main.go` is the CLI entry point. Core behavior lives in `syncer/`: `dir.go` discovers direct child repositories, `git.go` runs git commands, and `syncer.go` coordinates parallel execution. Output formatting lives in `printer/`. Tests sit next to the code as `*_test.go`. CI and templates are under `.github/`.
+
+## Build, Test, and Development Commands
+Use Go 1.26.
+
+- `make build`: builds a development binary as `./gih_dev`.
+- `make install`: installs `gih` from `./gih`.
+- `make test`: runs `go test -v -race ./...`.
+- `make lint`: runs `golangci-lint` as used in CI.
+- `make tidy`: normalizes `go.mod` and `go.sum`.
+- `DEBUG=* go run ./gih status`: handy for manual CLI checks.
+
+Before opening a PR, run `make build && make test && make lint`.
+
+## Coding Style & Naming Conventions
+Follow standard Go formatting with `gofmt`; keep packages small and focused. Prefer the standard library over new dependencies when possible. Use clear package scopes such as `feat(syncer)` or `fix(printer)` when naming changes. Keep filenames lowercase, and keep tests in the same package unless black-box coverage is required.
+
+## Testing Guidelines
+This repository uses Go’s `testing` package plus `github.com/matryer/is`. Write focused tests with `t.Run(...)`; avoid table-driven tests here. Use `t.Parallel()` whenever the case is safe to parallelize. Name receiver-related tests as `Test{Struct}_Xxx`, for example `TestSync_Execute`. Run targeted checks with `go test -v ./syncer/` and use `go test -cover ./...` when touching behavior broadly.
+
+## Commit & Pull Request Guidelines
+Recent history follows Conventional Commits, for example `feat(gih): ...`, `refactor(syncer): ...`, and `fix(printer): ...`. Keep commit titles in English and scoped when helpful. PRs should follow `.github/PULL_REQUEST_TEMPLATE.md` with `## Issue` and `## Overview`, link the related issue, and describe behavioral changes briefly in Japanese. Include tests for functional changes and update `README.md` when CLI behavior changes.
+
+## Worktree Workflow
+Do not work directly on the default branch (currently `master`). Start each non-trivial change from a dedicated branch and worktree under `.worktrees/`, for example `git worktree add .worktrees/feat-x -b feat/x`.

CLAUDE.md

@@ -0,0 +1 @@
+AGENTS.md

printer/print.go

@@ -114,27 +114,29 @@ func (p *Printer) PrintMsg(msg string) {
 	}
 }
 
-// PrintMsgErr prints error message
+// PrintMsgErr prints an error message to errWriter so that callers can
+// redirect stderr separately from stdout (consistent with Printer.Error).
 func (p *Printer) PrintMsgErr(msg string) {
 	type message struct {
 		Msg string
 	}
 	p.mu.Lock()
 	defer p.mu.Unlock()
-	if err := msgErrTpl.Execute(p.writer, message{Msg: msg}); err != nil {
+	if err := msgErrTpl.Execute(p.errWriter, message{Msg: msg}); err != nil {
 		log.Println(err)
 	}
 }
 
-// PrintRepoErr prints error message
+// PrintRepoErr prints a header plus a list of repository paths to errWriter
+// (matching the stderr semantics of PrintMsgErr / Error).
 func (p *Printer) PrintRepoErr(msg string, repos []string) {
 	type message struct {
 		Msg   string
 		Repos []string
 	}
 	p.mu.Lock()
 	defer p.mu.Unlock()
-	if err := repoErrTpl.Execute(p.writer, message{Msg: msg, Repos: repos}); err != nil {
+	if err := repoErrTpl.Execute(p.errWriter, message{Msg: msg, Repos: repos}); err != nil {
 		log.Println(err)
 	}
 }

syncer/git_test.go

@@ -17,6 +17,9 @@ func TestExistGit_NoStdoutSideEffect(t *testing.T) {
 
 	r, w, err := os.Pipe()
 	is.NoErr(err)
+	// io.ReadAll reaches EOF but the read-end fd still needs closing.
+	// Close error is irrelevant in tests — we only care about fd release.
+	defer func() { _ = r.Close() }()
 	orig := os.Stdout
 	os.Stdout = w
 	defer func() { os.Stdout = orig }()

syncer/syncer.go

@@ -55,18 +55,14 @@ type Sync struct {
 	Gitter Executor
 }
 
-// failedRepo carries the repository path together with the error that caused
-// the failure so that it can be reported in the final summary.
-type failedRepo struct {
-	Repo string
-	Err  error
-}
-
 // runStats accumulates per-repository outcomes safely from concurrent goroutines.
+// Each bucket is just a list of repository paths — the per-repo error message
+// is already streamed to stderr at execution time via PrintMsgErr, so there is
+// no need to retain it here.
 type runStats struct {
 	mu        sync.Mutex
 	succeeded []string
-	failed    []failedRepo
+	failed    []string
 	timedOut  []string
 }
 
@@ -76,10 +72,10 @@ func (s *runStats) addSuccess(r string) {
 	s.succeeded = append(s.succeeded, r)
 }
 
-func (s *runStats) addFailed(r string, e error) {
+func (s *runStats) addFailed(r string) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	s.failed = append(s.failed, failedRepo{Repo: r, Err: e})
+	s.failed = append(s.failed, r)
 }
 
 func (s *runStats) addTimedOut(r string) {
@@ -198,12 +194,12 @@ func (s *Sync) execute(parent context.Context, repos []string, perRepoTimeout ti
 			switch {
 			case err == nil:
 				stats.addSuccess(r)
-				s.Writer.PrintMsg(fmt.Sprintf("Success: %s\n", r))
+				s.Writer.PrintMsg(fmt.Sprintf("Success: %s", r))
 			case errors.Is(ctx.Err(), context.DeadlineExceeded):
 				stats.addTimedOut(r)
 				s.Writer.PrintMsgErr(fmt.Sprintf("Timeout: %s", r))
 			default:
-				stats.addFailed(r, err)
+				stats.addFailed(r)
 				s.Writer.PrintMsgErr(fmt.Sprintf("Failed: %s\n%v", r, err))
 			}
 
@@ -245,11 +241,7 @@ func (s *Sync) printSummary(stats *runStats) {
 	))
 
 	if len(stats.failed) > 0 {
-		repos := make([]string, len(stats.failed))
-		for i, f := range stats.failed {
-			repos[i] = f.Repo
-		}
-		s.Writer.PrintRepoErr("Failed repositories:", repos)
+		s.Writer.PrintRepoErr("Failed repositories:", stats.failed)
 	}
 	if len(stats.timedOut) > 0 {
 		s.Writer.PrintRepoErr("Timed out repositories:", stats.timedOut)

syncer/syncer_test.go

@@ -59,7 +59,7 @@ func TestSync_Execute(t *testing.T) {
 		stats := s.execute(context.Background(), []string{"a", "b", "c"}, time.Second)
 		is.Equal(len(stats.succeeded), 2)
 		is.Equal(len(stats.failed), 1)
-		is.Equal(filepath.Base(stats.failed[0].Repo), "b")
+		is.Equal(filepath.Base(stats.failed[0]), "b")
 	})
 
 	t.Run("timeout is classified separately", func(t *testing.T) {