Skip to content

コード品質レビュー: agent loop / gateway / CLI 周辺の改善提案 #1

@RNA4219

Description

@RNA4219

背景・目的

現状のコードベースを一通り確認したうえで、

  • 既に機能している品質ゲート・テストの強みを言語化する
  • 今後の拡張やコントリビューションの観点から「詰まりやすそうな箇所」「壊しやすい箇所」を特定する
  • 小さく着手できる改善タスクに分解する

ことを目的に、コード品質レビューをまとめました。

この Issue では 設計の大枠は変えず、保守性と変更容易性を上げる ためのリファクタリング候補を整理しています。


現状の品質ゲート(ざっくり把握)

  • メインの品質ゲートは Go に集中している:

    • make check が入口になっており、内部で

      • 依存関係チェック(deps)
      • go fmt
      • go vet ./...
      • go test ./...
        を実行する構成
    • CI も同等のコマンドを実行しており、ローカルと CI で挙動の差は小さい

  • Python / Node / TS などについては:

    • mypy / ruff / pytest / node:test などのツールは
      「標準の品質ゲート」としてはまだ使っていないように見える
    • 現状は「コアは Go / 周辺スクリプトは緩め」というバランス

→ すぐに問題になる状況ではないが、
 将来的に非 Go 部分が増える場合は、どこまでを品質ゲートに乗せるか一度方針決めが必要 になりそうです。


所見まとめ(強みと課題)

強み

  • pkg/gateway/server_test.go

    • 認証・部分更新・競合(楽観ロック)といった、設定 API まわりの主要リスクを押さえたテストが揃っている
    • 「設定 API の回帰」を検知する安全網としてはかなり強い
    • テストが仕様ドキュメント的な役割も兼ねている
  • pkg/agent/loop_test.go

    • コンテキスト圧縮・再試行・タイムアウトなど、LLM エージェント特有の境界条件がテストでカバーされている
    • “壊れやすくて気付きにくい” 挙動(N 回目で成功するケース・圧縮後の再利用など)に対する安全網になっている
  • 例外・エラー設計

    • fmt.Errorf("...: %w", err) によるラップが一貫しており、スタックトレースを追いやすい
    • CLI 側で回復不能エラーは終了、サービス層では上位に伝播、という大まかな方針が整理されている

課題(≒詰まりやすそうな箇所)

  1. pkg/agent/loop.gorunLLMIteration に責務が集中している
  2. cmd/clawdroid/main.go にコマンド実装が集約しつつあり、規模拡大時に詰まりそう
  3. pkg/gateway/server_test.go が巨大化しており、テスト追加時の衝突・重複リスクがある

いずれも「今すぐ壊れているわけではない」が、

  • 変更が重なったときにコンフリクトが増えやすい
  • 新しい貢献者が入りづらい
  • 挙動の理解コストが上がっていく

という意味で、長期保守の観点から優先度はそこそこ高い と感じました。


改善提案(生産的な打ち手)

1. pkg/agent/loop.go の責務分離

現状イメージ

runLLMIteration が以下を全部面倒見ているような構造になっている:

  • LLM への 1 ステップ実行要求
  • 再試行ロジック(リトライ回数・バックオフなど)
  • コンテキスト圧縮 / トークン管理
  • ツール実行(外部 API 呼び出し)
  • ログ / トレースの記録
  • ステート・履歴の保存

リスク

  • 何か 1 つ仕様を変えたいだけでも、この大きな関数を触る必要が出てくる
  • リトライ戦略の変更・トレースの改善・ツール呼び出しの並列化など、
    性質の違う変更同士がコンフリクトしやすい

提案

  • 挙動互換を維持したまま、内部関数や小さな構造体に分解したい:

    • 例: retryLLMStep, compressContextIfNeeded, executeTools, persistState など
  • 外向きのシグネチャは極力変えず、「中のワークフロー」だけを分解するイメージ

