Skip to content

Commit abad035

Browse files
takemi-ohamaclaude
andcommitted
fix(up): 実コンテナ名 docker 取得+フォールバックと open_index 範囲検証
レビュー指摘 (PR #69) 対応: - 指摘A (major): PLAN31_3 §3 が約束した「compose バージョン差異への保険」を実装。 opener._query_container_name で docker compose ps --format json を実行し、 NDJSON / JSON 配列の両形式から実 .Name を取得。失敗・非0・例外・空は None に 握り潰し、resolve_container_name が決定的名へフォールバックする。runner 差替で テスト可能。 - 指摘B (minor): _maybe_open_editor に scale を渡し open_index を 1..scale で 検証。0・負数・scale 超過は warning を出して既定 (1) へフォールバックする。 DEVBASE_OPEN_INDEX 経由も同じ検証を通る。 756 passed (+8 tests)。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2f7511d commit abad035

4 files changed

Lines changed: 173 additions & 9 deletions

File tree

lib/devbase/commands/container.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -356,11 +356,15 @@ def _auto_snapshot() -> None:
356356

357357

358358
def _maybe_open_editor(project_name: str, open_flag: Optional[bool],
359-
open_index: Optional[int]) -> None:
359+
open_index: Optional[int], scale: int) -> None:
360360
"""`up` 完了後に dev コンテナへ接続したエディタを開く ([6/6])。
361361
362362
有効判定は ``open_flag`` (CLI ``--open``/``--no-open``) が優先、None なら env
363363
``DEVBASE_OPEN_EDITOR``。エディタ起動の成否は ``up`` の戻り値に影響させない。
364+
365+
``open_index`` は起動済みインスタンス範囲 ``1..scale`` 内である必要がある。
366+
0・負数・``scale`` 超過は存在しないコンテナ URI になり原因不明な起動失敗を招くため、
367+
警告を出して既定 (1) へフォールバックする。
364368
"""
365369
from devbase.editor import opener
366370

