feat: add log rotation via newsyslog

[s4na]

Summary

• Add ldcron log setup-rotation subcommand that prints a newsyslog(8) configuration snippet
• Uses the G (glob) flag to cover all ~/Library/Logs/ldcron/*.log files with a single entry
• Config: rotate at 1 MB, keep 3 gzip archives, no process signaling (GNB flags)
• One-time setup: ldcron log setup-rotation | sudo tee /etc/newsyslog.d/com.ldcron.conf
• newsyslog already runs hourly via a system launchd job — no extra scheduling needed
• Update both README.md and README.ja.md with log rotation documentation

Test plan

• go test ./... passes
• golangci-lint run passes (no new lint issues)
• Run ldcron log setup-rotation and verify output format
• Install config and verify newsyslog rotates logs correctly

🤖 Generated with Claude Code

[s4na]

Code Review (Google Style)

TL;DR

ログローテーション機能は newsyslog を活用した堅実な設計で、launchd 完全互換の原則にも沿っており、全体的にコード品質を向上させる良い CL です。いくつか改善の余地がある点を以下に記載します。

• ログパス構築ロジックが paths.go と重複している
• テストで cobra コマンドオブジェクトを直接変更しており、テスト間の副作用リスクがある
• newsyslog の N フラグ（シグナル不要）の選択理由がコード内で明示されていない

Details

🚨 issue

特になし。マージをブロックする致命的な問題は見当たりません。

💡 suggestion

1. ログパス構築ロジックの重複（cmd/log.go / cmd/paths.go）

cmd/log.go の runLogSetupRotation では os.UserHomeDir() + filepath.Join(home, "Library", "Logs", "ldcron", ...) でログディレクトリのパスを独自に組み立てていますが、cmd/paths.go の logDir() に同じロジックが既に存在します。パスの定義が 2 箇所に分散すると、将来ログディレクトリを変更した際に片方だけ更新される恐れがあります。

logDir() はディレクトリの MkdirAll も行いますが、setup-rotation は出力するだけなのでディレクトリ作成は不要という事情は理解できます。それでも、パス文字列の構築部分を共通化する（例: logDirPath() を切り出す）か、logDir() を再利用するのが望ましいです。

// 案: paths.go にパスだけ返す関数を追加
func logDirPath() (string, error) {
    home, err := os.UserHomeDir()
    if err != nil {
        return "", fmt.Errorf("failed to get home directory: %w", err)
    }
    return filepath.Join(home, "Library", "Logs", "ldcron"), nil
}

2. テストでグローバルな cobra.Command を直接操作している

cmd/log_test.go で logSetupRotationCmd を直接取得し cmd.SetOut(&buf) を呼んでいます。cobra の Command はパッケージレベルの var なので、SetOut の変更がテスト間で残留し、並列実行時に干渉する可能性があります。

安全策として、rootCmd 経由で Execute するか、テスト終了時に SetOut をリセットする t.Cleanup を追加することを検討してください。

t.Cleanup(func() { logSetupRotationCmd.SetOut(nil) })

3. newsyslog 設定のパラメータをフラグで変更可能にする検討

現在、ローテーションの count（3）や size（1024 KB）はハードコードされています。将来的にユーザーがカスタマイズしたくなる可能性を考えると、--count / --size フラグを用意しておくと柔軟性が上がります。ただし、現時点では YAGNI（まだ必要ない）とも言えるので、今の実装のままでも問題ありません。これは将来への備えとしてのメモです。

🔧 nitpick

4. newsyslog フラグ GNB の意味をコードコメントに記載する

cmd/log.go 53行目の出力行:

_, _ = fmt.Fprintf(w, "%s\t644\t3\t1024\t*\tGNB\n", logPattern)

GNB の各フラグの意味（G=glob, N=no signal, B=binary/gzip）がコード内のコメントで説明されていると、将来メンテナンスする人にとって分かりやすいです。Long description には記載がありますが、実際の出力コード付近にも一行コメントがあると良いでしょう。

5. ヘッダーコメント行のタブ整列

_, _ = fmt.Fprintf(w, "# logfilename\t\t\t\t\t\tmode\tcount\tsize\twhen\tflags\n")

ヘッダーのタブ数がハードコードされており、logPattern の長さによっては実際の出力と列が揃わない場合があります。text/tabwriter を使うか、ヘッダーはあくまで参考情報であると割り切るかの判断ですが、newsyslog 自体がヘッダーコメントを無視するので実害はありません。

6. README.md と README.ja.md の内容の粒度の差

英語版 README には "keeps 3 compressed (gzip) archives, and requires no process signaling" とありますが、日本語版では "gzip圧縮したアーカイブを3世代保持します" のみで "no process signaling" に相当する記述がありません。CLAUDE.md の README 管理規約（「常に同期する」）に照らすと、日本語版にも同等の情報を含めるのが望ましいです。

❓ question

7. N フラグ（no signal）の選択について

newsyslog の N フラグは「ローテーション後にプロセスにシグナルを送らない」という意味ですが、launchd で管理されるジョブが書き込み中のログファイルがローテーションされた場合、ジョブ側でファイルディスクリプタが古いファイルを指し続ける可能性はないでしょうか？ launchd の plist で StandardOutPath / StandardErrorPath を指定している場合、launchd がジョブ実行ごとにファイルを開き直すので問題ないという理解で合っていますか？

👏 praise

8. newsyslog の活用は優れた設計判断

macOS 標準の newsyslog を活用することで、ldcron 自体にローテーションロジックを持たせる必要がなく、launchd 完全互換の設計原則に沿った素晴らしい判断です。ldcron をアンインストールしても newsyslog の設定はそのまま動作し続けます。

9. テストカバレッジが適切

出力内容の主要な要素（パターン、フラグ、サイズ）を検証するテストが書かれており、適切なカバレッジです。

10. README の両言語同時更新

CLAUDE.md のガイドラインに従い、英語・日本語の README を同時に更新している点は良い習慣です。

[s4na]

Code Review (Google Style)

TL;DR

newsyslog を活用したログローテーション機能は、macOS 標準のツールを活かした堅実な設計であり、launchd 完全互換の原則にも沿っています。全体としてコード品質を向上させる良い CL です。以下にいくつかの改善点を記載します。

• README 日本語版に英語版と同等の情報が欠落している箇所がある
• テストで mode（644）の検証が不足している
• newsyslog ヘッダーコメントのタブ整列が実際の出力と揃わない可能性がある

Details

🚨 issue

致命的な問題は見当たりません。マージをブロックする項目はありません。

💡 suggestion

1. README 日本語版に "no process signaling" の説明が不足している

README.md（英語版）には以下の記述があります:

keeps 3 compressed (gzip) archives, and requires no process signaling.
newsyslog runs automatically every hour via a system launchd job, so no additional scheduling is needed.

一方 README.ja.md（日本語版）では「プロセスへのシグナル送信は不要です」の記述はありますが、その理由（launchd はジョブ実行ごとにログファイルを開き直すため）が日本語版にのみ追加されており、英語版には記載されていません。逆に英語版の段落構成（2段落に分かれている）と日本語版（1段落）で構造が異なります。CLAUDE.md の README 管理規約（「常に同期する」）に照らすと、両言語で情報量と構造を揃えるのが望ましいです。

2. テストで mode（644）の検証が不足している

cmd/log_test.go では wantPattern、GNB、1024 を検証していますが、ファイルモード 644 の検証がありません。newsyslog 設定のすべてのフィールドが正しく出力されることを確認するために、644 も検証に加えることを検討してください。

if !strings.Contains(output, "644") {
    t.Errorf("output should contain mode 644, got:\n%s", output)
}

3. テストで出力全体の構造を検証する方が堅牢

現在のテストは strings.Contains で個別の要素を検証していますが、出力全体のフォーマット（タブ区切り、フィールドの順序）が正しいかは検証できません。正規表現やフルライン比較で、newsyslog 設定行全体の構造を検証するとより堅牢になります。ただし、現時点のテストでも十分な実用性があるため、これは将来の改善として記載します。

4. newsyslog 設定パラメータのカスタマイズ可能性の検討

count（3）や size（1024 KB）がハードコードされています。将来的にユーザーがカスタマイズしたくなる可能性を考えると --count / --size フラグの導入が考えられますが、現時点では YAGNI とも言えるので、今の実装のままでも問題ありません。将来への備えとしてのメモです。

🔧 nitpick

5. newsyslog ヘッダーコメントのタブ整列

_, _ = fmt.Fprintf(w, "# logfilename\t\t\t\t\t\tmode\tcount\tsize\twhen\tflags\n")

ヘッダーのタブ数がハードコードされており、logPattern の長さ（ホームディレクトリのパスに依存）によっては実際のデータ行と列が揃わない場合があります。text/tabwriter を使えば自動整列が可能ですが、newsyslog 自体がヘッダーコメントを無視するため実害はありません。見栄えの問題として記載します。

6. logCmd の init() の配置場所

cmd/log.go の init() で logCmd.AddCommand(logSetupRotationCmd) を実行していますが、rootCmd.AddCommand(logCmd) は cmd/root.go の init() にあります。cobra のサブコマンド登録が 2 つのファイルに分散しているのは現在の規模では問題ありませんが、サブコマンドが増えた場合の可読性を考慮すると、将来的にはコメントで関係性を明示するとよいかもしれません。

7. B フラグの意味の正確性

52行目のコメント:

// Flags: G=glob pattern, N=no signal to any process, B=no rotation message in log

newsyslog(5) において B フラグは "don't write a message to the log file when it is rotated" の意味です。コメントの「no rotation message in log」は正確ですが、もう少し明確に「don't insert a rotation-marker line into the new log file」と書くと、なぜこのフラグが必要か（launchd が出力するログに余計な行を混ぜたくない）がより伝わります。

❓ question

8. N フラグ（no signal）と実行中ジョブの関係

newsyslog の N フラグは「ローテーション後にプロセスにシグナルを送らない」意味ですが、launchd ジョブが長時間実行中の場合、ファイルディスクリプタが古いファイルを指し続ける可能性はないでしょうか？ launchd の StandardOutPath / StandardErrorPath はジョブ実行ごとにファイルを開き直すため、短時間ジョブなら問題ないという理解で合っていますか？長時間実行ジョブの場合の動作についてコメントがあると安心です。

9. x-man-page:// URL スキームの互換性

README の両バージョンで [newsyslog(8)](x-man-page://newsyslog) というリンクを使用していますが、x-man-page:// スキームは macOS のターミナル.app 固有の機能です。GitHub 上で表示した場合、このリンクは機能しません。ウェブ上の man page（例: https://www.freebsd.org/cgi/man.cgi?newsyslog）へのリンクに変更するか、リンクなしのプレーンテキストにすることを検討されましたか？

👏 praise

10. newsyslog の活用は優れた設計判断

macOS 標準の newsyslog を活用することで、ldcron 自体にローテーションロジックを持たせる必要がなく、launchd 完全互換の設計原則に沿った素晴らしい判断です。ldcron をアンインストールしても newsyslog の設定はそのまま動作し続けます。

11. logDirPath() の分離リファクタリングが適切

paths.go で logDirPath()（パス構築のみ）と logDir()（パス構築 + ディレクトリ作成）を分離したリファクタリングは適切です。setup-rotation コマンドはディレクトリ作成が不要なため、この分離により不要な副作用（ディレクトリの自動作成）を回避しています。

12. テストで t.Cleanup を使用した適切な後処理

テストで t.Cleanup(func() { cmd.SetOut(nil) }) を使ってグローバルな cobra.Command の状態をリセットしている点は、テスト間の副作用を防ぐ良い実践です。

13. カスタムヘルプテキストへの適切な統合

cmd/root.go のカスタムヘルプテキストに log setup-rotation コマンドと「Log rotation」セクションを追加しており、ユーザーが機能を発見しやすくなっています。

[s4na]

Code Review (Google Style)

TL;DR

newsyslog を活用したログローテーション機能は、macOS 標準ツールを活かした堅実な設計であり、launchd 完全互換の原則にも沿っています。前回2ラウンドの指摘事項（logDirPath() の分離、t.Cleanup の追加、644 テスト追加、B フラグコメント改善）がすべて対応済みであり、全体としてコード品質を向上させる良い CL です。

• README 英語版と日本語版の構造に軽微な差異あり
• newsyslog ヘッダーコメントのタブ整列がパスの長さに依存する点

Details

🚨 issue

致命的な問題は見当たりません。マージをブロックする項目はありません。

💡 suggestion

1. README 英語版と日本語版の段落構成の差異

英語版 README.md では説明が2段落に分かれています（1段落目：設定内容、2段落目：newsyslog の自動実行について）。一方、日本語版 README.ja.md では1段落にまとめられています。情報量自体はほぼ同等ですが、日本語版には英語版にない「launchdはジョブ実行ごとにログファイルを開き直すため」という理由説明が含まれており、逆に英語版にはその記述がありません。

CLAUDE.md の README 管理規約（「常に同期する」）に照らすと、以下のいずれかに統一するのが望ましいです：

• 英語版にも「launchd reopens log files on each job execution」の理由を含める（日本語版に合わせる）
• または両方の段落構成を揃える

2. テストで出力全体の構造をより堅牢に検証する検討

現在のテストは strings.Contains で個別の要素（パターン、GNB、1024、644）を検証しています。これで実用上十分ですが、newsyslog 設定行全体のフォーマット（タブ区切り、フィールドの順序）が正しいかは検証できません。将来的にフィールド順序が変わった場合にテストが見逃す可能性があります。

正規表現でデータ行全体の構造を検証すると、より堅牢になります：

// 例: データ行全体の構造を正規表現で検証
dataLineRe := regexp.MustCompile(`(?m)^.+/\*\.log\t644\t3\t1024\t\*\tGNB$`)
if !dataLineRe.MatchString(output) {
    t.Errorf("data line format mismatch, got:\n%s", output)
}

ただし、現時点のテストでも十分な実用性があるため、これは将来の改善としての提案です。

🔧 nitpick

3. newsyslog ヘッダーコメントのタブ整列

_, _ = fmt.Fprintf(w, "# logfilename\t\t\t\t\t\tmode\tcount\tsize\twhen\tflags\n")

ヘッダーのタブ数（6個）がハードコードされており、logPattern（~/Library/Logs/ldcron/*.log）の長さ（ホームディレクトリのパスに依存）によっては、実際のデータ行と列が揃わない場合があります。text/tabwriter を使えば自動整列が可能ですが、newsyslog 自体がヘッダーコメントを無視するため実害はありません。見栄えの問題として記載します。

4. カスタムヘルプテキストの log コマンド表記

cmd/root.go の80行目：

  log     setup-rotation                   Print newsyslog config for log rotation

他のコマンド（add, list, remove, run）は引数を <id> や <schedule> のように示していますが、log はサブコマンド名 setup-rotation をそのまま表示しています。現時点ではサブコマンドが1つだけなので問題ありませんが、将来サブコマンドが増えた場合の表記方針を念頭に置いておくと良いかもしれません。

❓ question

5. 長時間実行ジョブでの N フラグの動作

newsyslog の N フラグは「ローテーション後にプロセスにシグナルを送らない」意味ですが、launchd で StandardOutPath / StandardErrorPath を指定した長時間実行ジョブの場合、ファイルディスクリプタが古いファイル（ローテーション前のファイル）を指し続ける可能性はないでしょうか？ cron 的な短時間ジョブ（実行ごとにファイルを開き直す）なら問題ないという理解ですが、ldcron の設計上、長時間実行ジョブは想定外という前提で合っていますか？

Long description や README にこの前提を一言添えると、将来のユーザーにとって有益かもしれません。

👏 praise

6. newsyslog の活用は優れた設計判断

macOS 標準の newsyslog を活用することで、ldcron 自体にローテーションロジックを持たせる必要がなく、launchd 完全互換の設計原則に沿った素晴らしい判断です。ldcron をアンインストールしても newsyslog の設定はそのまま動作し続けます。

7. 前回レビュー指摘の対応が適切

logDirPath() の分離リファクタリング、t.Cleanup によるグローバル状態のリセット、644 のテスト追加、B フラグのコメント改善（「don't insert a rotation-marker line into the new log file」）など、前回2ラウンドの指摘がすべて丁寧に対応されています。

8. テストカバレッジが適切

出力内容の主要な要素（パターン、フラグ、サイズ、モード）をすべて検証するテストが書かれており、適切なカバレッジです。

9. README の両言語同時更新

CLAUDE.md のガイドラインに従い、英語・日本語の README を同時に更新している点は良い習慣です。

10. logDirPath() と logDir() の分離が適切

paths.go で logDirPath()（パス構築のみ）と logDir()（パス構築 + ディレクトリ作成）を分離したリファクタリングは適切です。setup-rotation コマンドはディレクトリ作成が不要なため、この分離により不要な副作用を回避しています。

[s4na]

Code Review (Google Style)

TL;DR

軽微な指摘のみ。マージ可能な状態です。

Details

🚨 issue

特になし。マージをブロックする致命的な問題は見当たりません。

💡 suggestion

特になし。

🔧 nitpick

1. newsyslog ヘッダーコメントのタブ整列がホームディレクトリの長さに依存する

cmd/log.go 51行目:

_, _ = fmt.Fprintf(w, "# logfilename\t\t\t\t\t\tmode\tcount\tsize\twhen\tflags\n")

ヘッダーのタブ数がハードコードされているため、ホームディレクトリのパスの長さによっては実際のデータ行と列が揃わない場合があります。text/tabwriter を使えば自動整列できますが、newsyslog 自体がヘッダーコメントを無視するため実害はありません。見栄えの問題として記載します。

2. logCmd の RunE でヘルプ表示に cmd.Help() を使用

cmd/log.go 14行目:

RunE: func(cmd *cobra.Command, args []string) error {
    return cmd.Help()
},

cobra では RunE を設定せずに logCmd をサブコマンドのみの親コマンドとすると、引数なし実行時にデフォルトでヘルプが表示されます。ただし、他のコマンド（rootCmd）でも同様のパターンを使っているため、プロジェクト内の一貫性は保たれています。

❓ question

3. 長時間実行ジョブでの N フラグの挙動

newsyslog の N フラグは「ローテーション後にプロセスにシグナルを送らない」意味ですが、launchd ジョブが長時間実行中の場合、ファイルディスクリプタが古いファイルを指し続ける可能性について — launchd の StandardOutPath/StandardErrorPath はジョブ実行ごとにファイルを開き直すため短時間ジョブでは問題ないという理解で合っていますか？長時間ジョブについての注意書きがあると安心です（前回レビューでも指摘あり）。

👏 praise

4. newsyslog の活用は優れた設計判断

macOS 標準の newsyslog を活用することで、ldcron 自体にローテーションロジックを持たせる必要がなく、launchd 完全互換の設計原則に沿った素晴らしい判断です。

5. logDirPath() の分離リファクタリングが適切

paths.go で logDirPath()（パス構築のみ）と logDir()（パス構築 + ディレクトリ作成）を分離しており、setup-rotation コマンドが不要な副作用（ディレクトリ作成）を回避しています。

6. テストカバレッジが充実

出力内容の主要な要素（パターン、フラグ、サイズ、モード）すべてを検証し、t.Cleanup でグローバル状態をリセットしている点は良い実践です。

7. README の両言語同時更新

CLAUDE.md のガイドラインに従い、英語・日本語の README を同時に更新し、情報量も同期されています。