Skip to content

リファクタリング #323

New issue
New issue
Closed
Closed
リファクタリング#323
@Kewton

Description

@Kewton
Kewton
opened on Feb 20, 2026

Note: このIssueは 2026-02-21 にレビュー結果（Stage 1, Stage 3, Stage 5, Stage 7）を反映して更新されました。
詳細: dev-reports/issue/323/issue-review/

概要

src/lib/auto-yes-manager.ts（706行）の複雑度を下げるリファクタリング。特に pollAutoYes() 関数（139行、条件分岐14個、責務7つ）を責務別に分割し、テスタビリティを向上させる。

背景・動機

  • pollAutoYes() 関数が139行・7責務を担っており、単体テストが困難
    • タイマー停止確認、auto-yes有効性チェック、tmux出力キャプチャ、ANSI処理、thinking状態検出、停止条件チェック、プロンプト検出・自動応答の7責務が1関数に集中
  • 外部依存が6つ（captureSessionOutput, detectThinking, detectPrompt, resolveAutoAnswer, sendPromptAnswer, CLIToolManager）あり、全てモック必須
  • stopCheckBaselineLength の初期値 -1 と3パターン分岐（> = <）のデルタベース判定が不明瞭
  • pollerState の存在確認が複数箇所で重複（L457, L606, L680等）
  • グローバルstate（globalThis）依存でテスト時の状態リセットが困難

リファクタリング方針

pollAutoYes() を責務別に分割し、各関数を個別にテスト可能にする。

主要な変更点

  1. pollAutoYes() の責務分割（139行 → 4-5関数）

    • validatePollingContext() - ポーリング前提条件チェック（pollAutoYes()冒頭のL457-465に相当する「pollerState存在確認 + autoYes有効性チェック」のみを抽出。startAutoYesPolling()側のworktreeId検証やMAX_CONCURRENT_POLLERS制限チェックは含まない）
    • captureAndCleanOutput() - tmux出力取得・ANSIクリーニング（captureSessionOutput呼び出し + stripAnsiによるANSIエスケープ除去までを担う。thinkingチェック用のウィンドウイング処理（L492のsplit→slice→join）やdetectThinking呼び出しは含まない。戻り値はcleanOutput文字列）
    • processStopConditionDelta() - 停止条件のデルタベース判定（注意: 既存のL409 checkStopCondition(worktreeId, cleanOutput) 関数（Issue #314で追加済み、@internal export）は正規表現パターンマッチングを担う別関数として存続する。本関数はデルタ計算（baseline比較によるnewContent抽出）を行い、内部で既存checkStopCondition()を呼び出す上位関数。引数設計: pollerStateを引数として受け取る設計とする。これにより、テスト側でAutoYesPollerState型のオブジェクトリテラルを直接生成して渡せるため、startAutoYesPolling()やsetTimeout副作用を伴わないタイマー非依存テストが自然に記述可能となる）
    • detectAndRespondToPrompt() - プロンプト検出・重複チェック・自動応答・後処理（具体的にはプロンプト検出（detectPrompt呼び出し）、no-promptリセット（promptKey null化）、重複チェック（isDuplicatePrompt判定）、回答解決（resolveAutoAnswer呼び出し）、tmux送信（sendPromptAnswer呼び出し）、タイムスタンプ更新、エラーカウントリセット、promptKey記録、ログ出力、クールダウンスケジューリングの一連の処理を担う。現在のL527-582に相当する約55行の処理を包含するため、「各関数が単一責務（20-40行）」の目標を超過する可能性があるが、これらのステップは「プロンプトを検出して応答する」という一連のフローとして凝集度が高く、途中で分割すると状態受け渡しが複雑化するため、許容範囲内とする。ただし、実装時にsendAndRecordResponse()等のサブ関数への追加分割が自然な場合は、その判断に委ねる）

  2. 停止条件ロジックの抽出

    • stopCheckBaselineLength の -1 初期化・3パターン分岐を専用の関数群またはクラスで隠蔽
    • setBaseline(), shouldCheck(), getDelta() で意図を明確化
    • 設計選択肢: (A) StopConditionCheckerクラスとして導入する場合、pollerState内にインスタンスを保持する方式を採用し、globalThisベースの状態管理との二重管理を避けること。(B) クラスではなく関数群（setBaseline, shouldCheck, getDelta等）として実装し、状態はpollerState内のフィールドとして管理する方式も可。auto-yes-manager.ts全体が関数ベースのモジュール設計であるため、プロジェクトの設計パターンとの整合性を考慮して選択すること
    • AutoYesPollerStateへの影響: 設計選択肢(B)を採用する場合、AutoYesPollerStateインターフェースへのフィールド追加が発生する可能性がある。既存のstopCheckBaselineLengthフィールドの再利用で十分か、追加フィールド（例: stopCheckEnabled等）が必要かを検討し、AutoYesPollerStateの変更は最小限に留めること。変更する場合はglobalThis.__autoYesPollerStatesの型宣言、startAutoYesPolling()内の初期化処理、tests/integration/auto-yes-persistence.test.tsのglobalThis参照テストへの波及を確認すること

  3. 分割関数の引数設計方針

    • processStopConditionDelta(), validatePollingContext(), captureAndCleanOutput(), detectAndRespondToPrompt() はいずれも pollerState を引数として受け取る設計とする
    • これにより、テスト側で各関数を直接呼び出す際に AutoYesPollerState 型のオブジェクトリテラルを生成して渡すことが可能となり、startAutoYesPolling() 経由の状態セットアップや setTimeout 副作用を回避したタイマー非依存テストが実現できる

  4. pollerState 存在確認の共通化

    • 重複する存在確認パターンをヘルパー関数に抽出

Before / After

Before:

  • pollAutoYes(): 139行、責務7つ、条件分岐14個
  • テスト時に6つの外部依存を全てモック必要
  • stopCheckBaselineLength の判定ロジックが不透明

After:

  • 各関数が単一責務（20-40行目安、detectAndRespondToPromptは凝集度の高い一連フローとして最大55行程度を許容）
  • 分割された各関数を個別にテスト可能
  • 停止条件ロジック（クラスまたは関数群）で停止条件判定が自己文書化

影響範囲

変更対象ファイル

ファイル 変更内容
src/lib/auto-yes-manager.ts pollAutoYes() の責務分割、停止条件ロジック抽出
tests/unit/lib/auto-yes-manager.test.ts 既存テストの維持 + 分割関数の個別テスト追加 + importブロック拡張（新規@internal export関数の追加に伴い、import対象が22個→26-30個程度に増加） + checkStopCondition/processStopConditionDelta describeブロック整理
CLAUDE.md auto-yes-manager.tsモジュール説明の更新（分割関数名、設計選択結果の追記）
docs/implementation-history.md Issue #323エントリの追加（リファクタリング概要・主要変更ファイル・設計書リンク）

関連コンポーネント（変更なし）

  • src/lib/auto-yes-resolver.ts - 依存先（変更不要）
  • src/hooks/useAutoYes.ts - クライアント側（変更不要、auto-yes-manager.tsからの直接importなし）
  • src/app/api/worktrees/[id]/auto-yes/route.ts - API（変更不要、exportされた関数のみ使用）
  • src/app/api/worktrees/[id]/current-output/route.ts - Current Output API（変更不要）
  • src/lib/session-cleanup.ts - セッションクリーンアップ（変更不要、stopAutoYesPollingのみ使用）
  • server.ts - カスタムサーバー（変更不要、stopAllAutoYesPollingのみ使用）

テスト方針

既存テストの維持

  • 既存のvi.useFakeTimers + vi.advanceTimersByTimeAsyncベースの統合的テストは全てパスすること
  • リファクタリング作業中に既存テストが失敗した場合、テストの修正は最小限にとどめ、テストの意図は変えないこと

分割関数の個別テスト

  • 分割された各関数（validatePollingContext, captureAndCleanOutput, processStopConditionDelta, detectAndRespondToPrompt等）の個別テストはタイマー非依存（直接関数呼び出し）で記述すること
  • 各関数はpollerStateを引数として受け取る設計であるため、テスト側でAutoYesPollerState型のオブジェクトリテラルを直接生成して渡すことで、startAutoYesPolling()やsetTimeout副作用を回避できる
  • これにより、既存の統合的テスト（pollAutoYes全体の動作確認）と新規の単体テスト（各関数の個別動作確認）が補完関係になり、リファクタリング後のテスト安定性が向上する

processStopConditionDelta() のテスト設計

  • processStopConditionDelta() は内部で checkStopCondition() を呼び出す上位関数であり、同一モジュール内の関数モック化はVitestのvi.spyOnでは制約がある（ESModulesのnamed exportモック化が困難な場合がある）
  • 推奨アプローチ: checkStopCondition() をモック化せず、processStopConditionDelta() の入出力（pollerState.stopCheckBaselineLengthの変化、checkStopConditionの副作用としてのautoYesState変更）を検証する統合的な単体テストとする
  • processStopConditionDelta() はpollerStateを引数として受け取るため、テスト側でAutoYesPollerState型のオブジェクトリテラルを生成して渡すことが可能

