fix: smoke-test agent images after build, add start --no-cache
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.
This commit is contained in:
@@ -61,6 +61,13 @@ class AgentProviderRuntime:
|
|||||||
prompt_mode: PromptMode
|
prompt_mode: PromptMode
|
||||||
bypass_args: tuple[str, ...]
|
bypass_args: tuple[str, ...]
|
||||||
resume_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)
|
@dataclass(frozen=True)
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ from contextlib import ExitStack, contextmanager
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Callable, Generator
|
from typing import Callable, Generator
|
||||||
|
|
||||||
|
from ...agent_provider import runtime_for
|
||||||
from ...egress import egress_resolve_token_values
|
from ...egress import egress_resolve_token_values
|
||||||
from ...git_gate import (
|
from ...git_gate import (
|
||||||
provision_git_gate_dynamic_keys,
|
provision_git_gate_dynamic_keys,
|
||||||
@@ -111,6 +112,9 @@ def launch(
|
|||||||
plan.image, _REPO_DIR,
|
plan.image, _REPO_DIR,
|
||||||
dockerfile=plan.dockerfile_path,
|
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)
|
internal_network = network_mod.network_name_for_slug(plan.slug)
|
||||||
egress_network = network_mod.network_egress_name_for_slug(plan.slug)
|
egress_network = network_mod.network_egress_name_for_slug(plan.slug)
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ existence, and building images."""
|
|||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import os
|
||||||
import re
|
import re
|
||||||
import shutil
|
import shutil
|
||||||
import subprocess
|
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
|
`dockerfile` is an optional path (relative to `context`, or
|
||||||
absolute) for callers that need to build from a non-default
|
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)")
|
info(f"building image {ref} from {context} (layer cache keeps repeat builds fast)")
|
||||||
args = ["docker", "build", "-t", ref]
|
args = ["docker", "build", "-t", ref]
|
||||||
|
if os.environ.get("BOT_BOTTLE_NO_CACHE") == "1":
|
||||||
|
args.append("--no-cache")
|
||||||
if dockerfile:
|
if dockerfile:
|
||||||
args.extend(["-f", dockerfile])
|
args.extend(["-f", dockerfile])
|
||||||
args.append(context)
|
args.append(context)
|
||||||
subprocess.run(args, check=True)
|
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(
|
# def build_image_with_cwd(
|
||||||
# derived: str,
|
# derived: str,
|
||||||
# base: str,
|
# base: str,
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ from contextlib import ExitStack, contextmanager
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Callable, Generator
|
from typing import Callable, Generator
|
||||||
|
|
||||||
|
from ...agent_provider import runtime_for
|
||||||
from ...bottle_state import (
|
from ...bottle_state import (
|
||||||
egress_state_dir,
|
egress_state_dir,
|
||||||
git_gate_state_dir,
|
git_gate_state_dir,
|
||||||
@@ -169,6 +170,9 @@ def _build_images(plan: MacosContainerBottlePlan) -> MacosContainerBottlePlan:
|
|||||||
_REPO_DIR,
|
_REPO_DIR,
|
||||||
dockerfile=plan.dockerfile_path,
|
dockerfile=plan.dockerfile_path,
|
||||||
)
|
)
|
||||||
|
container_mod.verify_agent_image(
|
||||||
|
plan.image, runtime_for(plan.agent_provider_template).smoke_test,
|
||||||
|
)
|
||||||
return plan
|
return plan
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -60,13 +60,21 @@ def dns_server() -> str:
|
|||||||
|
|
||||||
|
|
||||||
def build_image(ref: str, context: str, *, dockerfile: str = "") -> None:
|
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(
|
info(
|
||||||
f"building image {ref} from {context} with Apple Container "
|
f"building image {ref} from {context} with Apple Container "
|
||||||
"(layer cache keeps repeat builds fast)"
|
"(layer cache keeps repeat builds fast)"
|
||||||
)
|
)
|
||||||
_ensure_builder_dns()
|
_ensure_builder_dns()
|
||||||
args = [_CONTAINER, "build", "-t", ref, "--dns", dns_server()]
|
args = [_CONTAINER, "build", "-t", ref, "--dns", dns_server()]
|
||||||
|
if os.environ.get("BOT_BOTTLE_NO_CACHE") == "1":
|
||||||
|
args.append("--no-cache")
|
||||||
if dockerfile:
|
if dockerfile:
|
||||||
# `container build` resolves -f relative to the current working
|
# `container build` resolves -f relative to the current working
|
||||||
# directory, not the build context. Anchor a relative Dockerfile to
|
# 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)
|
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:
|
def commit_container(container_name: str, image_tag: str) -> None:
|
||||||
"""Snapshot a running Apple Container as a local image.
|
"""Snapshot a running Apple Container as a local image.
|
||||||
|
|
||||||
|
|||||||
@@ -46,6 +46,17 @@ def cmd_start(argv: list[str]) -> int:
|
|||||||
parser = argparse.ArgumentParser(prog=f"{PROG} start", add_help=True)
|
parser = argparse.ArgumentParser(prog=f"{PROG} start", add_help=True)
|
||||||
parser.add_argument("--dry-run", action="store_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("--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(
|
parser.add_argument(
|
||||||
"--backend",
|
"--backend",
|
||||||
choices=known_backend_names(),
|
choices=known_backend_names(),
|
||||||
@@ -97,6 +108,11 @@ def cmd_start(argv: list[str]) -> int:
|
|||||||
args = parser.parse_args(argv)
|
args = parser.parse_args(argv)
|
||||||
|
|
||||||
dry_run = args.dry_run or os.environ.get("BOT_BOTTLE_DRY_RUN") == "1"
|
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)
|
manifest = ManifestIndex.resolve(USER_CWD)
|
||||||
backend_name: str | None = args.backend
|
backend_name: str | None = args.backend
|
||||||
|
|||||||
@@ -91,6 +91,7 @@ _RUNTIME = AgentProviderRuntime(
|
|||||||
prompt_mode="append_file",
|
prompt_mode="append_file",
|
||||||
bypass_args=("--dangerously-skip-permissions",),
|
bypass_args=("--dangerously-skip-permissions",),
|
||||||
resume_args=("--continue",),
|
resume_args=("--continue",),
|
||||||
|
smoke_test=("claude", "--version"),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -61,6 +61,7 @@ _RUNTIME = AgentProviderRuntime(
|
|||||||
prompt_mode="read_prompt_file",
|
prompt_mode="read_prompt_file",
|
||||||
bypass_args=("--dangerously-bypass-approvals-and-sandbox",),
|
bypass_args=("--dangerously-bypass-approvals-and-sandbox",),
|
||||||
resume_args=("resume", "--last"),
|
resume_args=("resume", "--last"),
|
||||||
|
smoke_test=(_CODEX_CLI, "--version"),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -103,6 +103,8 @@ class TestLaunchCommittedImage(unittest.TestCase):
|
|||||||
launch_mod.docker_mod, "image_exists", return_value=image_present,
|
launch_mod.docker_mod, "image_exists", return_value=image_present,
|
||||||
), mock.patch.object(
|
), mock.patch.object(
|
||||||
launch_mod.docker_mod, "build_image", side_effect=fake_build,
|
launch_mod.docker_mod, "build_image", side_effect=fake_build,
|
||||||
|
), mock.patch.object(
|
||||||
|
launch_mod.docker_mod, "verify_agent_image",
|
||||||
), mock.patch.object(
|
), mock.patch.object(
|
||||||
launch_mod, "egress_tls_init",
|
launch_mod, "egress_tls_init",
|
||||||
return_value=(Path("/egress_ca"), Path("/egress_cert")),
|
return_value=(Path("/egress_ca"), Path("/egress_cert")),
|
||||||
|
|||||||
@@ -87,6 +87,7 @@ class TestTeardownWarning(unittest.TestCase):
|
|||||||
buf = io.StringIO()
|
buf = io.StringIO()
|
||||||
|
|
||||||
with mock.patch.object(launch_mod.docker_mod, "build_image"), \
|
with mock.patch.object(launch_mod.docker_mod, "build_image"), \
|
||||||
|
mock.patch.object(launch_mod.docker_mod, "verify_agent_image"), \
|
||||||
mock.patch.object(
|
mock.patch.object(
|
||||||
launch_mod, "egress_tls_init",
|
launch_mod, "egress_tls_init",
|
||||||
return_value=(Path("/egress_ca"), Path("/egress_cert")),
|
return_value=(Path("/egress_ca"), Path("/egress_cert")),
|
||||||
|
|||||||
@@ -357,12 +357,17 @@ class TestMacosContainerLaunchCommittedImage(unittest.TestCase):
|
|||||||
launch.container_mod, "image_exists", return_value=False,
|
launch.container_mod, "image_exists", return_value=False,
|
||||||
), patch.object(
|
), patch.object(
|
||||||
launch.container_mod, "build_image", side_effect=fake_build,
|
launch.container_mod, "build_image", side_effect=fake_build,
|
||||||
):
|
), patch.object(
|
||||||
|
launch.container_mod, "verify_agent_image",
|
||||||
|
) as verify:
|
||||||
updated = launch._build_images(plan)
|
updated = launch._build_images(plan)
|
||||||
|
|
||||||
self.assertEqual("bot-bottle-agent:latest", updated.image)
|
self.assertEqual("bot-bottle-agent:latest", updated.image)
|
||||||
self.assertEqual(2, len(calls))
|
self.assertEqual(2, len(calls))
|
||||||
self.assertEqual("bot-bottle-agent:latest", calls[1][0])
|
self.assertEqual("bot-bottle-agent:latest", calls[1][0])
|
||||||
|
verify.assert_called_once_with(
|
||||||
|
"bot-bottle-agent:latest", ("claude", "--version"),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
|
|||||||
Reference in New Issue
Block a user