@@ -375,6 +379,14 @@ def _maybe_open_editor(project_name: str, open_flag: Optional[bool],
375379
except ValueError:
376380
open_index = 1
377381

382+
# 起動済みインスタンス範囲 (1..scale) の検証。範囲外は既定 (1) へフォールバック。
383+
if not (1 <= open_index <= scale):
384+
logger.warning(
385+
"open index %d is out of range (1..%d); falling back to 1",
386+
open_index, scale,
387+
)
388+
open_index = 1
389+
378390
dev_service_name = get_dev_service_name()
379391
workdir = opener.resolve_workdir(os.environ, project_name)
380392
logger.info("[6/6] Opening editor attached to the dev container...")
@@ -458,7 +470,7 @@ def cmd_up(project_name: str = None, scale: int = None,
458470
if deploy_script.exists() and deploy_script.is_file():
459471
_run_deploy_script_for_instances(deploy_script, range(1, scale + 1))
460472

461-
_maybe_open_editor(project_name, open_editor, open_index)
473+
_maybe_open_editor(project_name, open_editor, open_index, scale)
462474

463475
logger.info("=== Deploy completed successfully ===")
464476
return 0

lib/devbase/editor/opener.py

Lines changed: 77 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,13 +135,87 @@ def build_attach_uri(container_name: str, workdir: str) -> str:
135135
return f"vscode-remote://attached-container+{hexname}{path}"
136136

137137

138-
def resolve_container_name(dev_service_name: str, project_name: str, index: int = 1) -> str:
138+
def _parse_compose_ps_name(stdout: str) -> Optional[str]:
139+
"""``docker compose ps --format json`` の出力から ``.Name`` を 1 件取り出す。
140+
141+
docker compose のバージョン差で出力形式が異なる:
142+
143+
- 新しめ: 1 行 1 JSON オブジェクト (改行区切り NDJSON)
144+
- 古め: JSON 配列 ``[{...}, ...]``
145+
146+
どちらでも先頭インスタンスの ``Name`` を返す。解釈不能・空なら None。
147+
"""
148+
text = (stdout or "").strip()
149+
if not text:
150+
return None
151+
# まず JSON 配列としてのパースを試す。
152+
try:
153+
obj = json.loads(text)
154+
except ValueError:
155+
obj = None
156+
if isinstance(obj, list):
157+
for item in obj:
158+
if isinstance(item, dict) and item.get("Name"):
159+
return item["Name"]
160+
return None
161+
if isinstance(obj, dict) and obj.get("Name"):
162+
return obj["Name"]
163+
# NDJSON (1 行 1 JSON) として行ごとにパース。
164+
for line in text.splitlines():
165+
line = line.strip()
166+
if not line:
167+
continue
168+
try:
169+
item = json.loads(line)
170+
except ValueError:
171+
continue
172+
if isinstance(item, dict) and item.get("Name"):
173+
return item["Name"]
174+
return None
175+
176+
177+
def _query_container_name(dev_service_name: str, index: int,
178+
runner: Optional[Callable] = None) -> Optional[str]:
179+
"""実 docker へ問い合わせて dev インスタンスの実コンテナ名を取得する (保険)。
180+
181+
scale 生成 compose ではサービス名が ``{dev}-{index}`` (例 ``dev-1``) になるため
182+
その service token を指定して ``docker compose ps --format json`` を実行する。
183+
取得できなければ None。docker 不在・非0・例外・空はすべて None に握り潰し、
184+
呼び出し側が決定的名へフォールバックできるようにする。
185+
"""
186+
run = runner or subprocess.run
187+
service_token = f"{dev_service_name}-{index}"
188+
try:
189+
proc = run(
190+
["docker", "compose", "ps", "--format", "json", service_token],
191+
capture_output=True, text=True, timeout=10,
192+
)
193+
except Exception: # noqa: BLE001 - docker 不在等は保険なので握り潰す
194+
return None
195+
if getattr(proc, "returncode", 1) != 0:
196+
return None
197+
try:
198+
return _parse_compose_ps_name(proc.stdout)
199+
except Exception: # noqa: BLE001 - パース失敗も決定的名へフォールバック
200+
return None
201+
202+
203+
def resolve_container_name(dev_service_name: str, project_name: str, index: int = 1,
204+
runner: Optional[Callable] = None) -> str:
139205
"""dev コンテナの実コンテナ名を返す。
140206
141-
scale 生成 compose が ``container_name = ${COMPOSE_PROJECT_NAME}-{dev}-{index}``
207+
PLAN31_3 §3: compose バージョン差異への保険として、まず実 docker へ
208+
``docker compose ps --format json`` で問い合わせて実 ``Name`` を取得する。
209+
取得できなければ決定的名 ``{project_name}-{dev_service_name}-{index}`` へ
210+
フォールバックする。
211+
212+
scale 生成 compose は ``container_name = ${COMPOSE_PROJECT_NAME}-{dev}-{index}``
142213
を全インスタンスへ設定する (volume/compose.py)。COMPOSE_PROJECT_NAME は
143-
project_name と一致するため決定的に組み立てられる
214+
project_name と一致するため、docker 問い合わせに失敗しても決定的に組み立てられる
144215
"""
216+
queried = _query_container_name(dev_service_name, index, runner=runner)
217+
if queried:
218+
return queried
145219
return f"{project_name}-{dev_service_name}-{index}"
146220

147221

tests/cli/test_project_dispatch.py

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ def test_maybe_open_editor_disabled_by_default(monkeypatch):
359359
called = []
360360
monkeypatch.setattr(opener, 'open_editor',
361361
lambda **kw: called.append(kw) or 'launch')
362-
container._maybe_open_editor('carmo', None, None)
362+
container._maybe_open_editor('carmo', None, None, 1)
363363
assert called == []
364364

365365

@@ -372,7 +372,7 @@ def test_maybe_open_editor_flag_overrides_env(monkeypatch):
372372
monkeypatch.setattr(opener, 'open_editor',
373373
lambda **kw: called.append(kw) or 'launch')
374374
monkeypatch.setattr(container, 'get_dev_service_name', lambda: 'dev')
375-
container._maybe_open_editor('carmo', True, 1)
375+
container._maybe_open_editor('carmo', True, 1, 1)
376376
assert len(called) == 1
377377
assert called[0]['project_name'] == 'carmo'
378378

@@ -388,4 +388,32 @@ def boom(**kw):
388388
raise RuntimeError("x")
389389

390390
monkeypatch.setattr(opener, 'open_editor', boom)
391-
container._maybe_open_editor('carmo', None, None) # 例外が出なければ OK
391+
container._maybe_open_editor('carmo', None, None, 1) # 例外が出なければ OK
392+
393+
394+
@pytest.mark.parametrize('bad_index', [0, -1, 3])
395+
def test_maybe_open_editor_out_of_range_index_falls_back(monkeypatch, bad_index):
396+
"""0・負数・scale 超過の index は既定 (1) へフォールバックする (scale=2)。"""
397+
from devbase.commands import container
398+
from devbase.editor import opener
399+
monkeypatch.setattr(opener, 'is_open_enabled', lambda environ=None: True)
400+
monkeypatch.setattr(container, 'get_dev_service_name', lambda: 'dev')
401+
called = []
402+
monkeypatch.setattr(opener, 'open_editor',
403+
lambda **kw: called.append(kw) or 'launch')
404+
container._maybe_open_editor('carmo', True, bad_index, 2)
405+
assert len(called) == 1
406+
assert called[0]['index'] == 1
407+
408+
409+
def test_maybe_open_editor_valid_index_within_scale(monkeypatch):
410+
"""範囲内 (1..scale) の index はそのまま使われる。"""
411+
from devbase.commands import container
412+
from devbase.editor import opener
413+
monkeypatch.setattr(opener, 'is_open_enabled', lambda environ=None: True)
414+
monkeypatch.setattr(container, 'get_dev_service_name', lambda: 'dev')
415+
called = []
416+
monkeypatch.setattr(opener, 'open_editor',
417+
lambda **kw: called.append(kw) or 'launch')
418+
container._maybe_open_editor('carmo', True, 2, 3)
419+
assert called[0]['index'] == 2

tests/editor/test_opener.py

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,20 @@
33
from __future__ import annotations
44

55
import json
6+
from dataclasses import dataclass
67

78
import pytest
89

910
from devbase.editor import opener
1011

1112

13+
@dataclass
14+
class _Proc:
15+
"""subprocess.run 互換の軽量スタブ (returncode / stdout のみ)。"""
16+
returncode: int = 0
17+
stdout: str = ""
18+
19+
1220
# ---------------------------------------------------------------------------
1321
# detect_context
1422
# ---------------------------------------------------------------------------
@@ -109,8 +117,50 @@ def test_build_attach_uri_adds_leading_slash():
109117
# ---------------------------------------------------------------------------
110118

111119
def test_resolve_container_name_deterministic():
120+
"""docker 問い合わせが失敗 (非0) する場合は決定的名へフォールバックする。"""
121+
def failing_runner(cmd, **kw):
122+
return _Proc(returncode=1, stdout="")
123+
124+
assert opener.resolve_container_name("dev", "carmo", 1, runner=failing_runner) \
125+
== "carmo-dev-1"
126+
assert opener.resolve_container_name("app", "carmo", 3, runner=failing_runner) \
127+
== "carmo-app-3"
128+
129+
130+
def test_resolve_container_name_falls_back_when_docker_absent(monkeypatch):
131+
"""docker 不在 (例外) でも決定的名で必ず動く。"""
132+
monkeypatch.setattr(opener, "_query_container_name",
133+
lambda *a, **kw: None)
112134
assert opener.resolve_container_name("dev", "carmo", 1) == "carmo-dev-1"
113-
assert opener.resolve_container_name("app", "carmo", 3) == "carmo-app-3"
135+
136+
137+
def test_resolve_container_name_prefers_docker_name_ndjson():
138+
"""docker から取得できた実 Name (NDJSON) を決定的名より優先する。"""
139+
def runner(cmd, **kw):
140+
# service token は dev-2 を指定しているはず
141+
assert cmd[:4] == ["docker", "compose", "ps", "--format"]
142+
assert cmd[-1] == "dev-2"
143+
return _Proc(returncode=0,
144+
stdout='{"Name":"real-dev-2","Service":"dev-2"}\n')
145+
146+
assert opener.resolve_container_name("dev", "carmo", 2, runner=runner) \
147+
== "real-dev-2"
148+
149+
150+
def test_resolve_container_name_prefers_docker_name_json_array():
151+
"""JSON 配列形式の docker compose ps 出力にも対応する。"""
152+
def runner(cmd, **kw):
153+
return _Proc(returncode=0,
154+
stdout='[{"Name":"real-dev-1","Service":"dev-1"}]')
155+
156+
assert opener.resolve_container_name("dev", "carmo", 1, runner=runner) \
157+
== "real-dev-1"
158+
159+
160+
def test_parse_compose_ps_name_empty_and_invalid():
161+
assert opener._parse_compose_ps_name("") is None
162+
assert opener._parse_compose_ps_name("not json") is None
163+
assert opener._parse_compose_ps_name("[]") is None
114164

115165

116166
def test_resolve_workdir_prefers_work_dir_env():

0 commit comments

Comments
 (0)