From b852302742bd56dbeccb7e3cff7383f6646986fb Mon Sep 17 00:00:00 2001 From: takeshi yamashita <220619+takecy@users.noreply.github.com> Date: Sun, 26 Apr 2026 11:54:13 +0900 Subject: [PATCH 1/6] fix(printer): emit error and repo-list messages on stderr MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PrintMsgErr and PrintRepoErr were writing to p.writer (stdout) despite their names and despite the parallel Error method routing its output to p.errWriter. That meant the gih CLI's error notices ("Failed: ", "Timeout: ") and the final failed/timed-out repository list landed on stdout — indistinguishable from successful results when the caller used `2>` to capture errors or `>` to capture results. Switch both methods to p.errWriter to match the Error method and the implicit contract suggested by the *Err naming. Existing tests use NewPrinter(&buf, &buf) so they keep passing; the new behaviour shows up only when callers wire distinct writers, which is the use case this fix enables. Reported by GitHub Copilot on PR #26. Co-Authored-By: Claude --- printer/print.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/printer/print.go b/printer/print.go index 5d1cc97..828aa0d 100644 --- a/printer/print.go +++ b/printer/print.go @@ -114,19 +114,21 @@ 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 @@ -134,7 +136,7 @@ func (p *Printer) PrintRepoErr(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) } } From ef5be0163e56939ab9ca9417973989f77e6c6f9f Mon Sep 17 00:00:00 2001 From: takeshi yamashita <220619+takecy@users.noreply.github.com> Date: Sun, 26 Apr 2026 11:55:27 +0900 Subject: [PATCH 2/6] refactor(syncer): drop unused Err field from failed repo records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit failedRepo carried both the repo path and the error returned by the git invocation, but the error field was never read after assignment. The per-repo failure reason is already streamed to stderr at execution time via PrintMsgErr ("Failed: \n"), and printSummary only needs the repo path to print the trailing failed-repo list. Replace the failedRepo struct with a plain []string and update addFailed and the summary printer accordingly. The test that asserted on stats.failed[0].Repo becomes stats.failed[0] — same coverage, slightly cleaner. Reported by GitHub Copilot on PR #26. Co-Authored-By: Claude --- syncer/syncer.go | 24 ++++++++---------------- syncer/syncer_test.go | 2 +- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/syncer/syncer.go b/syncer/syncer.go index 67a4679..56f634d 100644 --- a/syncer/syncer.go +++ b/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) { @@ -203,7 +199,7 @@ func (s *Sync) execute(parent context.Context, repos []string, perRepoTimeout ti 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) diff --git a/syncer/syncer_test.go b/syncer/syncer_test.go index c0f0a6e..9a4f43b 100644 --- a/syncer/syncer_test.go +++ b/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) { From 7d420ad69c43e541f67921a13f0837d9e89418e6 Mon Sep 17 00:00:00 2001 From: takeshi yamashita <220619+takecy@users.noreply.github.com> Date: Sun, 26 Apr 2026 11:39:38 +0900 Subject: [PATCH 3/6] docs: add AGENTS.md and symlink CLAUDE.md and copilot-instructions.md Co-Authored-By: Claude --- .github/copilot-instructions.md | 1 + AGENTS.md | 28 ++++++++++++++++++++++++++++ CLAUDE.md | 1 + 3 files changed, 30 insertions(+) create mode 120000 .github/copilot-instructions.md create mode 100644 AGENTS.md create mode 120000 CLAUDE.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 120000 index 0000000..be77ac8 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1 @@ +../AGENTS.md \ No newline at end of file diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..a8ffc3a --- /dev/null +++ b/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 `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`. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 120000 index 0000000..47dc3e3 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1 @@ +AGENTS.md \ No newline at end of file From 70ccedcae8183a2ea90088a54dcc3b61c1fdc459 Mon Sep 17 00:00:00 2001 From: takeshi yamashita <220619+takecy@users.noreply.github.com> Date: Sun, 26 Apr 2026 12:02:40 +0900 Subject: [PATCH 4/6] fix(syncer): drop trailing newline from Success message to avoid blank line The Success message used "Success: %s\n" while Timeout and Failed used the trailing-newline-free form. Combined with msgTpl's own "\n", the explicit \n in the format string produced an empty line right after every successful repo. The output already includes a visible separator from the Print template, so the extra blank line was pure noise. Drop the \n from the format string. Output now matches the Timeout and Failed shapes: a single line ending in the template's trailing newline. Reported by GitHub Copilot on PR #26. Co-Authored-By: Claude --- syncer/syncer.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/syncer/syncer.go b/syncer/syncer.go index 56f634d..26f4fd8 100644 --- a/syncer/syncer.go +++ b/syncer/syncer.go @@ -194,7 +194,7 @@ 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)) From f6bea28f0a68de54457f35613d905a6170388482 Mon Sep 17 00:00:00 2001 From: takeshi yamashita <220619+takecy@users.noreply.github.com> Date: Sun, 26 Apr 2026 12:03:30 +0900 Subject: [PATCH 5/6] test(syncer): close pipe reader to avoid fd leak in TestExistGit_NoStdoutSideEffect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit os.Pipe returns two file descriptors. The test was already closing the writer to signal EOF for io.ReadAll, but the read end was leaked on every invocation. One leak per run is harmless on its own; under go test -count=N or as the suite grows it can exhaust the open-file ulimit, especially in CI containers with low limits. Add a deferred Close on the reader. Wrap it in a closure to discard the error explicitly so the errcheck linter stays happy — the close result has no diagnostic value in this test, only the fd release matters. Reported by GitHub Copilot on PR #26. Co-Authored-By: Claude --- syncer/git_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/syncer/git_test.go b/syncer/git_test.go index 22e7bbd..1e2d432 100644 --- a/syncer/git_test.go +++ b/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 }() From d8f54fb37bd3a9550be2975c08374857ddf65890 Mon Sep 17 00:00:00 2001 From: takeshi yamashita <220619+takecy@users.noreply.github.com> Date: Sun, 26 Apr 2026 12:11:32 +0900 Subject: [PATCH 6/6] docs(agents): clarify default-branch wording in worktree workflow Reported by GitHub Copilot on PR #27. Co-Authored-By: Claude --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index a8ffc3a..9e3e12b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,4 +25,4 @@ This repository uses Go’s `testing` package plus `github.com/matryer/is`. Writ 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 `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`. +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`.