Let a build agent be paused, have a build killed, and end an ask cancelled as it took it (hq ADR 0219)

A queued ask could only be waited out and a running build only ended by stopping the machine, which redelivered it elsewhere. The holder now serves current, kill, pause and resume on its own machine's subjects; a kill ends the build's process group and labelled containers and settles the ask as failed, killed by hand; pause is kept in the workspace across a restart and said on the bus. The controller writes cancelled ids to a cancelled set the holder reads on taking an ask, closing the race a delete alone leaves.
2026-10-05 19:17:42 +02:00
parent f80b6cdbd1
commit e610f2d92c
17 changed files with 1601 additions and 16 deletions
@@ -761,6 +761,9 @@ func Command(ctx context.Context, dir, name string, args ...string) (string, err
 	tell("run", "$ (%s) %s %s", short(filepath.Base(dir)), name, strings.Join(args, " "))
 	cmd := exec.CommandContext(ctx, name, args...)
 	cmd.Dir = dir
+	// **Ended whole when the build is** (novox/hq ADR 0219): its own process group, killed as one
+	// when the context ends, so a build killed by hand leaves no `git` or `docker` child running.
+	inItsOwnGroup(cmd)
 	out, err := cmd.CombinedOutput()
 	if err != nil {
 		tell("run", "! %s %s failed after %s", name, args[0], since(started))
@@ -0,0 +1,78 @@
+package builder
+
+import (
+	"context"
+	"os/exec"
+	"strings"
+	"syscall"
+	"time"
+)
+
+// A build killed by hand (novox/hq ADR 0219).
+//
+// A build is a tree of commands — git, then docker, and docker's own children — and a container the
+// docker client started keeps running when the client dies. So ending one by hand is three things:
+// the build's context is cancelled, which kills each command's whole process group rather than the
+// one process the context knows; every container the build started carries the build's id as a
+// label, so what outlived its client is found and removed by that label; and the holder announces
+// the outcome itself, since nothing else will.
+
+// BuildLabel is the label every container a build starts carries, valued with the build's id.
+const BuildLabel = "mesh.build"
+
+// KillWait is how long a command killed with its build may take to let go of its output before it
+// is abandoned: a grandchild holding the pipe open must not hold the build open with it.
+const KillWait = 10 * time.Second
+
+// inItsOwnGroup makes a command the leader of its own process group, killed as a group when its
+// context ends.
+func inItsOwnGroup(cmd *exec.Cmd) {
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	cmd.Cancel = func() error {
+		if cmd.Process == nil {
+			return nil
+		}
+		// The negative pid is the group: the command and everything it started.
+		if err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL); err != nil {
+			return cmd.Process.Kill()
+		}
+		return nil
+	}
+	cmd.WaitDelay = KillWait
+}
+
+// Labelled is a Runner that marks every container a build starts with the build's id, so a kill
+// finds what outlived the docker client (novox/hq ADR 0219). Applied at the one place every command
+// passes rather than at each `docker run` the builder composes: a run added later is labelled too.
+func Labelled(run Runner, id string) Runner {
+	return func(ctx context.Context, dir string, name string, args ...string) (string, error) {
+		return run(ctx, dir, name, LabelledArgs(name, args, id)...)
+	}
+}
+
+// LabelledArgs is a command's arguments with the build's label added, when it is a `docker run`.
+func LabelledArgs(name string, args []string, id string) []string {
+	if name != "docker" || len(args) == 0 || args[0] != "run" || id == "" {
+		return args
+	}
+	out := make([]string, 0, len(args)+2)
+	out = append(out, "run", "--label", BuildLabel+"="+id)
+	return append(out, args[1:]...)
+}
+
+// RemoveContainersOf removes every container labelled with the build's id, running or not, and
+// says how many. Run with a context of its own: the build's is the one that was just cancelled.
+func RemoveContainersOf(ctx context.Context, run Runner, id string) (int, error) {
+	out, err := run(ctx, "", "docker", "ps", "-aq", "--filter", "label="+BuildLabel+"="+id)
+	if err != nil {
+		return 0, err
+	}
+	ids := strings.Fields(out)
+	if len(ids) == 0 {
+		return 0, nil
+	}
+	if _, err := run(ctx, "", "docker", append([]string{"rm", "-f"}, ids...)...); err != nil {
+		return 0, err
+	}
+	return len(ids), nil
+}
@@ -0,0 +1,103 @@
+package builder
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"reflect"
+	"strconv"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+)
+
+// A build killed by hand (novox/hq ADR 0219) ends every command it started: the context's end kills
+// the command's whole process group, not the one process the context knows about.
+func TestAKilledBuildEndsItsWholeProcessGroup(t *testing.T) {
+	dir := t.TempDir()
+	child := filepath.Join(dir, "child")
+	ctx, cancel := context.WithCancel(context.Background())
+	done := make(chan error, 1)
+	began := time.Now()
+	go func() {
+		// A shell that starts a grandchild and waits on it: killing the shell alone would leave the
+		// grandchild running, holding the output open.
+		_, err := Command(ctx, dir, "sh", "-c", `sleep 60 & echo $! > child; wait`)
+		done <- err
+	}()
+	var pid int
+	for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(20 * time.Millisecond) {
+		if raw, err := os.ReadFile(child); err == nil && strings.TrimSpace(string(raw)) != "" {
+			pid, _ = strconv.Atoi(strings.TrimSpace(string(raw)))
+			break
+		}
+	}
+	if pid == 0 {
+		t.Fatal("the command never started its child")
+	}
+	cancel()
+	select {
+	case err := <-done:
+		if err == nil {
+			t.Fatal("a killed command reported success")
+		}
+	case <-time.After(KillWait + 5*time.Second):
+		t.Fatal("the killed command never returned")
+	}
+	if took := time.Since(began); took > KillWait {
+		t.Errorf("ending the command took %s: the group was not killed, the wait ran out", took)
+	}
+	for deadline := time.Now().Add(2 * time.Second); time.Now().Before(deadline); time.Sleep(20 * time.Millisecond) {
+		if err := syscall.Kill(pid, 0); errors.Is(err, syscall.ESRCH) {
+			return
+		}
+	}
+	_ = syscall.Kill(pid, syscall.SIGKILL)
+	t.Fatalf("the grandchild %d outlived the kill", pid)
+}
+
+// Every `docker run` a build starts carries its id as a label; nothing else is touched.
+func TestEveryContainerABuildStartsCarriesItsID(t *testing.T) {
+	got := LabelledArgs("docker", []string{"run", "--rm", "img", "sh"}, "build-1")
+	if want := []string{"run", "--label", "mesh.build=build-1", "--rm", "img", "sh"}; !reflect.DeepEqual(got, want) {
+		t.Errorf("docker run became %v", got)
+	}
+	for _, c := range []struct {
+		name string
+		args []string
+	}{{"docker", []string{"build", "."}}, {"git", []string{"run"}}, {"docker", nil}} {
+		if got := LabelledArgs(c.name, c.args, "build-1"); !reflect.DeepEqual(got, c.args) {
+			t.Errorf("%s %v became %v", c.name, c.args, got)
+		}
+	}
+	var ran [][]string
+	run := Labelled(func(_ context.Context, _ string, name string, args ...string) (string, error) {
+		ran = append(ran, append([]string{name}, args...))
+		return "", nil
+	}, "build-2")
+	_, _ = run(context.Background(), "", "docker", "run", "img")
+	if want := [][]string{{"docker", "run", "--label", "mesh.build=build-2", "img"}}; !reflect.DeepEqual(ran, want) {
+		t.Errorf("ran %v", ran)
+	}
+}
+
+// What a kill removes is found by the label, and only what it finds.
+func TestAKillRemovesTheContainersLabelledWithTheBuild(t *testing.T) {
+	var ran []string
+	run := func(_ context.Context, _ string, name string, args ...string) (string, error) {
+		ran = append(ran, name+" "+strings.Join(args, " "))
+		if args[0] == "ps" {
+			return "c1\nc2\n", nil
+		}
+		return "", nil
+	}
+	n, err := RemoveContainersOf(context.Background(), run, "build-3")
+	if err != nil || n != 2 {
+		t.Fatalf("%d %v", n, err)
+	}
+	if want := []string{"docker ps -aq --filter label=mesh.build=build-3", "docker rm -f c1 c2"}; !reflect.DeepEqual(ran, want) {
+		t.Errorf("ran %v", ran)
+	}
+}