From 2db286c079861b5913f3716658aca0bd79e91462 Mon Sep 17 00:00:00 2001
From: dawn <dawn@tangled.org>
Date: Wed, 26 Aug 2026 01:49:42 +0900
Subject: [PATCH] spindle/git: kill git children on cancellation

Signed-off-by: dawn <dawn@tangled.org>
---
 spindle/git/git.go      |  14 +++++
 spindle/git/git_test.go | 135 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 149 insertions(+)
 create mode 100644 spindle/git/git_test.go

diff --git a/spindle/git/git.go b/spindle/git/git.go
index 3ae1958a..e5438565 100644
--- a/spindle/git/git.go
+++ b/spindle/git/git.go
@@ -9,6 +9,8 @@ import (
 	"path/filepath"
 	"strings"
 	"sync"
+	"syscall"
+	"time"
 
 	"github.com/hashicorp/go-version"
 )
@@ -69,6 +71,18 @@ const WorkflowDir = `/.tangled/workflows`
 func runGit(ctx context.Context, args ...string) error {
 	var stderr bytes.Buffer
 	cmd := exec.CommandContext(ctx, "git", args...)
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	cmd.WaitDelay = 10 * time.Second
+	cmd.Cancel = func() error {
+		if cmd.Process == nil {
+			return nil
+		}
+		err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
+		if err == syscall.ESRCH {
+			return nil
+		}
+		return err
+	}
 	cmd.Stderr = &stderr
 	if err := cmd.Run(); err != nil {
 		return fmt.Errorf("%w: %s", err, strings.TrimSpace(stderr.String()))
diff --git a/spindle/git/git_test.go b/spindle/git/git_test.go
new file mode 100644
index 00000000..e418b9b3
--- /dev/null
+++ b/spindle/git/git_test.go
@@ -0,0 +1,135 @@
+package git
+
+import (
+	"bufio"
+	"context"
+	"io"
+	"net"
+	"path/filepath"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+)
+
+// hangingKnot keeps the child process alive until the client closes its socket
+func hangingKnot(t *testing.T) (url string, asked, dropped <-chan struct{}) {
+	t.Helper()
+
+	ln, err := net.Listen("tcp", "127.0.0.1:0")
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { ln.Close() })
+
+	askedCh := make(chan struct{})
+	droppedCh := make(chan struct{})
+	var askOnce, dropOnce sync.Once
+
+	go func() {
+		for {
+			conn, err := ln.Accept()
+			if err != nil {
+				return
+			}
+			go func() {
+				defer conn.Close()
+				br := bufio.NewReader(conn)
+				for {
+					line, err := br.ReadString('\n')
+					if err != nil {
+						return
+					}
+					if strings.TrimSpace(line) == "" {
+						break
+					}
+				}
+				io.WriteString(conn, "HTTP/1.1 200 OK\r\n"+
+					"Content-Type: application/x-git-upload-pack-advertisement\r\n"+
+					"Transfer-Encoding: chunked\r\n\r\n")
+				askOnce.Do(func() { close(askedCh) })
+				// this ends only when the child process dies
+				io.Copy(io.Discard, br)
+				dropOnce.Do(func() { close(droppedCh) })
+			}()
+		}
+	}()
+
+	return "http://" + ln.Addr().String(), askedCh, droppedCh
+}
+
+func TestRunGitCancelKillsChild(t *testing.T) {
+	knot, asked, dropped := hangingKnot(t)
+	path := filepath.Join(t.TempDir(), "repo")
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	done := make(chan error, 1)
+	go func() { done <- runGit(ctx, "clone", "--no-checkout", "--depth=1", knot+"/repo.git", path) }()
+
+	select {
+	case <-asked:
+	case err := <-done:
+		t.Fatalf("clone returned before it reached the knot: %v", err)
+	case <-time.After(30 * time.Second):
+		t.Fatal("git never asked the knot for refs")
+	}
+
+	cancel()
+
+	select {
+	case err := <-done:
+		if err == nil {
+			t.Fatal("a cancelled clone reported success")
+		}
+	case <-time.After(15 * time.Second):
+		t.Fatal("runGit is still blocked after cancellation")
+	}
+
+	select {
+	case <-dropped:
+	case <-time.After(15 * time.Second):
+		t.Fatal("git child outlived the cancelled clone")
+	}
+}
+
+func TestSparseSyncCancelReleasesRepoLock(t *testing.T) {
+	knot, asked, _ := hangingKnot(t)
+	path := filepath.Join(t.TempDir(), "repo")
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	done := make(chan error, 1)
+	go func() { done <- SparseSyncGitRepo(ctx, knot+"/repo.git", path, "HEAD") }()
+
+	select {
+	case <-asked:
+	case err := <-done:
+		t.Fatalf("sync returned before it reached the knot: %v", err)
+	case <-time.After(30 * time.Second):
+		t.Fatal("git never asked the knot for refs")
+	}
+
+	cancel()
+
+	select {
+	case <-done:
+	case <-time.After(15 * time.Second):
+		t.Fatal("sync is still blocked after cancellation")
+	}
+
+	// another request must be able to use this repo
+	dead, cancelDead := context.WithCancel(context.Background())
+	cancelDead()
+
+	next := make(chan error, 1)
+	go func() { next <- SparseSyncGitRepo(dead, knot+"/repo.git", path, "HEAD") }()
+
+	select {
+	case <-next:
+	case <-time.After(15 * time.Second):
+		t.Fatal("a cancelled sync never released the repo lock")
+	}
+}
-- 
2.51.2