diff --git a/cmd/clear.go b/cmd/clear.go index ea792815..dd8a1b52 100644 --- a/cmd/clear.go +++ b/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)) diff --git a/cmd/clear_dryrun_test.go b/cmd/clear_dryrun_test.go new file mode 100644 index 00000000..5563f244 --- /dev/null +++ b/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") + } +} diff --git a/docs/orchestration.md b/docs/orchestration.md index ac30f70c..c3e357da 100644 --- a/docs/orchestration.md +++ b/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. diff --git a/pkg/processgroup/registered.go b/pkg/processgroup/registered.go index 12c90958..51a72c79 100644 --- a/pkg/processgroup/registered.go +++ b/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 diff --git a/pkg/processgroup/registered_test.go b/pkg/processgroup/registered_test.go index e5606ff4..dd9360ed 100644 --- a/pkg/processgroup/registered_test.go +++ b/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) + } +}