refactor(PLAN65): cmd_scale の段階を前提の検査と段階の実行へ分ける (#192)

issues/PLAN65_scale-compose-path-impl2.md

@@ -0,0 +1,143 @@
+# PLAN65 実装 2 本目: `cmd_scale` の段階を分ける
+
+## 関連リンク
+
+- 課題: devbasex/devbase#192（この Pull Request が閉じる）
+- 要求と受け入れ条件: `issues/PLAN65_scale-compose-path.md`
+- 設計: `issues/PLAN65_scale-compose-path-design.md`（設計 PR #225。承認済み）
+- 1 本目: #231（`release/v3.7.0` へマージ済み。計画は `issues/PLAN65_scale-compose-path-impl1.md`）
+- release PR: #212（base は `release/v3.7.0`）
+- この計画が扱うのは設計の「実装の分け方」の **2 本目だけ**である
+
+## モード
+
+`standard`（本番の振る舞いを変えない構造変更で、対象に 1 本目で入れた現状固定テストが十分にある）。
+
+## 目的と非目的
+
+達成したい状態:
+
+- `cmd_scale` が「2 つの検査 → 段階 → 後処理」の 3 段に読め、`cmd_up`（`_run_pre_up_checks` →
+  `_run_deploy_pipeline` → 後処理）と並べて読める（設計の決定 8）
+
+やらないこと:
+
+- 段階ごとに 5 つの関数へ分けること（決定 8 が退けた案）
+- `_run_deploy_pipeline` と `_run_scale_pipeline` の統合（決定 8 が退けた案）
+- 後処理（`_push_bao_token`・`./deploy`・完了のログ）を関数へ出すこと（決定 8。`cmd_up` も本体に持つ）
+- 段階の番号の文字列・ログの文言・失敗の扱い・後処理の順序の変更（決定 5・7）
+- `cmd_scale` 以外の本番コードの変更（`_SCALE_COMPOSE_FILE` の有無の確認の重複などは範囲外）
+- `cmd_scale` から `_report_missing_repos` / `_apply_window_titles` を呼ぶこと（#224）
+
+## 前提
+
+- 前提 1: 実環境のプロジェクトで `devbase up` / `scale` / `login` を実行しない。確認はテスト（`subprocess.run` と
+  段階の関数を差し替えた水準）で行う
+- 前提 2: `release/v3.7.0` を base にした Pull Request では CI が動かない（#216）。`uv run --locked pytest tests/ -q` を
+  手元で実行し、結果と終了コードを Pull Request 本文の Test plan へ載せる
+- 前提 3: 変更前の全件は `2889 passed`、exit=0（この作業ツリーの起点 `eaa9e5d` + 空コミットで取り直した）
+- 前提 4: 行数は `ast` で `cmd_scale` の `def` の行から関数の最後の行までを数える（`end_lineno - lineno + 1`）。
+  あわせて設計の検証手段の「`def` から次の `def` まで」も並べて載せる
+
+## 受け入れ条件（この Pull Request で満たすもの）
+
+設計文書の「受け入れ条件とどちらの Pull Request が対応するか」の表から、2 本目で満たすものを写す。
+
+- [ ] D-3: `cmd_scale` の本体が 40 行以下になる（起点で 92 行）。抽出した段階の関数（`_run_scale_pipeline`）が
+  `[1/5]`〜`[5/5]` のログ文字列をそのまま持つ
+  - 結果: 段階のログ文字列はすべて `_run_scale_pipeline` へ移った（満たす）。行数は `def`〜最後の行で 92 → 54、
+    要求の「86 行」と同じ数え方（シグネチャとドキュメント文字列を除く）で 51 行。空行・コメントを除けば 40 行。
+    一度は空行を削り 2 つの文を約 100 文字の 1 行へ詰めて 40 行に合わせたが、検査の持ち場で元の書式へ戻した。
+    意図（段階の命名と `cmd_up` との形の一致）は満たしており、数字のために読みやすさを犠牲にしない。
+    要求の文書の D-3 にこの判断を書き足した
+  - 検証: `ast` で行数を数える / `grep -n "/5\]" lib/devbase/commands/container.py` の 6 行がすべて `_run_scale_pipeline` の範囲にある
+- [x] D-4: `cmd_scale` と `cmd_up` の段階の対応が設計文書の「段階の対応（変更後）」の表と一致する。段階の番号の
+  文字列（`[2.5/5]` を含む）は変えない
+  - 検証: 設計の表と `grep -n "/5\]"` の出力を並べる。`_check_scale_request` と `_run_scale_pipeline` の契約（設計の
+    「新設・変更する関数の契約」の表）を単体テストで固定する
+
+退行しないこと（1 本目で満たした条件を書き換えずに緑のまま通す）:
+
+- [x] `tests/commands/test_container_scale_order.py` の既存のテスト（順序・範囲・停止しないこと・`./deploy` の失敗の後も
+  続けること・`project_name` の明示・`cmd_login` の既定の引数ほか）を**書き換えずに**各コミットで通す — B-1〜B-5 / C-1 / C-2 / E-2 / E-3
+- [x] 既存の 4 か所の `cmd_scale` のテストを書き換えずに通す — C-3
+- [x] `uv run --locked pytest tests/ -q` の全件が変更の前後で同じ（足したテストの分だけ増える）— C-4 / E-1
+
+## 代替案と採否
+
+| 案 | 内容 | 採否 | 理由 |
+| --- | --- | --- | --- |
+| A | `_check_scale_request` と `_run_scale_pipeline` の 2 つを抽出し、後処理は本体に残す | 採用 | 設計の決定 8 |
+| B | 段階ごとに 5 つの関数へ分ける | 不採用 | 決定 8 が退けた |
+| C | `_run_deploy_pipeline` と統合して引数で分岐 | 不採用 | 決定 8 が退けた |
+| D | 1 本目の cross-refactoring で 3 者が挙げた extract_method の形をそのまま使う | 参考のみ | 決定 8 が優先する。関数の名前・境界・シグネチャは設計の「新設・変更する関数のシグネチャ」に従う |
+
+## 不変条件
+
+- `project.yml` の `scale` は、グループの不一致と `new_scale` の不適のときは書き換わらない
+- 起動が 0 以外のとき `Failed to start new containers` を出して 1 を返す（例外にしない）
+- 構成生成・既定のサービスの解決・ready 待ちの失敗は `DevbaseError` として `cmd_scale` の `except` が `Scale failed: ...` を出す
+- `[1/5]` → `[2/5]` → `[2.5/5]` → `[3/5]` → `default_services` → `[4/5]` → `[5/5]` → bao → `./deploy` の順
+
+## 互換性
+
+| 対象 | 変更 | 互換性の扱い |
+| --- | --- | --- |
+| `cmd_scale` のシグネチャ・戻り値・ログ | 変えない | 構造だけを変える |
+| 新設の 2 関数 | モジュール内の private 関数を足す | 公開インタフェースではない |
+
+## 修正対象
+
+- `lib/devbase/commands/container.py`（`cmd_scale` と、その直前に置く新設の 2 関数）
+- `tests/commands/test_container_scale_order.py`（新設の 2 関数の契約のテストを**追記**。既存のテストは書き換えない）
+
+## タスク分解
+
+### Task 1: `_check_scale_request` を抽出する
+
+- **対象ファイル:** `lib/devbase/commands/container.py`、`tests/commands/test_container_scale_order.py`
+- **変更内容:** `new_scale < 1`（error 1 行）と `new_scale <= current_scale`（warning + info 2 行）の判定を
+  `_check_scale_request(new_scale, current_scale) -> bool` へ移す。文言と出し分けは変えない
+- **満たす受け入れ条件:** D-4（前提の検査の段）・D-3 の一部
+- **進め方:** 契約のテスト（1 未満で False と error、現在以下で False と warning + info、上回れば True でログ無し）を
+  先に書き、関数が無いことで落ちるのを確かめてから抽出する。既存の現状固定テストが緑のままであることを確かめてコミット
+
+### Task 2: `_run_scale_pipeline` を抽出する
+
+- **対象ファイル:** 同上
+- **変更内容:** `[1/5]`〜`[5/5]`（`write_scale`・`ensure_volumes`・`ensure_network`・`_build_scaled_override`・
+  `default_services`・`docker_compose(['up', '-d', '--no-recreate', *services])`・`wait_for_containers_ready`）を
+  `_run_scale_pipeline(project_name, new_scale, current_scale, config, target, dev_service_name) -> Optional[Path]` へ移す。
+  起動が 0 以外なら `Failed to start new containers` を出して `None` を返す。それ以外の失敗は伝播する
+- **満たす受け入れ条件:** D-3・D-4
+- **進め方:** 契約のテスト（起動が 0 以外で `None` とエラーのログ、成功で生成物のパスを返し後処理を呼ばない）を先に書き、
+  関数が無いことで落ちるのを確かめてから抽出する。既存の現状固定テストが緑のままであることを確かめてコミット
+
+### Task 3: 本体の行数と段階の対応を確かめる
+
+- **対象ファイル:** 無し（検証のみ。必要なら本体のコメントを整える）
+- **変更内容:** `ast` で行数を数え、`grep -n "/5\]"` の行が `_run_scale_pipeline` の範囲にあることを確かめる。全件のテストを流す
+- **満たす受け入れ条件:** D-3・D-4、C-3・C-4
+- **進め方:** 検証のみ（テスト駆動は適用しない。数える対象が既にあるため）
+
+## 影響範囲
+
+- `devbase scale`（`bin/devbase` の dispatch → `cmd_scale`）。振る舞いは変えない
+
+## リスクと対処
+
+| リスク | 対処 |
+| --- | --- |
+| 抽出で `try` の範囲が変わり、`DevbaseError` の捕捉の範囲がずれる | `_run_scale_pipeline` の呼び出しから後処理までを本体の `try` に収める。既存の `default_services` の失敗・`_build_scaled_override` の例外のテストで確かめる |
+| テストが差し替える名前（`container.write_scale` など）を新しい関数が別の経路で引く | 新しい関数もモジュールの名前を実行時に引く。タスクごとに現状固定テストを通す |
+| 触る範囲 | 狭く（`cmd_scale` の 1 関数）、テストが厚い。「タスクごとにテストを通す」で足りる |
+
+## 切り戻し手順
+
+- 本番コードの変更は `container.py` の 1 関数の分割だけで、データの移行は無い。Pull Request を revert すれば戻る
+
+## 完了の定義
+
+- [x] D-4 を満たし、D-3 は意図を満たして数字は満たさない理由を記し、条件ごとに検証手段と結果が Pull Request 本文に対応している
+- [x] 既存の現状固定テストを書き換えずに、各コミットで緑
+- [x] `uv run --locked pytest tests/ -q` が exit=0

issues/PLAN65_scale-compose-path.md

@@ -212,6 +212,12 @@ lib/devbase/commands/container.py:1648
   `json.JSONDecodeError` を伝播する
 - [ ] **D-3: `cmd_scale` の本体が 40 行以下になる**（現状 86 行）。抽出した段階の関数が
   `[1/5]`〜`[5/5]` のログ文字列をそのまま持つ
+  - 2 本目（#232）での判断: この条件の意図は「長い関数の段階に名前を付け、`cmd_up` と形を揃える」ことで、
+    行数はその目安である。「86 行」は `def` から最後の行までの 89 行から、シグネチャの 2 行とドキュメント文字列の
+    1 行を除いた数（空行・コメントを含む）。同じ数え方で変更後は 51 行になり、40 行には届かない。
+    空行・コメントを除いたコードの行は 67 行から 40 行。40 行に合わせるには空行を削り文を 1 行へ詰めるか、
+    設計の決定 8 に無い 3 つ目の関数（後処理）を出す必要がある。**数字のために読みやすさを犠牲にせず、
+    設計からも外れない方を採り、40 行の数字は満たさないまま閉じる**
 - [ ] **D-4: `cmd_scale` と `cmd_up` の段階の対応が、設計文書の表と一致する。** 段階の番号の
   文字列（`[2.5/5]` を含む）は変えない
 

lib/devbase/commands/container.py

@@ -1594,6 +1594,74 @@ def cmd_profile_list(context: Optional[str] = None) -> int:
 # cmd_scale
 # ---------------------------------------------------------------------------
 
+def _check_scale_request(new_scale: int, current_scale: int) -> bool:
+    """``new_scale`` を受け付けるかを判定し、受け付けないときは案内を出す。
+
+    ``cmd_scale`` の前提の検査のうち、``_check_group_consistency`` の後に行う 2 つ
+    (1 未満・現在以下)。受け付けないときは ``project.yml`` を書き換える前に止まる。
+    """
+    if new_scale < 1:
+        logger.error("Scale must be at least 1")
+        return False
+
+    if new_scale <= current_scale:
+        logger.warning("New scale (%d) is not greater than current scale (%d)", new_scale, current_scale)
+        logger.info("To scale down, use 'devbase container down' first, then 'devbase container up' with desired scale")
+        return False
+
+    return True
+
+
+def _run_scale_pipeline(project_name: str, new_scale: int, current_scale: int,
+                        config, target: docker_context.DockerTarget,
+                        dev_service_name: str) -> Optional[Path]:
+    """``[1/5]``〜``[5/5]`` の本体。生成した override compose のパスを返す。
+
+    ``cmd_up`` の :func:`_run_deploy_pipeline` と対称の段 (PLAN65 決定 8)。``scale`` は
+    既存のコンテナを止めず、退避も取らない。起動が 0 以外で終わったときだけ
+    ``Failed to start new containers`` を出して ``None`` を返す。それ以外の失敗は
+    ``DevbaseError`` / ``DockerError`` のまま伝播する。後処理 (bao の token・``./deploy``)
+    は ``cmd_scale`` の本体が行う。
+    """
+    logger.info("[1/5] Updating %s: scale=%d -> %d...",
+                project_runtime.PROJECT_CONFIG_FILENAME, current_scale, new_scale)
+    project_runtime.write_scale(Path.cwd(), new_scale)
+
+    logger.info("[2/5] Ensuring volumes exist for scale=%d...", new_scale)
+    ensure_volumes(new_scale, project_name)
+
+    logger.info("[2.5/5] Ensuring network exists...")
+    ensure_network('devbase_net')
+
+    logger.info("[3/5] Generating scaled compose file...")
+    override_file = _build_scaled_override(new_scale, config, project_name, target)
+    logger.info("Generated: %s", override_file)
+    # up と同じく起動の対象を生成物の既定のサービスで明示する (PLAN65 決定 4)
+    services = default_services(override_file)
+
+    logger.info("[4/5] Starting new containers (%d..%d)...", current_scale + 1, new_scale)
+    logger.info("Using --no-recreate to avoid restarting existing containers...")
+
+    # 共通経路を通し、子プロセスの COMPOSE_PROFILES を打ち消す (PLAN65 決定 1)。
+    # docker_compose_up は check=True 固定で CalledProcessError が except DevbaseError を
+    # 素通りするため、check=False で終了コードを見る (決定 3)
+    result = docker_compose(['up', '-d', '--no-recreate', *services],
+                            compose_file=override_file, check=False)
+
+    if result.returncode != 0:
+        logger.error("Failed to start new containers")
+        return None
+
+    logger.info("[5/5] Waiting for new containers to be ready...")
+    wait_for_containers_ready(
+        container_prefix=dev_service_name,
+        scale=new_scale,
+        compose_file=override_file,
+        timeout=60
+    )
+    return override_file
+
+
 def cmd_scale(new_scale: int, project_name: str = None,
               context: Optional[str] = None) -> int:
     """Scale containers online without restarting existing ones"""
@@ -1618,53 +1686,15 @@ def cmd_scale(new_scale: int, project_name: str = None,
     logger.info("Scaling project '%s' from %d to %d containers (dev service: %s)",
                 project_name, current_scale, new_scale, dev_service_name)
 
-    if new_scale < 1:
-        logger.error("Scale must be at least 1")
-        return 1
-
-    if new_scale <= current_scale:
-        logger.warning("New scale (%d) is not greater than current scale (%d)", new_scale, current_scale)
-        logger.info("To scale down, use 'devbase container down' first, then 'devbase container up' with desired scale")
+    if not _check_scale_request(new_scale, current_scale):
         return 1
 
     try:
-        logger.info("[1/5] Updating %s: scale=%d -> %d...",
-                    project_runtime.PROJECT_CONFIG_FILENAME, current_scale, new_scale)
-        project_runtime.write_scale(Path.cwd(), new_scale)
-
-        logger.info("[2/5] Ensuring volumes exist for scale=%d...", new_scale)
-        ensure_volumes(new_scale, project_name)
-
-        logger.info("[2.5/5] Ensuring network exists...")
-        ensure_network('devbase_net')
-
-        logger.info("[3/5] Generating scaled compose file...")
-        override_file = _build_scaled_override(new_scale, config, project_name, target)
-        logger.info("Generated: %s", override_file)
-        # up と同じく起動の対象を生成物の既定のサービスで明示する (PLAN65 決定 4)
-        services = default_services(override_file)
-
-        logger.info("[4/5] Starting new containers (%d..%d)...", current_scale + 1, new_scale)
-        logger.info("Using --no-recreate to avoid restarting existing containers...")
-
-        # 共通経路を通し、子プロセスの COMPOSE_PROFILES を打ち消す (PLAN65 決定 1)。
-        # docker_compose_up は check=True 固定で CalledProcessError が except DevbaseError を
-        # 素通りするため、check=False で終了コードを見る (決定 3)
-        result = docker_compose(['up', '-d', '--no-recreate', *services],
-                                compose_file=override_file, check=False)
-
-        if result.returncode != 0:
-            logger.error("Failed to start new containers")
+        override_file = _run_scale_pipeline(project_name, new_scale, current_scale,
+                                            config, target, dev_service_name)
+        if override_file is None:
             return 1
 
-        logger.info("[5/5] Waiting for new containers to be ready...")
-        wait_for_containers_ready(
-            container_prefix=dev_service_name,
-            scale=new_scale,
-            compose_file=override_file,
-            timeout=60
-        )
-
         # 増やしたインスタンスにも bao の token を書く (PLAN54。既存のものは up で書いてある)
         _push_bao_token(project_name, new_scale, dev_service_name, compose_file=override_file,
                         start=current_scale + 1)