fix(processgroup): one undecidable group no longer hides the rest

cmd/clear.go

@@ -188,19 +188,23 @@ func clearCommand(ctx context.Context, args []string, options clearOptions) (ret
 // files under ~/.codefly/runs — or counts it on a dry run.
 func sweepStaleState(ctx context.Context, w *wool.Wool, options clearOptions) []error {
 	if options.dryRun {
+		var failures []error
 		if !options.keepProcesses {
+			// A group the preview cannot decide is named in err and kept; the
+			// rest are still listed, and the dry run goes on to count sockets.
 			groups, err := processgroup.PreviewManagedProcessGroups(ctx, options.scope)
+			w.Info("would stop registered process groups", wool.Field("count", len(groups)), wool.Field("pgids", groups))
 			if err != nil {
-				return []error{err}
+				w.Warn("would keep process groups whose ownership cannot be decided", wool.ErrField(err))
+				failures = append(failures, err)
 			}
-			w.Info("would stop registered process groups", wool.Field("count", len(groups)), wool.Field("pgids", groups))
 		}
 		if n := manager.CountStaleAgentSockets(); n > 0 {
 			w.Info("would remove stale agent sockets", wool.Field("count", n))
 		} else {
 			w.Info("no stale agent sockets")
 		}
-		return nil
+		return failures
 	}
 	if n := manager.SweepStaleAgentSockets(); n > 0 {
 		w.Info("removed stale agent sockets", wool.Field("count", n))

cmd/clear_dryrun_test.go

@@ -0,0 +1,136 @@
+package cmd
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"sync"
+	"syscall"
+	"testing"
+
+	runners "github.com/codefly-dev/core/runners/base"
+	"github.com/codefly-dev/core/wool"
+
+	"github.com/codefly-dev/cli/pkg/processgroup"
+)
+
+// capturedLogs keeps every line a command writes, with its fields.
+type capturedLogs struct {
+	mu   sync.Mutex
+	logs []*wool.Log
+}
+
+func (c *capturedLogs) Process(msg *wool.Log) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	c.logs = append(c.logs, msg)
+}
+
+// line is the captured log whose message is msg, with its fields as text.
+func (c *capturedLogs) line(msg string) (string, bool) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	for _, l := range c.logs {
+		if l.Message != msg {
+			continue
+		}
+		var fields []string
+		for _, f := range l.Fields {
+			fields = append(fields, fmt.Sprintf("%s=%v", f.Key, f.Value))
+		}
+		return strings.Join(fields, " "), true
+	}
+	return "", false
+}
+
+// trackedSleep starts a real tracked process group, killed with the test.
+func trackedSleep(t *testing.T) int {
+	t.Helper()
+	group, err := runners.StartTrackedProcessGroup(exec.Command("sleep", "60"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = syscall.Kill(-group.PGID(), syscall.SIGKILL) })
+	return group.PGID()
+}
+
+// recordedByAnOlderAgent rewrites a group's record as an older Darwin writer
+// left it: one clock-derived boot id for leader and owner, which Core refuses
+// to decide while the leader's pid and start are unchanged.
+func recordedByAnOlderAgent(t *testing.T, home string, pgid int) {
+	t.Helper()
+	paths, err := filepath.Glob(filepath.Join(home, ".codefly", "*", "authenticated-v2", strconv.Itoa(pgid)+".pgid"))
+	if err != nil || len(paths) != 1 {
+		t.Fatalf("record for %d: %v %v", pgid, paths, err)
+	}
+	raw, err := os.ReadFile(paths[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	var record map[string]any
+	if err := json.Unmarshal(raw, &record); err != nil {
+		t.Fatal(err)
+	}
+	for _, field := range []string{"leader", "owner"} {
+		identity, ok := record[field].(map[string]any)
+		if !ok {
+			t.Fatalf("record has no %s: %s", field, raw)
+		}
+		identity["boot_id"] = "1793000000:123456"
+	}
+	out, err := json.Marshal(record)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(paths[0], out, 0o600); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// A dry run with one group Core cannot decide still says everything it can:
+// the healthy group it would stop, a warning naming the kept group and its
+// recovery, and the stale sockets it would remove; and it returns the
+// refusal rather than hiding it (cli#1233).
+func TestClearDryRunReportsPastAnUndecidableGroup(t *testing.T) {
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	t.Setenv("TMPDIR", t.TempDir())
+	healthy, undecidable := trackedSleep(t), trackedSleep(t)
+	recordedByAnOlderAgent(t, home, undecidable)
+
+	// One stale agent socket: its owner's pid is dead.
+	gone := exec.Command("true")
+	if err := gone.Run(); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Mkdir(filepath.Join(os.TempDir(), fmt.Sprintf("codefly-uds-%d-x", gone.Process.Pid)), 0o700); err != nil {
+		t.Fatal(err)
+	}
+
+	logs := &capturedLogs{}
+	w := wool.Get(context.Background()).WithLogger(logs)
+	failures := sweepStaleState(context.Background(), w, clearOptions{dryRun: true, scope: processgroup.AllWorkspaces()})
+
+	groups, ok := logs.line("would stop registered process groups")
+	if !ok || !strings.Contains(groups, strconv.Itoa(healthy)) || strings.Contains(groups, strconv.Itoa(undecidable)) {
+		t.Fatalf("would stop %q (logged %v), want only the healthy group %d", groups, ok, healthy)
+	}
+	warning, ok := logs.line("would keep process groups whose ownership cannot be decided")
+	if !ok || !strings.Contains(warning, "codefly recover process-group "+strconv.Itoa(undecidable)) {
+		t.Fatalf("ownership warning %q (logged %v) does not name group %d's recovery", warning, ok, undecidable)
+	}
+	if sockets, ok := logs.line("would remove stale agent sockets"); !ok || !strings.Contains(sockets, "count=1") {
+		t.Fatalf("stale sockets %q (logged %v), want count 1", sockets, ok)
+	}
+	if len(failures) != 1 || !strings.Contains(failures[0].Error(), "inspect registered group "+strconv.Itoa(undecidable)) {
+		t.Fatalf("returned %v, want the undecidable group's refusal", failures)
+	}
+	if syscall.Kill(-healthy, 0) != nil || syscall.Kill(-undecidable, 0) != nil {
+		t.Fatal("the dry run signaled a group")
+	}
+}

docs/orchestration.md

@@ -1150,7 +1150,10 @@ recover its groups; upgrade the cleanup consumer along with the producer.
 error when a live leader has the recorded start identity but a different boot
 identity. Older Darwin agents wrote a boot timestamp where current agents write
 a boot-session UUID. This mismatch does not prove PID reuse: cleanup neither
-signals the group nor discards its recovery evidence. Recover the owning run
+signals the group nor discards its recovery evidence. Such a group is kept and
+named, with `codefly recover process-group PGID --preview`, in the error; it
+does not stop a preview (`codefly clear --dry-run`) from listing every other
+group, nor a stop from handling them. Recover the owning run
 and update its agent before restarting. A successful stop must not hide a
 process it could not authenticate. If registry cleanup fails, stop also skips
 the signature-based orphan fallback so it cannot bypass that refusal.

pkg/processgroup/registered.go

@@ -9,29 +9,45 @@ import (
 )
 
 // PreviewManagedProcessGroups lists live, authenticated, in-scope registrations
-// without signaling or removing records. Refusals match managed stop admission.
+// without signaling or removing records. Refusals match managed stop admission:
+// a group whose ownership cannot be decided is left out and named in the error,
+// with how to recover it, and every other group is still listed.
 func PreviewManagedProcessGroups(ctx context.Context, scope Scope) ([]int, error) {
 	groups, err := runners.ListProcessGroups(ctx)
 	if err != nil {
 		return nil, err
 	}
 	var selected []int
+	var kept []error
 	for _, group := range groups {
 		if !group.Alive() {
 			continue
 		}
 		ownership, err := group.InspectOwnership(ctx)
 		if err != nil {
-			return nil, fmt.Errorf("inspect registered group %d: %w", group.PGID(), err)
+			kept = append(kept, undecided(group.PGID(), err))
+			continue
 		}
 		if !ownership.Authenticated {
-			return nil, fmt.Errorf("group %d ownership is unproven", group.PGID())
+			kept = append(kept, undecided(group.PGID(), errUnproven))
+			continue
 		}
 		if registeredGroupInScope(group, ownership, scope) {
 			selected = append(selected, group.PGID())
 		}
 	}
-	return selected, nil
+	return selected, errors.Join(kept...)
+}
+
+// errUnproven: the group's members do not prove they belong to its record.
+var errUnproven = errors.New("live group ownership is unproven")
+
+// undecided is a live registered group Core cannot decide is the one its
+// record describes (an unproven owner, or a boot identity an older writer
+// recorded differently). It is kept without signaling; the error names it and
+// the supported way to recover it, so one such group never hides the others.
+func undecided(pgid int, err error) error {
+	return fmt.Errorf("inspect registered group %d: %w; kept without signaling, recover it with `codefly recover process-group %d --preview`", pgid, err, pgid)
 }
 
 // Core owns supported registry discovery, authentication and identity-bound
@@ -70,10 +86,10 @@ func reconcileRegisteredGroup(ctx context.Context, group *runners.TrackedProcess
 	}
 	ownership, err := group.InspectOwnership(ctx)
 	if err != nil {
-		return CleanupFailed, false, err
+		return CleanupFailed, false, undecided(group.PGID(), err)
 	}
 	if !ownership.Authenticated {
-		return CleanupFailed, false, fmt.Errorf("live group ownership is unproven; registration retained without signaling")
+		return CleanupFailed, false, undecided(group.PGID(), errUnproven)
 	}
 	if !registeredGroupInScope(group, ownership, policy.scope) {
 		return CleanupClean, true, nil

pkg/processgroup/registered_test.go

@@ -2,11 +2,13 @@ package processgroup
 
 import (
 	"context"
+	"encoding/json"
 	"io"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strconv"
+	"strings"
 	"syscall"
 	"testing"
 	"time"
@@ -92,3 +94,71 @@ func TestRegisteredStopUsesPersistedWorkspaceAfterOwnerAndEnvironmentDisappear(t
 		t.Fatalf("target missing from recovery evidence: %+v", evidence)
 	}
 }
+
+// legacyBootID rewrites a group's registry record as an older Darwin writer
+// left it: the boot id differs while the leader's pid and start are unchanged,
+// which Core refuses to decide (checkBootIdentity) and keeps the record for.
+func legacyBootID(t *testing.T, pgid int) {
+	t.Helper()
+	home, err := os.UserHomeDir()
+	if err != nil {
+		t.Fatal(err)
+	}
+	paths, err := filepath.Glob(filepath.Join(home, ".codefly", "*", "authenticated-v2", strconv.Itoa(pgid)+".pgid"))
+	if err != nil || len(paths) != 1 {
+		t.Fatalf("record for %d: %v %v", pgid, paths, err)
+	}
+	raw, err := os.ReadFile(paths[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	var record map[string]any
+	if err := json.Unmarshal(raw, &record); err != nil {
+		t.Fatal(err)
+	}
+	// An older writer recorded one boot id for the leader and the owner alike.
+	for _, field := range []string{"leader", "owner"} {
+		identity, ok := record[field].(map[string]any)
+		if !ok {
+			t.Fatalf("record has no %s: %s", field, raw)
+		}
+		identity["boot_id"] = "1793000000:123456"
+	}
+	out, err := json.Marshal(record)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(paths[0], out, 0o600); err != nil {
+		t.Fatal(err)
+	}
+}
+
+// One group whose boot identity Core cannot decide is kept and named, with
+// the command that recovers it, but it no longer stops the preview or the
+// stop from handling every other group in the workspace (cli#1233).
+func TestAnUndecidableGroupIsKeptWithoutStoppingTheRest(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	workspace := newWorkspace(t, "first")
+	undecidable := startUnobservableRun(t, workspace, workspace)
+	healthy := startUnobservableRun(t, workspace, workspace)
+	legacyBootID(t, undecidable)
+
+	preview, err := PreviewManagedProcessGroups(context.Background(), InWorkspace(workspace))
+	if len(preview) != 1 || preview[0] != healthy {
+		t.Fatalf("preview=%v err=%v, want only the healthy group %d", preview, err, healthy)
+	}
+	if err == nil || !strings.Contains(err.Error(), strconv.Itoa(undecidable)) || !strings.Contains(err.Error(), "codefly recover process-group "+strconv.Itoa(undecidable)) {
+		t.Fatalf("preview error %v does not name the undecidable group and its recovery", err)
+	}
+
+	_, err = StopManagedProcessGroups(context.Background(), InWorkspace(workspace))
+	if groupAlive(healthy) {
+		t.Fatal("the healthy group survived the stop")
+	}
+	if !groupAlive(undecidable) {
+		t.Fatal("the undecidable group was signaled")
+	}
+	if err == nil || !strings.Contains(err.Error(), "codefly recover process-group "+strconv.Itoa(undecidable)) {
+		t.Fatalf("stop error %v does not name the undecidable group's recovery", err)
+	}
+}