fix: address review findings for self-update (#15)

internal/agent/inventory/collector.go

@@ -22,12 +22,14 @@ import (
 )
 
 const (
-	labelProject     = "com.docker.compose.project"
-	labelService     = "com.docker.compose.service"
-	labelWorkingDir  = "com.docker.compose.project.working_dir"
-	labelConfigFiles = "com.docker.compose.project.config_files"
-	labelEnvFile     = "com.docker.compose.project.environment_file"
-	labelSwarmTask   = "com.docker.swarm.service.id"
+	labelProject       = "com.docker.compose.project"
+	labelService       = "com.docker.compose.service"
+	labelWorkingDir    = "com.docker.compose.project.working_dir"
+	labelConfigFiles   = "com.docker.compose.project.config_files"
+	labelEnvFile       = "com.docker.compose.project.environment_file"
+	labelSwarmTask     = "com.docker.swarm.service.id"
+	labelSwarmName     = "com.docker.swarm.service.name"
+	labelSelfUpdateJob = "se.shcizo.package-updater.self-update.job"
 )
 
 // DockerClient is the subset of the Docker SDK the collector needs.
@@ -61,6 +63,7 @@ type Collector struct {
 	now   func() time.Time
 
 	mu     sync.Mutex
+	self   *selfInfo             // the agent's own container, once inspected
 	cache  map[string]buildEntry // by Compose project
 	warned map[string]bool       // logged warnings: "<project>" (check failed) and "<project>/<service>" (service not in config)
 }
@@ -77,8 +80,15 @@ type buildEntry struct {
 // stackLabels are the Compose labels a build check needs.
 type stackLabels struct{ workingDir, configFiles, envFile string }
 
-// New returns a Collector. selfID is a prefix of the agent's own container
-// ID (inside a container, the hostname); that container is marked Self.
+// selfInfo is what the inspect of the agent's own container tells.
+type selfInfo struct {
+	id           string // full container ID
+	swarmService string // name of the Swarm service the container is a task of, if any
+}
+
+// New returns a Collector. selfID is the agent's own container: a prefix of
+// its ID (inside a container, the hostname) or its name. That container is
+// marked Self, and so is the Swarm service it is a task of.
 func New(cli DockerClient, selfID string) *Collector {
 	return &Collector{cli: cli, selfID: selfID, now: time.Now, cache: map[string]buildEntry{}, warned: map[string]bool{}}
 }
@@ -92,6 +102,29 @@ func (c *Collector) WithBuildCheck(check BuildCheck, log *slog.Logger) *Collecto
 	return c
 }
 
+// resolveSelf inspects the agent's own container once and remembers the
+// answer. It returns nil while that fails (or selfID is empty), and the
+// caller then compares container IDs with the selfID prefix.
+func (c *Collector) resolveSelf(ctx context.Context) *selfInfo {
+	if c.selfID == "" {
+		return nil
+	}
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if c.self != nil {
+		return c.self
+	}
+	insp, err := c.cli.ContainerInspect(ctx, c.selfID)
+	if err != nil || insp.ContainerJSONBase == nil || insp.ID == "" {
+		return nil
+	}
+	c.self = &selfInfo{id: insp.ID}
+	if insp.Config != nil {
+		c.self.swarmService = insp.Config.Labels[labelSwarmName]
+	}
+	return c.self
+}
+
 // Hello returns the machine facts sent on connect. AgentVersion is left for
 // the caller to fill in.
 func (c *Collector) Hello(ctx context.Context) (protocol.Hello, error) {
@@ -125,20 +158,21 @@ func (c *Collector) Snapshot(ctx context.Context) ([]protocol.Service, error) {
 		repoDigests[img.ID] = img.RepoDigests
 	}
 
+	self := c.resolveSelf(ctx)
 	byKey := make(map[string]protocol.Service)
 	stacks := make(map[string]stackLabels)
 	for _, ct := range containers {
 		if _, isTask := ct.Labels[labelSwarmTask]; isTask {
 			continue
 		}
-		if _, isHelper := ct.Labels[protocol.LabelSelfUpdateJob]; isHelper {
+		if _, isHelper := ct.Labels[labelSelfUpdateJob]; isHelper {
 			continue
 		}
 		ref, err := c.imageRef(ctx, ct)
 		if err != nil {
 			return nil, err
 		}
-		svc := c.fromContainer(ct, ref, repoDigests[ct.ImageID])
+		svc := c.fromContainer(ct, ref, repoDigests[ct.ImageID], self)
 		if svc.Kind == protocol.KindCompose {
 			if _, ok := stacks[svc.ComposeProject]; !ok {
 				stacks[svc.ComposeProject] = stackLabels{svc.WorkingDir, svc.ConfigFiles, ct.Labels[labelEnvFile]}
@@ -162,6 +196,7 @@ func (c *Collector) Snapshot(ctx context.Context) ([]protocol.Service, error) {
 				continue
 			}
 			svc := fromSwarmService(s)
+			svc.Self = self != nil && self.swarmService != "" && svc.SwarmService == self.swarmService
 			byKey[svc.Key()] = svc
 		}
 	}
@@ -192,12 +227,12 @@ func (c *Collector) imageRef(ctx context.Context, ct container.Summary) (string,
 	return insp.Config.Image, nil
 }
 
-func (c *Collector) fromContainer(ct container.Summary, ref string, repoDigests []string) protocol.Service {
+func (c *Collector) fromContainer(ct container.Summary, ref string, repoDigests []string, self *selfInfo) protocol.Service {
 	svc := protocol.Service{
 		Image:  ref,
 		Digest: digestFor(ref, repoDigests),
 		Status: containerStatus(ct.State),
-		Self:   c.selfID != "" && strings.HasPrefix(ct.ID, c.selfID),
+		Self:   c.isSelf(ct.ID, self),
 	}
 	project, service := ct.Labels[labelProject], ct.Labels[labelService]
 	if project != "" && service != "" {
@@ -216,6 +251,15 @@ func (c *Collector) fromContainer(ct container.Summary, ref string, repoDigests
 	return svc
 }
 
+// isSelf reports whether id is the agent's own container: by the inspected
+// ID when there is one, else by the selfID prefix.
+func (c *Collector) isSelf(id string, self *selfInfo) bool {
+	if self != nil {
+		return id == self.id
+	}
+	return c.selfID != "" && strings.HasPrefix(id, c.selfID)
+}
+
 func fromSwarmService(s swarm.Service) protocol.Service {
 	ref := s.Spec.TaskTemplate.ContainerSpec.Image
 	return protocol.Service{

internal/agent/inventory/collector_test.go

@@ -138,7 +138,7 @@ func TestSnapshot_PlainContainerAndSwarmTaskSkipped(t *testing.T) {
 func TestSnapshot_SelfUpdateHelperSkipped(t *testing.T) {
 	cli := &fakeDocker{containers: []container.Summary{
 		{ID: "h1", Names: []string{"/quirky_helper"}, Image: "sha256:agentimage", State: "running",
-			Labels: map[string]string{protocol.LabelSelfUpdateJob: "7"}},
+			Labels: map[string]string{labelSelfUpdateJob: "7"}},
 		{ID: "p1", Names: []string{"/adhoc"}, Image: "redis:alpine", State: "exited"},
 	}}
 	got, err := New(cli, "").Snapshot(context.Background())
@@ -158,6 +158,63 @@ func TestSnapshot_SelfFlag(t *testing.T) {
 	assert.True(t, got[0].Self)
 }
 
+func TestSnapshot_SelfFlagByContainerName(t *testing.T) {
+	cli := &fakeDocker{
+		containers: []container.Summary{
+			composeContainer("0123456789abcdef", "updater-agent", "agent", "pu:1", "sha256:i", "running"),
+			composeContainer("fedcba9876543210", "slusk", "web", "slusk:1", "sha256:j", "running"),
+		},
+		inspect: map[string]container.InspectResponse{
+			"updater-agent-agent-1": {ContainerJSONBase: &container.ContainerJSONBase{ID: "0123456789abcdef"}},
+		},
+	}
+	got, err := New(cli, "updater-agent-agent-1").Snapshot(context.Background())
+	require.NoError(t, err)
+	require.Len(t, got, 2)
+	assert.False(t, got[0].Self, "slusk")
+	assert.True(t, got[1].Self, "updater-agent")
+}
+
+func TestSnapshot_SelfFlagFallsBackToIDPrefixWhenInspectFails(t *testing.T) {
+	cli := &fakeDocker{
+		containers: []container.Summary{composeContainer("0123456789abcdef", "updater-agent", "agent", "pu:1", "sha256:i", "running")},
+		inspectErr: errors.New("boom"),
+	}
+	got, err := New(cli, "0123456789ab").Snapshot(context.Background())
+	require.NoError(t, err)
+	require.Len(t, got, 1)
+	assert.True(t, got[0].Self)
+}
+
+func TestSnapshot_AgentsOwnSwarmServiceIsSelf(t *testing.T) {
+	swarmSvc := func(name string) swarm.Service {
+		return swarm.Service{
+			Spec: swarm.ServiceSpec{
+				Annotations:  swarm.Annotations{Name: name},
+				TaskTemplate: swarm.TaskSpec{ContainerSpec: &swarm.ContainerSpec{Image: "pu:1"}},
+			},
+			ServiceStatus: &swarm.ServiceStatus{RunningTasks: 1, DesiredTasks: 1},
+		}
+	}
+	cli := &fakeDocker{
+		info:     managerInfo(),
+		services: []swarm.Service{swarmSvc("pu_agent"), swarmSvc("pu_master")},
+		inspect: map[string]container.InspectResponse{
+			"abc123": {
+				ContainerJSONBase: &container.ContainerJSONBase{ID: "abc123full"},
+				Config:            &container.Config{Labels: map[string]string{labelSwarmName: "pu_agent"}},
+			},
+		},
+	}
+	got, err := New(cli, "abc123").Snapshot(context.Background())
+	require.NoError(t, err)
+	require.Len(t, got, 2)
+	assert.Equal(t, "pu_agent", got[0].SwarmService)
+	assert.True(t, got[0].Self)
+	assert.Equal(t, "pu_master", got[1].SwarmService)
+	assert.False(t, got[1].Self)
+}
+
 func TestSnapshot_SwarmServicesOnlyOnManager(t *testing.T) {
 	svc := swarm.Service{
 		Spec: swarm.ServiceSpec{

internal/agent/updater/compose.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 	"os/exec"
+	"slices"
 	"strings"
 	"time"
 
@@ -98,10 +99,6 @@ func NewComposeExecutor(cli ComposeDocker, resolve *Resolver, selfID string, run
 // When the service is the agent's own container, the recreate runs in a
 // helper container instead (see selfUpdate), because it stops this process.
 func (e *ComposeExecutor) Execute(ctx context.Context, job Job, out io.Writer) (string, error) {
-	// What the job has printed so far goes into the helper's labels.
-	soFar := &tailBuffer{max: selfUpdateOutputMax}
-	out = io.MultiWriter(out, tailWriter{soFar})
-
 	project, service := job.ComposeProject, job.ComposeService
 	cts, err := e.cli.ContainerList(ctx, container.ListOptions{
 		All: true,
@@ -116,14 +113,23 @@ func (e *ComposeExecutor) Execute(ctx context.Context, job Job, out io.Writer) (
 	if len(cts) == 0 {
 		return "", fmt.Errorf("no container for compose service %s/%s on this machine", project, service)
 	}
-	workingDir := cts[0].Labels[labelWorkingDir]
-	configFiles := cts[0].Labels[labelConfigFiles]
-	envFileLabel := cts[0].Labels[labelEnvFile]
-
 	self, err := inspectSelf(ctx, e.cli, e.selfID)
 	if err != nil {
 		return "", err
 	}
+	isSelf := self.ContainerJSONBase != nil && self.ID != "" &&
+		slices.ContainsFunc(cts, func(ct container.Summary) bool { return ct.ID == self.ID })
+	// What an update of the agent's own service has printed so far goes into
+	// the helper's labels.
+	var soFar *tailBuffer
+	if isSelf {
+		soFar = &tailBuffer{max: selfUpdateOutputMax}
+		out = io.MultiWriter(out, tailWriter{soFar})
+	}
+	workingDir := cts[0].Labels[labelWorkingDir]
+	configFiles := cts[0].Labels[labelConfigFiles]
+	envFileLabel := cts[0].Labels[labelEnvFile]
+
 	roots := rootsOf(self)
 	files, dir, err := composeFiles(roots, configFiles)
 	if err != nil {
@@ -170,22 +176,29 @@ func (e *ComposeExecutor) Execute(ctx context.Context, job Job, out io.Writer) (
 	}
 
 	base := composeBase(project, workingDir, files, envs)
-	isSelf := false
-	if self.ContainerJSONBase != nil && self.ID != "" {
-		for _, ct := range cts {
-			isSelf = isSelf || ct.ID == self.ID
-		}
-	}
-	for _, step := range [][]string{{"pull", service}, {"up", "-d", service}} {
-		args := append(append([]string(nil), base...), step...)
-		if isSelf && step[0] == "up" {
-			return e.selfUpdate(ctx, job, self, workingDir, args, pinned, out, soFar)
-		}
+	run := func(args []string) error {
 		fmt.Fprintf(out, "$ docker %s\n", strings.Join(args, " "))
 		tail := &tailBuffer{max: errorOutputTail}
 		if err := e.run(ctx, workingDir, io.MultiWriter(out, tailWriter{tail}), args...); err != nil {
-			return "", fmt.Errorf("docker compose %s: %w (output: %s)", step[0], err, strings.TrimSpace(tail.String()))
+			return composeError(args[len(base)], err, tail)
 		}
+		return nil
+	}
+	if err := run(append(append([]string(nil), base...), "pull", service)); err != nil {
+		return "", err
+	}
+	up := append(append([]string(nil), base...), "up", "-d", service)
+	if isSelf {
+		return e.selfUpdate(ctx, job, self, workingDir, up, pinned, out, soFar)
+	}
+	if err := run(up); err != nil {
+		return "", err
 	}
 	return pinned, nil
 }
+
+// composeError describes a failed `docker compose <step>` call with the tail
+// of its output.
+func composeError(step string, err error, tail *tailBuffer) error {
+	return fmt.Errorf("docker compose %s: %w (output: %s)", step, err, strings.TrimSpace(tail.String()))
+}

internal/agent/updater/compose_test.go

@@ -64,6 +64,7 @@ type fakeHelper struct {
 	hold      bool   // ContainerWait never reports an exit
 	logs      string // what the helper printed
 	onStart   func()
+	state     container.ContainerState // what an inspect reports; exited when empty
 
 	created  []createdHelper
 	started  []string
@@ -79,6 +80,14 @@ func (h *fakeHelper) ContainerCreate(_ context.Context, cfg *container.Config, h
 	return container.CreateResponse{ID: "helper1"}, nil
 }
 
+func (h *fakeHelper) ContainerInspect(context.Context, string) (container.InspectResponse, error) {
+	state := h.state
+	if state == "" {
+		state = container.StateExited
+	}
+	return container.InspectResponse{ContainerJSONBase: &container.ContainerJSONBase{State: &container.State{Status: state}}}, nil
+}
+
 func (h *fakeHelper) ContainerStart(_ context.Context, id string, _ container.StartOptions) error {
 	h.started = append(h.started, id)
 	if h.onStart != nil {
@@ -116,6 +125,14 @@ type fakeHelperCompose struct {
 	h *fakeHelper
 }
 
+// ContainerInspect answers for the agent's own container ("me") like fakeCompose
+// and for any other ID like the helper.
+func (f fakeHelperCompose) ContainerInspect(ctx context.Context, id string) (container.InspectResponse, error) {
+	if id == "me" {
+		return f.fakeCompose.ContainerInspect(ctx, id)
+	}
+	return f.h.ContainerInspect(ctx, id)
+}
 func (f fakeHelperCompose) ContainerCreate(ctx context.Context, cfg *container.Config, host *container.HostConfig, n *network.NetworkingConfig, p *ocispec.Platform, name string) (container.CreateResponse, error) {
 	return f.h.ContainerCreate(ctx, cfg, host, n, p, name)
 }
@@ -528,13 +545,13 @@ func TestComposeSelfUpdate(t *testing.T) {
 			assert.Equal(t, container.WaitConditionNextExit, c.helper.waitCond)
 			labels := cfg.Labels
 			assert.Len(t, labels, 6, "no compose labels")
-			assert.Equal(t, "7", labels[protocol.LabelSelfUpdateJob])
-			assert.Equal(t, pinned, labels[protocol.LabelSelfUpdateRef])
-			assert.Equal(t, "2026-10-13T09:00:00Z", labels[protocol.LabelSelfUpdateStarted])
-			assert.Equal(t, "updater", labels[protocol.LabelSelfUpdateProject])
-			assert.Equal(t, "agent", labels[protocol.LabelSelfUpdateService])
-			assert.Contains(t, labels[protocol.LabelSelfUpdateOutput], "resolved "+pinned)
-			assert.Contains(t, labels[protocol.LabelSelfUpdateOutput], "$ docker "+strings.Join(withStep("pull", "agent"), " "))
+			assert.Equal(t, "7", labels[labelSelfUpdateJob])
+			assert.Equal(t, pinned, labels[labelSelfUpdateRef])
+			assert.Equal(t, "2026-10-13T09:00:00Z", labels[labelSelfUpdateStarted])
+			assert.Equal(t, "updater", labels[labelSelfUpdateProject])
+			assert.Equal(t, "agent", labels[labelSelfUpdateService])
+			assert.Contains(t, labels[labelSelfUpdateOutput], "resolved "+pinned)
+			assert.Contains(t, labels[labelSelfUpdateOutput], "$ docker "+strings.Join(withStep("pull", "agent"), " "))
 		})
 	}
 

internal/agent/updater/runner_test.go

@@ -260,7 +260,7 @@ func TestRunner_SelfJobsRunLast(t *testing.T) {
 	assert.Equal(t, []string{"a:2", "a:4", "self:1", "self:3"}, order)
 }
 
-func TestRunner_SelfJobRunsWhenAloneAndNonSelfSubmittedLater(t *testing.T) {
+func TestRunner_SelfJobRunsWhenAlone(t *testing.T) {
 	fe := newFakeExec()
 	r := startRunner(t, fe, time.Minute)
 	r.Submit("1", protocol.JobStart{Ref: "self:1", Self: true})

internal/agent/updater/selfupdate.go

@@ -15,8 +15,17 @@ import (
 	"github.com/docker/docker/api/types/network"
 	"github.com/docker/docker/pkg/stdcopy"
 	ocispec "github.com/opencontainers/image-spec/specs-go/v1"
+)
 
-	"github.com/shcizo/package-updater/internal/protocol"
+// Labels on a self-update helper container. The new agent finds the helper
+// by them, and the inventory skips containers that carry the job label.
+const (
+	labelSelfUpdateJob     = "se.shcizo.package-updater.self-update.job"
+	labelSelfUpdateRef     = "se.shcizo.package-updater.self-update.ref"
+	labelSelfUpdateStarted = "se.shcizo.package-updater.self-update.started"
+	labelSelfUpdateProject = "se.shcizo.package-updater.self-update.project"
+	labelSelfUpdateService = "se.shcizo.package-updater.self-update.service"
+	labelSelfUpdateOutput  = "se.shcizo.package-updater.self-update.output"
 )
 
 // selfUpdateOutputMax caps the job output stored in the helper container's
@@ -32,6 +41,7 @@ const resumeSeparator = "--- agent restarted; output of the update helper ---\n"
 // HelperDocker is the subset of the Docker SDK the self-update helper
 // container needs. *client.Client satisfies it.
 type HelperDocker interface {
+	ContainerInspect(ctx context.Context, containerID string) (container.InspectResponse, error)
 	ContainerCreate(ctx context.Context, config *container.Config, hostConfig *container.HostConfig, networkingConfig *network.NetworkingConfig, platform *ocispec.Platform, containerName string) (container.CreateResponse, error)
 	ContainerStart(ctx context.Context, containerID string, options container.StartOptions) error
 	ContainerWait(ctx context.Context, containerID string, condition container.WaitCondition) (<-chan container.WaitResponse, <-chan error)
@@ -57,12 +67,12 @@ func (e *ComposeExecutor) selfUpdate(ctx context.Context, job Job, self containe
 		WorkingDir: workingDir,
 		Env:        dockerEnv(),
 		Labels: map[string]string{
-			protocol.LabelSelfUpdateJob:     job.ID,
-			protocol.LabelSelfUpdateRef:     pinned,
-			protocol.LabelSelfUpdateStarted: job.Started.UTC().Format(time.RFC3339Nano),
-			protocol.LabelSelfUpdateProject: job.ComposeProject,
-			protocol.LabelSelfUpdateService: job.ComposeService,
-			protocol.LabelSelfUpdateOutput:  outputLabel(soFar),
+			labelSelfUpdateJob:     job.ID,
+			labelSelfUpdateRef:     pinned,
+			labelSelfUpdateStarted: job.Started.UTC().Format(time.RFC3339Nano),
+			labelSelfUpdateProject: job.ComposeProject,
+			labelSelfUpdateService: job.ComposeService,
+			labelSelfUpdateOutput:  outputLabel(soFar),
 		},
 	}
 	created, err := e.cli.ContainerCreate(ctx, cfg, &container.HostConfig{Mounts: helperMounts(self)}, nil, nil, "")
@@ -72,7 +82,9 @@ func (e *ComposeExecutor) selfUpdate(ctx context.Context, job Job, self containe
 	// Wait is registered before the start so a fast exit is not missed.
 	waitCh, errCh := e.cli.ContainerWait(ctx, created.ID, container.WaitConditionNextExit)
 	if err := e.cli.ContainerStart(ctx, created.ID, container.StartOptions{}); err != nil {
-		e.cli.ContainerRemove(context.WithoutCancel(ctx), created.ID, container.RemoveOptions{Force: true})
+		if rmErr := e.cli.ContainerRemove(context.WithoutCancel(ctx), created.ID, container.RemoveOptions{Force: true}); rmErr != nil {
+			fmt.Fprintf(out, "warning: remove the update helper: %v\n", rmErr)
+		}
 		return "", fmt.Errorf("start update helper: %w", err)
 	}
 	code, err := awaitExit(ctx, waitCh, errCh)
@@ -154,7 +166,7 @@ func finishHelper(ctx context.Context, cli HelperDocker, id string, w io.Writer)
 
 // exitError describes a failed helper like a failed `docker compose` call.
 func exitError(code int64, tail *tailBuffer) error {
-	return fmt.Errorf("docker compose up: exit status %d (output: %s)", code, strings.TrimSpace(tail.String()))
+	return composeError("up", fmt.Errorf("exit status %d", code), tail)
 }
 
 // ResumeDocker is the subset of the Docker SDK ResumeSelfUpdates needs.
@@ -186,16 +198,16 @@ func ResumeSelfUpdates(ctx context.Context, cli ResumeDocker, selfID string, run
 	}
 	cts, err := cli.ContainerList(ctx, container.ListOptions{
 		All:     true,
-		Filters: filters.NewArgs(filters.Arg("label", protocol.LabelSelfUpdateJob)),
+		Filters: filters.NewArgs(filters.Arg("label", labelSelfUpdateJob)),
 	})
 	if err != nil {
 		return fmt.Errorf("list update helpers: %w", err)
 	}
 	for _, ct := range cts {
-		if ct.Labels[protocol.LabelSelfUpdateProject] != project || ct.Labels[protocol.LabelSelfUpdateService] != service {
+		if ct.Labels[labelSelfUpdateProject] != project || ct.Labels[labelSelfUpdateService] != service {
 			continue
 		}
-		jobID := ct.Labels[protocol.LabelSelfUpdateJob]
+		jobID := ct.Labels[labelSelfUpdateJob]
 		if jobID == "" {
 			log.Warn("self_update_helper_without_job", "helper", ct.ID)
 			continue
@@ -205,7 +217,7 @@ func ResumeSelfUpdates(ctx context.Context, cli ResumeDocker, selfID string, run
 			started = time.Now()
 		}
 		log.Info("self_update_resumed", "job_id", jobID, "helper", ct.ID)
-		runner.Resume(jobID, started, resumeFunc(cli, ct.ID, ct.Labels[protocol.LabelSelfUpdateOutput], ref, labelErr))
+		runner.Resume(jobID, started, resumeFunc(cli, ct.ID, ct.Labels[labelSelfUpdateOutput], ref, labelErr))
 	}
 	return nil
 }
@@ -214,11 +226,11 @@ func ResumeSelfUpdates(ctx context.Context, cli ResumeDocker, selfID string, run
 // labels. started is zero when it is missing or unreadable, and err then says
 // what is wrong.
 func parseHelperLabels(labels map[string]string) (ref string, started time.Time, err error) {
-	ref = labels[protocol.LabelSelfUpdateRef]
+	ref = labels[labelSelfUpdateRef]
 	if ref == "" {
 		err = errors.New("update helper has no ref label")
 	}
-	raw := labels[protocol.LabelSelfUpdateStarted]
+	raw := labels[labelSelfUpdateStarted]
 	if t, perr := time.Parse(time.RFC3339Nano, raw); perr == nil {
 		started = t
 	} else if err == nil {
@@ -233,6 +245,15 @@ func parseHelperLabels(labels map[string]string) (ref string, started time.Time,
 // failed or its labels were unusable.
 func resumeFunc(cli HelperDocker, helperID, earlier, ref string, labelErr error) ResumeFunc {
 	return func(ctx context.Context, out io.Writer) (string, error) {
+		// A helper that was created but never started would read as exited
+		// with code 0, so it must not be mistaken for a finished update.
+		if insp, err := cli.ContainerInspect(ctx, helperID); err == nil && insp.ContainerJSONBase != nil &&
+			insp.State != nil && insp.State.Status == container.StateCreated {
+			if err := cli.ContainerRemove(context.WithoutCancel(ctx), helperID, container.RemoveOptions{Force: true}); err != nil {
+				fmt.Fprintf(out, "warning: remove the update helper: %v\n", err)
+			}
+			return "", errors.New("update helper was never started")
+		}
 		waitCh, errCh := cli.ContainerWait(ctx, helperID, container.WaitConditionNotRunning)
 		code, err := awaitExit(ctx, waitCh, errCh)
 		if err != nil {

internal/agent/updater/selfupdate_test.go

@@ -22,12 +22,12 @@ func TestResumeSelfUpdates(t *testing.T) {
 
 	helper := func(id, project, service string, extra map[string]string) container.Summary {
 		labels := map[string]string{
-			protocol.LabelSelfUpdateJob:     "j-" + id,
-			protocol.LabelSelfUpdateRef:     pinned,
-			protocol.LabelSelfUpdateStarted: started.Format(time.RFC3339Nano),
-			protocol.LabelSelfUpdateProject: project,
-			protocol.LabelSelfUpdateService: service,
-			protocol.LabelSelfUpdateOutput:  "resolved " + pinned + "\n$ docker compose pull agent\n",
+			labelSelfUpdateJob:     "j-" + id,
+			labelSelfUpdateRef:     pinned,
+			labelSelfUpdateStarted: started.Format(time.RFC3339Nano),
+			labelSelfUpdateProject: project,
+			labelSelfUpdateService: service,
+			labelSelfUpdateOutput:  "resolved " + pinned + "\n$ docker compose pull agent\n",
 		}
 		for k, v := range extra {
 			if v == "" {
@@ -44,6 +44,8 @@ func TestResumeSelfUpdates(t *testing.T) {
 		name       string
 		helper     container.Summary
 		exit       int64
+		state      container.ContainerState
+		noStarted  bool // the old agent's start time is lost with the label
 		wantStatus string
 		wantErr    string
 		wantOutput string
@@ -52,16 +54,19 @@ func TestResumeSelfUpdates(t *testing.T) {
 			wantOutput: "resolved " + pinned + "\n$ docker compose pull agent\n" + separator + "Recreated\n"},
 		{name: "helper failed", helper: helper("h1", "updater", "agent", nil), exit: 1, wantStatus: protocol.ResultFailed,
 			wantErr: "docker compose up: exit status 1 (output: Recreated)"},
-		{name: "no ref label", helper: helper("h1", "updater", "agent", map[string]string{protocol.LabelSelfUpdateRef: ""}),
+		{name: "no ref label", helper: helper("h1", "updater", "agent", map[string]string{labelSelfUpdateRef: ""}),
 			wantStatus: protocol.ResultFailed, wantErr: "update helper has no ref label"},
-		{name: "no started label", helper: helper("h1", "updater", "agent", map[string]string{protocol.LabelSelfUpdateStarted: ""}),
+		{name: "no started label", helper: helper("h1", "updater", "agent", map[string]string{labelSelfUpdateStarted: ""}),
+			noStarted:  true,
 			wantStatus: protocol.ResultFailed, wantErr: `update helper has no usable started label ""`},
-		{name: "no output label", helper: helper("h1", "updater", "agent", map[string]string{protocol.LabelSelfUpdateOutput: ""}),
+		{name: "helper never started", helper: helper("h1", "updater", "agent", nil), state: container.StateCreated,
+			wantStatus: protocol.ResultFailed, wantErr: "update helper was never started"},
+		{name: "no output label", helper: helper("h1", "updater", "agent", map[string]string{labelSelfUpdateOutput: ""}),
 			wantStatus: protocol.ResultSucceeded, wantOutput: separator + "Recreated\n"},
 	}
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			h := &fakeHelper{exit: c.exit, logs: "Recreated\n"}
+			h := &fakeHelper{exit: c.exit, state: c.state, logs: "Recreated\n"}
 			unrelated := []container.Summary{
 				helper("other-project", "other", "agent", nil),
 				helper("other-service", "updater", "web", nil),
@@ -86,7 +91,9 @@ func TestResumeSelfUpdates(t *testing.T) {
 			assert.Equal(t, "j-h1", got[0].ID)
 			res := got[0].Result
 			assert.Equal(t, c.wantStatus, res.Status)
-			assert.Equal(t, container.WaitConditionNotRunning, h.waitCond)
+			if c.state != container.StateCreated {
+				assert.Equal(t, container.WaitConditionNotRunning, h.waitCond)
+			}
 			assert.Equal(t, []string{"h1"}, h.removed, "only the matching helper is touched")
 			if c.wantStatus == protocol.ResultSucceeded {
 				assert.Equal(t, pinned, res.Ref)
@@ -94,7 +101,7 @@ func TestResumeSelfUpdates(t *testing.T) {
 			} else {
 				assert.Equal(t, c.wantErr, res.Error)
 			}
-			if c.name != "no started label" {
+			if !c.noStarted {
 				assert.True(t, started.Equal(res.StartedAt), "the old agent's start time is kept")
 			}
 		})