From 9d78484e3c985817225c67ff364cb09a6d61b7a3 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Wed, 22 Jul 2026 15:02:23 -0400 Subject: [PATCH 1/6] feat(cli): warn before af stop interrupts running executions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `af stop ` gracefully killed nodes with long-running executions in flight with no warning. Running executions are queryable from the control plane, so query them before signalling the process: - running executions found + TTY → list count/ids/age and prompt to confirm - running executions found + no TTY → warn (count/ids/age) and proceed - --force → stop immediately, no query/warning/prompt - control plane unreachable → note it and proceed (best-effort, as before) The check runs only once we have confirmed the process is genuinely ours and alive, so a dead/stale node still reconciles cleanly without spurious queries. Co-Authored-By: Claude Fable 5 --- control-plane/internal/cli/stop.go | 205 ++++++++++++++- .../internal/cli/stop_executions_test.go | 234 ++++++++++++++++++ 2 files changed, 436 insertions(+), 3 deletions(-) create mode 100644 control-plane/internal/cli/stop_executions_test.go diff --git a/control-plane/internal/cli/stop.go b/control-plane/internal/cli/stop.go index a9d713902..2fd20d79a 100644 --- a/control-plane/internal/cli/stop.go +++ b/control-plane/internal/cli/stop.go @@ -1,6 +1,7 @@ package cli import ( + "bufio" "bytes" "encoding/json" "errors" @@ -9,6 +10,7 @@ import ( "net/http" "os" "path/filepath" + "strings" "time" "github.com/Agent-Field/agentfield/control-plane/internal/packages" @@ -16,7 +18,10 @@ import ( "gopkg.in/yaml.v3" ) -var stopJSON bool +var ( + stopJSON bool + stopForce bool +) // NewStopCommand creates the stop command func NewStopCommand() *cobra.Command { @@ -28,15 +33,22 @@ func NewStopCommand() *cobra.Command { The agent node process will be terminated gracefully and its status will be updated in the registry. +Before stopping, the control plane is queried for executions still running on +the node. On a terminal you are asked to confirm; when not attached to a +terminal a warning is printed and the stop proceeds. Pass --force to skip the +check entirely. + Examples: af stop email-helper af stop data-analyzer - af stop email-helper --json`, + af stop email-helper --json + af stop email-helper --force`, Args: cobra.ExactArgs(1), RunE: runStopCommand, } cmd.Flags().BoolVar(&stopJSON, "json", false, "Emit a machine-readable JSON envelope instead of progress output") + cmd.Flags().BoolVar(&stopForce, "force", false, "Stop immediately without warning about or confirming in-flight executions") return cmd } @@ -46,6 +58,11 @@ func runStopCommand(cmd *cobra.Command, args []string) error { stopper := &AgentNodeStopper{ AgentFieldHome: getAgentFieldHomeDir(), Quiet: stopJSON, + Force: stopForce, + ServerURL: GetServerURL(), + // A JSON invocation is a machine caller: never prompt. Otherwise honor + // whether stdin is a real terminal. + Interactive: !stopJSON && isInputTerminal(), } if err := stopper.StopAgentNode(agentNodeName); err != nil { @@ -69,8 +86,22 @@ type AgentNodeStopper struct { AgentFieldHome string // Quiet suppresses human progress output (used by --json mode). Quiet bool + // Force skips the running-execution check entirely (no query, no warning, + // no prompt). + Force bool + // ServerURL is the control plane queried for in-flight executions. Empty + // disables the check. + ServerURL string + // Interactive is true when the caller may prompt on a real terminal. When + // false, a running-execution warning is printed and the stop proceeds. + Interactive bool + // In is where an interactive confirmation is read from (defaults to stdin). + In io.Reader + // ErrOut receives execution warnings/prompts (defaults to stderr) so they + // never pollute a --json stdout envelope. + ErrOut io.Writer // Outcome records the result of the last StopAgentNode call: - // "stopped" or "not_running". + // "stopped", "not_running", or "aborted". Outcome string } @@ -82,6 +113,24 @@ func (as *AgentNodeStopper) printf(format string, args ...interface{}) { fmt.Printf(format, args...) } +// warnf writes an execution warning/prompt. Unlike printf it is never gated by +// Quiet — a potential-data-loss warning must not be silently swallowed — and it +// goes to ErrOut (stderr) so --json stdout stays clean. +func (as *AgentNodeStopper) warnf(format string, args ...interface{}) { + w := as.ErrOut + if w == nil { + w = os.Stderr + } + fmt.Fprintf(w, format, args...) +} + +func (as *AgentNodeStopper) input() io.Reader { + if as.In != nil { + return as.In + } + return os.Stdin +} + // StopAgentNode stops a running agent node func (as *AgentNodeStopper) StopAgentNode(agentNodeName string) error { // Load registry @@ -130,6 +179,22 @@ func (as *AgentNodeStopper) StopAgentNode(agentNodeName string) error { } } + // The process is genuinely ours and running — a graceful stop from here on + // will interrupt whatever it is doing. Warn about (or, on a terminal, + // confirm) any in-flight executions before we signal it, so a long build or + // analysis is not silently killed. Resolve the manifest node_id for the + // query since the control plane keys executions by node_id, which may differ + // from the registry install name. + queryID := agentNodeName + if md, err := packages.ParsePackageMetadata(agentNode.Path); err == nil && md.AgentNode.NodeID != "" { + queryID = md.AgentNode.NodeID + } + if !as.confirmStopWithRunningExecutions(queryID) { + as.Outcome = "aborted" + as.printf("🚫 Stop aborted — %s left running to protect its in-flight executions\n", agentNodeName) + return nil + } + // Try HTTP shutdown first if port is available httpShutdownSuccess := false if agentNode.Runtime.Port != nil { @@ -221,6 +286,140 @@ func (as *AgentNodeStopper) markStopped(registry *packages.InstallationRegistry, return nil } +// runningExecution is the subset of an execution record the stop guard needs. +type runningExecution struct { + ExecutionID string `json:"execution_id"` + AgentNodeID string `json:"agent_node_id"` + Status string `json:"status"` + StartedAt time.Time `json:"started_at"` +} + +// executionsQueryResponse mirrors the /api/v1/agentic/query envelope for the +// executions resource. +type executionsQueryResponse struct { + OK bool `json:"ok"` + Data struct { + Results []runningExecution `json:"results"` + Total int `json:"total"` + } `json:"data"` +} + +// queryRunningExecutions asks the control plane for executions still running on +// nodeID. An unreachable control plane or an unusable response returns an error; +// callers treat that as "proceed, best-effort". +func queryRunningExecutions(serverURL, nodeID string) ([]runningExecution, error) { + serverURL = strings.TrimRight(strings.TrimSpace(serverURL), "/") + if serverURL == "" { + return nil, fmt.Errorf("no control plane URL configured") + } + + payload := map[string]interface{}{ + "resource": "executions", + "filters": map[string]string{"status": "running", "agent_id": nodeID}, + "limit": 100, + } + body, err := json.Marshal(payload) + if err != nil { + return nil, err + } + + client := &http.Client{Timeout: 3 * time.Second} + req, err := http.NewRequest(http.MethodPost, serverURL+"/api/v1/agentic/query", bytes.NewReader(body)) + if err != nil { + return nil, err + } + req.Header.Set("Content-Type", "application/json") + if key := GetAPIKey(); key != "" { + req.Header.Set("Authorization", "Bearer "+key) + } + + resp, err := client.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("control plane returned status %d", resp.StatusCode) + } + + var parsed executionsQueryResponse + if err := json.NewDecoder(io.LimitReader(resp.Body, 1<<20)).Decode(&parsed); err != nil { + return nil, err + } + return parsed.Data.Results, nil +} + +// confirmStopWithRunningExecutions decides whether to proceed with stopping a +// node given its in-flight executions. It returns true to proceed, false to +// abort (only reachable via an interactive "no"): +// +// - --force → proceed silently (no query, no warning) +// - control plane unreachable → note it, proceed (best-effort, as before) +// - no running executions → proceed silently +// - running executions, TTY → list them and prompt; abort on anything but yes +// - running executions, non-TTY → warn with count + ids + age, then proceed +func (as *AgentNodeStopper) confirmStopWithRunningExecutions(nodeID string) bool { + if as.Force { + return true + } + + execs, err := queryRunningExecutions(as.ServerURL, nodeID) + if err != nil { + as.warnf("⚠️ Could not check the control plane for running executions on %s (%v) — proceeding with stop\n", nodeID, err) + return true + } + if len(execs) == 0 { + return true + } + + as.warnf("⚠️ %d running execution(s) on %s will be interrupted by stopping it:\n", len(execs), nodeID) + now := time.Now() + for _, e := range execs { + age := "unknown age" + if !e.StartedAt.IsZero() { + age = "age " + formatExecutionAge(now.Sub(e.StartedAt)) + } + as.warnf(" • %s (%s)\n", e.ExecutionID, age) + } + + if as.Interactive { + as.warnf("Proceed and interrupt %d running execution(s)? [y/N]: ", len(execs)) + return readAffirmative(as.input()) + } + + as.warnf(" Proceeding (non-interactive). Pass --force to stop without this warning, or wait for the executions to finish.\n") + return true +} + +// formatExecutionAge renders an execution's age compactly (e.g. "45s", "12m30s", +// "1h15m"). +func formatExecutionAge(d time.Duration) string { + if d < 0 { + d = 0 + } + d = d.Round(time.Second) + switch { + case d < time.Minute: + return fmt.Sprintf("%ds", int(d.Seconds())) + case d < time.Hour: + return fmt.Sprintf("%dm%ds", int(d.Minutes()), int(d.Seconds())%60) + default: + return fmt.Sprintf("%dh%dm", int(d.Hours()), int(d.Minutes())%60) + } +} + +// readAffirmative reads one line and reports whether it is an affirmative +// ("y"/"yes", case-insensitive). Anything else — including EOF — is a "no". +func readAffirmative(r io.Reader) bool { + line, _ := bufio.NewReader(r).ReadString('\n') + switch strings.ToLower(strings.TrimSpace(line)) { + case "y", "yes": + return true + default: + return false + } +} + // probeHealthNodeID asks /health on a local port and returns the node_id the // responder claims — "" when nothing answers or the payload carries none // (custom health endpoints are not required to identify themselves). diff --git a/control-plane/internal/cli/stop_executions_test.go b/control-plane/internal/cli/stop_executions_test.go new file mode 100644 index 000000000..b8864dbe8 --- /dev/null +++ b/control-plane/internal/cli/stop_executions_test.go @@ -0,0 +1,234 @@ +package cli + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +// executionsQueryServer stands up a control plane that answers the +// /api/v1/agentic/query endpoint with the given running executions and records +// how many times it was called. +func executionsQueryServer(t *testing.T, execs []runningExecution, calls *int) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if calls != nil { + *calls++ + } + if r.URL.Path != "/api/v1/agentic/query" { + w.WriteHeader(http.StatusNotFound) + return + } + var body map[string]interface{} + _ = json.NewDecoder(r.Body).Decode(&body) + if body["resource"] != "executions" { + t.Errorf("expected resource=executions, got %v", body["resource"]) + } + resp := executionsQueryResponse{OK: true} + resp.Data.Results = execs + resp.Data.Total = len(execs) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + })) +} + +func twoRunningExecutions() []runningExecution { + now := time.Now() + return []runningExecution{ + {ExecutionID: "exec-aaa", AgentNodeID: "swe-af", Status: "running", StartedAt: now.Add(-75 * time.Minute)}, + {ExecutionID: "exec-bbb", AgentNodeID: "swe-af", Status: "running", StartedAt: now.Add(-30 * time.Second)}, + } +} + +// Contract: non-TTY stop with running executions warns, includes the count, and +// proceeds (returns true) without prompting. +func TestConfirmStop_NonInteractive_WarnsWithCount(t *testing.T) { + srv := executionsQueryServer(t, twoRunningExecutions(), nil) + defer srv.Close() + + var out bytes.Buffer + stopper := &AgentNodeStopper{ + ServerURL: srv.URL, + Interactive: false, + ErrOut: &out, + } + + if proceed := stopper.confirmStopWithRunningExecutions("swe-af"); !proceed { + t.Fatalf("non-interactive stop must proceed, got proceed=false") + } + + got := out.String() + if !strings.Contains(got, "2 running execution") { + t.Errorf("warning should mention the count of 2, got:\n%s", got) + } + if !strings.Contains(got, "exec-aaa") || !strings.Contains(got, "exec-bbb") { + t.Errorf("warning should list the execution ids, got:\n%s", got) + } + if strings.Contains(got, "[y/N]") { + t.Errorf("non-interactive stop must not prompt, got:\n%s", got) + } +} + +// Contract: --force skips the check entirely — no warning is printed and the +// control plane is never queried. +func TestConfirmStop_Force_NoWarnNoQuery(t *testing.T) { + calls := 0 + srv := executionsQueryServer(t, twoRunningExecutions(), &calls) + defer srv.Close() + + var out bytes.Buffer + stopper := &AgentNodeStopper{ + ServerURL: srv.URL, + Force: true, + Interactive: false, + ErrOut: &out, + } + + if proceed := stopper.confirmStopWithRunningExecutions("swe-af"); !proceed { + t.Fatalf("--force stop must proceed, got proceed=false") + } + if out.Len() != 0 { + t.Errorf("--force must not warn, got:\n%s", out.String()) + } + if calls != 0 { + t.Errorf("--force must not query the control plane, got %d calls", calls) + } +} + +// Contract: with no running executions the stop proceeds silently. +func TestConfirmStop_NoRunningExecutions_Silent(t *testing.T) { + srv := executionsQueryServer(t, nil, nil) + defer srv.Close() + + var out bytes.Buffer + stopper := &AgentNodeStopper{ServerURL: srv.URL, ErrOut: &out} + + if proceed := stopper.confirmStopWithRunningExecutions("swe-af"); !proceed { + t.Fatalf("stop with no running executions must proceed") + } + if out.Len() != 0 { + t.Errorf("no executions should produce no warning, got:\n%s", out.String()) + } +} + +// Contract: an unreachable control plane notes the failure and proceeds. +func TestConfirmStop_ControlPlaneUnreachable_Proceeds(t *testing.T) { + var out bytes.Buffer + stopper := &AgentNodeStopper{ + // A port nothing is listening on. + ServerURL: "http://127.0.0.1:1", + ErrOut: &out, + } + + if proceed := stopper.confirmStopWithRunningExecutions("swe-af"); !proceed { + t.Fatalf("unreachable control plane must proceed (best-effort)") + } + if !strings.Contains(out.String(), "Could not check") { + t.Errorf("unreachable control plane should note it, got:\n%s", out.String()) + } +} + +// Contract: on a terminal the operator is prompted; a "no" aborts. +func TestConfirmStop_Interactive_DeclineAborts(t *testing.T) { + srv := executionsQueryServer(t, twoRunningExecutions(), nil) + defer srv.Close() + + var out bytes.Buffer + stopper := &AgentNodeStopper{ + ServerURL: srv.URL, + Interactive: true, + In: strings.NewReader("n\n"), + ErrOut: &out, + } + + if proceed := stopper.confirmStopWithRunningExecutions("swe-af"); proceed { + t.Fatalf("declining the prompt must abort (proceed=false)") + } + if !strings.Contains(out.String(), "[y/N]") { + t.Errorf("interactive stop should prompt, got:\n%s", out.String()) + } +} + +func TestConfirmStop_Interactive_AcceptProceeds(t *testing.T) { + srv := executionsQueryServer(t, twoRunningExecutions(), nil) + defer srv.Close() + + stopper := &AgentNodeStopper{ + ServerURL: srv.URL, + Interactive: true, + In: strings.NewReader("y\n"), + ErrOut: io.Discard, + } + + if proceed := stopper.confirmStopWithRunningExecutions("swe-af"); !proceed { + t.Fatalf("accepting the prompt must proceed") + } +} + +func TestQueryRunningExecutions_ParsesEnvelope(t *testing.T) { + srv := executionsQueryServer(t, twoRunningExecutions(), nil) + defer srv.Close() + + execs, err := queryRunningExecutions(srv.URL, "swe-af") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(execs) != 2 { + t.Fatalf("expected 2 executions, got %d", len(execs)) + } + if execs[0].ExecutionID != "exec-aaa" { + t.Errorf("expected first execution exec-aaa, got %s", execs[0].ExecutionID) + } +} + +func TestQueryRunningExecutions_EmptyServerURL(t *testing.T) { + if _, err := queryRunningExecutions("", "swe-af"); err == nil { + t.Fatalf("empty server URL should error") + } +} + +func TestReadAffirmative(t *testing.T) { + cases := []struct { + in string + want bool + }{ + {"y\n", true}, + {"Y\n", true}, + {"yes\n", true}, + {"YES\n", true}, + {" y \n", true}, + {"n\n", false}, + {"no\n", false}, + {"\n", false}, + {"", false}, + {"maybe\n", false}, + } + for _, tc := range cases { + if got := readAffirmative(strings.NewReader(tc.in)); got != tc.want { + t.Errorf("readAffirmative(%q) = %v, want %v", tc.in, got, tc.want) + } + } +} + +func TestFormatExecutionAge(t *testing.T) { + cases := []struct { + d time.Duration + want string + }{ + {-5 * time.Second, "0s"}, + {45 * time.Second, "45s"}, + {90 * time.Second, "1m30s"}, + {75 * time.Minute, "1h15m"}, + {2 * time.Hour, "2h0m"}, + } + for _, tc := range cases { + if got := formatExecutionAge(tc.d); got != tc.want { + t.Errorf("formatExecutionAge(%v) = %q, want %q", tc.d, got, tc.want) + } + } +} From f0b428487bf05506e9f1bb5a7bdf587f326b7e84 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Wed, 22 Jul 2026 15:05:49 -0400 Subject: [PATCH 2/6] feat(cli): surface af run startup logs and retry once on strict-port bind conflict Two operational failures in the `af run` path made a failed start opaque and non-recoverable: - On any startup failure the CLI only printed "did not become ready within 30s"; the real traceback/exit reason lived in the node log, reachable only via a separate `af logs`. Now the last ~15 lines of the node's log are printed inline on failure, plus a "Full logs: af logs " pointer. - When a node was assigned a port that looked free but lost the bind race (a just-stopped node's port lingering under mirrored networking), the SDK exited with AGENTFIELD_STRICT_PORT "assigned port N is unavailable" and nothing retried. The run path now detects that strict-port exit from the node log and retries exactly once on a fresh port (the failed port is reserved first so it is never reused), logging "Port

unavailable, retrying on a fresh port". The port-alloc/start/wait section is refactored into attemptStart + startWithPortRetry so the retry decision and port-change logic are unit-testable without the real health-poll. Co-Authored-By: Claude Fable 5 --- .../internal/core/services/agent_service.go | 154 ++++++++-- .../services/agent_service_startup_test.go | 271 ++++++++++++++++++ 2 files changed, 399 insertions(+), 26 deletions(-) create mode 100644 control-plane/internal/core/services/agent_service_startup_test.go diff --git a/control-plane/internal/core/services/agent_service.go b/control-plane/internal/core/services/agent_service.go index 68e39dadb..cb23538d7 100644 --- a/control-plane/internal/core/services/agent_service.go +++ b/control-plane/internal/core/services/agent_service.go @@ -104,20 +104,64 @@ func (as *DefaultAgentService) runAgentGuarded(name string, options domain.RunOp } } + // 4-5. Start the process and wait for readiness. Retry exactly once on a + // fresh port if the node exits with a strict-port bind conflict: the + // assigned port looked free but the bind lost a race (commonly a + // just-stopped node's port lingering on mirrored-network setups), and the + // SDK exits fast so the control plane can reallocate — which is what we do. + pid, port, startErr := as.startWithPortRetry(port, func(p int) (int, error, bool) { + return as.attemptStart(agentNode, name, p) + }) + if startErr != nil { + // Surface the node's own log inline so the real traceback / exit reason + // is visible without a separate `af logs` round-trip. + as.printStartupFailureDiagnostics(agentNode, name) + return nil, startErr + } + + fmt.Printf("🧠 Agent node registered with AgentField Server\n") + + // 6. Update registry with runtime info + if err := as.updateRuntimeInfo(name, port, pid); err != nil { + return nil, fmt.Errorf("failed to update runtime info: %w", err) + } + + // 7. Display agent node capabilities + if err := as.displayCapabilities(agentNode, port); err != nil { + fmt.Printf("⚠️ Could not fetch capabilities: %v\n", err) + } + + fmt.Printf("\n💡 Agent node running in background (PID: %d)\n", pid) + fmt.Printf("💡 View logs: af logs %s\n", name) + fmt.Printf("💡 Stop agent node: af stop %s\n", name) + + // Convert to domain model and return + runningAgent := as.convertToRunningAgent(agentNode) + runningAgent.PID = pid + runningAgent.Port = port + runningAgent.StartedAt = time.Now() + + return &runningAgent, nil +} + +// attemptStart builds the process config, starts the node on the given port, +// and waits for it to answer its health check. On failure it stops the process +// and reports whether the failure was a strict-port bind conflict (detected +// from the node's own log), which the caller uses to decide on a fresh-port +// retry. +func (as *DefaultAgentService) attemptStart(agentNode packages.InstalledPackage, name string, port int) (pid int, err error, portConflict bool) { fmt.Printf("✅ Assigned port: %d\n", port) - // 4. Start agent node process fmt.Printf("📡 Starting agent node process...\n") processConfig, err := as.buildProcessConfig(agentNode, port) if err != nil { - return nil, err + return 0, err, false } - pid, err := as.processManager.Start(processConfig) + pid, err = as.processManager.Start(processConfig) if err != nil { - return nil, fmt.Errorf("failed to start agent node: %w", err) + return 0, fmt.Errorf("failed to start agent node: %w", err), false } - // 5. Wait for agent node to be ready healthPath := "/health" expectedNodeID := name if metadata, err := packages.ParsePackageMetadata(agentNode.Path); err == nil { @@ -126,37 +170,95 @@ func (as *DefaultAgentService) runAgentGuarded(name string, options domain.RunOp expectedNodeID = metadata.AgentNode.NodeID } } - if err := as.waitForAgentNode(port, healthPath, expectedNodeID, nodeReadyTimeout()); err != nil { - // Kill the process if it failed to start properly + + if waitErr := as.waitForAgentNode(port, healthPath, expectedNodeID, nodeReadyTimeout()); waitErr != nil { + // Read the log before killing so a strict-port exit is still visible. + conflict := logIndicatesPortConflict(readLogTailLines(agentNode.Runtime.LogFile, 40)) if stopErr := as.processManager.Stop(pid); stopErr != nil { - return nil, fmt.Errorf("agent node failed to start: %w (additionally failed to stop process: %v)", err, stopErr) + return 0, fmt.Errorf("agent node failed to start: %w (additionally failed to stop process: %v)", waitErr, stopErr), conflict } - return nil, fmt.Errorf("agent node failed to start: %w", err) + return 0, fmt.Errorf("agent node failed to start: %w", waitErr), conflict } + return pid, nil, false +} - fmt.Printf("🧠 Agent node registered with AgentField Server\n") - - // 6. Update registry with runtime info - if err := as.updateRuntimeInfo(name, port, pid); err != nil { - return nil, fmt.Errorf("failed to update runtime info: %w", err) +// startWithPortRetry runs attemptFn on the initial port and, if it fails with a +// strict-port conflict, retries exactly once on a fresh port. It returns the +// final pid, the port actually used, and any error. +func (as *DefaultAgentService) startWithPortRetry(initialPort int, attemptFn func(port int) (pid int, err error, portConflict bool)) (int, int, error) { + port := initialPort + pid, err, conflict := attemptFn(port) + if err == nil || !conflict { + return pid, port, err } - // 7. Display agent node capabilities - if err := as.displayCapabilities(agentNode, port); err != nil { - fmt.Printf("⚠️ Could not fetch capabilities: %v\n", err) + retryPort, rerr := as.freshRetryPort(port) + if rerr != nil || retryPort == port { + // No distinct fresh port to try — keep the original failure. + return pid, port, err } - fmt.Printf("\n💡 Agent node running in background (PID: %d)\n", pid) - fmt.Printf("💡 View logs: af logs %s\n", name) - fmt.Printf("💡 Stop agent node: af stop %s\n", name) + fmt.Printf("⚠️ Port %d unavailable, retrying on a fresh port\n", port) + pid, err, _ = attemptFn(retryPort) + return pid, retryPort, err +} - // Convert to domain model and return - runningAgent := as.convertToRunningAgent(agentNode) - runningAgent.PID = pid - runningAgent.Port = port - runningAgent.StartedAt = time.Now() +// freshRetryPort excludes the port that just failed to bind, then asks the port +// manager for a new one, so the retry never reuses the conflicting port. +func (as *DefaultAgentService) freshRetryPort(failedPort int) (int, error) { + _ = as.portManager.ReservePort(failedPort) + return as.portManager.FindFreePort(8001) +} - return &runningAgent, nil +// printStartupFailureDiagnostics prints the tail of the node's log so the real +// exit reason (traceback, bind error, missing dependency, …) is visible inline, +// plus a pointer to the full logs. +func (as *DefaultAgentService) printStartupFailureDiagnostics(agentNode packages.InstalledPackage, name string) { + lines := readLogTailLines(agentNode.Runtime.LogFile, 15) + if len(lines) > 0 { + fmt.Printf("\n📄 Last %d line(s) of %s log:\n", len(lines), name) + for _, line := range lines { + fmt.Printf(" %s\n", line) + } + } + fmt.Printf("💡 Full logs: af logs %s\n", name) +} + +// readLogTailLines returns the last n lines of the file at path. A missing or +// unreadable file yields nil so callers treat "no log" and "no diagnostic info" +// the same way. +func readLogTailLines(path string, n int) []string { + if path == "" { + return nil + } + data, err := os.ReadFile(path) + if err != nil { + return nil + } + text := strings.TrimRight(string(data), "\n") + if text == "" { + return nil + } + lines := strings.Split(text, "\n") + if n > 0 && len(lines) > n { + lines = lines[len(lines)-n:] + } + return lines +} + +// logIndicatesPortConflict reports whether the node's log tail shows the SDK's +// strict-port bind failure — the node was assigned a port that turned out to be +// unavailable at bind time and exited so the control plane can reallocate. The +// SDK logs "AGENTFIELD_STRICT_PORT set but the assigned port N is unavailable" +// and raises "assigned port N is unavailable"; both contain "assigned port" and +// "unavailable". +func logIndicatesPortConflict(lines []string) bool { + for _, line := range lines { + if strings.Contains(line, "assigned port") && strings.Contains(line, "unavailable") { + return true + } + } + return false } // StopAgent stops a running agent with robust error handling diff --git a/control-plane/internal/core/services/agent_service_startup_test.go b/control-plane/internal/core/services/agent_service_startup_test.go new file mode 100644 index 000000000..47f82f628 --- /dev/null +++ b/control-plane/internal/core/services/agent_service_startup_test.go @@ -0,0 +1,271 @@ +package services + +import ( + "errors" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/Agent-Field/agentfield/control-plane/internal/packages" +) + +// captureStdout runs fn while capturing everything written to os.Stdout. +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + orig := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + os.Stdout = w + done := make(chan string, 1) + go func() { + b, _ := io.ReadAll(r) + done <- string(b) + }() + fn() + _ = w.Close() + os.Stdout = orig + return <-done +} + +func newStartupTestService(t *testing.T, pm *mockPortManager) *DefaultAgentService { + t.Helper() + return NewAgentService( + newMockProcessManager(), + pm, + newMockRegistryStorage(), + newMockAgentClient(), + t.TempDir(), + ).(*DefaultAgentService) +} + +// Contract: a strict-port failure triggers exactly one retry, on a different +// port than the one that failed. +func TestStartWithPortRetry_RetriesOnceOnPortConflict(t *testing.T) { + pm := newMockPortManager() + reserved := -1 + pm.reserveFunc = func(p int) error { reserved = p; return nil } + pm.findFreePortFunc = func(int) (int, error) { return 8002, nil } + service := newStartupTestService(t, pm) + + var attemptPorts []int + attempt := func(p int) (int, error, bool) { + attemptPorts = append(attemptPorts, p) + if len(attemptPorts) == 1 { + // First attempt: strict-port conflict. + return 0, errors.New("agent node failed to start: assigned port unavailable"), true + } + // Retry: success. + return 777, nil, false + } + + pid, port, err := captureRetry(t, service, 8001, attempt) + if err != nil { + t.Fatalf("expected retry to succeed, got %v", err) + } + if len(attemptPorts) != 2 { + t.Fatalf("expected exactly 2 attempts, got %d: %v", len(attemptPorts), attemptPorts) + } + if attemptPorts[0] == attemptPorts[1] { + t.Errorf("retry must use a different port, both were %d", attemptPorts[0]) + } + if attemptPorts[0] != 8001 || attemptPorts[1] != 8002 { + t.Errorf("expected attempts on 8001 then 8002, got %v", attemptPorts) + } + if reserved != 8001 { + t.Errorf("failed port 8001 must be reserved before retry, reserved=%d", reserved) + } + if pid != 777 || port != 8002 { + t.Errorf("expected pid=777 port=8002, got pid=%d port=%d", pid, port) + } +} + +// Contract: a non-conflict startup failure is NOT retried. +func TestStartWithPortRetry_NoRetryOnNonConflictFailure(t *testing.T) { + pm := newMockPortManager() + pm.findFreePortFunc = func(int) (int, error) { return 8002, nil } + service := newStartupTestService(t, pm) + + var attempts int + attempt := func(p int) (int, error, bool) { + attempts++ + return 0, errors.New("boom: import error"), false + } + + _, _, err := captureRetry(t, service, 8001, attempt) + if err == nil { + t.Fatalf("expected failure to propagate") + } + if attempts != 1 { + t.Errorf("non-conflict failure must not retry, got %d attempts", attempts) + } +} + +// Contract: a first-attempt success runs exactly once. +func TestStartWithPortRetry_SuccessRunsOnce(t *testing.T) { + pm := newMockPortManager() + service := newStartupTestService(t, pm) + + var attempts int + attempt := func(p int) (int, error, bool) { + attempts++ + return 55, nil, false + } + + pid, port, err := captureRetry(t, service, 8001, attempt) + if err != nil || attempts != 1 { + t.Fatalf("expected one successful attempt, got attempts=%d err=%v", attempts, err) + } + if pid != 55 || port != 8001 { + t.Errorf("expected pid=55 port=8001, got pid=%d port=%d", pid, port) + } +} + +// Contract: when no distinct fresh port is available, the conflict failure is +// returned without a (pointless) retry on the same port. +func TestStartWithPortRetry_NoDistinctPortDoesNotRetry(t *testing.T) { + pm := newMockPortManager() + pm.reserveFunc = func(int) error { return nil } + pm.findFreePortFunc = func(int) (int, error) { return 8001, nil } // same port back + service := newStartupTestService(t, pm) + + var attempts int + attempt := func(p int) (int, error, bool) { + attempts++ + return 0, errors.New("assigned port unavailable"), true + } + + _, _, err := captureRetry(t, service, 8001, attempt) + if err == nil { + t.Fatalf("expected failure to propagate") + } + if attempts != 1 { + t.Errorf("must not retry when the fresh port equals the failed port, got %d attempts", attempts) + } +} + +// captureRetry runs startWithPortRetry while swallowing its progress output. +func captureRetry(t *testing.T, s *DefaultAgentService, initial int, fn func(int) (int, error, bool)) (int, int, error) { + t.Helper() + var pid, port int + var err error + _ = captureStdout(t, func() { + pid, port, err = s.startWithPortRetry(initial, fn) + }) + return pid, port, err +} + +func TestFreshRetryPort_ExcludesFailedPort(t *testing.T) { + pm := newMockPortManager() + reserved := -1 + pm.reserveFunc = func(p int) error { reserved = p; return nil } + pm.findFreePortFunc = func(start int) (int, error) { + if start != 8001 { + t.Errorf("expected FindFreePort(8001), got %d", start) + } + return 8003, nil + } + service := newStartupTestService(t, pm) + + got, err := service.freshRetryPort(8001) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if reserved != 8001 { + t.Errorf("failed port must be reserved, reserved=%d", reserved) + } + if got != 8003 { + t.Errorf("expected fresh port 8003, got %d", got) + } +} + +// Contract: logIndicatesPortConflict classifies the SDK's strict-port exit as a +// conflict and other failures as not. +func TestLogIndicatesPortConflict(t *testing.T) { + cases := []struct { + name string + lines []string + want bool + }{ + {"sdk log_error line", []string{"INFO boot", "AGENTFIELD_STRICT_PORT set but the assigned port 8001 is unavailable; exiting so the control plane can reallocate and retry"}, true}, + {"sdk runtime error", []string{"RuntimeError: assigned port 8005 is unavailable"}, true}, + {"unrelated traceback", []string{"Traceback (most recent call last):", "ModuleNotFoundError: No module named 'foo'"}, false}, + {"empty", nil, false}, + {"port mentioned but not unavailable", []string{"listening on assigned port 8001"}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := logIndicatesPortConflict(tc.lines); got != tc.want { + t.Errorf("logIndicatesPortConflict(%v) = %v, want %v", tc.lines, got, tc.want) + } + }) + } +} + +func TestReadLogTailLines(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "node.log") + var b strings.Builder + for i := 1; i <= 30; i++ { + fmt.Fprintf(&b, "line-%d\n", i) + } + if err := os.WriteFile(path, []byte(b.String()), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + + lines := readLogTailLines(path, 15) + if len(lines) != 15 { + t.Fatalf("expected 15 lines, got %d", len(lines)) + } + if lines[0] != "line-16" || lines[14] != "line-30" { + t.Errorf("expected tail line-16..line-30, got %s..%s", lines[0], lines[14]) + } + + // Missing file yields nil, not an error. + if got := readLogTailLines(filepath.Join(dir, "missing.log"), 10); got != nil { + t.Errorf("missing file should yield nil, got %v", got) + } + if got := readLogTailLines("", 10); got != nil { + t.Errorf("empty path should yield nil, got %v", got) + } +} + +// Contract: the startup-failure path prints the tail of the node's log file and +// the "af logs" pointer. +func TestPrintStartupFailureDiagnostics(t *testing.T) { + dir := t.TempDir() + logPath := filepath.Join(dir, "swe.log") + var b strings.Builder + for i := 1; i <= 20; i++ { + fmt.Fprintf(&b, "boot-line-%d\n", i) + } + b.WriteString("RuntimeError: assigned port 8001 is unavailable\n") + if err := os.WriteFile(logPath, []byte(b.String()), 0o644); err != nil { + t.Fatalf("write: %v", err) + } + + service := newStartupTestService(t, newMockPortManager()) + node := packages.InstalledPackage{ + Name: "swe-af", + Runtime: packages.RuntimeInfo{LogFile: logPath}, + } + + out := captureStdout(t, func() { + service.printStartupFailureDiagnostics(node, "swe-af") + }) + + if !strings.Contains(out, "RuntimeError: assigned port 8001 is unavailable") { + t.Errorf("diagnostics should include the failing log tail, got:\n%s", out) + } + if !strings.Contains(out, "Full logs: af logs swe-af") { + t.Errorf("diagnostics should point at `af logs`, got:\n%s", out) + } + // Only the last ~15 lines — the earliest boot line must be trimmed. + if strings.Contains(out, "boot-line-1\n") { + t.Errorf("diagnostics should show only the tail, but included boot-line-1:\n%s", out) + } +} From 4ad6ba9680285ed3fd0c998b8fb639e9a0881fed Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Wed, 22 Jul 2026 15:09:23 -0400 Subject: [PATCH 3/6] feat(cli): reconcile af list registry status with control-plane health MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `af list` showed only the local registry's view, which is a claim, not a fact: a node the registry calls "running" can be dead on the control plane (or a registry "stopped" node can still be live). Add a HEALTH column that fetches the control plane's node view (GET /api/v1/nodes?show_all=true, so inactive nodes are included) and reconciles it with each node's registry status: - statuses agree → plain health (e.g. "active") - statuses disagree → health + "(mismatch)" and a footer explaining it - node absent from CP → "not on control plane (mismatch)" when registry running - control plane unreachable → "unknown (control plane unreachable)", never an error The same health/health_discrepancy fields are added to `af list --json` for the agent-driven flow. A missing control plane never fails the command. Co-Authored-By: Claude Fable 5 --- control-plane/internal/cli/list.go | 164 +++++++++++++++++- .../internal/cli/list_health_test.go | 147 ++++++++++++++++ 2 files changed, 306 insertions(+), 5 deletions(-) create mode 100644 control-plane/internal/cli/list_health_test.go diff --git a/control-plane/internal/cli/list.go b/control-plane/internal/cli/list.go index 6b42da686..61cf36c47 100644 --- a/control-plane/internal/cli/list.go +++ b/control-plane/internal/cli/list.go @@ -1,14 +1,20 @@ package cli import ( + "encoding/json" "errors" "fmt" + "io" + "net/http" "os" "path/filepath" "sort" + "strings" + "time" "github.com/Agent-Field/agentfield/control-plane/internal/packages" "github.com/Agent-Field/agentfield/control-plane/internal/ui" + "github.com/Agent-Field/agentfield/control-plane/pkg/types" "github.com/spf13/cobra" "gopkg.in/yaml.v3" ) @@ -63,14 +69,19 @@ func runListCommandJSON() error { } sort.Strings(names) + health := resolveNodeHealth(GetServerURL(), registry) + nodes := make([]map[string]interface{}, 0, len(names)) for _, name := range names { pkg := registry.Installed[name] + h := health[name] node := map[string]interface{}{ - "name": name, - "version": pkg.Version, - "status": pkg.Status, - "description": pkg.Description, + "name": name, + "version": pkg.Version, + "status": pkg.Status, + "description": pkg.Description, + "health": h.Display, + "health_discrepancy": h.Discrepancy, } if pkg.Status == "running" && pkg.Runtime.Port != nil { node["port"] = *pkg.Runtime.Port @@ -84,6 +95,135 @@ func runListCommandJSON() error { }) } +// controlPlaneNode is the subset of the /api/v1/nodes payload the health +// reconciliation needs. +type controlPlaneNode struct { + ID string `json:"id"` + HealthStatus string `json:"health_status"` + LastHeartbeat time.Time `json:"last_heartbeat"` +} + +// nodeHealth is the reconciled health of one installed node for display. +type nodeHealth struct { + Display string + Discrepancy bool +} + +// resolveNodeHealth fetches the control plane's node view and reconciles it with +// each installed node's local registry status, keyed by registry name. When the +// control plane is unreachable every node reports "unknown (control plane +// unreachable)" without erroring — `af list` must never fail just because the +// control plane is down. +func resolveNodeHealth(serverURL string, registry *packages.InstallationRegistry) map[string]nodeHealth { + cpNodes, reachable := fetchControlPlaneNodes(serverURL) + out := make(map[string]nodeHealth, len(registry.Installed)) + for name, pkg := range registry.Installed { + // The control plane keys nodes by manifest node_id, which may differ + // from the local install name — prefer it, fall back to the name. + candidates := []string{name} + if md, err := packages.ParsePackageMetadata(pkg.Path); err == nil && md.AgentNode.NodeID != "" { + candidates = append([]string{md.AgentNode.NodeID}, candidates...) + } + cpNode, found := lookupControlPlaneNode(cpNodes, candidates...) + display, discrepancy := reconcileHealth(pkg.Status, cpNode.HealthStatus, found, reachable) + out[name] = nodeHealth{Display: display, Discrepancy: discrepancy} + } + return out +} + +// reconcileHealth compares a node's local registry status with the control +// plane's reported health, returning the display string and whether they +// disagree. A node the registry calls "running" but the control plane reports +// as inactive/absent (or vice versa) is a discrepancy. +func reconcileHealth(registryStatus, cpHealth string, foundInCP, cpReachable bool) (string, bool) { + if !cpReachable { + return "unknown (control plane unreachable)", false + } + registryRunning := registryStatus == "running" + if !foundInCP { + if registryRunning { + return "not on control plane (mismatch)", true + } + return "—", false + } + if cpHealth == "" { + cpHealth = string(types.HealthStatusUnknown) + } + // Active and degraded both mean the node is registered and heartbeating. + cpAlive := cpHealth == string(types.HealthStatusActive) || cpHealth == string(types.HealthStatusDegraded) + if registryRunning != cpAlive { + return cpHealth + " (mismatch)", true + } + return cpHealth, false +} + +// lookupControlPlaneNode finds a control-plane node matching any of the given id +// candidates, first by exact id then by node-id equivalence (hyphen/underscore +// normalization). +func lookupControlPlaneNode(cpNodes map[string]controlPlaneNode, candidates ...string) (controlPlaneNode, bool) { + for _, cand := range candidates { + if cand == "" { + continue + } + if n, ok := cpNodes[cand]; ok { + return n, true + } + } + for _, cand := range candidates { + if cand == "" { + continue + } + for id, n := range cpNodes { + if packages.NodeIDsEquivalent(id, cand) { + return n, true + } + } + } + return controlPlaneNode{}, false +} + +// fetchControlPlaneNodes returns the control plane's view of every agent node +// (show_all=true includes inactive nodes so a dead-but-registered node is +// visible), keyed by node id. reachable is false when the control plane cannot +// be reached or answers unusably. +func fetchControlPlaneNodes(serverURL string) (map[string]controlPlaneNode, bool) { + serverURL = strings.TrimRight(strings.TrimSpace(serverURL), "/") + if serverURL == "" { + return nil, false + } + + client := &http.Client{Timeout: 3 * time.Second} + req, err := http.NewRequest(http.MethodGet, serverURL+"/api/v1/nodes?show_all=true", nil) + if err != nil { + return nil, false + } + if key := GetAPIKey(); key != "" { + req.Header.Set("Authorization", "Bearer "+key) + } + + resp, err := client.Do(req) + if err != nil { + return nil, false + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return nil, false + } + + var parsed struct { + Nodes []controlPlaneNode `json:"nodes"` + } + if err := json.NewDecoder(io.LimitReader(resp.Body, 4<<20)).Decode(&parsed); err != nil { + return nil, false + } + + nodes := make(map[string]controlPlaneNode, len(parsed.Nodes)) + for _, n := range parsed.Nodes { + nodes[n.ID] = n + } + return nodes, true +} + func runListCommand(cmd *cobra.Command, args []string) { agentfieldHome := getAgentFieldHomeDir() registryPath := filepath.Join(agentfieldHome, "installed.yaml") @@ -115,6 +255,12 @@ func runListCommand(cmd *cobra.Command, args []string) { } sort.Strings(names) + // Reconcile the local registry status against the control plane's health so + // a "running" registry entry backed by a dead node (or vice versa) is + // visible rather than silently trusted. + health := resolveNodeHealth(GetServerURL(), registry) + anyDiscrepancy := false + rows := make([][]string, 0, len(names)) for _, name := range names { pkg := registry.Installed[name] @@ -122,20 +268,28 @@ func runListCommand(cmd *cobra.Command, args []string) { if pkg.Status == "running" && pkg.Runtime.Port != nil { port = fmt.Sprintf("%d", *pkg.Runtime.Port) } + h := health[name] + if h.Discrepancy { + anyDiscrepancy = true + } rows = append(rows, []string{ name, "v" + pkg.Version, ui.StatusBadge(pkg.Status), port, + h.Display, pkg.Description, }) } fmt.Println(ui.Table( fmt.Sprintf("Installed agent nodes (%d)", len(rows)), - []string{"NODE", "VERSION", "STATUS", "PORT", "DESCRIPTION"}, + []string{"NODE", "VERSION", "STATUS", "PORT", "HEALTH", "DESCRIPTION"}, rows, )) fmt.Println() + if anyDiscrepancy { + fmt.Println(ui.Muted("STATUS is the local registry; HEALTH is the control plane. (mismatch) marks a disagreement — reconcile with `af stop`/`af run`.")) + } fmt.Println(ui.Muted("af run · af stop · af logs ")) } diff --git a/control-plane/internal/cli/list_health_test.go b/control-plane/internal/cli/list_health_test.go new file mode 100644 index 000000000..dba891166 --- /dev/null +++ b/control-plane/internal/cli/list_health_test.go @@ -0,0 +1,147 @@ +package cli + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/Agent-Field/agentfield/control-plane/internal/packages" +) + +// Contract: reconcileHealth renders agreement plainly, flags every disagreement, +// and never treats an unreachable control plane as an error. +func TestReconcileHealth(t *testing.T) { + cases := []struct { + name string + registryStatus string + cpHealth string + found bool + reachable bool + wantDisplay string + wantDiscrep bool + }{ + {"unreachable", "running", "", false, false, "unknown (control plane unreachable)", false}, + {"agree-active", "running", "active", true, true, "active", false}, + {"agree-degraded", "running", "degraded", true, true, "degraded", false}, + {"running-but-inactive", "running", "inactive", true, true, "inactive (mismatch)", true}, + {"running-but-absent", "running", "", false, true, "not on control plane (mismatch)", true}, + {"stopped-but-active", "stopped", "active", true, true, "active (mismatch)", true}, + {"agree-stopped-absent", "stopped", "", false, true, "—", false}, + {"stopped-and-inactive", "stopped", "inactive", true, true, "inactive", false}, + {"found-empty-health-running", "running", "", true, true, "unknown (mismatch)", true}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + display, discrep := reconcileHealth(tc.registryStatus, tc.cpHealth, tc.found, tc.reachable) + if display != tc.wantDisplay { + t.Errorf("display = %q, want %q", display, tc.wantDisplay) + } + if discrep != tc.wantDiscrep { + t.Errorf("discrepancy = %v, want %v", discrep, tc.wantDiscrep) + } + }) + } +} + +func nodesServer(t *testing.T, nodes []controlPlaneNode) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/nodes" { + w.WriteHeader(http.StatusNotFound) + return + } + if r.URL.Query().Get("show_all") != "true" { + t.Errorf("expected show_all=true so inactive nodes are visible, got %q", r.URL.RawQuery) + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]interface{}{"nodes": nodes, "count": len(nodes)}) + })) +} + +func fixtureRegistry() *packages.InstallationRegistry { + // Paths are intentionally nonexistent so metadata resolution falls back to + // the registry name (which equals the control-plane id in these fixtures). + mk := func(status string) packages.InstalledPackage { + return packages.InstalledPackage{Status: status, Path: "/nonexistent/path"} + } + return &packages.InstallationRegistry{ + Installed: map[string]packages.InstalledPackage{ + "agree-running": mk("running"), + "dead-node": mk("running"), // CP says inactive + "orphan-running": mk("running"), // absent from CP + "revived": mk("stopped"), // CP says active + "agree-stopped": mk("stopped"), // absent from CP + }, + } +} + +// Contract: af list merges the local registry with /api/v1/nodes — agreeing +// statuses render plainly, disagreements are flagged. +func TestResolveNodeHealth_Merge(t *testing.T) { + srv := nodesServer(t, []controlPlaneNode{ + {ID: "agree-running", HealthStatus: "active"}, + {ID: "dead-node", HealthStatus: "inactive"}, + {ID: "revived", HealthStatus: "active"}, + }) + defer srv.Close() + + got := resolveNodeHealth(srv.URL, fixtureRegistry()) + + checks := map[string]struct { + display string + discrep bool + }{ + "agree-running": {"active", false}, + "dead-node": {"inactive (mismatch)", true}, + "orphan-running": {"not on control plane (mismatch)", true}, + "revived": {"active (mismatch)", true}, + "agree-stopped": {"—", false}, + } + for name, want := range checks { + g := got[name] + if g.Display != want.display || g.Discrepancy != want.discrep { + t.Errorf("%s: got (%q, %v), want (%q, %v)", name, g.Display, g.Discrepancy, want.display, want.discrep) + } + } +} + +// Contract: an unreachable control plane yields "unknown" health for every node +// without erroring. +func TestResolveNodeHealth_Unreachable(t *testing.T) { + got := resolveNodeHealth("http://127.0.0.1:1", fixtureRegistry()) + if len(got) != 5 { + t.Fatalf("expected 5 nodes, got %d", len(got)) + } + for name, h := range got { + if !strings.Contains(h.Display, "unknown") { + t.Errorf("%s: expected unknown health, got %q", name, h.Display) + } + if h.Discrepancy { + t.Errorf("%s: unreachable control plane must not flag a discrepancy", name) + } + } +} + +func TestFetchControlPlaneNodes_Reachable(t *testing.T) { + srv := nodesServer(t, []controlPlaneNode{{ID: "n1", HealthStatus: "active"}}) + defer srv.Close() + + nodes, reachable := fetchControlPlaneNodes(srv.URL) + if !reachable { + t.Fatalf("expected reachable=true") + } + if n, ok := nodes["n1"]; !ok || n.HealthStatus != "active" { + t.Errorf("expected n1 active, got %+v (ok=%v)", n, ok) + } +} + +func TestFetchControlPlaneNodes_Unreachable(t *testing.T) { + if _, reachable := fetchControlPlaneNodes("http://127.0.0.1:1"); reachable { + t.Errorf("expected reachable=false for a dead control plane") + } + if _, reachable := fetchControlPlaneNodes(""); reachable { + t.Errorf("expected reachable=false for an empty URL") + } +} From 279582da652714e859d9e784eee755b82e0443ac Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Wed, 22 Jul 2026 15:13:30 -0400 Subject: [PATCH 4/6] feat(cli): add af doctor --probe to smoke-test detected provider CLIs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `af doctor` reported a provider as available whenever its binary was on PATH, but a present binary can still return instant empty completions (broken auth, model outage) — the doctor called it healthy while every real call failed. Add an opt-in `--probe` flag that runs a minimal one-shot prompt against each DETECTED provider CLI (claude -p, codex exec, gemini -p, opencode run) with a per-provider 60s timeout and classifies the result: - ok → non-empty completion - empty → exit 0 but no output (the silently-broken case a PATH check misses) - error → non-zero exit (stderr head captured) - timeout → no response within the timeout Probes run only for providers doctor already detects; without --probe the command is unchanged. Output and help note that a probe consumes a trivial amount of provider quota. The classifier is a pure function so ok/empty/error/ timeout are table-tested from (exit code, stdout, timed-out) tuples. Co-Authored-By: Claude Fable 5 --- control-plane/internal/cli/doctor.go | 169 +++++++++++++++++- .../internal/cli/doctor_probe_test.go | 135 ++++++++++++++ 2 files changed, 298 insertions(+), 6 deletions(-) create mode 100644 control-plane/internal/cli/doctor_probe_test.go diff --git a/control-plane/internal/cli/doctor.go b/control-plane/internal/cli/doctor.go index 3e6038368..c7effca68 100644 --- a/control-plane/internal/cli/doctor.go +++ b/control-plane/internal/cli/doctor.go @@ -28,6 +28,9 @@ type DoctorReport struct { ProviderKeys map[string]ProviderKey `json:"provider_keys"` ControlPlane ControlPlaneStatus `json:"control_plane"` Recommendation Recommendation `json:"recommendation"` + // HarnessProbes is populated only when `--probe` is passed: a live smoke + // test of each detected provider CLI, keyed by provider name. + HarnessProbes map[string]HarnessProbeResult `json:"harness_probes,omitempty"` } // ToolStatus describes whether a CLI is available and, if so, where. @@ -78,19 +81,37 @@ var providerEnvVars = []struct { // harnessProviders is the canonical list of CLIs `app.harness()` knows how to drive. var harnessProviders = []struct { - Name string // value passed to provider= in app.harness() - Binary string // executable name to look up on PATH + Name string // value passed to provider= in app.harness() + Binary string // executable name to look up on PATH + ProbeArgs []string // minimal one-shot invocation used by `--probe` }{ - {Name: "claude-code", Binary: "claude"}, - {Name: "codex", Binary: "codex"}, - {Name: "gemini", Binary: "gemini"}, - {Name: "opencode", Binary: "opencode"}, + {Name: "claude-code", Binary: "claude", ProbeArgs: []string{"-p", "Say OK"}}, + {Name: "codex", Binary: "codex", ProbeArgs: []string{"exec", "Say OK"}}, + {Name: "gemini", Binary: "gemini", ProbeArgs: []string{"-p", "Say OK"}}, + {Name: "opencode", Binary: "opencode", ProbeArgs: []string{"run", "Say OK"}}, +} + +// harnessProbeTimeout bounds a single provider smoke test. Coding-agent CLIs +// cold-start slowly (model download, auth handshake), so this is generous. +const harnessProbeTimeout = 60 * time.Second + +// HarnessProbeResult is the outcome of smoke-testing one provider CLI with a +// minimal one-shot prompt. +type HarnessProbeResult struct { + Provider string `json:"provider"` + Binary string `json:"binary,omitempty"` + // Status is one of: ok, empty, error, timeout. + Status string `json:"status"` + DurationMS int64 `json:"duration_ms"` + // Detail carries a short explanation on failure (stderr head / note). + Detail string `json:"detail,omitempty"` } // NewDoctorCommand builds the `af doctor` command. func NewDoctorCommand() *cobra.Command { var jsonOut bool var controlPlaneURL string + var probe bool cmd := &cobra.Command{ Use: "doctor", @@ -108,13 +129,25 @@ Coding agents and skills (e.g. agentfield) should call this once at the start of a build to learn ground truth instead of probing each tool by hand. +With --probe, each DETECTED provider CLI is additionally smoke-tested with a +minimal one-shot prompt (e.g. codex exec "Say OK") and classified as ok, empty +(exit 0 but no completion — a silently broken provider), error, or timeout. +Detecting a binary on PATH does not prove it can actually complete; --probe +catches providers that look installed but return nothing. Each probe runs a real +one-shot request and consumes a trivial amount of provider quota. + Examples: af doctor # Pretty human-readable output af doctor --json # Machine-readable JSON for tooling/skills + af doctor --probe # Also smoke-test each detected provider CLI af doctor --json | jq # Pipe to jq for filtering`, RunE: func(cmd *cobra.Command, args []string) error { report := buildDoctorReport(controlPlaneURL) + if probe { + report.HarnessProbes = runHarnessProbes(report) + } + if jsonOut { enc := json.NewEncoder(os.Stdout) enc.SetIndent("", " ") @@ -128,10 +161,104 @@ Examples: cmd.Flags().BoolVar(&jsonOut, "json", false, "Output the report as JSON (recommended for tools and skills)") cmd.Flags().StringVar(&controlPlaneURL, "server", "http://localhost:8080", "Control plane URL to probe for /api/v1/health") + cmd.Flags().BoolVar(&probe, "probe", false, "Smoke-test each detected provider CLI with a minimal one-shot prompt (consumes trivial provider quota)") return cmd } +// runHarnessProbes smoke-tests every provider CLI that doctor detected on PATH. +// Providers that were not detected are skipped entirely — probing runs only for +// what doctor already reports as available. +func runHarnessProbes(report DoctorReport) map[string]HarnessProbeResult { + results := map[string]HarnessProbeResult{} + for _, h := range harnessProviders { + if !report.HarnessProviders[h.Name].Available { + continue + } + results[h.Name] = probeHarnessProvider(h.Name, h.Binary, h.ProbeArgs, harnessProbeTimeout) + } + return results +} + +// probeHarnessProvider runs one provider CLI's minimal one-shot invocation and +// classifies the outcome. +func probeHarnessProvider(name, binary string, args []string, timeout time.Duration) HarnessProbeResult { + start := time.Now() + stdout, stderr, exitCode, timedOut := runProbeCommand(binary, args, timeout) + status := classifyProbe(exitCode, stdout, timedOut) + + result := HarnessProbeResult{ + Provider: name, + Binary: binary, + Status: status, + DurationMS: time.Since(start).Milliseconds(), + } + switch status { + case "error": + result.Detail = firstLine(stderr) + case "timeout": + result.Detail = fmt.Sprintf("no response within %s", timeout) + case "empty": + result.Detail = "exited 0 with no completion output" + } + return result +} + +// runProbeCommand executes bin with args under a timeout, returning stdout, +// stderr, the process exit code, and whether the timeout fired. A timeout is +// reported distinctly so it is never misclassified as a plain error. +func runProbeCommand(bin string, args []string, timeout time.Duration) (stdout, stderr string, exitCode int, timedOut bool) { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + + cmd := exec.CommandContext(ctx, bin, args...) + var outBuf, errBuf strings.Builder + cmd.Stdout = &outBuf + cmd.Stderr = &errBuf + err := cmd.Run() + + if ctx.Err() == context.DeadlineExceeded { + return outBuf.String(), errBuf.String(), -1, true + } + if err != nil { + if exitErr, ok := err.(*exec.ExitError); ok { + return outBuf.String(), errBuf.String(), exitErr.ExitCode(), false + } + // Could not start (binary vanished between detection and probe) — treat + // as a non-zero exit so it classifies as error. + return outBuf.String(), err.Error(), 1, false + } + return outBuf.String(), errBuf.String(), 0, false +} + +// classifyProbe maps a probe outcome to a status. Order matters: a timeout is +// checked before the exit code (a killed process also exits non-zero), and an +// empty completion on a clean exit is the real-world "silently broken provider" +// case that a mere PATH check misses. +func classifyProbe(exitCode int, stdout string, timedOut bool) string { + switch { + case timedOut: + return "timeout" + case exitCode != 0: + return "error" + case strings.TrimSpace(stdout) == "": + return "empty" + default: + return "ok" + } +} + +// firstLine returns the first non-empty line of s, trimmed, for compact error +// detail. +func firstLine(s string) string { + for _, line := range strings.Split(s, "\n") { + if trimmed := strings.TrimSpace(line); trimmed != "" { + return trimmed + } + } + return "" +} + // buildDoctorReport collects the full environment snapshot. func buildDoctorReport(controlPlaneURL string) DoctorReport { report := DoctorReport{ @@ -284,6 +411,19 @@ func printDoctorReport(r DoctorReport) { } fmt.Println() + if len(r.HarnessProbes) > 0 { + bold.Println("Harness provider probes (--probe)") + for _, h := range harnessProviders { + probe, ok := r.HarnessProbes[h.Name] + if !ok { + continue + } + printProbeLine(probe, green, red, yellow) + } + fmt.Println(" note: probes ran a real one-shot request and consumed a trivial amount of provider quota") + fmt.Println() + } + bold.Println("Provider API keys") for _, p := range providerEnvVars { key := r.ProviderKeys[p.Name] @@ -329,6 +469,23 @@ func printDoctorReport(r DoctorReport) { fmt.Println("Tip: pipe to jq for tooling — `af doctor --json | jq`") } +// printProbeLine renders one provider probe result with a status-colored mark. +func printProbeLine(p HarnessProbeResult, ok, fail, warn *color.Color) { + mark, c := "✓", ok + switch p.Status { + case "empty", "timeout": + mark, c = "⚠", warn + case "error": + mark, c = "✗", fail + } + c.Printf(" %s %-12s %s", mark, p.Provider, p.Status) + fmt.Printf(" (%dms)", p.DurationMS) + if p.Detail != "" { + fmt.Printf(" — %s", p.Detail) + } + fmt.Println() +} + func printToolLine(name string, status ToolStatus, ok, fail *color.Color) { if status.Available { ok.Printf(" ✓ %-12s %s", name, status.Path) diff --git a/control-plane/internal/cli/doctor_probe_test.go b/control-plane/internal/cli/doctor_probe_test.go new file mode 100644 index 000000000..e4274b400 --- /dev/null +++ b/control-plane/internal/cli/doctor_probe_test.go @@ -0,0 +1,135 @@ +package cli + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "testing" + "time" +) + +// Contract: classifyProbe maps (exit code, stdout, timed-out) to the four probe +// statuses, with timeout taking precedence over a non-zero exit and an empty +// completion on a clean exit reported distinctly from an error. +func TestClassifyProbe(t *testing.T) { + cases := []struct { + name string + exitCode int + stdout string + timedOut bool + want string + }{ + {"ok", 0, "OK\n", false, "ok"}, + {"empty exit zero", 0, "", false, "empty"}, + {"empty whitespace only", 0, " \n\t", false, "empty"}, + {"error nonzero", 1, "partial", false, "error"}, + {"error nonzero no output", 127, "", false, "error"}, + {"timeout wins over exit code", -1, "", true, "timeout"}, + {"timeout wins even with output", 1, "some", true, "timeout"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := classifyProbe(tc.exitCode, tc.stdout, tc.timedOut); got != tc.want { + t.Errorf("classifyProbe(%d, %q, %v) = %q, want %q", tc.exitCode, tc.stdout, tc.timedOut, got, tc.want) + } + }) + } +} + +// End-to-end wiring of runProbeCommand -> classifyProbe over real processes, so +// each classification path is exercised through the actual command runner. +func TestProbeHarnessProvider_RealProcesses(t *testing.T) { + cases := []struct { + name string + bin string + args []string + timeout time.Duration + want string + }{ + {"ok", "echo", []string{"OK"}, 5 * time.Second, "ok"}, + {"empty", "true", nil, 5 * time.Second, "empty"}, + {"error", "false", nil, 5 * time.Second, "error"}, + {"timeout", "sleep", []string{"5"}, 200 * time.Millisecond, "timeout"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if _, err := exec.LookPath(tc.bin); err != nil { + t.Skipf("%s not available: %v", tc.bin, err) + } + res := probeHarnessProvider("prov-"+tc.name, tc.bin, tc.args, tc.timeout) + if res.Status != tc.want { + t.Errorf("status = %q, want %q (detail=%q)", res.Status, tc.want, res.Detail) + } + if res.Provider != "prov-"+tc.name { + t.Errorf("provider label lost: %q", res.Provider) + } + }) + } +} + +// Contract: probes run ONLY for providers doctor already detected — unavailable +// providers are never invoked. +func TestRunHarnessProbes_SkipsUndetected(t *testing.T) { + report := DoctorReport{ + HarnessProviders: map[string]ToolStatus{ + "claude-code": {Available: false}, + "codex": {Available: false}, + "gemini": {Available: false}, + "opencode": {Available: false}, + }, + } + got := runHarnessProbes(report) + if len(got) != 0 { + t.Errorf("no detected providers should mean no probes, got %v", got) + } +} + +// Contract: `af doctor` without --probe performs no probe (no harness_probes in +// the report). +func TestDoctorCommand_NoProbeByDefault(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + out := captureStdoutCLI(t, func() { + cmd := NewDoctorCommand() + cmd.SetArgs([]string{"--json", "--server", srv.URL}) + if err := cmd.Execute(); err != nil { + t.Fatalf("doctor failed: %v", err) + } + }) + + var report DoctorReport + if err := json.Unmarshal([]byte(out), &report); err != nil { + t.Fatalf("parse report: %v\noutput:\n%s", err, out) + } + if len(report.HarnessProbes) != 0 { + t.Errorf("without --probe there must be no probes, got %v", report.HarnessProbes) + } +} + +// captureStdoutCLI captures os.Stdout while fn runs. +func captureStdoutCLI(t *testing.T, fn func()) string { + t.Helper() + orig := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + os.Stdout = w + done := make(chan string, 1) + go func() { + var b bytes.Buffer + _, _ = io.Copy(&b, r) + done <- b.String() + }() + fn() + _ = w.Close() + os.Stdout = orig + return <-done +} From 2a8d7da05da4d8546e49c06d66e44bd31638ceca Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 24 Jul 2026 12:26:19 -0400 Subject: [PATCH 5/6] fix: preserve explicit agent ports on startup conflict --- .../internal/core/services/agent_service.go | 26 ++++++----- .../services/agent_service_startup_test.go | 44 +++++++++++++++---- 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/control-plane/internal/core/services/agent_service.go b/control-plane/internal/core/services/agent_service.go index cb23538d7..e34082434 100644 --- a/control-plane/internal/core/services/agent_service.go +++ b/control-plane/internal/core/services/agent_service.go @@ -97,19 +97,20 @@ func (as *DefaultAgentService) runAgentGuarded(name string, options domain.RunOp // 3. Allocate port fmt.Printf("🔍 Searching for available port...\n") port := options.Port - if port == 0 { + autoAssignedPort := port == 0 + if autoAssignedPort { port, err = as.portManager.FindFreePort(8001) if err != nil { return nil, fmt.Errorf("failed to allocate port: %w", err) } } - // 4-5. Start the process and wait for readiness. Retry exactly once on a - // fresh port if the node exits with a strict-port bind conflict: the - // assigned port looked free but the bind lost a race (commonly a - // just-stopped node's port lingering on mirrored-network setups), and the - // SDK exits fast so the control plane can reallocate — which is what we do. - pid, port, startErr := as.startWithPortRetry(port, func(p int) (int, error, bool) { + // 4-5. Start the process and wait for readiness. Automatically allocated + // ports retry exactly once on a fresh port if the node exits with a + // strict-port bind conflict. A caller-supplied port is never reassigned: + // callers may have firewall, proxy, or service-discovery configuration that + // targets that exact port. + pid, port, startErr := as.startWithPortRetry(port, autoAssignedPort, func(p int) (int, error, bool) { return as.attemptStart(agentNode, name, p) }) if startErr != nil { @@ -182,13 +183,14 @@ func (as *DefaultAgentService) attemptStart(agentNode packages.InstalledPackage, return pid, nil, false } -// startWithPortRetry runs attemptFn on the initial port and, if it fails with a -// strict-port conflict, retries exactly once on a fresh port. It returns the -// final pid, the port actually used, and any error. -func (as *DefaultAgentService) startWithPortRetry(initialPort int, attemptFn func(port int) (pid int, err error, portConflict bool)) (int, int, error) { +// startWithPortRetry runs attemptFn on the initial port. When retryOnConflict +// is true (for an automatically allocated port), a strict-port conflict is +// retried exactly once on a fresh port. It returns the final pid, the port +// actually used, and any error. +func (as *DefaultAgentService) startWithPortRetry(initialPort int, retryOnConflict bool, attemptFn func(port int) (pid int, err error, portConflict bool)) (int, int, error) { port := initialPort pid, err, conflict := attemptFn(port) - if err == nil || !conflict { + if err == nil || !conflict || !retryOnConflict { return pid, port, err } diff --git a/control-plane/internal/core/services/agent_service_startup_test.go b/control-plane/internal/core/services/agent_service_startup_test.go index 47f82f628..2e9960c2a 100644 --- a/control-plane/internal/core/services/agent_service_startup_test.go +++ b/control-plane/internal/core/services/agent_service_startup_test.go @@ -43,8 +43,9 @@ func newStartupTestService(t *testing.T, pm *mockPortManager) *DefaultAgentServi ).(*DefaultAgentService) } -// Contract: a strict-port failure triggers exactly one retry, on a different -// port than the one that failed. +// Contract: an automatically allocated port that encounters a strict-port +// failure triggers exactly one retry on a different port than the one that +// failed. func TestStartWithPortRetry_RetriesOnceOnPortConflict(t *testing.T) { pm := newMockPortManager() reserved := -1 @@ -63,7 +64,7 @@ func TestStartWithPortRetry_RetriesOnceOnPortConflict(t *testing.T) { return 777, nil, false } - pid, port, err := captureRetry(t, service, 8001, attempt) + pid, port, err := captureRetry(t, service, 8001, true, attempt) if err != nil { t.Fatalf("expected retry to succeed, got %v", err) } @@ -96,7 +97,7 @@ func TestStartWithPortRetry_NoRetryOnNonConflictFailure(t *testing.T) { return 0, errors.New("boom: import error"), false } - _, _, err := captureRetry(t, service, 8001, attempt) + _, _, err := captureRetry(t, service, 8001, true, attempt) if err == nil { t.Fatalf("expected failure to propagate") } @@ -116,7 +117,7 @@ func TestStartWithPortRetry_SuccessRunsOnce(t *testing.T) { return 55, nil, false } - pid, port, err := captureRetry(t, service, 8001, attempt) + pid, port, err := captureRetry(t, service, 8001, true, attempt) if err != nil || attempts != 1 { t.Fatalf("expected one successful attempt, got attempts=%d err=%v", attempts, err) } @@ -139,7 +140,7 @@ func TestStartWithPortRetry_NoDistinctPortDoesNotRetry(t *testing.T) { return 0, errors.New("assigned port unavailable"), true } - _, _, err := captureRetry(t, service, 8001, attempt) + _, _, err := captureRetry(t, service, 8001, true, attempt) if err == nil { t.Fatalf("expected failure to propagate") } @@ -148,13 +149,40 @@ func TestStartWithPortRetry_NoDistinctPortDoesNotRetry(t *testing.T) { } } +// Contract: a caller-supplied port is never reassigned after a strict-port +// conflict because external configuration can depend on that exact port. +func TestStartWithPortRetry_ExplicitPortDoesNotRetry(t *testing.T) { + pm := newMockPortManager() + service := newStartupTestService(t, pm) + + var attempts int + attempt := func(p int) (int, error, bool) { + attempts++ + if p != 9123 { + t.Errorf("attempted port = %d, want explicitly requested port 9123", p) + } + return 0, errors.New("assigned port unavailable"), true + } + + _, port, err := captureRetry(t, service, 9123, false, attempt) + if err == nil { + t.Fatal("expected conflict failure to propagate") + } + if attempts != 1 { + t.Errorf("explicit port must not retry, got %d attempts", attempts) + } + if port != 9123 { + t.Errorf("returned port = %d, want explicitly requested port 9123", port) + } +} + // captureRetry runs startWithPortRetry while swallowing its progress output. -func captureRetry(t *testing.T, s *DefaultAgentService, initial int, fn func(int) (int, error, bool)) (int, int, error) { +func captureRetry(t *testing.T, s *DefaultAgentService, initial int, retryOnConflict bool, fn func(int) (int, error, bool)) (int, int, error) { t.Helper() var pid, port int var err error _ = captureStdout(t, func() { - pid, port, err = s.startWithPortRetry(initial, fn) + pid, port, err = s.startWithPortRetry(initial, retryOnConflict, fn) }) return pid, port, err } From 4e1268fb447ec8f7f1d5076bc35439fe256c94db Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Fri, 24 Jul 2026 13:08:30 -0400 Subject: [PATCH 6/6] chore(skills): sync embedded skill mirrors on branch Realigns embedded mirrors with skills/ sources inherited from the main merge so skillkit drift tests pass branch-locally. Co-Authored-By: Claude Fable 5 --- .../skillkit/skill_data/agentfield-use/SKILL.md | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/control-plane/internal/skillkit/skill_data/agentfield-use/SKILL.md b/control-plane/internal/skillkit/skill_data/agentfield-use/SKILL.md index d9dc1f156..e39568501 100644 --- a/control-plane/internal/skillkit/skill_data/agentfield-use/SKILL.md +++ b/control-plane/internal/skillkit/skill_data/agentfield-use/SKILL.md @@ -128,7 +128,9 @@ curl -s -X POST http://localhost:8080/api/v1/execute/swe-planner.plan \ Async dispatch is cheap: fire all independent calls up front, then poll them together. Do NOT serialize multi-agent work — the whole point of the control -plane is managing many agents at once. What to know: +plane is managing many agents at once. When a batch of independent jobs arrives +(ten PRs to review, five repos to scan), the default is to dispatch the whole +batch now and poll as a group — not one-at-a-time. What to know: - Concurrent calls to the **same reasoner** are safe when the agent is (e.g. pr-af isolates concurrent reviews per PR). If an agent's docs don't say it's @@ -147,6 +149,17 @@ launching more heavy runs — if `active_executions >= recommended_max_concurren finish or await in-flight work first rather than starting more, and tell the user you're throttling to avoid overloading the machine. +**Canary after reconfiguration, then fan out.** The one exception to +fire-everything-up-front: you just changed a node's runtime config (provider, +model, bin path — `af secrets set` + restart). A misconfigured harness can fail +*silently* — the run reports `succeeded` with empty results in seconds, and an +agent that posts externally (GitHub reviews, Slack, tickets) will publish that +garbage under the user's identity, once per dispatched call. So after any +config change: send ONE representative call, confirm it did real work (nonzero +cost/duration, plausible output — not just `succeeded`), then fan out the rest +at full width. This is a gate on the first call after a config change, not a +reason to serialize steady-state work. + ## 4. Get the result **What's in flight right now** — no IDs needed (also answers "how many agents