perf(deploy): layer-incremental publish path + image diet for make redeploy

[jwbron]

Fixes #2999

Problem

make redeploy moved ~40 GB of disk I/O per code edit: any source change busts the sandbox image id (COPY . /opt/egg-runtime/), and k3s-import then docker saved + ctr imported the full ~9.3 GB image once per tag (:latest + :<sha>). Beyond being slow, that serialization churn is the btrfs chunk-over-allocation engine behind the recurring DiskPressure wedges #2999 documents.

Two compounding bugs made it far worse than it had to be:

• the build context silently included .claude/ (3.1 GB of worktrees), .mypy_cache, .pytest_cache, .egg-state, .hypothesis — the per-edit COPY . layer was 1.91 GB for a 65 MB repo;
• Stage 3 cp -a'd the persisted system dirs over a tree the image already contained, storing ~2 GB twice.

What this PR does

Publish path

• New loopback-only registry flow: make registry-setup (one-time, also run by make k3s-setup) starts a registry:2 container bound to 127.0.0.1 and points k3s at it via registries.yaml; make k3s-push pushes + pre-pulls. Push and pull are both layer-aware — measured 31 s for the cold 442 MB gateway push, 0.03 s for a second tag of the same image. Nothing is ever published off-host, and push-egg-images.sh hard-refuses non-loopback registries with no override.
• The sandbox image is excluded from the registry by default (EGG_REGISTRY_IMAGES = egg-gateway egg-orchestrator egg-litellm): it bakes in private repo content, so it stays on the store-to-store k3s-import path unless the operator opts in. k3s-import now serializes each image once (single multi-tag tarball) instead of once per tag, halving its I/O.
• EGG_IMAGE_REGISTRY= (empty) falls back to pure save+import everywhere; CI pins that (plus BUILD_JOBS=1) and keeps its inline import step unchanged.

Image diet

• .dockerignore exclusions: per-edit COPY . layer 1.91 GB → 23 MB.
• persist_system_dirs moves to /opt/egg-system-dirs + COPY --from restore: kills the ~2 GB duplicate layer. Sandbox image 9.29 GB → 5.33 GB.
• The four image builds run under make -j (BUILD_JOBS, default 4).

