fix(generate): reap the agents generate contracts starts

cmd/generate/contracts.go

@@ -21,6 +21,7 @@ import (
 	basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0"
 	resources "github.com/codefly-dev/core/resources"
 	runners "github.com/codefly-dev/core/runners/dockerrun"
+	"github.com/codefly-dev/core/services"
 	"github.com/codefly-dev/core/shared"
 	"github.com/codefly-dev/core/standards"
 	"github.com/spf13/cobra"
@@ -76,6 +77,22 @@ against what's on disk, without writing anything.
 		defer done()
 		ctx, stop := common.SignalContext(ctx)
 		defer stop()
+		// This command loads service builders to read their endpoints
+		// (generators.LoadServiceEndpoints -> services.Load, which spawns the
+		// agent, then instance.LoadBuilder), so an agent subprocess exists per
+		// service. ClearAgents closes the cached connections, and Core's close
+		// is cooperative on its own account: it asks the agent to stop, waits
+		// for the agent to reap its own children, and forces the process group
+		// down only if that wait expires. Which signal it sends is Core's to
+		// decide and is not restated here — the comments around it in Core name
+		// SIGTERM while the code sends os.Interrupt. Without this,
+		// every `generate contracts` pass left its agents running — eight live
+		// private go-grpc records accumulated across two regenerations of one
+		// workspace. `generate client` already reaps the same way.
+		//
+		// It ends the processes THIS command started, which is all it claims:
+		// an agent already running from another command is not its business.
+		defer services.ClearAgents()
 
 		if contractsFormat != "text" && contractsFormat != "json" {
 			return fmt.Errorf("--format must be text or json, got %q", contractsFormat)

docs/commands.md

@@ -3034,6 +3034,40 @@ release commit also:
   in full. A catalog that no longer matches its contract files is refused —
   run `codefly generate contracts` and commit it first.
 
+**Agents the command starts, and what ends them.** Reading each service's
+exported endpoints loads that service, which starts its agent as a child
+process. The command retires them on return by closing the cached connections.
+That close is Core's, and what it attempts is cooperative: Core asks the agent
+to stop, gives it time to retire its own children (user binaries, containers),
+and forces the process group down if that wait expires. The signal it sends is
+Core's own choice and is not restated here.
+
+What that does **not** establish is that every child was retired. The close
+reports no error, the agent ignores the error from stopping its children, and
+the agent has a hard-deadline exit of its own, so an agent process that has
+gone away is evidence the agent exited — not proof that nothing it started
+survived it. Treat this as best-effort cleanup with a forced fallback.
+
+Cancelling the command once (Ctrl-C) takes the same path: the reap is deferred
+rather than conditional on success, so it still runs. A **second** Ctrl-C does
+not — it exits the process immediately and no deferred cleanup runs at all,
+which leaves exactly the agents this retirement exists to prevent.
+
+It retires the agents **that run of the command started**. An agent left
+running by an earlier command — a version of the CLI before this retirement
+existed, for instance — is not adopted or reaped here: it is a separate process
+that outlived its own command. Retiring one is `codefly recover process <PID>
+--preview` to read its identity, then `--expect FILE` with an explicit mode to
+act on exactly that birth, one process at a time, with no group or registry
+effect.
+
+The only mode this version offers is `--force`, an irreversible SIGKILL. That
+is **not** a substitute for a cooperative shutdown: it gives the agent no
+chance to reap its own children, so a leftover agent's user binaries and
+containers can survive it. A graceful mode, and what should happen when a
+target refuses to exit or is signalled twice, are not covered at this version.
+
+
 The services and modules are found through `workspace.codefly.yaml` and each
 `module.codefly.yaml` — never by searching the tree, so a fixture workspace
 elsewhere in the repository is not touched, and never through the machine-local

pkg/conformance/agent_reaping_test.go

@@ -0,0 +1,119 @@
+package conformance
+
+import (
+	"bytes"
+	"go/parser"
+	"go/printer"
+	"go/token"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+	"testing"
+)
+
+// codeOf returns the file's source with every comment removed, by parsing it
+// and printing the syntax tree back out. The checks below are textual, and a
+// comment is text: the first version of this guard matched the word
+// "ClearAgents" anywhere in the file, so the explanatory comment that sits
+// above the real `defer services.ClearAgents()` was itself enough to satisfy
+// it — deleting the defer and keeping the comment passed. Stripping comments
+// first is what makes a reap have to be a reap.
+func codeOf(t *testing.T, path, body string) string {
+	t.Helper()
+	fset := token.NewFileSet()
+	file, err := parser.ParseFile(fset, path, body, 0)
+	if err != nil {
+		t.Fatalf("%s: %v", path, err)
+	}
+	var code bytes.Buffer
+	if err := printer.Fprint(&code, fset, file); err != nil {
+		t.Fatalf("%s: %v", path, err)
+	}
+	return code.String()
+}
+
+// TestNoCommandFileOmitsAgentReaping is a TEXTUAL omission detector, and
+// nothing more: it reads source as text — comments stripped — and asks whether
+// a reaping call appears. It catches the omission that actually happened: a
+// command that loads agents with no reap anywhere in its file. It is a cheap
+// backstop against the same slip recurring, not proof that any command reaps
+// correctly, and it is deliberately not a call-graph analysis: a reap written
+// behind a condition that never runs would still satisfy it.
+//
+// Loading a service starts an agent process. ClearAgents closes the cached
+// connections, and Core's close is cooperative in itself — it asks the agent
+// to stop, waits for the agent to reap its own children, and forces the
+// process group down only if that wait times out. (The signal is Core's
+// choice; the comments around it in Core do not match the code it runs, so
+// this does not restate a mechanism.) A command that loads agents and never
+// reaps leaks one process per service, every invocation. It retires the
+// processes that command started; agents owned by another command are not
+// affected.
+//
+// This is not hypothetical: `generate contracts` loaded builders through
+// generators.LoadServiceEndpoints and reaped nothing, while its sibling
+// `generate client` reaped correctly. Two regenerations of one workspace left
+// eight live private go-grpc agents behind. The omission was invisible because
+// nothing failed — the command exited 0 and the processes simply stayed.
+//
+// A command that deliberately hands its agents to something longer-lived
+// (a server, the MCP host) still has to say so by name below, so the exemption
+// is a decision on the record rather than an absence.
+func TestNoCommandFileOmitsAgentReaping(t *testing.T) {
+	root := repositoryRoot(t)
+	// Calls that start an agent subprocess for a service.
+	// services.Load is where the agent process is actually started;
+	// LoadBuilder/LoadRuntime then attach to it. All of them are listed because
+	// a command reaching any of them has an agent to answer for.
+	spawns := []string{"services.Load(", "LoadBuilder(", "LoadRuntime(", "LoadServiceEndpoints("}
+	// Reaping, however the command spells it.
+	reaps := []string{"ClearAgents", "ClearAgent(", "RegisterCleanup(services.ClearAgents"}
+
+	// Per COMMAND FILE, which is the granularity that actually holds.
+	//
+	// Per-file alone is too strict: cmd/sourcefix/fix.go loads while its
+	// sibling commands.go defers the reap, and that is correct. Per-package is
+	// too loose: cmd/generate hosts many independent commands, so
+	// client.go's reap masked contracts.go having none — the very bug this
+	// guard exists for passed a per-package check.
+	//
+	// So the invariant is: a file that declares a command (RunE) AND reaches a
+	// spawn must reap in that same file. A helper file with no RunE is cleared,
+	// because whichever RunE calls it owns the lifetime.
+	var offenders []string
+	err := filepath.WalkDir(filepath.Join(root, "cmd"), func(path string, entry os.DirEntry, err error) error {
+		if err != nil || entry.IsDir() || !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") {
+			return err
+		}
+		body := codeOf(t, path, readFile(t, path))
+		if !strings.Contains(body, "RunE:") {
+			return nil
+		}
+		loads := false
+		for _, call := range spawns {
+			loads = loads || strings.Contains(body, call)
+		}
+		if !loads {
+			return nil
+		}
+		for _, reap := range reaps {
+			if strings.Contains(body, reap) {
+				return nil
+			}
+		}
+		relative, relErr := filepath.Rel(root, path)
+		if relErr != nil {
+			relative = path
+		}
+		offenders = append(offenders, filepath.ToSlash(relative))
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	sort.Strings(offenders)
+	for _, offender := range offenders {
+		t.Errorf("%s declares a command that loads service agents but never reaps them: add `defer services.ClearAgents()` beside the command's other defers, or name the file here with the reason its agents outlive it", offender)
+	}
+}