feat: parallel execution hardening with run summary and exit codes#26
Merged
feat: parallel execution hardening with run summary and exit codes#26
Conversation
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace channel-based throttle with errgroup.SetLimit so the main goroutine no longer blocks while enqueueing repos beyond the concurrency limit. Move the timeout boundary into each goroutine via context.WithTimeout(parent, perRepoTimeout) so the README's "timeout per directory" semantics actually hold and so a single slow repo cannot starve the others. Switch Gitter.Git to exec.CommandContext and introduce an Executor interface so cancellation reaches the spawned git process (and its helper subprocesses via SIGPIPE) and so tests can swap in a fake executor without forking real git. Aggregate per-repo outcomes into runStats (succeeded / failed / timedOut) and emit a final summary line plus repository lists for failed and timed-out runs, reusing the previously-unused printer.PrintRepoErr formatter. Add focused unit tests for execute() and filterRepos() using a mutex-wrapped io.Discard writer so -race stays clean even though the Printer itself is not yet mutex-protected. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each Print* method previously re-parsed its template on every call via template.Must(template.New(...).Funcs(helpers).Parse(...)). Move the six templates to package-level vars so parsing happens once at init. The t() helper that selected between successTmpl and errTmpl is no longer needed; Print and Error reference successTpl / errTpl directly. Pure refactor — no observable behaviour change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each Print* method calls text/template.Execute, which in turn issues multiple io.Writer.Write calls. Without external synchronisation, two goroutines emitting messages through the Printer can interleave their Writes — observed empirically in the previous commit's E2E run as "Timeout: r1Done: 1/3" (a result line and a Done line concatenated without the intervening newline). Add a single sync.Mutex to Printer, locked around every Execute call. A single mutex (rather than separate mutexes for writer and errWriter) is intentional: in typical CLI use both end up at the same TTY, and we want stdout/stderr messages to also stay non-overlapping. Drop the syncWriter helper from syncer_test.go: it was wrapping io.Discard purely to defend against the unprotected Printer, which is no longer needed. Add printer/print_test.go covering each public method concurrently, plus a TestPrinter_SharedMutex_AcrossWriterAndErrWriter test that wires both writer and errWriter to the same probe writer and asserts that the maximum number of in-flight Write calls observed is 1 — this directly verifies the single-mutex design rather than just exercising the methods in parallel. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous Gitter.IsExist had two unrelated jobs glued together: report whether `git` is on PATH, and write that path to os.Stdout. The stdout write was an invisible side effect — production code never called IsExist, so the only consumer was a test, and even there the write only ever surfaced as noise next to the test summary. Promote the check to a package-level ExistGit() error and drop the side effect entirely. The receiver was empty anyway, so a method added no value. Update TestIsExist to call the new function. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ut output TestFetch used to run `git fetch --all -p` against ".", i.e. whatever directory the test happened to start in. On CI that meant fetching the git-here repo itself, which required network access and mutated the local clone's remote-tracking refs. Replace this with a fully self-contained fixture: a bare repo built in t.TempDir() acts as the remote, a sibling working repo pushes one empty commit to it, and the fetch under test pulls those refs back. The whole test runs in well under a second and never touches the network. After the fetch, resolve refs/remotes/origin/main; without this check the test would still pass on a no-op fetch because err==nil alone is not strong enough to detect "fetch succeeded but transferred nothing". Replace TestIsExist with TestExistGit_NoStdoutSideEffect, which redirects os.Stdout through an os.Pipe and asserts the captured output is empty. This guards the #11 contract ("ExistGit has no side effect") in a way that does not depend on which path resolves for git on the test host (Homebrew, Xcode, /usr/bin, etc.). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Exit Sync.Run had three early os.Exit(1) calls for setup failures (ListDirs error, no git repositories present, invalid timeout duration). Calling os.Exit from inside Run skips deferred work and makes the function untestable from a test process — the public signature (err error) was already there, so the exits were also inconsistent with the declared contract. Change Run's signature to (*RunSummary, error). Setup failures now return an error; the new RunSummary type carries Succeeded / Failed / TimedOut counts plus a HasFailures predicate for callers that need to distinguish a clean run from a partial-failure run. Filter narrowing to zero matches returns an empty summary and nil error (it is not an error — there is just no work to do). Update gih/main.go to print the error to stderr and exit 1 in the err != nil case, replacing the earlier panic(err) which produced a stack trace and exit code 2 — the wrong code per the README spec. The summary value is currently ignored at the call site; the next commit wires it up to the exit-code 2 path for partial failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The README has documented an exit-code-2 behaviour for partial repository failures since the project's first release, but the implementation has been silent: gih returned 0 whether all repos succeeded or only some did, so CI couldn't distinguish the two. Now that Sync.Run returns a RunSummary, inspect summary.HasFailures() after the err == nil path and exit 2 if any repository ended up in the Failed or TimedOut buckets. Pure setup failures continue to exit 1, and a clean run still exits 0. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
IsRepo previously called os.ReadDir(dirName) and walked every entry looking for a name match against ".git". That has two problems: it pays full directory-listing cost just to answer a yes/no question about a single fixed name, and the name-only match doesn't make the worktree case explicit even though git worktrees place ".git" as a *file* (a "gitdir: ..." pointer) rather than a directory. Replace the loop with a single os.Stat on the joined path. One syscall instead of N, and stat doesn't care whether the entry is a file or a directory, so a worktree's .git file is correctly accepted as a repository indicator. Add syncer/dir_test.go covering both layouts plus the negative cases (non-git directory, non-existent path), pinning the worktree-support contract so a future "must be a directory" change would surface as a test failure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ListDirs used `strings.Index(f.Name(), ".") != 0` to drop hidden entries — a slightly indirect way of asking the same question that strings.HasPrefix asks more idiomatically. Switch to HasPrefix. While here, reorder the filter chain so the cheap HasPrefix check runs before IsRepo. IsRepo now performs an os.Stat per call, so short-circuiting on hidden directories avoids a syscall for each .something entry in the parent. Rewrite the conjunction as a sequence of early-continue guards so the intent of each step reads top-down. Pure refactor — for non-empty names, "name starts with ." and "strings.Index(name, \".\") == 0" are exactly the same predicate, so ListDirs returns the same set of paths it always did. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
並列実行時の信頼性向上（per-repo timeout / プロセス kill / 実行サマリ）、CLI の exit code 仕様の実装、出力の並列安全化、およびテストの安定化・高速化を行うPRです。
Changes:
Sync.Run()を(*RunSummary, error)返却に変更し、README 仕様に沿った exit code (0/1/2) を main 側で決定- errgroup の並列制御・per-repo timeout 化・実行結果サマリ（成功/失敗/timeout と一覧）出力を追加
- Printer のテンプレート再利用 + 単一 mutex による並列出力混線対策、dir 判定の syscall 最適化と worktree 対応、テストの hermetic 化
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
syncer/syncer.go |
並列実行の再設計（errgroup.SetLimit / per-repo timeout / 実行サマリ / RunSummary 返却） |
syncer/syncer_test.go |
execute の成功/失敗/timeout 分類と ConNum throttle をユニットテストで検証 |
syncer/git.go |
exec.CommandContext による ctx キャンセル対応、Executor インターフェース化、ExistGit 追加 |
syncer/git_test.go |
ExistGit の stdout 副作用なしの検証、Fetch テストの hermetic 化（ローカル bare remote） |
syncer/dir.go |
IsRepo を os.Stat 化（worktree .git file 対応）、ListDirs の効率化 |
syncer/dir_test.go |
通常 repo / worktree / 非repo の IsRepo 判定テスト追加 |
printer/print.go |
テンプレートを package var で再利用、単一 mutex で Execute を直列化 |
printer/print_test.go |
Printer の並列安全性（行構造・stdout/stderr 共通 mutex）をテストで担保 |
gih/main.go |
RunSummary/err を受けて exit code (0/1/2) を実装、panic を排除 |
.gitignore |
.worktrees/ と tasks/ を追加で無視 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
takecy
added a commit
that referenced
this pull request
Apr 26, 2026
fix(printer,syncer): address Copilot review feedback from PR #26
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
なし（リポジトリ全体のコードレビューに基づく改善 /
tasks/improvements-review-2026-04-26.mdに対応する 13 項目中 12 項目を本 PR で対応）。Overview
並列実行の信頼性、CLI 挙動の README 整合、内部品質を中心とした多領域の改善。各グループは独立した目的で、コミット粒度も分離してある（合計 11 コミット、A〜D グループに対応）。
A. 並列実行の信頼性（99ae654 / 99ae654 / 99ae654 / 99ae654）
exec.CommandContext化)。git fetchが hang しても ctx cancel で SIGKILL が届く。HTTPS の補助プロセス (git remote-https階層) も SIGPIPE 経由で連鎖終了することを E2E で確認済みerrgroup.SetLimitに置き換えて、-c <ConNum>を超える repos がある場合に main goroutine が throttle 詰まりで全体ハングする既存バグを解消Summary: success=N failed=M timeout=K+ 失敗・timeout リポジトリ一覧を表示。並列で逐次出力が混ざっても最後にサマリで把握可能にB. Printer の堅牢化（599fa1d / 38063dd）
text/template.Executeをsync.Mutexで直列化。Executeが内部で複数回io.Writer.Writeするため mutex なしだと goroutine 間で出力が混線していた（修正前 E2E でTimeout: r1Done: 1/3のような連結を実観測）concurrencyProbeを使って peak inflight Write が常に 1 であることを直接アサートtemplate.Must(...).Parse(...)を排除C. テスト独立性（fdd273e / 9f35617）
TestFetchをt.TempDir()+ ローカル bare remote 構成に書き換え。ネットワーク不要・冪等になり、テスト所要時間が 2.21s → 0.15s（約14倍高速化）。refs/remotes/origin/mainの解決可否で実質的な fetch 成功も強アサートGitter.IsExistの stdout 副作用（git のパスを直接書き出していた）を除去し、package-levelExistGit() error関数に置き換え。TestExistGit_NoStdoutSideEffectでos.Pipe()経由で stdout が空であることを直接アサート（git のパスや CI 環境に依存しない検証）D. error handling 整理（d67107a / 77e85c4）
Sync.Run()から 3 箇所のos.Exit(1)を排除し、(*RunSummary, error)を返すシグネチャに変更。setup error は error 経由、partial failure は summary 経由で main に伝える0全成功 /1setup error (no repos / invalid timeout / invalid regex) /2一部 repo の失敗または timeoutpanic(err)(= exit 2 + stack trace) をfmt.Fprintln + os.Exit(1)に修正E. dir.go 改善（f38ae87 / 298e76e）
IsRepoをos.ReadDirループからos.Stat1 発呼び出し に変更。1 syscall で済む + git worktree (.gitがファイル形式) を意味的に明示対応。TestIsRepoのworktree with .git fileサブテストで pinListDirsでstrings.Index(name, ".") != 0をstrings.HasPrefixに置換。さらに評価順をHasPrefix → IsRepoに入れ替えて、隠しディレクトリで syscall を skip。if a && b && cを early-continue形式に整理検証結果
make test(-race): printer 5 + syncer 26 + main 0 = 全テスト PASSgolangci-lint run: 0 issuesmake build: 成功スコープ外（別 issue で対応予定）
gih/main.goの細かい点（goversionをruntime.Version()化 /concurrency: %d timeout:の余計な2スペース / subcommand 設計の将来検討）🤖 Generated with Claude Code