Skip to content

Commit eed868d

Browse files
takemi-ohamaclaude
andcommitted
fix(container): Codex 第二回 review 指摘 (HIGH/MED) 対応
[HIGH] 既存 image-only サービスで marker bootstrap PR 適用前から手動 docker pull 済みの公開 image は marker が無く、 _pull_age_days() が永遠に None を返して定期 pull が発火しない不具合。 image-only かつ marker 不在のケースで marker を「now」で bootstrap し、 次回以降 threshold-based pull が正しく作動するよう修正。 option (a) (即 pull) ではなく option (b) (現時点を起点に記録) を選択。 理由: 初回 up での予期せぬネットワーク呼び出しを避け、 最大 7 日の更新遅延 (SLA 厳密ではない) を受容する方が UX として穏便。 [MED] marker filename の衝突を hash で防止 re.sub による単純置換は lossy で、 'a/b:c' と 'a_b/c' がともに 'a_b_c' に衝突する問題があった。 filename を `<sanitized[:60]>--<sha256[:12]>` 形式に変更し、 人間可読性と衝突耐性を両立。 [LOW] 未来 mtime のクランプ 時刻巻き戻しや手動 mtime 操作で marker mtime が未来を指すと、 時刻差が負値になり pull_age < max_age が常に成立して refresh が永続抑止される問題があった。 負値検出時は warning ログを出して 0 を返すよう修正。 動作確認: - collision: a/b:c, a_b/c, a:b/c が全て異なる hash で識別される - bootstrap: marker 無し既存 image で初回 up 時に marker 作成、age=0 - future mtime: 5 日先の mtime → warning + age=0 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent decf80f commit eed868d

1 file changed

Lines changed: 34 additions & 5 deletions

File tree

lib/devbase/commands/container.py

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Container lifecycle commands (up, down, ps, login, logs, scale, build)"""
22

3+
import hashlib
34
import json
45
import os
56
import re
@@ -507,7 +508,19 @@ def _ensure_images() -> bool:
507508
# 'Created' reflects upstream build time, not local pull time.
508509
if not has_build:
509510
pull_age = _pull_age_days(image_name)
510-
if pull_age is None or pull_age < max_age:
511+
if pull_age is None:
512+
# Pre-existing image with no marker (e.g., upgrade from a
513+
# devbase version without touch-file tracking). Bootstrap a
514+
# marker now so future runs can apply the threshold. We do
515+
# not auto-pull here to avoid surprising network calls on
516+
# the first `up` after upgrade.
517+
logger.info(
518+
"First time tracking image '%s'; recording marker (no pull this run)",
519+
image_name
520+
)
521+
_mark_pulled(image_name)
522+
return True
523+
if pull_age < max_age:
511524
return True
512525
logger.info(
513526
"Image '%s' last pulled %d days ago (>= %d days threshold), re-pulling...",
@@ -596,21 +609,37 @@ def _run_pull(image_name: str) -> bool:
596609
def _pull_marker_path(image_name: str) -> Optional[Path]:
597610
"""Path of the touch-file recording the last pull time of `image_name`.
598611
612+
Filename format: ``<sanitized>--<sha12>`` to keep the human-readable part
613+
while preventing collisions between distinct image references that
614+
sanitize to the same string (e.g., ``a/b:c`` vs ``a_b/c``).
615+
599616
Returns None when DEVBASE_ROOT is not set so callers can no-op safely.
600617
"""
601618
devbase_root = os.environ.get('DEVBASE_ROOT')
602619
if not devbase_root:
603620
return None
604-
safe = re.sub(r'[^A-Za-z0-9._-]', '_', image_name)
605-
return Path(devbase_root) / '.cache' / 'pulls' / safe
621+
sanitized = re.sub(r'[^A-Za-z0-9._-]', '_', image_name)[:60]
622+
digest = hashlib.sha256(image_name.encode('utf-8')).hexdigest()[:12]
623+
return Path(devbase_root) / '.cache' / 'pulls' / f'{sanitized}--{digest}'
606624

607625

608626
def _pull_age_days(image_name: str) -> Optional[int]:
609-
"""Days since the last successful pull of `image_name`. None if never."""
627+
"""Days since the last successful pull of `image_name`. None if never.
628+
629+
Negative ages (clock skew or future-dated marker) are clamped to 0 with
630+
a warning so they do not silently suppress refresh forever.
631+
"""
610632
marker = _pull_marker_path(image_name)
611633
if marker is None or not marker.exists():
612634
return None
613-
return int((time.time() - marker.stat().st_mtime) / 86400)
635+
delta = time.time() - marker.stat().st_mtime
636+
if delta < 0:
637+
logger.warning(
638+
"Pull marker for '%s' has a future mtime (clock skew?); treating as 0 days",
639+
image_name
640+
)
641+
return 0
642+
return int(delta / 86400)
614643

615644

616645
def _mark_pulled(image_name: str) -> None:

0 commit comments

Comments
 (0)