From e862b823224162031fc32535758f32c36cd7b82d Mon Sep 17 00:00:00 2001 From: didericis Date: Mon, 13 Jul 2026 04:04:08 -0400 Subject: [PATCH] fix: smoke-test agent images after build, add start --no-cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit npm treats optionalDependencies failures as non-fatal, so a transient network blip fetching claude-code's platform-native binary during `npm install -g` left a stub CLI in an image that still "built" successfully — then got baked into the Docker/Container layer cache until forced to rebuild. Post-build smoke test (provider-declared argv, run in a throwaway container of the freshly built image) fails the launch loudly instead of shipping a broken image; --no-cache gives an escape hatch to force a from-scratch rebuild. Closes #353. --- bot_bottle/agent_provider.py | 7 ++++ bot_bottle/backend/docker/launch.py | 4 +++ bot_bottle/backend/docker/util.py | 33 ++++++++++++++++++- bot_bottle/backend/macos_container/launch.py | 4 +++ bot_bottle/backend/macos_container/util.py | 32 +++++++++++++++++- bot_bottle/cli/start.py | 16 +++++++++ bot_bottle/contrib/claude/agent_provider.py | 1 + bot_bottle/contrib/codex/agent_provider.py | 1 + .../test_docker_launch_committed_image.py | 2 ++ tests/unit/test_docker_launch_teardown.py | 1 + tests/unit/test_macos_container_launch.py | 7 +++- 11 files changed, 105 insertions(+), 3 deletions(-) diff --git a/bot_bottle/agent_provider.py b/bot_bottle/agent_provider.py index 398f239..9f9d0d7 100644 --- a/bot_bottle/agent_provider.py +++ b/bot_bottle/agent_provider.py @@ -61,6 +61,13 @@ class AgentProviderRuntime: prompt_mode: PromptMode bypass_args: tuple[str, ...] resume_args: tuple[str, ...] + # argv run inside a throwaway container of a freshly built agent + # image, right after `build_image()`, to catch a build that + # exited 0 but produced a broken CLI (e.g. an npm + # optionalDependencies fetch for a platform-native binary that + # silently no-ops on a transient failure). Empty tuple skips the + # check — not every provider has opted in yet. + smoke_test: tuple[str, ...] = () @dataclass(frozen=True) diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index fe49903..0739f19 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -36,6 +36,7 @@ from contextlib import ExitStack, contextmanager from pathlib import Path from typing import Callable, Generator +from ...agent_provider import runtime_for from ...egress import egress_resolve_token_values from ...git_gate import ( provision_git_gate_dynamic_keys, @@ -111,6 +112,9 @@ def launch( plan.image, _REPO_DIR, dockerfile=plan.dockerfile_path, ) + docker_mod.verify_agent_image( + plan.image, runtime_for(plan.agent_provider_template).smoke_test, + ) internal_network = network_mod.network_name_for_slug(plan.slug) egress_network = network_mod.network_egress_name_for_slug(plan.slug) diff --git a/bot_bottle/backend/docker/util.py b/bot_bottle/backend/docker/util.py index 6a4abdd..e827c24 100644 --- a/bot_bottle/backend/docker/util.py +++ b/bot_bottle/backend/docker/util.py @@ -4,6 +4,7 @@ existence, and building images.""" from __future__ import annotations +import os import re import shutil import subprocess @@ -108,15 +109,45 @@ def build_image(ref: str, context: str, *, dockerfile: str = "") -> None: `dockerfile` is an optional path (relative to `context`, or absolute) for callers that need to build from a non-default - Dockerfile in the same context — e.g. `Dockerfile.git-gate`.""" + Dockerfile in the same context — e.g. `Dockerfile.git-gate`. + + Set `BOT_BOTTLE_NO_CACHE=1` (the `start --no-cache` flag) to force + `--no-cache`. The npm/curl installers some provider Dockerfiles + shell out to can silently no-op on a transient network failure — + e.g. an `optionalDependencies` fetch for a platform-native binary — + and Docker will then cache that broken layer indefinitely.""" info(f"building image {ref} from {context} (layer cache keeps repeat builds fast)") args = ["docker", "build", "-t", ref] + if os.environ.get("BOT_BOTTLE_NO_CACHE") == "1": + args.append("--no-cache") if dockerfile: args.extend(["-f", dockerfile]) args.append(context) subprocess.run(args, check=True) +def verify_agent_image(image: str, argv: tuple[str, ...]) -> None: + """Run `argv` inside a throwaway container of a freshly built agent + image and die loudly if it fails, instead of shipping an image + whose CLI only breaks at first real use. No-op when the provider + hasn't declared a smoke test (`AgentProviderRuntime.smoke_test`).""" + if not argv: + return + result = subprocess.run( + ["docker", "run", "--rm", "--entrypoint", argv[0], image, *argv[1:]], + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + detail = (result.stderr or result.stdout or "").strip() + die( + f"agent image {image!r} failed its post-build smoke test " + f"({' '.join(argv)}): {detail}\n" + f"Try rebuilding from scratch: bot-bottle start --no-cache" + ) + + # def build_image_with_cwd( # derived: str, # base: str, diff --git a/bot_bottle/backend/macos_container/launch.py b/bot_bottle/backend/macos_container/launch.py index 6e9034f..0abb39f 100644 --- a/bot_bottle/backend/macos_container/launch.py +++ b/bot_bottle/backend/macos_container/launch.py @@ -17,6 +17,7 @@ from contextlib import ExitStack, contextmanager from pathlib import Path from typing import Callable, Generator +from ...agent_provider import runtime_for from ...bottle_state import ( egress_state_dir, git_gate_state_dir, @@ -169,6 +170,9 @@ def _build_images(plan: MacosContainerBottlePlan) -> MacosContainerBottlePlan: _REPO_DIR, dockerfile=plan.dockerfile_path, ) + container_mod.verify_agent_image( + plan.image, runtime_for(plan.agent_provider_template).smoke_test, + ) return plan diff --git a/bot_bottle/backend/macos_container/util.py b/bot_bottle/backend/macos_container/util.py index ccb8f06..5853948 100644 --- a/bot_bottle/backend/macos_container/util.py +++ b/bot_bottle/backend/macos_container/util.py @@ -60,13 +60,21 @@ def dns_server() -> str: def build_image(ref: str, context: str, *, dockerfile: str = "") -> None: - """Build an OCI image with Apple's BuildKit-backed `container build`.""" + """Build an OCI image with Apple's BuildKit-backed `container build`. + + Set `BOT_BOTTLE_NO_CACHE=1` (the `start --no-cache` flag) to force + `--no-cache`. The npm/curl installers some provider Dockerfiles + shell out to can silently no-op on a transient network failure — + e.g. an `optionalDependencies` fetch for a platform-native binary — + and the builder will then cache that broken layer indefinitely.""" info( f"building image {ref} from {context} with Apple Container " "(layer cache keeps repeat builds fast)" ) _ensure_builder_dns() args = [_CONTAINER, "build", "-t", ref, "--dns", dns_server()] + if os.environ.get("BOT_BOTTLE_NO_CACHE") == "1": + args.append("--no-cache") if dockerfile: # `container build` resolves -f relative to the current working # directory, not the build context. Anchor a relative Dockerfile to @@ -78,6 +86,28 @@ def build_image(ref: str, context: str, *, dockerfile: str = "") -> None: subprocess.run(args, check=True) +def verify_agent_image(image: str, argv: tuple[str, ...]) -> None: + """Run `argv` inside a throwaway container of a freshly built agent + image and die loudly if it fails, instead of shipping an image + whose CLI only breaks at first real use. No-op when the provider + hasn't declared a smoke test (`AgentProviderRuntime.smoke_test`).""" + if not argv: + return + result = subprocess.run( + [_CONTAINER, "run", "--rm", "--entrypoint", argv[0], image, *argv[1:]], + capture_output=True, + text=True, + check=False, + ) + if result.returncode != 0: + detail = (result.stderr or result.stdout or "").strip() + die( + f"agent image {image!r} failed its post-build smoke test " + f"({' '.join(argv)}): {detail}\n" + f"Try rebuilding from scratch: bot-bottle start --no-cache" + ) + + def commit_container(container_name: str, image_tag: str) -> None: """Snapshot a running Apple Container as a local image. diff --git a/bot_bottle/cli/start.py b/bot_bottle/cli/start.py index 5a22ce0..4101804 100644 --- a/bot_bottle/cli/start.py +++ b/bot_bottle/cli/start.py @@ -46,6 +46,17 @@ def cmd_start(argv: list[str]) -> int: parser = argparse.ArgumentParser(prog=f"{PROG} start", add_help=True) parser.add_argument("--dry-run", action="store_true") parser.add_argument("--cwd", action="store_true", help="copy host cwd into the running bottle") + parser.add_argument( + "--no-cache", + action="store_true", + help=( + "rebuild agent/sidecar images from scratch, bypassing the " + "build layer cache. Use when an image looks broken after a " + "dependency bump — e.g. an installer's optionalDependencies " + "fetch silently no-op'd on a transient failure and got baked " + "into a cached layer." + ), + ) parser.add_argument( "--backend", choices=known_backend_names(), @@ -97,6 +108,11 @@ def cmd_start(argv: list[str]) -> int: args = parser.parse_args(argv) dry_run = args.dry_run or os.environ.get("BOT_BOTTLE_DRY_RUN") == "1" + if args.no_cache or os.environ.get("BOT_BOTTLE_NO_CACHE") == "1": + # Read by build_image() in each backend's util module — set here + # so both the interactive and --headless paths pick it up without + # threading a no_cache field through every backend's plan dataclass. + os.environ["BOT_BOTTLE_NO_CACHE"] = "1" manifest = ManifestIndex.resolve(USER_CWD) backend_name: str | None = args.backend diff --git a/bot_bottle/contrib/claude/agent_provider.py b/bot_bottle/contrib/claude/agent_provider.py index 204357b..a218c1a 100644 --- a/bot_bottle/contrib/claude/agent_provider.py +++ b/bot_bottle/contrib/claude/agent_provider.py @@ -91,6 +91,7 @@ _RUNTIME = AgentProviderRuntime( prompt_mode="append_file", bypass_args=("--dangerously-skip-permissions",), resume_args=("--continue",), + smoke_test=("claude", "--version"), ) diff --git a/bot_bottle/contrib/codex/agent_provider.py b/bot_bottle/contrib/codex/agent_provider.py index 0a681a5..07eb0f6 100644 --- a/bot_bottle/contrib/codex/agent_provider.py +++ b/bot_bottle/contrib/codex/agent_provider.py @@ -61,6 +61,7 @@ _RUNTIME = AgentProviderRuntime( prompt_mode="read_prompt_file", bypass_args=("--dangerously-bypass-approvals-and-sandbox",), resume_args=("resume", "--last"), + smoke_test=(_CODEX_CLI, "--version"), ) diff --git a/tests/unit/test_docker_launch_committed_image.py b/tests/unit/test_docker_launch_committed_image.py index 1152e63..eadd378 100644 --- a/tests/unit/test_docker_launch_committed_image.py +++ b/tests/unit/test_docker_launch_committed_image.py @@ -103,6 +103,8 @@ class TestLaunchCommittedImage(unittest.TestCase): launch_mod.docker_mod, "image_exists", return_value=image_present, ), mock.patch.object( launch_mod.docker_mod, "build_image", side_effect=fake_build, + ), mock.patch.object( + launch_mod.docker_mod, "verify_agent_image", ), mock.patch.object( launch_mod, "egress_tls_init", return_value=(Path("/egress_ca"), Path("/egress_cert")), diff --git a/tests/unit/test_docker_launch_teardown.py b/tests/unit/test_docker_launch_teardown.py index 983bbc6..6ee15c8 100644 --- a/tests/unit/test_docker_launch_teardown.py +++ b/tests/unit/test_docker_launch_teardown.py @@ -87,6 +87,7 @@ class TestTeardownWarning(unittest.TestCase): buf = io.StringIO() with mock.patch.object(launch_mod.docker_mod, "build_image"), \ + mock.patch.object(launch_mod.docker_mod, "verify_agent_image"), \ mock.patch.object( launch_mod, "egress_tls_init", return_value=(Path("/egress_ca"), Path("/egress_cert")), diff --git a/tests/unit/test_macos_container_launch.py b/tests/unit/test_macos_container_launch.py index 7dd9de7..4a5db43 100644 --- a/tests/unit/test_macos_container_launch.py +++ b/tests/unit/test_macos_container_launch.py @@ -357,12 +357,17 @@ class TestMacosContainerLaunchCommittedImage(unittest.TestCase): launch.container_mod, "image_exists", return_value=False, ), patch.object( launch.container_mod, "build_image", side_effect=fake_build, - ): + ), patch.object( + launch.container_mod, "verify_agent_image", + ) as verify: updated = launch._build_images(plan) self.assertEqual("bot-bottle-agent:latest", updated.image) self.assertEqual(2, len(calls)) self.assertEqual("bot-bottle-agent:latest", calls[1][0]) + verify.assert_called_once_with( + "bot-bottle-agent:latest", ("claude", "--version"), + ) if __name__ == "__main__":