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,
|
||||||
@@ -110,6 +111,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,
|
||||||
|
)
|
||||||
|
|
||||||
# Step 2: mint the git-gate dynamic (gitea) deploy keys, if any, before
|
# Step 2: mint the git-gate dynamic (gitea) deploy keys, if any, before
|
||||||
# provisioning the bottle's repos into the shared gateway.
|
# provisioning the bottle's repos into the shared gateway.
|
||||||
|
|||||||
@@ -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,
|
||||||
|
|||||||
@@ -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"),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -88,9 +88,11 @@ class TestLaunchCommittedImage(unittest.TestCase):
|
|||||||
del ctx, dockerfile
|
del ctx, dockerfile
|
||||||
built.append(image)
|
built.append(image)
|
||||||
|
|
||||||
|
<<<<<<< HEAD
|
||||||
with mock.patch.object(launch_mod, "read_committed_image", return_value=committed_tag), \
|
with mock.patch.object(launch_mod, "read_committed_image", return_value=committed_tag), \
|
||||||
mock.patch.object(launch_mod.docker_mod, "image_exists", return_value=image_present), \
|
mock.patch.object(launch_mod.docker_mod, "image_exists", return_value=image_present), \
|
||||||
mock.patch.object(launch_mod.docker_mod, "build_image", side_effect=_build), \
|
mock.patch.object(launch_mod.docker_mod, "build_image", side_effect=_build), \
|
||||||
|
mock.patch.object(launch_mod.docker_mod, "verify_agent_image"), \
|
||||||
mock.patch.object(launch_mod, "launch_consolidated", return_value=_CTX), \
|
mock.patch.object(launch_mod, "launch_consolidated", return_value=_CTX), \
|
||||||
mock.patch.object(launch_mod, "teardown_consolidated"), \
|
mock.patch.object(launch_mod, "teardown_consolidated"), \
|
||||||
mock.patch.object(launch_mod, "DockerGateway", return_value=gw), \
|
mock.patch.object(launch_mod, "DockerGateway", return_value=gw), \
|
||||||
|
|||||||
@@ -94,6 +94,7 @@ class TestTeardownWarning(unittest.TestCase):
|
|||||||
)
|
)
|
||||||
|
|
||||||
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(launch_mod, "launch_consolidated", return_value=ctx), \
|
mock.patch.object(launch_mod, "launch_consolidated", return_value=ctx), \
|
||||||
mock.patch.object(launch_mod, "teardown_consolidated"), \
|
mock.patch.object(launch_mod, "teardown_consolidated"), \
|
||||||
mock.patch.object(launch_mod, "DockerGateway", return_value=gw), \
|
mock.patch.object(launch_mod, "DockerGateway", return_value=gw), \
|
||||||
|
|||||||
Reference in New Issue
Block a user