fix(ci): orphan sweep spares other homes, dep gate blocks on failed deps (D-1117, D-1119)

free/ci/internal/exec/sweep.go

@@ -15,6 +15,13 @@ type StaleChecker interface {
 	Stale(context.Context, string) (bool, error)
 }
 
+// KnownChecker is an optional StaleChecker extension: Known reports whether the coordinator belongs to the state
+// this sweep serves. The container listing is daemon-wide, so a container of a coordinator that is not known
+// here may be live in another CI home and is never removed (D-1117).
+type KnownChecker interface {
+	Known(context.Context, string) (bool, error)
+}
+
 // Sweep removes only labelled containers (docker or podman) owned by stale coordinators.
 func (e *Executor) Sweep(ctx context.Context, current string) error {
 	home := os.Getenv("NSELF_CI_HOME")
@@ -126,6 +133,15 @@ func (e *Executor) sweepRuntime(ctx context.Context, current, binary string) err
 		if len(parts) != 2 || parts[1] == current {
 			continue
 		}
+		if k, ok := e.Stale.(KnownChecker); ok {
+			known, err := k.Known(ctx, parts[1])
+			if err != nil {
+				return err
+			}
+			if !known {
+				continue
+			}
+		}
 		stale, err := e.Stale.Stale(ctx, parts[1])
 		if err != nil {
 			return err

free/ci/internal/run/depok_test.go

@@ -0,0 +1,55 @@
+package run
+
+// Purpose: what lets a job's waiters run (D-1119). A failed allow_failure job
+// blocks them as plan/dag.go promises; a prerequisite that never ran blocks them too.
+
+import (
+	"testing"
+
+	"github.com/nself-org/plugins/free/ci/internal/model"
+)
+
+func TestDepOK(t *testing.T) {
+	t.Parallel()
+	cases := []struct {
+		name string
+		d    JobOutcome
+		want bool
+	}{
+		{"passed", JobOutcome{State: StatePassed}, true},
+		{"failed", JobOutcome{State: StateFailed}, false},
+		{"failed allow_failure", JobOutcome{State: StateFailed, AllowFailure: true}, false},
+		{"cancelled", JobOutcome{State: StateCancelled}, false},
+		{"skipped not affected", JobOutcome{State: StateSkipped, Reason: "selection.affected"}, true},
+		{"skipped by when", JobOutcome{State: StateSkipped, Reason: "when.paths"}, true},
+		{"skipped dep.failed", JobOutcome{State: StateSkipped, Reason: "dep.failed"}, false},
+		{"skipped fail-fast", JobOutcome{State: StateSkipped, Reason: "fail_fast.cancelled"}, false},
+		{"skipped unplaceable", JobOutcome{State: StateSkipped, Reason: "unplaceable.tool"}, false},
+		{"skipped admission", JobOutcome{State: StateSkipped, Reason: "admission.battery"}, false},
+		{"skipped advisory", JobOutcome{State: StateSkipped, Reason: "advisory.tool_missing"}, false},
+	}
+	for _, tc := range cases {
+		if got := depOK(tc.d); got != tc.want {
+			t.Errorf("%s: depOK = %v, want %v", tc.name, got, tc.want)
+		}
+	}
+}
+
+func TestFailedAllowFailureJobBlocksItsWaiters(t *testing.T) {
+	t.Parallel()
+	opt := func(script string, deps ...string) model.Job {
+		j := sh(script, deps...)
+		j.Required = no()
+		return j
+	}
+	build := opt("exit 1")
+	build.AllowFailure = true
+	r := newRig(t, map[string]model.Job{"build": build, "publish": opt("true", "build")}, gitFake{})
+	res := r.run(t)
+	if o := outcome(res, "build"); o.State != StateFailed {
+		t.Fatalf("build: %+v", o)
+	}
+	if o := outcome(res, "publish"); o.State != StateSkipped || o.Reason != "dep.failed" {
+		t.Fatalf("publish ran or was skipped for the wrong reason: %+v", o)
+	}
+}

free/ci/internal/run/loop.go

@@ -13,6 +13,7 @@ package run
 
 import (
 	"context"
+	"strings"
 	"sync"
 
 	"github.com/nself-org/plugins/free/ci/internal/model"
@@ -99,14 +100,20 @@ func cancelled(o JobOutcome, why model.Reason, detail string) JobOutcome {
 	return o
 }
 
+// depOK reports whether a finished dependency lets its waiters run. A failed
+// job always blocks them, allow_failure or not (plan/dag.go: a failing
+// allow_failure job still blocks everything that waits on it). A skipped job
+// lets them run only when it was scoped out by design (selection, when.*,
+// pipeline); one that never ran for want of a place, a tool or admission, or
+// that was cancelled, blocks them like a failure.
 func depOK(d JobOutcome) bool {
 	switch d.State {
 	case StatePassed:
 		return true
-	case StateFailed:
-		return d.AllowFailure
 	case StateSkipped:
-		return d.Reason != "dep.failed" && d.Reason != "fail_fast.cancelled"
+		r := string(d.Reason)
+		return r != "dep.failed" && r != "fail_fast.cancelled" &&
+			!strings.HasPrefix(r, "unplaceable.") && !strings.HasPrefix(r, "admission.") && !strings.HasPrefix(r, "advisory.")
 	}
 	return false
 }

free/ci/internal/run/recovery.go

@@ -54,7 +54,23 @@ func (c staleChecker) Stale(ctx context.Context, id string) (bool, error) {
 			return false, nil
 		}
 	}
-	return true, nil // no such coordinator: its leftovers belong to nobody
+	return true, nil // no such coordinator: its own-home leftovers (process markers) belong to nobody; containers are gated by Known
+}
+
+// Known reports whether the store has a coordinator with this (safe) id. The
+// container sweep lists every labelled container on the daemon, so an id this
+// store has never seen may be a live coordinator of another CI home.
+func (c staleChecker) Known(ctx context.Context, id string) (bool, error) {
+	all, err := c.st.Coordinators(ctx)
+	if err != nil {
+		return false, err
+	}
+	for _, a := range all {
+		if safeID(a.ID) == id {
+			return true, nil
+		}
+	}
+	return false, nil
 }
 
 // Recover cleans up after coordinators that died. current is the live

free/ci/internal/run/recovery_sweep_test.go

@@ -0,0 +1,53 @@
+//go:build !windows
+
+package run
+
+// Purpose: the container sweep of Recover removes only containers of coordinators
+// this store knows to be stale; another CI home's container is left alone (D-1117).
+// Constraints: a fake docker script on a PATH that holds nothing else; no real daemon.
+
+import (
+	"context"
+	"os"
+	osexec "os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	ciexec "github.com/nself-org/plugins/free/ci/internal/exec"
+)
+
+func TestRecoverSweepSkipsContainersOfUnknownCoordinators(t *testing.T) {
+	ciexec.ResetRuntimeInfoCache()
+	st := openState(t)
+	ctx := context.Background()
+	gone := osexec.Command("true")
+	if err := gone.Run(); err != nil {
+		t.Fatal(err)
+	}
+	dead, err := st.RegisterCoordinator(ctx, gone.Process.Pid, "t0", bootID())
+	if err != nil {
+		t.Fatal(err)
+	}
+	bin := t.TempDir()
+	t.Setenv("PATH", bin)
+	home := t.TempDir()
+	t.Setenv("NSELF_CI_HOME", home)
+	listing := "dead1 " + safeID(dead.ID) + "\nforeign1 another-home-coordinator\n"
+	script := "#!/bin/sh\necho \"$*\" >> \"${0%/*}/docker.calls\"\n" +
+		"case \"$1\" in\ninfo) echo '{}';;\nps) printf '%s' '" + listing + "';;\nesac\nexit 0\n"
+	if err := os.WriteFile(filepath.Join(bin, "docker"), []byte(script), 0700); err != nil {
+		t.Fatal(err)
+	}
+	if rep := Recover(ctx, st, "", home); len(rep.Errors) > 0 {
+		t.Fatalf("recover errors: %v", rep.Errors)
+	}
+	calls, _ := os.ReadFile(filepath.Join(bin, "docker.calls"))
+	out := string(calls)
+	if !strings.Contains(out, "rm -f dead1") {
+		t.Errorf("the container of a stale known coordinator was not removed:\n%s", out)
+	}
+	if strings.Contains(out, "foreign1") {
+		t.Errorf("a container of a coordinator unknown to this store was removed:\n%s", out)
+	}
+}