期待効果

  • 変更範囲を局所化できる(1 か所を直して別の挙動を壊すリスクを減らせる)
  • 将来、LLM 切り替え・リトライ方針の変更・メトリクス追加などをするときのコストが下がる
  • レビューする側も、ロジック単位で読めるようになり負担が減る

2. cmd/clawdroid/main.go のコマンド単位分割

現状イメージ

  • エントリポイントの main.go に、複数サブコマンドの実装や引数パースがまとまりつつある
  • サブコマンドが増えるほどロジックが雪だるま式に積み上がる構造

リスク

  • 新しいサブコマンド追加が心理的に重くなる(= 書き足すと main.go がさらにカオスになる)
  • CLI 挙動のレビューがしづらくなる(差分が大きくなりがち)
  • --help の挙動変更などをうっかり紛れ込ませる可能性が高まる

提案

  • コマンドごとにファイルを切り出す:

    • 例: cmd/clawdroid/root.go, cmd/clawdroid/cmd_run.go, cmd/clawdroid/cmd_eval.go など
  • 引数パース部分とビジネスロジック部分を、ある程度レイヤー分けする

期待効果

  • コマンド追加・削除・変更の差分が小さくなり、レビューしやすくなる
  • CLI の仕様を追いやすくなり、ドキュメント代わりにも使える
  • 他の人がサブコマンドを追加しやすくなる(コントリビューションの敷居を下げられる)

3. pkg/gateway/server_test.go の観点別分割

現状イメージ

  • 設定 API に関するテストが 1 ファイルに集中しており、

    • 認証
    • 部分更新
    • 競合
    • そのほか周辺ケース
      などがすべて詰め込まれている

リスク

  • どこに新しいテストを追加すべきか迷いやすい
  • 類似シナリオの重複が起きやすい
  • コミットが増えると、このファイルだけコンフリクト多発ゾーンになりやすい

提案

  • 観点ベースでファイルを分割する:

    • 例: server_auth_test.go, server_update_test.go, server_conflict_test.go など
  • テスト関数名はできるだけ維持し、履歴の追跡がしやすいようにする

期待効果

  • テスト追加時の判断がシンプルになる(「これは auth 系だからこのファイル」など)
  • コンフリクトが減り、Issue / PR の分割もしやすくなる
  • 今後仕様が増えたときに、どの観点が手薄か把握しやすくなる

優先度と進め方の提案

個人的には、以下の順番で進めるのがコスパ良さそうです:

  1. pkg/agent/loop.go の責務分離

    • ここを整理しておくと、今後の機能追加・改善のたびに悩まなくて済む
    • LLM 周りは今後も変更頻度が高い領域になりがちなので、先に基礎を固めておきたい
  2. cmd/clawdroid/main.go の分割

    • CLI は「入り口」なので、読みやすさ・変更容易性のメリットが大きい
    • 将来 CLI を触る人のためにも、今のうちにパターンを作っておく意味がある
  3. pkg/gateway/server_test.go の観点別分割

    • 今すぐ困っているわけではないが、テストが増えたタイミングで急に辛くなるゾーン
    • 分割は挙動互換を保ったまま機械的に進めやすいので、空き時間タスクにも向いている

ToDo(提案)

ひとまず、こんな感じのチェックリストを想定しています。

  • A: pkg/agent/loop.go の責務分離(挙動互換・テストパスを前提)
  • B: cmd/clawdroid/main.go のコマンド単位分割(CLI 互換・--help 出力不変)
  • C: pkg/gateway/server_test.go の観点別分割(テスト名維持・カバレッジ維持)

参考: レビュー時に実行したコマンド

再現可能性のため、今回確認に使った主なコマンドを残しておきます。

# 品質ゲート / CI の挙動確認
rg -n "make check|go vet|go test|mypy|ruff|pytest|node:test" \
  -- Makefile README.md README.ja.md .github/workflows/*.yml

# エラーラップ / エラー判定の利用状況確認
rg -n "fmt\.Errorf\(|errors\.Is|errors\.As" pkg cmd

# 生成処理・テストの再実行
GOTOOLCHAIN=auto make generate
GOTOOLCHAIN=auto go test ./...

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions