Skip to content

Commit 5f91183

Browse files
Fix stale cached image causing tests to not run
The prepare command was caching images with source code baked in, causing newly added tests to not be found when using --cached. The cached image would have old source code, so nextest filter expressions wouldn't match tests that were added after the cache was created. Fix: Cache only the BASE image (Dockerfile build), then add --include-cwd and --copy-dir on top of the cached base image. This ensures: - Expensive Dockerfile builds are still cached - Source code is always fresh, even when using --cached - Newly added tests are always included 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 1b3e7fa commit 5f91183

File tree

1 file changed

+79
-63
lines changed

1 file changed

+79
-63
lines changed

scripts/modal_sandbox.py

Lines changed: 79 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -180,11 +180,11 @@ def write_cached_image_id(image_id: str) -> None:
180180

181181
@cli.command("prepare")
182182
@click.argument("dockerfile_path", required=False, default=None)
183-
@click.option("--cached", is_flag=True, help="Use cached image_id if available")
183+
@click.option("--cached", is_flag=True, help="Use cached BASE image if available")
184184
@click.option(
185185
"--include-cwd",
186186
is_flag=True,
187-
help="Include current directory in the image build context",
187+
help="Include current directory in the image (added after cache lookup)",
188188
)
189189
@click.option(
190190
"--copy-dir",
@@ -198,86 +198,105 @@ def prepare(dockerfile_path: str | None, cached: bool, include_cwd: bool, copy_d
198198
DOCKERFILE_PATH: Optional path to a Dockerfile. If provided, builds from
199199
that Dockerfile. If omitted, builds the default pytest image.
200200
201+
The --cached flag caches only the BASE image (Dockerfile build). The --include-cwd
202+
and --copy-dir options are applied AFTER cache lookup, ensuring fresh source code
203+
is always used even when the base image is cached.
204+
201205
Prints the image_id to stdout for use with 'create'.
202206
"""
203-
# Check cache first if --cached flag is set
204-
if cached:
205-
cached_id = read_cached_image_id()
206-
if cached_id:
207-
logger.info("Using cached image_id: %s", cached_id)
208-
sys.stdout.write("%s\n" % cached_id)
209-
return
210-
211207
# Read ignore patterns from .dockerignore
212208
ignore_patterns = read_dockerignore_patterns()
213209
if ignore_patterns:
214210
logger.debug(
215211
"Using %d ignore patterns from %s", len(ignore_patterns), DOCKERIGNORE_FILE
216212
)
217213

218-
# NOTE(Danver): App name here should be injectable from the Config.
214+
base_image = None
215+
base_image_id = None
216+
217+
# Step 1: Get or build the BASE image (without cwd/copy-dirs)
218+
if cached:
219+
base_image_id = read_cached_image_id()
220+
if base_image_id:
221+
logger.info("Using cached base image_id: %s", base_image_id)
222+
219223
if dockerfile_path is None:
220-
# Build default image with cwd baked in
224+
# Build default base image
221225
with modal.enable_output():
222-
logger.info("Building default image with cwd baked in...")
223226
app = modal.App.lookup("offload-sandbox", create_if_missing=True)
224-
image = (
225-
modal.Image.debian_slim(python_version="3.11")
226-
.pip_install("pytest")
227-
.add_local_dir(".", "/app", copy=True, ignore=ignore_patterns)
228-
)
229-
# Add user-specified directories
230-
for copy_spec in copy_dirs:
231-
if ":" not in copy_spec:
232-
logger.warning("Invalid copy-dir format '%s', expected 'local:remote'", copy_spec)
233-
continue
234-
local_path, remote_path = copy_spec.split(":", 1)
235-
if not os.path.isdir(local_path):
236-
logger.warning("Local directory '%s' not found, skipping", local_path)
237-
continue
238-
logger.info("Adding %s -> %s to image", local_path, remote_path)
239-
image = image.add_local_dir(local_path, remote_path, copy=True, ignore=ignore_patterns)
240227

241-
image.build(app)
242-
# Create temp sandbox to materialize image_id, then terminate
243-
temp_sandbox = modal.Sandbox.create(app=app, image=image, timeout=10)
244-
temp_sandbox.terminate()
245-
image_id = image.object_id
228+
if base_image_id:
229+
# Load cached base image
230+
base_image = modal.Image.from_id(base_image_id)
231+
else:
232+
# Build fresh base image
233+
logger.info("Building default base image...")
234+
base_image = (
235+
modal.Image.debian_slim(python_version="3.11")
236+
.pip_install("pytest")
237+
)
238+
base_image.build(app)
239+
# Materialize to get base image_id for caching
240+
temp_sandbox = modal.Sandbox.create(app=app, image=base_image, timeout=10)
241+
temp_sandbox.terminate()
242+
base_image_id = base_image.object_id
243+
# Cache the base image
244+
write_cached_image_id(base_image_id)
245+
logger.info("Cached base image_id to %s", CACHE_FILE)
246246
else:
247247
if not os.path.isfile(dockerfile_path):
248248
logger.error("Error: Dockerfile not found: %s", dockerfile_path)
249249
sys.exit(1)
250250

251251
with modal.enable_output():
252252
app = modal.App.lookup("offload-dockerfile-sandbox", create_if_missing=True)
253-
logger.info("Building image from %s with context_dir=.", dockerfile_path)
254-
image = modal.Image.from_dockerfile(dockerfile_path, context_dir=".")
255-
if include_cwd:
256-
logger.info("Including current directory as /app")
257-
image = image.add_local_dir(
258-
".", "/app", copy=True, ignore=ignore_patterns
259-
)
260-
# Add user-specified directories
261-
for copy_spec in copy_dirs:
262-
if ":" not in copy_spec:
263-
logger.warning("Invalid copy-dir format '%s', expected 'local:remote'", copy_spec)
264-
continue
265-
local_path, remote_path = copy_spec.split(":", 1)
266-
if not os.path.isdir(local_path):
267-
logger.warning("Local directory '%s' not found, skipping", local_path)
268-
continue
269-
logger.info("Adding %s -> %s to image", local_path, remote_path)
270-
image = image.add_local_dir(local_path, remote_path, copy=True, ignore=ignore_patterns)
271253

272-
image.build(app)
273-
# Create temp sandbox to materialize image_id, then terminate
274-
temp_sandbox = modal.Sandbox.create(app=app, image=image, timeout=10)
275-
temp_sandbox.terminate()
276-
image_id = image.object_id
254+
if base_image_id:
255+
# Load cached base image
256+
base_image = modal.Image.from_id(base_image_id)
257+
else:
258+
# Build fresh base image from Dockerfile
259+
logger.info("Building base image from %s with context_dir=.", dockerfile_path)
260+
base_image = modal.Image.from_dockerfile(dockerfile_path, context_dir=".")
261+
base_image.build(app)
262+
# Materialize to get base image_id for caching
263+
temp_sandbox = modal.Sandbox.create(app=app, image=base_image, timeout=10)
264+
temp_sandbox.terminate()
265+
base_image_id = base_image.object_id
266+
# Cache the base image
267+
write_cached_image_id(base_image_id)
268+
logger.info("Cached base image_id to %s", CACHE_FILE)
269+
270+
# Step 2: Add cwd and copy-dirs on top of the base image (always fresh)
271+
final_image = base_image
272+
273+
with modal.enable_output():
274+
if include_cwd:
275+
logger.info("Adding current directory as /app...")
276+
final_image = final_image.add_local_dir(
277+
".", "/app", copy=True, ignore=ignore_patterns
278+
)
277279

278-
# Write to cache file
279-
write_cached_image_id(image_id)
280-
logger.info("Cached image_id to %s", CACHE_FILE)
280+
# Add user-specified directories
281+
for copy_spec in copy_dirs:
282+
if ":" not in copy_spec:
283+
logger.warning("Invalid copy-dir format '%s', expected 'local:remote'", copy_spec)
284+
continue
285+
local_path, remote_path = copy_spec.split(":", 1)
286+
if not os.path.isdir(local_path):
287+
logger.warning("Local directory '%s' not found, skipping", local_path)
288+
continue
289+
logger.info("Adding %s -> %s to image", local_path, remote_path)
290+
final_image = final_image.add_local_dir(local_path, remote_path, copy=True, ignore=ignore_patterns)
291+
292+
# Build and materialize the final image if we added anything
293+
if final_image is not base_image:
294+
final_image.build(app)
295+
temp_sandbox = modal.Sandbox.create(app=app, image=final_image, timeout=10)
296+
temp_sandbox.terminate()
297+
image_id = final_image.object_id
298+
else:
299+
image_id = base_image_id
281300

282301
sys.stdout.write("%s\n" % image_id)
283302

@@ -432,9 +451,6 @@ def create_from_image(image_id: str, copy_dirs: tuple[str, ...] = ()):
432451
)
433452
logger.debug("[%.2fs] Sandbox created", time.time() - t0)
434453

435-
# Copy files based on sandbox type
436-
cwd = os.getcwd()
437-
438454
# Copy user-specified directories
439455
logger.debug(
440456
"[%.2fs] Processing %d user-specified copy-dir(s)",

0 commit comments

Comments
 (0)