Disk hygiene (levers B + C of #2999)

• reap-stale-egg-images.sh grows three scopes: docker-store stale-tag reap + BuildKit cache cap (EGG_BUILDKIT_CACHE_CAP, 40 GB default), registry stale-manifest reap + garbage-collect --delete-untagged, and a btrfs unallocated-chunk check (warn <16 GiB, auto-balance <4 GiB). The containerd reap understands per-image authoritative ref forms (registry-qualified vs bare) and treats non-authoritative leftovers as stale, digest-guarded as before.
• New make btrfs-reclaim (the make redeploy recurringly wedges local k3s (DiskPressure / 'images not in k3s') — root cause: btrfs chunk over-allocation from image churn #2999 runbook command, first-class).

Net effect

Default config (sandbox on import): per-edit redeploy I/O drops from ~40 GB to ~11 GB (one 5.33 GB save+import instead of two 9.29 GB ones) plus ~25 MB of registry traffic for the core images. Opting the sandbox into the registry drops it to ~tens of MB total. Either way the tarball/serialization churn that fragments btrfs is mostly or entirely gone, and the reap + balance hooks bound what remains.

Verification

• Live on the dev host: registry container + push + manifest checks + stale-tag reap + GC (planted a stale tag, watched it reaped and its blob collected; digest-shared tags spared).
• Full reap dry-run against a stubbed containerd listing exercising the hybrid prefix logic (registry-subset keeps, bare sandbox keeps, legacy/non-authoritative refs reaped, digest guard).
• make -n renders verified in all three configs (default hybrid / sandbox opt-in / CI no-registry).
• Full make build with the new Dockerfile: green, sizes above; 194 targeted tests pass (test_docker_setup, test_entrypoint); make lint green.
• NOT exercised here (needs attended sudo): make registry-setup's registries.yaml write + k3s restart, and an end-to-end make redeploy against the live cluster.

Operator notes

One-time: make registry-setup (writes /etc/rancher/k3s/registries.yaml, restarts the k3s service — pods keep running). Then make redeploy as usual. To opt the sandbox into the registry path: EGG_REGISTRY_IMAGES="egg-gateway egg-orchestrator egg-sandbox egg-litellm" make redeploy (or export it).

[james-in-a-box]

Autofix tracking

{"Test/Unit Tests": 1}

[egg-reviewer]

No agent-mode design concerns. This PR is scoped to deployment infrastructure (Docker .dockerignore, parallel image builds, loopback registry publish path, btrfs reclaim, image reaping) and doesn't touch agent prompts, LLM call sites, model identifiers, or output-parsing pipelines.

— Authored by egg

[egg-reviewer]

Review

Substantial perf/disk-hygiene refactor of the make redeploy publish path. I traced the diff end-to-end against the existing Makefile/scripts and the consuming code paths (deploy sed rewrites, pre-flight check, post-deploy reap, CI workflow). The implementation is careful and well-commented, the safety invariants from the prior reap script (full-four-image safety gate, digest guard) are preserved, and the privacy guard around off-host registries is strong. No security or correctness blockers.

Detail below — all non-blocking.

What I verified end-to-end

• Three configurations are consistent: default hybrid (registry-subset = gateway/orch/litellm; sandbox via import), full opt-in (EGG_REGISTRY_IMAGES includes egg-sandbox), and no-registry (EGG_IMAGE_REGISTRY="", CI). For each, the image_tags macro, the deploy target's sed rewrites, the check-egg-images-present.sh lookup arm, and the reap-stale-egg-images.sh authority logic all line up.
• Containerd reap auth logic: a bare docker.io/library/egg-gateway:KEEP_TAG ref under hybrid mode (where egg-gateway is in the registry subset) is not treated as authoritative — it falls into the candidate list, but the digest guard then spares it because it shares the authoritative localhost:5000/egg-gateway:KEEP_TAG digest. That's the right outcome and matches the comment.
• Privacy guard: push-egg-images.sh validates ${REGISTRY%%:*} against localhost/127.0.0.1 and refuses anything else with no override. The setup-local-registry.sh script docker run -p 127.0.0.1:${PORT}:5000 so even host-internal IPs other than loopback can't reach the registry.
• CI path: EGG_IMAGE_REGISTRY="" correctly forces EGG_IMAGE_PREFIX="" → reg_prefix returns empty → bare tags everywhere → check-egg-images-present.sh skips the registry HEAD check → matches the inline docker save | ctr import pipeline. CI's reap call also passes "" as registry, so the script's REGISTRY_SUBSET ends up empty and the no-registry behavior is preserved bit-for-bit.

Non-blocking suggestions

1. Testing gap for the new reap paths

tests/scripts/test_reap_stale_egg_images.py was updated to track the awk extractor change (good — the AUTH_BARE_RE=...awk ' anchor is robust), but the existing test cases all exercise only the no-registry case (REGISTRY="", AUTH_REG_RE='^$'). The PR description acknowledges the hybrid path was exercised by a one-time manual dry-run.

The hybrid logic is the heart of the new behavior — registry-subset authoritative form, bare-prefix non-authoritative form on subset images, mixed authority across IMAGES[], the auth-aware safety-gate prefix selection (reap-stale-egg-images.sh:128-137), and the digest guard's interaction with refs that match match_re but neither auth_reg_re nor auth_bare_re. A regression in any of those would silently stop reaping rather than mis-reap (so it's not destructive), but a slow disk fill-up creeping back is the exact thing #2999 was about. Worth a TestReapHybridMode class with at least:

• registry-subset image kept via authoritative localhost:5000/... ref, bare leftover docker.io/library/...:KEEP_TAG with same digest → bare ref left alone (digest guard).
• registry-subset image kept via authoritative ref, bare stale docker.io/library/...:OLD with different digest → bare ref reaped.
• bare-subset image (egg-sandbox) authoritative as bare, registry-qualified leftover with same digest → registry ref left alone.

The docker-store reap and registry-side reap are likewise untested. Less critical (no destructive failure mode) but worth at least a smoke test that the DOCKER_REF_RE interpolation doesn't accidentally match unrelated images when REGISTRY contains a name like localhost:5000 — the regex anchors on ^ so it's safe, but a future tweak could regress.

2. EGG_REGISTRY_IMAGES="" + non-empty EGG_IMAGE_REGISTRY is a footgun

Makefile:722-730:

k3s-publish: sudo-keepalive
	@if [ -n "$(EGG_IMAGE_REGISTRY)" ]; then \
		$(MAKE) --no-print-directory k3s-push; \
		if [ -n "$(strip $(EGG_IMPORT_IMAGES))" ]; then \
			$(MAKE) --no-print-directory k3s-import K3S_IMPORT_IMAGES="$(EGG_IMPORT_IMAGES)"; \
		fi; \
	else \
		$(MAKE) --no-print-directory k3s-import; \
	fi

If an operator empties the registry-subset (EGG_REGISTRY_IMAGES="") but leaves EGG_IMAGE_REGISTRY set (thinking "I want all images via import"), k3s-push is still invoked, and push-egg-images.sh:28 : "${3:?$usage}" aborts with usage: — make redeploy fails. The canonical way to "import everything" is EGG_IMAGE_REGISTRY="", but the empty-subset variant fails non-obviously. Suggest:

@if [ -n "$(EGG_IMAGE_REGISTRY)" ] && [ -n "$(strip $(EGG_REGISTRY_IMAGES))" ]; then \
    $(MAKE) --no-print-directory k3s-push; \
    ...

and fall through to the all-import branch otherwise.

3. "Restart bounces the service, not the pods" — approximate at best

scripts/setup-local-registry.sh:119-120:

echo "    (k3s only reads registries.yaml at startup; the restart bounces the"
echo "    k3s service itself, not the running pods)"

K3s's embedded containerd is a child of the k3s server process. systemctl restart k3s does take down containerd; running pod sandboxes survive (their network namespaces are not torn down), but kubelet briefly disconnects and individual container processes can be affected depending on probe configuration. The script is only ever called against a fresh k3s (no egg pods deployed yet), so this is academic in practice — but the operator-facing claim is more confident than the behavior warrants. Suggest softening to e.g. "the restart is brief; running pod containers reattach when containerd comes back" — or just remove the parenthetical so an operator who does run it against a populated cluster isn't surprised.

4. Parallel docker builds still ship repo-deps/ to non-sandbox contexts

The .dockerignore tightening is great (1.91 GB → 23 MB on the COPY . layer), but repo-deps/ (generated by prepare-sandbox-build-context.py, only used by sandbox/Dockerfile) is not in .dockerignore. Each of the four parallel docker build invocations sends it as part of the context. On a typical checkout with full webapp/mobile/etc. that's 50-200 MB of context the gateway/orchestrator/litellm builds don't read.

Two options:

• Add repo-deps/ to .dockerignore and override per-image with a sandbox/.dockerignore (Docker reads the dockerfile-adjacent one when -f points outside the context root — but only in BuildKit, and even then it's the combined effect). Simplest: ignore repo-deps/ in the root and use COPY --from= from a separate prep stage in sandbox.
• Or, easier, accept the cost — it's small relative to the ~5 GB sandbox base layers and the savings here would be a one-time optimization.

Not worth blocking the PR over; flagging because the diet effort is what motivated the rest of #2999.

5. Redundant :latest push when EGG_IMAGE_TAG == "latest"

scripts/push-egg-images.sh:71-77:

for image in "${IMAGES[@]}"; do
  echo ">>> pushing ${REGISTRY}/${image}:${TAG}"
  docker push "${REGISTRY}/${image}:${TAG}"
  echo ">>> pushing ${REGISTRY}/${image}:latest"
  docker push "${REGISTRY}/${image}:latest"
done

When EGG_IMAGE_TAG == "latest" (the git describe fallback outside a git checkout), the two pushes hit the same ref. The second is a manifest-only no-op so it's harmless, but logs are misleading. Mirror the Makefile's k3s-import pattern:

tags=("$TAG")
[ "$TAG" != "latest" ] && tags+=("latest")
for image in "${IMAGES[@]}"; do
  for tag in "${tags[@]}"; do
    docker push "${REGISTRY}/${image}:${tag}"
  done
done

6. Cross-script image-set duplication (pre-existing, not introduced here)

EGG_ALL_IMAGES in the Makefile, ALL_IMAGES in check-egg-images-present.sh, IMAGES in reap-stale-egg-images.sh — three copies of the same list, with comments asking to keep them in sync. The PR doesn't make this worse and explicitly calls out the contract in the comments, but adding a 5th image down the road still requires three coordinated edits. Out of scope for this PR; worth a follow-up issue.

Things I checked and found fine

• The escape_re character class (s/[][\\.*^$+?(){}|]/\\&/g) is correct — ] first, \\ as literal backslash, no / (the comment justification holds).
• The (A && B) || C exit gate at reap-stale-egg-images.sh:278 works correctly for all four input combinations.
• The awk programs use ENVIRON[] (not -v) for the regex fragments containing \. — the test extractor's anchor (AUTH_BARE_RE="$AUTH_BARE_RE"\s+awk\s+'(.+?)'\s*<<<) uniquely identifies the containerd reap block and won't drift onto the later docker-store awk.
• The mkdir -p /opt/prebuilt-deps /opt/egg-system-dirs in sandbox/Dockerfile:65 makes the Stage 3 COPY --from=repo-deps /opt/egg-system-dirs/ / safe even when no persist_system_dirs were declared — empty source dir, no-op copy.
• entrypoint.py:858-863 keeps the __egg_system_dirs__ skip for backward compat with images built before this change. Correct: a current entrypoint running against an older image would otherwise restore system dirs into a repo mount.
• The CI workflow's existing inline import is untouched; with EGG_IMAGE_REGISTRY="" set as a job env, the rest of the make machinery (sed rewrites, pre-flight check, reap) degrades correctly to the pre-registry behavior.
• The registry-side reap's manifest_digest function's || true is load-bearing under set -e -o pipefail; the comment correctly identifies why.

Approving. The non-blocking items above (especially #1 and #2) are worth a follow-up but don't gate this merge.

— Authored by egg

[james-in-a-box]

Thanks for the thorough review. Addressing the non-blocking suggestions:

1. Testing gap for the new reap paths — fixed-in-PR (commit 6bcaf26)
Added TestReapHybridMode (7 tests) covering the cases you called out plus
:latest-on-authoritative-prefix sharing-digest protection and authoritative-
form stale reaps under the hybrid split. Generalized _run_awk to take
registry / registry_subset kwargs so the PREFIX_ALT_RE / AUTH_REG_RE /
AUTH_BARE_RE construction in the test mirrors reap-stale-egg-images.sh:88-114
(noted in the helper docstring so a future regex tweak there flags the test).
The docker-store reap and registry-side reap are still un-tested — agreed they
have no destructive failure mode and would need a different harness (the
docker-store reap shells out to docker rmi); leaving those for a future
harness improvement.

2. EGG_REGISTRY_IMAGES="" + non-empty EGG_IMAGE_REGISTRY footgun — fixed-in-PR (commit 6bcaf26)
Applied your suggested gating: k3s-publish now requires both
EGG_IMAGE_REGISTRY AND non-empty EGG_REGISTRY_IMAGES for the push branch,
otherwise falls through to the all-import branch. Updated the comment block
above the target to reflect the extended fall-through condition.

3. "Restart bounces the service, not the pods" — fixed-in-PR (commit 6bcaf26)
Softened to "the restart is brief and running pod containers reattach when
containerd comes back" — keeps the operator-facing meaning ("you don't have to
re-kubectl apply") without overstating the isolation guarantee.

4. Parallel docker builds shipping repo-deps/ to non-sandbox contexts — fixed-in-PR (commit 6bcaf26)
Went with the per-Dockerfile dockerignore approach: repo-deps/ excluded in
root .dockerignore, and sandbox/Dockerfile.dockerignore (BuildKit's
<Dockerfile>.dockerignore lookup, adjacent to the Dockerfile) re-includes it
by mirroring the root rules minus that one line. Drift risk acknowledged in
comments at the top of both files. Drops 50-200 MB of context transfer per
non-sandbox build × 3 builds — small relative to the per-edit COPY . savings
already in the PR, but cheap and aligned with the diet effort that motivated
the rest of #2999.

5. Redundant :latest push when EGG_IMAGE_TAG == "latest" — fixed-in-PR (commit 6bcaf26)
Adopted the k3s-import pattern verbatim: build a tags array, append
"latest" only when TAG != "latest", then loop. Comment clarifies the
"git describe fallback outside a checkout" trigger.

6. Cross-script image-set duplication — deferred-to #3102
Explicit defer per your "Out of scope for this PR; worth a follow-up issue."
Filed #3102 with the three
duplication sites, the proposed centralization approach (canonicalize in
EGG_ALL_IMAGES, pass into both scripts), and an acceptance criterion that a
test enforces the single-source-of-truth invariant.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns. The re-review delta (fe96364..6bcaf26, single commit) is scoped to deployment infrastructure: .dockerignore (plus a sandbox/Dockerfile.dockerignore BuildKit override), Makefile k3s-publish gating, push-egg-images.sh :latest-push dedup, setup-local-registry.sh wording softening, and new TestReapHybridMode pytest cases for the reap script. Nothing in the delta touches agent prompts, LLM call sites, model identifiers, the Agent SDK boundary, or output-parsing pipelines.

— Authored by egg

[egg-reviewer]

Re-review

Re-reviewed the delta fe96364..6bcaf26 (the "Address review feedback" commit). All five non-blocking suggestions from the previous review were addressed cleanly, and the deferral of #6 was filed as a follow-up. I traced each fix against the production code paths it touches; CI is green on every required check.

Verification of previous-review items

1. TestReapHybridMode (suggestion #1) — Added with 7 cases that pin the hybrid-mode reap logic against the script's auth-aware regex construction:

   • subset image, bare leftover sharing digest → spared (digest guard)
   • subset image, bare stale with distinct digest → reaped
   • bare-subset (sandbox), registry-qualified leftover sharing digest → spared
   • bare-subset, registry-qualified leftover distinct digest → reaped
   • mixed stale refs across all four images → all reaped
   • authoritative-form stale refs with distinct digests → reaped
   • :latest-on-authoritative-prefix sharing-digest protection (a bonus case beyond what I asked for)

   The _run_awk helper is generalized correctly: registry=""/registry_subset=() keeps the existing no-registry tests' behavior bit-for-bit (verified by reading the existing five test classes — their call sites are unchanged). With non-empty kwargs, the PREFIX_ALT_RE / AUTH_REG_RE / AUTH_BARE_RE construction at tests/scripts/test_reap_stale_egg_images.py:88-98 mirrors reap-stale-egg-images.sh:88-114 line-for-line, and the docstring explicitly points at both line ranges to keep the test in sync on future tweaks. Docker-store and registry-side reap deferrals are correctly justified (no destructive failure mode, would need a different harness).

2. Empty-subset footgun (suggestion #2) — Makefile:723 now requires both EGG_IMAGE_REGISTRY AND non-empty EGG_REGISTRY_IMAGES for the push branch; otherwise falls through to bare k3s-import (which defaults K3S_IMPORT_IMAGES := $(EGG_ALL_IMAGES) at line 801). Traced the empty-subset case through the rest of the pipeline:

   • reg_prefix(X) returns empty for every X → deploy sed rewrites produce bare egg-X:$TAG tags
   • check-egg-images-present.sh puts every image into CONTAINERD_IMAGES (no registry HEAD check)
   • k3s-import imports as docker.io/library/egg-X:$TAG, which k3s resolves bare names to

   All three legs degrade consistently to the no-registry path. Good.

3. Registry restart wording (suggestion #3) — setup-local-registry.sh:119-120 softened to "the restart is brief and running pod containers reattach when containerd comes back." Accurate.

4. Per-Dockerfile dockerignore for repo-deps/ (suggestion #4) — repo-deps/ added to root .dockerignore, and sandbox/Dockerfile.dockerignore mirrors the root minus that one line. Verified with diff — the only content differences are the documented header comments and the single repo-deps/ exclusion. The KEEP-IN-SYNC comment is in both files.

5. Redundant :latest push (suggestion #5) — push-egg-images.sh:74-82 now uses the k3s-import array-loop pattern. When TAG="latest" (git describe fallback outside a checkout), the array stays ("latest") and only one push fires.

6. Cross-script image duplication (#6) — Deferred to #3102 as flagged.

One new non-blocking observation in the delta

Per-Dockerfile .dockerignore is a BuildKit-only feature; nothing enforces BuildKit.

The new sandbox/Dockerfile.dockerignore is honored only when BuildKit is the active builder. Docker made BuildKit the default in 23.0 (Feb 2023). On Docker 20.10-22.x with no DOCKER_BUILDKIT=1, the legacy builder ignores <Dockerfile>.dockerignore and falls back to the root .dockerignore — which now excludes repo-deps/. The sandbox build's Stage 1 COPY repo-deps/ /tmp/repo-deps/ would then fail with COPY failed: file not found in build context or excluded by .dockerignore.

skills/egg-setup/SKILL.md:33 still documents Docker 20.10+ as the minimum. This PR implicitly bumps that to 23.0+ for anyone not setting DOCKER_BUILDKIT=1. In practice everyone on 2026 Docker is on 23+ (Docker Desktop auto-updates, modern distro packages ship 24+), but the dependency is silent — the failure mode is a confusing COPY failed rather than "you need BuildKit."

Three ways to harden, pick whichever:

• Bump the documented minimum to Docker 23.0 in skills/egg-setup/SKILL.md
• Add DOCKER_BUILDKIT=1 to build-gateway / build-orchestrator / build-sandbox / build-litellm in the Makefile (also nudges legacy builds toward modern caching behavior)
• Add an early check in make build that errors with a clear "BuildKit required for per-Dockerfile dockerignore (since #2999)" message if the env doesn't have it

The comment at the top of sandbox/Dockerfile.dockerignore ("BuildKit feature") acknowledges awareness, but the runtime path doesn't gate on it.

Optional follow-up: drift test for the two dockerignore files

The KEEP-IN-SYNC contract between root .dockerignore and sandbox/Dockerfile.dockerignore is currently enforced by comments only. A 5-line test that diffs the two files modulo repo-deps/ would catch drift mechanically:

def test_sandbox_dockerignore_mirrors_root():
    root = set(read_pattern_lines(".dockerignore"))
    sandbox = set(read_pattern_lines("sandbox/Dockerfile.dockerignore"))
    assert root - sandbox == {"repo-deps/"}, \
        "Drift between root .dockerignore and sandbox/Dockerfile.dockerignore — see KEEP-IN-SYNC comment"

Cheap, mechanically enforces the invariant, runs in milliseconds.

Things I checked and found fine

• Test extraction's re.escape(f"{registry}/") vs script's escape_re are output-identical for all plausible registry hostnames (localhost, 127.0.0.1, hostnames with ., hostnames with -) — both escape . and neither escapes : or / or -. Confirmed against the script's sed metachar class [][\\.*^$+?(){}|] and Python 3.7+ re.escape semantics.
• The AUTH_BARE_RE="$AUTH_BARE_RE"\s+awk\s+'(.+?)'\s*<<< anchor in _extract_awk_program still uniquely identifies the containerd reap block; it doesn't drift onto the later docker-store awk (which uses DOCKER_REF_RE instead).
• Existing five test classes (TestReapAwkDigestGuard and below) still pass through the generalized _run_awk with default kwargs — verified by reading their call sites; none pass registry=/registry_subset=.

Approving with the BuildKit-version note and the drift-test idea as non-blocking suggestions. The PR is ready to ship.

— Authored by egg

[james-in-a-box]

Thanks for the re-review. Addressing both non-blocking suggestions:

1. BuildKit dependency for per-Dockerfile <Dockerfile>.dockerignore — fixed-in-PR (commit f03d895)

Went with option 2 of the three you offered (pin DOCKER_BUILDKIT=1 on the build invocations) rather than option 1 (bump documented Docker minimum) or option 3 (early check in make build). Reasoning:

• BuildKit-as-builder is honored by Docker 18.09+ when DOCKER_BUILDKIT=1 is set — Docker 23.0 only changed the default. Pinning the env var widens the supported floor (back to 18.09) instead of narrowing it, and works regardless of whether the operator has set up daemon.json features.
• It moves the guarantee from "you must read the docs and bump Docker, and remember to do so on every machine" to "the Makefile makes it true." Less drift potential than a documented minimum that nothing enforces.
• An early make build check would fire correctly but the failure mode without the env-var pin is precisely the confusing COPY failed: file not found in build context or excluded by .dockerignore you flagged — pinning the env var fixes the root cause; a check would just label it.

Applied to all four build sub-targets (build-gateway / build-orchestrator / build-sandbox / build-litellm). The comment block above the four targets explains why and what would break without it (Makefile:523-532).

I considered also bumping skills/egg-setup/SKILL.md:33 from "Docker 20.10+" to "Docker 23.0+" but with the env-var pin in place, the older floor remains correct — and unnecessarily narrowing the documented requirement would frustrate any operator on a Docker version that does work. Left the doc unchanged.

2. Drift test for the two dockerignore files — fixed-in-PR (commit f03d895)

Added tests/scripts/test_dockerignore_drift.py. The single test (test_sandbox_dockerignore_mirrors_root) diffs the pattern sets of .dockerignore and sandbox/Dockerfile.dockerignore:

• root - sandbox - {"repo-deps/"} must be empty (no exclusion missing from the sandbox override)
• sandbox - root must be empty (no exclusion in the sandbox override that's not in root)
• The allowed delta (repo-deps/) must be in root and must not be in sandbox (catches the case where someone deletes the override entirely without removing the sandbox file)

Cross-referenced the test from the comment headers in both .dockerignore files so future editors find the guard rail. Verified the test passes locally (pytest tests/scripts/test_dockerignore_drift.py -v → 1 passed in 0.41s) and that the existing 14 reap tests still pass alongside it.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns. The re-review delta (6bcaf26..f03d895, single commit) is scoped to deployment infrastructure: pinning DOCKER_BUILDKIT=1 on the four build-* Makefile targets, a new tests/scripts/test_dockerignore_drift.py mechanical drift guard, and cross-reference comment updates in both .dockerignore files. Nothing in the delta touches agent prompts, LLM call sites, model identifiers, the Agent SDK boundary, or output-parsing pipelines.

— Authored by egg

[egg-reviewer]

Re-review (delta 6bcaf26..f03d895)

Re-reviewed the single new commit f03d895 ("Address re-review: force BuildKit + drift guard for dockerignore"). Both non-blocking suggestions from my prior review are addressed cleanly; no new issues introduced.

Verification of previous-review items

1. BuildKit dependency (suggestion #1) — fixed. Makefile:535-545 now pins DOCKER_BUILDKIT=1 on build-gateway, build-orchestrator, build-sandbox, and build-litellm. The new comment block at Makefile:527-534 explains the constraint and points future editors at sandbox/Dockerfile.dockerignore. Choosing option 2 (env-var pin) over option 1 (docs bump) or option 3 (early check) is the right call — it makes the contract Makefile-enforced rather than docs-enforced.

Verified the pin reaches every path that builds these images:

• Local: make build → make -j build-{gateway,orchestrator,sandbox,litellm} (all pinned)
• CI: .github/workflows/test-integration.yml:85 invokes make build, so it inherits the pin
• No other docker build invocations remain in the repo's build paths — only documentation examples (docs/guides/deploy-migration.md:63, docs/guides/deployment.md:85-88) that are not executed

2. Drift test (suggestion #2) — fixed. tests/scripts/test_dockerignore_drift.py parses pattern sets from both files (stripping blanks and comments) and asserts the symmetric-difference is exactly {"repo-deps/"}. The assertion structure is the right shape:

• (root - sandbox) - ALLOWED_SANDBOX_DELTA empty → no new root exclusion missing from sandbox
• sandbox - root empty → no sandbox-only exclusion sneaking in
• ALLOWED_SANDBOX_DELTA.issubset(root) → catches "someone deleted the override entirely but left the sandbox file"
• not ALLOWED_SANDBOX_DELTA & sandbox → catches "someone added repo-deps/ to the sandbox override"

Manually verified the files are in sync today (only delta is repo-deps/). Cross-references from the comment headers of both .dockerignore files point future editors at the test, which closes the loop.

One minor inaccuracy in the new comment block (non-blocking)

Makefile:533-534 says "older Docker (18.09–22.x, which supports BuildKit but doesn't default to it) on the same path." That's true for invoking BuildKit as the builder, but the per-Dockerfile <Dockerfile>.dockerignore feature specifically requires BuildKit ≥ 0.10 (released March 2022). Docker bundles BuildKit, and 18.09–early-22.x shipped older BuildKit (0.3–0.9). So the real floor for this feature is roughly Docker 20.10.20+ / 22.x-late, not 18.09.

In practice this is academic — skills/egg-setup/SKILL.md:33 documents Docker 20.10+ as the minimum, and anyone in 2026 is on Docker 24+. But the comment overstates the support window; if you want to be precise, "older Docker (20.10.x late / 22.x, which ships BuildKit but doesn't default to it)" is more accurate. Not worth a follow-up commit on its own.

Things I checked and found fine

• Drift test handles edge cases correctly: blank lines skipped, comment lines skipped (.lstrip().startswith("#") handles indented comments too, though there are none), set semantics make order-independence explicit. No false positives on the current files.
• The pin doesn't introduce a cache-invalidation regression: BuildKit and legacy builder use separate caches, so first build under BuildKit pays a one-time cost. Subsequent rebuilds reuse the BuildKit cache as designed.
• CI is green on f03d895 (all required checks COMPLETED/SUCCESS).
• The _read_patterns helper correctly handles !-negation patterns as opaque strings — if anyone adds negations to either file, the test still catches drift on string-equality.

The PR is ready to ship.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

11 previous review(s) hidden.