受入条件

  • 既存テスト（tests/unit/lib/auto-yes-manager.test.ts）が全てパスすること
  • pollAutoYes() が4-5個の単一責務関数に分割されていること
  • 停止条件チェックロジックがクラスまたは専用関数群に抽出されていること
  • 分割関数の命名が既存関数と衝突しないこと（特に既存 checkStopCondition() との区別）
  • pollerState 存在確認の重複が解消されていること
  • 機能変更がないこと（外部インターフェースは維持）
  • 分割された各関数（validatePollingContext, captureAndCleanOutput, processStopConditionDelta, detectAndRespondToPrompt等）に対する個別テストが追加されていること
  • 分割関数の個別テストはタイマー非依存（直接関数呼び出し）で記述すること
  • 分割関数はpollerStateを引数として受け取る設計であること
  • CLAUDE.mdのauto-yes-manager.tsモジュール説明が更新されていること（分割関数名、設計選択結果の追記）
  • docs/implementation-history.mdにIssue #323のエントリが追加されていること

レビュー履歴

Stage 1 レビュー反映 (2026-02-21)

  • F001 (must_fix): checkStopCondition() → processStopConditionDelta() に命名変更。既存L409 checkStopCondition() との衝突を回避し、両者の関係を明記
  • F002 (should_fix): StopConditionCheckerクラスの代替案として関数群方式を追記。設計選択肢(A)(B)を提示し、プロジェクトの設計パターンとの整合性を考慮するよう記載
  • F003 (should_fix): validatePollingContext() の責務範囲を明確化。pollAutoYes()冒頭のL457-465（pollerState存在確認 + autoYes有効性チェック）のみを抽出する旨を明記
  • F004 (should_fix): 受入条件に「分割された各関数に対する個別テストが追加されていること」を追加
  • F005 (should_fix): captureAndCleanOutput() の責務範囲を明確化。captureSessionOutput + stripAnsiまでを担い、thinkingチェック用ウィンドウイングやdetectThinking呼び出しは含まない旨を明記。戻り値はcleanOutput文字列

Stage 3 影響範囲レビュー反映 (2026-02-21)

  • IF003 (must_fix): 受入条件に「分割関数の個別テストはタイマー非依存（直接関数呼び出し）で記述すること」を追加。テスト方針セクションを新設
  • IF001 (should_fix): 変更対象ファイルテーブルで、テストファイルの変更内容を詳細化（importブロック拡張、describeブロック整理を明記）
  • IF002 (should_fix): 停止条件ロジック設計選択肢にAutoYesPollerStateへの影響を明記。変更時の波及先（globalThis型宣言、初期化処理、永続性テスト）を記載
  • IF005 (should_fix): テスト方針セクションにprocessStopConditionDelta()とcheckStopCondition()の同一モジュール内モック制約と推奨アプローチを記載
  • IF006 (should_fix): 変更対象ファイルテーブルにCLAUDE.mdを追加。受入条件にCLAUDE.md更新を追加

Stage 5 レビュー反映 (2026-02-21)

  • S2F001 (should_fix): テストファイルのimport数を「17個→21-25個程度に増加」から「22個→26-30個程度に増加」に修正。現在のテストファイルのimport対象が15関数+6定数+1型=22個であることを反映
  • S2F002 (should_fix): detectAndRespondToPrompt() の責務範囲を具体化。プロンプト検出→no-promptリセット→重複チェック→回答解決→tmux送信→タイムスタンプ更新→エラーカウントリセット→promptKey記録→ログ出力→クールダウンスケジューリングの一連フロー（約55行）を包含し、20-40行目標の超過を凝集度の観点から許容範囲とする旨を明記。After欄にも許容範囲の注記を追加
  • S2F003 (nice_to_have): thinkingチェック処理（L492-496）の所属関数については実装時の判断に委ねるためスキップ

Stage 7 影響範囲レビュー反映 (2026-02-21)

  • S2IF001 (should_fix): processStopConditionDelta()の引数設計方針として「pollerStateを引数として受け取る」ことをIssue本文に明記。processStopConditionDelta()の説明、主要な変更点セクションの「分割関数の引数設計方針」項目、テスト方針セクション、受入条件に反映
  • S2IF002 (nice_to_have): 変更対象ファイルテーブルにdocs/implementation-history.mdを追加。受入条件にもエントリ追加を追加
  • S2IF003 (nice_to_have): E2E手動確認手順はスコープ外のためスキップ。自動テストでの品質担保で十分

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions