From f965d785a70891a39f999534983ead2b755622ab Mon Sep 17 00:00:00 2001 From: Lio Lunesu Date: Sun, 2 Aug 2026 08:54:13 +0000 Subject: [PATCH] fix(sessions): close the delete/respawn race; deflake the webhook dispatch test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Master CI run 30740226645 failed both ways this race can bite: 1. sessions VM test: a session deleted through the settings page came back. The delete delists then kills, but start_session had already read the session's JSON and was preparing the spawn (a dozen jq calls, claude state seeding) — its tmux new-session then resurrected the just-killed session. The reconcile loop deliberately tolerates unmanaged sessions, so the leak is permanent and invisible to the supervisor. start_session now re-checks the CURRENT file immediately before spawning AND verifies again after: a delist landing before the post-check is honored by killing the fresh spawn; one landing after sees the session live and kills it itself. The post-check is ordered before mark_started, whose jq assignment would otherwise re-create a just-deleted session as a stub entry. 2. webhook VM test (added in #164): it read initialPrompt out of sessions.json after waiting for the hook-* session to appear — but the supervisor consumes a kickoff prompt (mark_started nulls it) within ~2s of spawning, so the assert raced the very consumption it sat next to; it passed on the PR run by timing luck and lost on master. The dispatch section now STOPS agent-box-agent.service before the signed delivery (the spawn wrapper writes sessions.json either way — that is the point of the file-based contract), asserts the full prompt deterministically, then restarts the supervisor and asserts the other half: the hook session comes up in tmux, hasRun flips true, and initialPrompt is consumed to null. Verification: nix run .#assemble committed; module-generated-up-to-date, multi-user, module-single-file, webhook-route pass natively on aarch64-linux. The two VM tests run in this PR's CI on x86 — the webhook test now exercises spawn-while-supervisor-down plus consume-on-start, and the sessions test keeps stochastic coverage of the delete race with the window shrunk from ~1s to the jq→tmux gap and backstopped by the post-spawn kill. Refs run 30740226645. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_013VoCn5tqBY3hKKwmtCH1kx --- modules/agent-box.nix | 16 ++++++++++++++++ modules/agent-box.nix.in | 16 ++++++++++++++++ tests/webhook.nix | 15 +++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/modules/agent-box.nix b/modules/agent-box.nix index 91602b5..2a695a6 100644 --- a/modules/agent-box.nix +++ b/modules/agent-box.nix @@ -1620,10 +1620,26 @@ ${lib.optionalString (agentsMdPointer != null) '' # nested inspection bash. postmortem=" || exec ${pkgs.bashInteractive}/bin/bash" [ "$agent" = shell ] && postmortem="" + # A delete (settings page / agent-box-session rm: delist THEN kill) can + # land while this function is preparing the spawn — their kill hits the + # OLD session and this spawn would resurrect it as a live, delisted + # session the reconcile loop never kills (it tolerates unmanaged + # sessions on purpose): a permanent leak. Re-check the CURRENT file at + # the last moment, and verify again AFTER the spawn — a delist landing + # before the post-check is honored here by killing what we just + # started; one landing after it sees the session live and kills it + # itself. (Seen as a CI flake in the sessions VM test, run 30740226645.) + $JQ -e --arg s "$sname" '.sessions | has($s)' "$SESSIONS_FILE" >/dev/null 2>&1 || return 0 if $TMUX new-session -d -s "$sname" -c "$wd" ${webhookSessionEnvArgs name} \ "${envExecWrapper name} $cmd$postmortem"; then + if ! $JQ -e --arg s "$sname" '.sessions | has($s)' "$SESSIONS_FILE" >/dev/null 2>&1; then + $TMUX kill-session -t "=$sname" 2>/dev/null || true + return 0 + fi # First spawn only: persist the id + hasRun and consume the kickoff # prompt, so the next respawn resumes instead of redoing the task. + # (Ordered after the delist post-check: mark_started's jq assignment + # would otherwise re-create a just-deleted session as a stub entry.) [ "$resuming" = true ] || mark_started "$sname" "$bid" fi } diff --git a/modules/agent-box.nix.in b/modules/agent-box.nix.in index cea30af..d9c5452 100644 --- a/modules/agent-box.nix.in +++ b/modules/agent-box.nix.in @@ -1616,10 +1616,26 @@ ${lib.optionalString (agentsMdPointer != null) '' # nested inspection bash. postmortem=" || exec ${pkgs.bashInteractive}/bin/bash" [ "$agent" = shell ] && postmortem="" + # A delete (settings page / agent-box-session rm: delist THEN kill) can + # land while this function is preparing the spawn — their kill hits the + # OLD session and this spawn would resurrect it as a live, delisted + # session the reconcile loop never kills (it tolerates unmanaged + # sessions on purpose): a permanent leak. Re-check the CURRENT file at + # the last moment, and verify again AFTER the spawn — a delist landing + # before the post-check is honored here by killing what we just + # started; one landing after it sees the session live and kills it + # itself. (Seen as a CI flake in the sessions VM test, run 30740226645.) + $JQ -e --arg s "$sname" '.sessions | has($s)' "$SESSIONS_FILE" >/dev/null 2>&1 || return 0 if $TMUX new-session -d -s "$sname" -c "$wd" ${webhookSessionEnvArgs name} \ "${envExecWrapper name} $cmd$postmortem"; then + if ! $JQ -e --arg s "$sname" '.sessions | has($s)' "$SESSIONS_FILE" >/dev/null 2>&1; then + $TMUX kill-session -t "=$sname" 2>/dev/null || true + return 0 + fi # First spawn only: persist the id + hasRun and consume the kickoff # prompt, so the next respawn resumes instead of redoing the task. + # (Ordered after the delist post-check: mark_started's jq assignment + # would otherwise re-create a just-deleted session as a stub entry.) [ "$resuming" = true ] || mark_started "$sname" "$bid" fi } diff --git a/tests/webhook.nix b/tests/webhook.nix index f0c5b85..99d13c4 100644 --- a/tests/webhook.nix +++ b/tests/webhook.nix @@ -329,6 +329,13 @@ # A signed delivery on the watched repo → a fresh hook-* session appears in # sessions.json, primed with the framed event text plus the trusted # preamble, and the supervisor starts it as a real tmux session. + # + # The supervisor is STOPPED for the delivery: it consumes a kickoff prompt + # within ~2s of spawning (mark_started nulls initialPrompt), so asserting + # the prompt via sessions.json is a race otherwise (lost on master run + # 30740226645). With it stopped, the wrapper's write is the only actor; + # restarting it afterwards proves the spawn + consumption half. + machine.succeed("systemctl stop agent-box-agent.service") client.succeed( f"{post} -H 'x-hub-signature-256: sha256={sig}' " f"https://box.test/agent/webhook/github | grep -x 200" @@ -352,11 +359,19 @@ "jq -e '.sessions | keys[] | select(startswith(\"hook-defangdevs-agent-box-\"))'" " /home/agent/.config/agent-box/sessions.json" ) + # Supervisor back up: it starts the hook session and consumes the prompt. + machine.succeed("systemctl start agent-box-agent.service") machine.wait_until_succeeds( "sudo -u agent env TMUX_TMPDIR=/run/agent-box-agent tmux -L agent-box" " list-sessions -F '#S' | grep -q '^hook-'", timeout=60, ) + machine.wait_until_succeeds( + "jq -e '.sessions | to_entries[] | select(.key | startswith(\"hook-\"))" + " | .value | (.hasRun == true and .initialPrompt == null)'" + " /home/agent/.config/agent-box/sessions.json", + timeout=60, + ) # The event that spawned the watch session was NOT also a session delivery # for the peer (its filter has someone else's repo) — dispatch and session