From 622ec6d18d91b5886597d5b438031d618fcf34b5 Mon Sep 17 00:00:00 2001
From: sneak <sneak@sneak.berlin>
Date: Wed, 23 Sep 2026 09:28:11 +0000
Subject: [PATCH] Remove the clone container even when the deploy is cancelled
 (closes #215)

The deferred removal used the deploy context, so a cancelled deploy
(a newer push, or the build timeout) never sent the remove request and
left the clone container and its volume behind. It now uses
context.WithoutCancel, and the test gains a case that cancels the
context while the clone is running.

Model: opus-5-5
---
 internal/docker/client.go          |  5 +++--
 internal/docker/validation_test.go | 28 +++++++++++++++++++++++-----
 2 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/internal/docker/client.go b/internal/docker/client.go
index eb56a73..9e6abc5 100644
--- a/internal/docker/client.go
+++ b/internal/docker/client.go
@@ -657,10 +657,11 @@ func (c *Client) performClone(
 	}
 
 	// The git image declares a volume, so Docker gives each clone container
-	// an anonymous volume; remove it with the container.
+	// an anonymous volume; remove it with the container. The removal must
+	// still run when the deploy is cancelled.
 	defer func() {
 		_ = c.docker.ContainerRemove(
-			ctx,
+			context.WithoutCancel(ctx),
 			gitContainerID.String(),
 			container.RemoveOptions{Force: true, RemoveVolumes: true},
 		)
diff --git a/internal/docker/validation_test.go b/internal/docker/validation_test.go
index 9a57b82..3ea3378 100644
--- a/internal/docker/validation_test.go
+++ b/internal/docker/validation_test.go
@@ -1,6 +1,7 @@
 package docker //nolint:testpackage // tests unexported regexps and Client struct
 
 import (
+	"context"
 	"errors"
 	"fmt"
 	"log/slog"
@@ -160,14 +161,27 @@ func TestCloneRepoRejectsInjection(t *testing.T) {
 
 // TestPerformCloneRemovesContainerVolumes runs a clone against a fake Docker
 // API and checks that the clone container is removed together with its
-// anonymous volumes, whether the clone succeeds or fails.
+// anonymous volumes, whether the clone succeeds, fails, or is cancelled.
 func TestPerformCloneRemovesContainerVolumes(t *testing.T) {
 	t.Parallel()
 
-	for _, exitCode := range []int{0, 1} {
-		t.Run(fmt.Sprintf("exit code %d", exitCode), func(t *testing.T) {
+	tests := []struct {
+		name     string
+		exitCode int
+		cancel   bool
+	}{
+		{name: "succeeds", exitCode: 0},
+		{name: "fails", exitCode: 1},
+		{name: "cancelled", cancel: true},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
 
+			ctx, cancel := context.WithCancel(t.Context())
+			t.Cleanup(cancel)
+
 			removeQuery := make(chan url.Values, 1)
 
 			srv := httptest.NewServer(http.HandlerFunc(
@@ -179,8 +193,12 @@ func TestPerformCloneRemovesContainerVolumes(t *testing.T) {
 						removeQuery <- r.URL.Query()
 					case strings.HasSuffix(r.URL.Path, "/containers/create"):
 						_, _ = w.Write([]byte(`{"Id":"gitcontainer"}`))
+					case strings.HasSuffix(r.URL.Path, "/wait") && tt.cancel:
+						// Cancel the deploy while the clone is running.
+						cancel()
+						<-r.Context().Done()
 					case strings.HasSuffix(r.URL.Path, "/wait"):
-						_, _ = fmt.Fprintf(w, `{"StatusCode":%d}`, exitCode)
+						_, _ = fmt.Fprintf(w, `{"StatusCode":%d}`, tt.exitCode)
 					default:
 						_, _ = w.Write([]byte(`{}`))
 					}
@@ -208,7 +226,7 @@ func TestPerformCloneRemovesContainerVolumes(t *testing.T) {
 				hostKeyFile:   filepath.Join(dir, "deploy_key"),
 			}
 
-			_, _ = c.performClone(t.Context(), cfg)
+			_, _ = c.performClone(ctx, cfg)
 
 			select {
 			case query := <-removeQuery: