Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -232,9 +232,11 @@ Decouples a session's lifetime from the launching client, so a session survives

**D9 — container existence is the durable source of truth; the lock is the live-operation guard.** A running labeled daemon container = "this session exists," *even with no supervisor* (after a reboot, `--restart unless-stopped` resurrects the container on the same fixed proxy IP but the supervisor does not come back). So idempotency (`--start`), the busy-check (bare `sandy`, `--attach`), and `--stop` all key off the **container**, not lock/supervisor liveness. `--start` refuses headless (`-p`/`--print`/`--prompt`) — a one-shot under `--restart unless-stopped` would restart-loop.

**D6 refinement — "container-as-truth" means a container with a LIVE inner session.** A container that is up but whose agent session has *exited* (a zombie: the #47 supervisor died, or the agent exited and a restart raced the ~60s teardown-detection window) must not read as "already running" and wedge the user out. So the `--start` idempotency check and the bare-`sandy` DEC-B check both **probe the inner session** (`docker exec … tmux has-session`, with a short mid-startup retry so a container that's merely still-launching isn't misread and reaped out from under a concurrent start) before honoring the container. A dead-session zombie is **reaped via `"$0" --stop`** and the operation proceeds fresh (`--start` starts a new session; bare `sandy` falls through to interactive). Guarded by `run-tests.sh §73` (structural) + `acceptance-daemon.sh §6.5` (real-docker: kill the inner session → assert `--start` reaps and replaces the container).

**Decisions (documented for the `sandy-ui` consumer contract):**
- **DEC-A — concurrent attach = last-wins** via `tmux attach -d` (a second client cleanly displaces the first; the displaced client exits `3`). Never plain `tmux attach` (that mirrors — the one banned outcome).
- **DEC-B — bare `sandy` over a live daemon session = error-with-hint + exit `1`** (points at `--attach` / `--stop`); keyed off the container label so a supervisor-less rebooted session is respected, not clobbered.
- **DEC-B — bare `sandy` over a *live* daemon session = error-with-hint + exit `1`** (points at `--attach` / `--stop`); keyed off the container label so a supervisor-less rebooted session is respected, not clobbered. "Live" is now verified by an inner-session probe (D6 refinement above) — a dead-session zombie is reaped and bare `sandy` proceeds to interactive rather than erroring.
- **DEC-C — exit codes.** `--attach`: `0` = session ended while attached, `3` = clean detach (session lives), `4` = no such session, `5` = attach failed. `--stop`: `0` = stopped, `4` = no such session, `5` = teardown failed. A client attached when `--stop` runs elsewhere sees the container vanish → exits `0`. **Post-attach, `5` is reserved for a failure to *establish* the attach** — once `tmux attach` returns, the outcome is only `3` (session still up) or `0` (session gone). The session-gone case includes the brief window where the container is *still up* but the inner session has ended (the agent exited and the #47 supervisor watch-loop is mid-teardown): that maps to `0`, not `5`, so sandy-ui stops sticky-reconnecting a dying session. A single transient `docker exec` probe failure is absorbed by a one-shot re-probe before the `0` verdict.

**`--stop` interplay with the #14 lock:** if the supervisor PID is alive, `--stop` signals it (`kill -TERM`) so the supervisor's *own* trap releases the lock (nothing else ever removes a live-owned lock). If the supervisor is dead (D9 reboot case), `--stop` tears the container/networks down directly and reaps the now-stale lock (whose holder PID is provably dead). This is the only unavoidable cleanup duplication, bounded to container+network+lock.
Expand Down
44 changes: 40 additions & 4 deletions sandy
Original file line number Diff line number Diff line change
Expand Up @@ -3567,8 +3567,28 @@ if [ "$SANDY_START" = "true" ] && [ "${SANDY_DAEMON_SUPERVISOR:-0}" != "1" ]; th
_sandy_daemon_existing="$(docker ps -q "${_sandy_daemon_filter[@]}" 2>/dev/null | head -1)" || true
if [ -n "$_sandy_daemon_existing" ]; then
_sandy_daemon_name="$(docker inspect -f '{{ index .Config.Labels "sandy.session" }}' "$_sandy_daemon_existing" 2>/dev/null || true)"
info "Daemon session already running for $WORK_DIR: ${_sandy_daemon_name:-$_sandy_daemon_existing} (container ${_sandy_daemon_existing:0:12})"
exit 0
# D6 refinement: "container-as-truth" must mean a container with a LIVE
# inner session. A container that is up but whose agent session has
# exited (a zombie — the #47 supervisor died, or the agent exited and a
# restart raced the teardown-detection window) must NOT read as "already
# running" and block the restart. Probe the session, retrying briefly so
# a container that is merely mid-startup (session not created yet) is not
# misread as a zombie and reaped out from under a concurrent start.
_sandy_session_live=0
for _sandy_probe in 1 2 3 4 5; do
if docker exec -u "$(id -u)" "$_sandy_daemon_existing" tmux has-session -t sandy >/dev/null 2>&1; then
_sandy_session_live=1
break
fi
sleep 1
done
if [ "$_sandy_session_live" = 1 ]; then
info "Daemon session already running for $WORK_DIR: ${_sandy_daemon_name:-$_sandy_daemon_existing} (container ${_sandy_daemon_existing:0:12})"
exit 0
fi
warn "Found a daemon container with no live session for $WORK_DIR (agent exited; container ${_sandy_daemon_existing:0:12}) — reaping it and starting fresh."
"$0" --stop --workspace "$WORK_DIR" >/dev/null 2>&1 || true
# fall through to a fresh --start below
fi

# D1 — normalize the currently-parsed flags back into an argv for the
Expand Down Expand Up @@ -5352,8 +5372,24 @@ if [ "$SANDY_START" != "true" ] && [ "$SANDY_ATTACH" != "true" ] && [ "$SANDY_ST
&& [ "${SANDY_DAEMON_SUPERVISOR:-0}" != "1" ]; then
_sandy_bare_existing="$(docker ps -q --filter "label=sandy.daemon=true" --filter "label=sandy.workspace_path=$WORK_DIR" 2>/dev/null | head -1)" || true
if [ -n "$_sandy_bare_existing" ]; then
error "A daemon session is already running for this workspace. Attach with 'sandy --attach', or end it with 'sandy --stop'."
exit 1
# Only a LIVE daemon session blocks a bare interactive launch (DEC-B). A
# container that is up but whose agent session has exited (a zombie) must
# not wedge the user out — reap it and fall through to the interactive
# flow. Same session-liveness probe (with mid-startup retry) as --start.
_sandy_bare_live=0
for _sandy_probe in 1 2 3 4 5; do
if docker exec -u "$(id -u)" "$_sandy_bare_existing" tmux has-session -t sandy >/dev/null 2>&1; then
_sandy_bare_live=1
break
fi
sleep 1
done
if [ "$_sandy_bare_live" = 1 ]; then
error "A daemon session is already running for this workspace. Attach with 'sandy --attach', or end it with 'sandy --stop'."
exit 1
fi
warn "Found a daemon container with no live session for this workspace (agent exited) — reaping it before starting."
"$0" --stop --workspace "$WORK_DIR" >/dev/null 2>&1 || true
fi
fi

Expand Down
22 changes: 22 additions & 0 deletions test/acceptance-daemon.sh
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,28 @@ echo "== 6. idempotent second --start =="
"$SANDY" --start --workspace "$WS"; ck "second --start is a no-op, exit 0" "[ $? -eq 0 ]"
ck "still exactly one daemon container" "[ \$(docker ps -q --filter label=sandy.daemon=true --filter label=sandy.workspace_path=$WS | wc -l | tr -d ' ') -eq 1 ]"

echo "== 6.5 zombie recovery: --start reaps a session-less container, restarts fresh =="
# Reproduce the observed failure: the agent exits but the container stays up
# (supervisor missed it, or a restart raced the #47 teardown window) → a zombie
# (container alive, inner session dead). A restart must NOT read it as "already
# running" and block — it must probe the session, find it dead, reap the
# container, and start a fresh one.
Z="$(cid)"
docker exec -u "$(id -u)" "$Z" tmux kill-server >/dev/null 2>&1 || true
sleep 1
ck "zombie: container still up but session gone" \
"[ -n \"$Z\" ] && [ \"\$(cid)\" = \"$Z\" ] && ! docker exec -u \"$(id -u)\" \"$Z\" tmux has-session -t sandy 2>/dev/null"
"$SANDY" --start --workspace "$WS"; RCZ=$?
ck "--start over a zombie exits 0 (recovered, not blocked)" "[ $RCZ -eq 0 ]"
Z2="$(cid)"
ck "a FRESH container replaced the zombie (different id)" "[ -n \"$Z2\" ] && [ \"$Z2\" != \"$Z\" ]"
ck "the fresh session is live" "docker exec -u \"$(id -u)\" \"$Z2\" tmux has-session -t sandy"
# Re-point the teardown markers at the fresh session so §7 stops the NEW one.
C="$Z2"
DPID="$(docker inspect -f '{{index .Config.Labels "sandy.daemon_pid"}}' "$C" 2>/dev/null)"
SESS="$(docker inspect -f '{{index .Config.Labels "sandy.session"}}' "$C" 2>/dev/null)"
LOCK="$SANDY_HOME_DIR/sandboxes/.$SESS.lock"

echo "== 7. sandy --stop — full teardown =="
"$SANDY" --stop --workspace "$WS"; ck "--stop exits 0" "[ $? -eq 0 ]"
sleep 2
Expand Down
21 changes: 21 additions & 0 deletions test/run-tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5435,6 +5435,27 @@ check "base build is quieted under the daemon supervisor, bash-3.2-safe (#51)" \
bash -c 'grep -qF "_BASE_BUILD_Q=(-q)" "$1" \
&& grep -qF "docker build \${_BASE_BUILD_Q[@]+" "$1"' -- "$_S72"

# ============================================================
echo ""
echo "§73: daemon zombie recovery — restart over a session-less container reaps + restarts"
# ============================================================
# A daemon container that is up but whose agent session has exited (a zombie:
# the #47 supervisor died, or a restart raced the teardown window) must NOT read
# as "already running" and block a restart. Both the --start idempotency check
# and the bare-sandy DEC-B check must probe the INNER session (tmux has-session),
# not just container existence, and reap a dead-session zombie via `$0 --stop`.
# Behavioral end-to-end coverage is test/acceptance-daemon.sh §6.5 (real docker);
# these are structural guards so a revert fails CI too.
_S73="$(cd "$(dirname "$0")/.." && pwd)/sandy"
check "--start verifies inner-session liveness before 'already running' (#47/D6)" \
grep -q '_sandy_session_live' "$_S73"
check "bare sandy verifies inner-session liveness before the DEC-B block" \
grep -q '_sandy_bare_live' "$_S73"
check "restart probes the session with a mid-startup retry (not a one-shot)" \
bash -c 'grep -A6 "_sandy_session_live=0" "$1" | grep -q "tmux has-session -t sandy"' -- "$_S73"
check "a dead-session daemon zombie is reaped via \$0 --stop (both --start + bare)" \
bash -c '[ "$(grep -c "\"\$0\" --stop --workspace \"\$WORK_DIR\" >/dev/null 2>&1 || true" "$1")" -ge 2 ]' -- "$_S73"

# ============================================================
# Summary
# ============================================================
Expand Down