From f4ce03b58880811257dbabd1ec1104014777cd44 Mon Sep 17 00:00:00 2001 From: MarMar Labs Date: Fri, 21 Aug 2026 05:21:31 -0500 Subject: [PATCH 1/3] fix(sandbox): default the pool size when tearing down, as startup does start.sh defaults SANDBOX_EXECUTOR_MANAGER_POOL_SIZE to 5 whether or not .env supplies it, so it creates and cleans indices 0..4. stop.sh and the Makefile clean target source .env and then use the variable bare, so an .env that omits it leaves the range 0..-1 and the containers startup created are not removed, while both still report cleanup complete. Mirror the default start.sh already applies. An .env that sets the value is unaffected. --- agent/sandbox/Makefile | 1 + agent/sandbox/scripts/stop.sh | 1 + 2 files changed, 2 insertions(+) diff --git a/agent/sandbox/Makefile b/agent/sandbox/Makefile index bcd5eef76a9..cc8afa91bb6 100644 --- a/agent/sandbox/Makefile +++ b/agent/sandbox/Makefile @@ -104,6 +104,7 @@ clean: @docker compose down -v || true @if [ -f .env ]; then \ source .env && \ + SANDBOX_EXECUTOR_MANAGER_POOL_SIZE="$${SANDBOX_EXECUTOR_MANAGER_POOL_SIZE:-5}" && \ for i in $$(seq 0 $$((SANDBOX_EXECUTOR_MANAGER_POOL_SIZE - 1))); do \ echo "🧹 Deleting sandbox_python_$$i..." && \ docker rm -f sandbox_python_$$i 2>/dev/null || true && \ diff --git a/agent/sandbox/scripts/stop.sh b/agent/sandbox/scripts/stop.sh index 51bd2b6e93b..02a4a056645 100755 --- a/agent/sandbox/scripts/stop.sh +++ b/agent/sandbox/scripts/stop.sh @@ -26,6 +26,7 @@ docker compose down echo "🧹 Deleting sandbox containers..." if [ -f .env ]; then source .env + SANDBOX_EXECUTOR_MANAGER_POOL_SIZE="${SANDBOX_EXECUTOR_MANAGER_POOL_SIZE:-5}" # Default to 5 if not set in .env, as in start.sh for i in $(seq 0 $((SANDBOX_EXECUTOR_MANAGER_POOL_SIZE - 1))); do echo "🧹 Deleting sandbox_python_$i..." docker rm -f "sandbox_python_$i" >/dev/null 2>&1 || true From 6fabdf8354fce21ae0f38153e2b8029514bf5b7e Mon Sep 17 00:00:00 2001 From: MarMar Labs Date: Tue, 25 Aug 2026 01:30:47 -0500 Subject: [PATCH 2/3] fix(sandbox): remove the sandbox containers that exist, not a pool-size range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Teardown walked `seq 0 $((SANDBOX_EXECUTOR_MANAGER_POOL_SIZE - 1))`, so cleanup was only correct when the current .env still described the pool that was actually created: - a pool started at size 8 and torn down after .env said 5 left sandbox_python_5..7 and sandbox_nodejs_5..7 running, - a missing .env skipped cleanup altogether rather than falling back, - a container that was created but never joined the executor-manager queue was never inside the range to begin with. Both teardown paths now list the containers docker actually has and remove the ones whose names match the generated pool exactly. The pattern is anchored, so a container that merely contains "sandbox_python_" in its name — say my_sandbox_python_1 or sandbox_python_1_backup — is left alone. Defaulting the pool size, which this branch did before, fixed only the reported symptom and kept teardown coupled to a value that can no longer describe the pool. --- agent/sandbox/Makefile | 22 ++++++++++++---------- agent/sandbox/scripts/stop.sh | 25 ++++++++++++++----------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/agent/sandbox/Makefile b/agent/sandbox/Makefile index cc8afa91bb6..d903e2361b4 100644 --- a/agent/sandbox/Makefile +++ b/agent/sandbox/Makefile @@ -102,15 +102,17 @@ logs: clean: @echo "🧹 Cleaning all containers and volumes..." @docker compose down -v || true - @if [ -f .env ]; then \ - source .env && \ - SANDBOX_EXECUTOR_MANAGER_POOL_SIZE="$${SANDBOX_EXECUTOR_MANAGER_POOL_SIZE:-5}" && \ - for i in $$(seq 0 $$((SANDBOX_EXECUTOR_MANAGER_POOL_SIZE - 1))); do \ - echo "🧹 Deleting sandbox_python_$$i..." && \ - docker rm -f sandbox_python_$$i 2>/dev/null || true && \ - echo "🧹 Deleting sandbox_nodejs_$$i..." && \ - docker rm -f sandbox_nodejs_$$i 2>/dev/null || true; \ - done; \ + @# Same discovery as scripts/stop.sh: the containers that exist, not a range derived from + @# SANDBOX_EXECUTOR_MANAGER_POOL_SIZE, which misses a pool created at a different size, skips + @# cleanup entirely when .env is gone, and never covers a container that failed to join the + @# executor-manager queue. + @sandbox_containers="$$(docker ps -a --format '{{.Names}}' 2>/dev/null | \ + grep -E '^sandbox_(python|nodejs)_[0-9]+$$' || true)"; \ + if [ -n "$$sandbox_containers" ]; then \ + while IFS= read -r container; do \ + echo "🧹 Deleting $$container..."; \ + docker rm -f "$$container" >/dev/null 2>&1 || true; \ + done <<<"$$sandbox_containers"; \ else \ - echo "⚠️ .env not found, skipping container cleanup"; \ + echo "✅ No sandbox containers found"; \ fi diff --git a/agent/sandbox/scripts/stop.sh b/agent/sandbox/scripts/stop.sh index 02a4a056645..d6fec56b376 100755 --- a/agent/sandbox/scripts/stop.sh +++ b/agent/sandbox/scripts/stop.sh @@ -24,18 +24,21 @@ echo "🛑 Stopping all services..." docker compose down echo "🧹 Deleting sandbox containers..." -if [ -f .env ]; then - source .env - SANDBOX_EXECUTOR_MANAGER_POOL_SIZE="${SANDBOX_EXECUTOR_MANAGER_POOL_SIZE:-5}" # Default to 5 if not set in .env, as in start.sh - for i in $(seq 0 $((SANDBOX_EXECUTOR_MANAGER_POOL_SIZE - 1))); do - echo "🧹 Deleting sandbox_python_$i..." - docker rm -f "sandbox_python_$i" >/dev/null 2>&1 || true - - echo "🧹 Deleting sandbox_nodejs_$i..." - docker rm -f "sandbox_nodejs_$i" >/dev/null 2>&1 || true - done +# Remove the sandbox containers that actually exist instead of walking a range derived from +# SANDBOX_EXECUTOR_MANAGER_POOL_SIZE. A pool created at one size and torn down after .env changed +# left every container above the new size running; a missing .env skipped cleanup altogether; and a +# container that never joined the executor-manager queue was never in the range to begin with. +# The pattern is anchored to the generated pool names, so an unrelated container that merely +# contains "sandbox_python_" in its name is not touched. +sandbox_containers="$(docker ps -a --format '{{.Names}}' 2>/dev/null | + grep -E '^sandbox_(python|nodejs)_[0-9]+$' || true)" +if [ -n "$sandbox_containers" ]; then + while IFS= read -r container; do + echo "🧹 Deleting $container..." + docker rm -f "$container" >/dev/null 2>&1 || true + done <<<"$sandbox_containers" else - echo "⚠️ .env not found, skipping container cleanup" + echo "✅ No sandbox containers found" fi echo "✅ Stopping and cleanup complete" From ce5a0a5e8682486fe7e5b4800b16108ea3fccda2 Mon Sep 17 00:00:00 2001 From: MarMar Labs Date: Tue, 25 Aug 2026 01:39:57 -0500 Subject: [PATCH 3/3] fix(sandbox): fail teardown when container discovery fails The discovery added for the pool-size fix used `|| true` to tolerate grep's no-match exit, which also swallowed a failing `docker ps`. An unreachable daemon then produced an empty list, and teardown reported "No sandbox containers found" and exited 0 while every container was still running. Only grep's exit is tolerated now; a discovery failure prints why and exits non-zero, so a successful empty result stays distinguishable from a failed lookup. --- agent/sandbox/Makefile | 8 +++++++- agent/sandbox/scripts/stop.sh | 9 ++++++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/agent/sandbox/Makefile b/agent/sandbox/Makefile index d903e2361b4..f7aa3ac8117 100644 --- a/agent/sandbox/Makefile +++ b/agent/sandbox/Makefile @@ -106,7 +106,13 @@ clean: @# SANDBOX_EXECUTOR_MANAGER_POOL_SIZE, which misses a pool created at a different size, skips @# cleanup entirely when .env is gone, and never covers a container that failed to join the @# executor-manager queue. - @sandbox_containers="$$(docker ps -a --format '{{.Names}}' 2>/dev/null | \ + @# A failed `docker ps` and an empty result both collapse to an empty string, and only the + @# first means the containers are still running. Tolerate grep's no-match exit, not docker's. + @if ! all_containers="$$(docker ps -a --format '{{.Names}}')"; then \ + echo "❌ Could not list Docker containers; sandbox containers were NOT removed" >&2; \ + exit 1; \ + fi; \ + sandbox_containers="$$(printf '%s\n' "$$all_containers" | \ grep -E '^sandbox_(python|nodejs)_[0-9]+$$' || true)"; \ if [ -n "$$sandbox_containers" ]; then \ while IFS= read -r container; do \ diff --git a/agent/sandbox/scripts/stop.sh b/agent/sandbox/scripts/stop.sh index d6fec56b376..8e2ec02ff23 100755 --- a/agent/sandbox/scripts/stop.sh +++ b/agent/sandbox/scripts/stop.sh @@ -30,7 +30,14 @@ echo "🧹 Deleting sandbox containers..." # container that never joined the executor-manager queue was never in the range to begin with. # The pattern is anchored to the generated pool names, so an unrelated container that merely # contains "sandbox_python_" in its name is not touched. -sandbox_containers="$(docker ps -a --format '{{.Names}}' 2>/dev/null | +# `docker ps` failing and finding nothing look identical once both become an empty string, and +# the difference matters: the first leaves every sandbox container running while reporting +# success. Only grep's no-match exit is tolerated here. +if ! all_containers="$(docker ps -a --format '{{.Names}}')"; then + echo "❌ Could not list Docker containers; sandbox containers were NOT removed" >&2 + exit 1 +fi +sandbox_containers="$(printf '%s\n' "$all_containers" | grep -E '^sandbox_(python|nodejs)_[0-9]+$' || true)" if [ -n "$sandbox_containers" ]; then while IFS= read -r container; do