From 8b6fd79b7fc606dc8e72f692d8476e51305efc25 Mon Sep 17 00:00:00 2001 From: MK Date: Thu, 2 Apr 2026 13:48:35 -0400 Subject: [PATCH 1/4] fix: scope KUBECONFIG and NO_PROXY to kubectl/helm binaries kubectl fails with "the server has asked for the client to provide credentials" because cli_execute overrides HOME to workDir, so kubectl can't find ~/.kube/config. Fix: - Set KUBECONFIG to real ~/.kube/config for kubectl/helm only (same pattern as GH_CONFIG_DIR for gh) - Set NO_PROXY with K8s API server host extracted from kubeconfig, plus localhost/common local addresses, so kubectl's mTLS/bearer auth isn't broken by the egress proxy - Both env vars scoped to kubectl/helm binaries only --- forge-cli/tools/cli_execute.go | 98 ++++++++++++++++++++++-- forge-cli/tools/cli_execute_test.go | 115 ++++++++++++++++++++++++++++ 2 files changed, 208 insertions(+), 5 deletions(-) diff --git a/forge-cli/tools/cli_execute.go b/forge-cli/tools/cli_execute.go index 9940411..1728999 100644 --- a/forge-cli/tools/cli_execute.go +++ b/forge-cli/tools/cli_execute.go @@ -276,13 +276,27 @@ func (t *CLIExecuteTool) buildEnv(binary string) []string { "HOME=" + homeVal, "LANG=" + os.Getenv("LANG"), } - // When HOME is overridden, preserve GH_CONFIG_DIR ONLY for the gh binary. - // Scoping to gh prevents other binaries from accessing GitHub credentials. - if binary == "gh" && t.workDir != "" && realHome != "" { - if _, ok := os.LookupEnv("GH_CONFIG_DIR"); !ok { - env = append(env, "GH_CONFIG_DIR="+filepath.Join(realHome, ".config", "gh")) + + // Per-binary credential scoping: only the binary that needs credentials gets them. + if t.workDir != "" && realHome != "" { + switch binary { + case "gh": + // Preserve GH_CONFIG_DIR so gh CLI finds auth at real ~/.config/gh. + if _, ok := os.LookupEnv("GH_CONFIG_DIR"); !ok { + env = append(env, "GH_CONFIG_DIR="+filepath.Join(realHome, ".config", "gh")) + } + case "kubectl", "helm": + // Preserve KUBECONFIG so kubectl/helm find cluster credentials at + // the real ~/.kube/config when HOME has been overridden. + if _, ok := os.LookupEnv("KUBECONFIG"); !ok { + defaultKubeconfig := filepath.Join(realHome, ".kube", "config") + if _, err := os.Stat(defaultKubeconfig); err == nil { + env = append(env, "KUBECONFIG="+defaultKubeconfig) + } + } } } + for _, key := range t.config.EnvPassthrough { if val, ok := os.LookupEnv(key); ok { env = append(env, key+"="+val) @@ -296,9 +310,83 @@ func (t *CLIExecuteTool) buildEnv(binary string) []string { "https_proxy="+t.proxyURL, ) } + + // kubectl/helm manage their own TLS (mTLS client certs, bearer tokens). + // Routing through the egress proxy breaks K8s API auth. Set NO_PROXY to + // bypass the proxy for addresses kubectl commonly connects to. + if t.proxyURL != "" && (binary == "kubectl" || binary == "helm") { + noProxy := buildK8sNoProxy(env) + if noProxy != "" { + env = append(env, + "NO_PROXY="+noProxy, + "no_proxy="+noProxy, + ) + } + } + return env } +// buildK8sNoProxy extracts the K8s API server address from the KUBECONFIG +// file (if available) and returns a NO_PROXY value that includes it along +// with standard local addresses. This prevents the egress proxy from +// intercepting kubectl's mTLS/bearer-token auth to the API server. +func buildK8sNoProxy(env []string) string { + // Always bypass proxy for loopback and common K8s local addresses. + entries := []string{"localhost", "127.0.0.1", "::1", "*.local", "kubernetes.docker.internal", "host.docker.internal"} + + // Try to extract the API server host from KUBECONFIG env we just set. + var kubeconfigPath string + for _, e := range env { + if strings.HasPrefix(e, "KUBECONFIG=") { + kubeconfigPath = strings.TrimPrefix(e, "KUBECONFIG=") + break + } + } + if kubeconfigPath == "" { + // Check the real env (user may have set KUBECONFIG explicitly). + kubeconfigPath = os.Getenv("KUBECONFIG") + } + if kubeconfigPath != "" { + if host := extractK8sAPIHost(kubeconfigPath); host != "" { + entries = append(entries, host) + } + } + return strings.Join(entries, ",") +} + +// extractK8sAPIHost reads the kubeconfig and extracts the server hostname +// of the current context. Returns just the hostname (no port, no scheme). +func extractK8sAPIHost(kubeconfigPath string) string { + data, err := os.ReadFile(kubeconfigPath) + if err != nil { + return "" + } + // Lightweight extraction: find "server:" lines and parse the hostname. + // Full YAML parsing would require a dependency; a simple scan suffices. + for _, line := range strings.Split(string(data), "\n") { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "server:") { + serverURL := strings.TrimSpace(strings.TrimPrefix(trimmed, "server:")) + // Strip scheme + serverURL = strings.TrimPrefix(serverURL, "https://") + serverURL = strings.TrimPrefix(serverURL, "http://") + // Strip port + if idx := strings.LastIndex(serverURL, ":"); idx > 0 { + serverURL = serverURL[:idx] + } + // Strip trailing path + if idx := strings.Index(serverURL, "/"); idx > 0 { + serverURL = serverURL[:idx] + } + if serverURL != "" { + return serverURL + } + } + } + return "" +} + // deniedShells is a hardcoded set of shell interpreters that are never allowed // regardless of the allowlist. Shells defeat the security model by // reintroducing shell interpretation, bypassing path validation and the diff --git a/forge-cli/tools/cli_execute_test.go b/forge-cli/tools/cli_execute_test.go index 78d7957..0b877ac 100644 --- a/forge-cli/tools/cli_execute_test.go +++ b/forge-cli/tools/cli_execute_test.go @@ -572,3 +572,118 @@ func TestCLIExecute_HomeOverriddenToWorkDir(t *testing.T) { t.Errorf("expected %q in env output, got:\n%s", expected, res.Stdout) } } + +func TestBuildEnv_GHConfigDirScopedToGh(t *testing.T) { + tmpDir := t.TempDir() + t.Setenv("HOME", "/Users/testuser") + + tool := NewCLIExecuteTool(CLIExecuteConfig{ + AllowedBinaries: []string{"env", "gh", "curl"}, + WorkDir: tmpDir, + }) + + // gh binary should get GH_CONFIG_DIR + ghEnv := tool.buildEnv("gh") + found := false + for _, e := range ghEnv { + if strings.HasPrefix(e, "GH_CONFIG_DIR=") { + found = true + break + } + } + if !found { + t.Error("expected GH_CONFIG_DIR for gh binary") + } + + // curl binary should NOT get GH_CONFIG_DIR + curlEnv := tool.buildEnv("curl") + for _, e := range curlEnv { + if strings.HasPrefix(e, "GH_CONFIG_DIR=") { + t.Error("GH_CONFIG_DIR should not be set for curl binary") + } + } +} + +func TestBuildEnv_KubeconfigScopedToKubectl(t *testing.T) { + tmpDir := t.TempDir() + realHome := t.TempDir() + t.Setenv("HOME", realHome) + + // Create a fake kubeconfig + kubeDir := filepath.Join(realHome, ".kube") + os.MkdirAll(kubeDir, 0o700) + os.WriteFile(filepath.Join(kubeDir, "config"), []byte("apiVersion: v1\nclusters:\n- cluster:\n server: https://192.168.1.100:6443\n"), 0o600) + + tool := NewCLIExecuteTool(CLIExecuteConfig{ + AllowedBinaries: []string{"env", "kubectl", "curl"}, + WorkDir: tmpDir, + }) + + // kubectl should get KUBECONFIG + kubectlEnv := tool.buildEnv("kubectl") + found := false + for _, e := range kubectlEnv { + if strings.HasPrefix(e, "KUBECONFIG=") { + found = true + if !strings.Contains(e, filepath.Join(realHome, ".kube", "config")) { + t.Errorf("expected KUBECONFIG to point to real home, got: %s", e) + } + break + } + } + if !found { + t.Error("expected KUBECONFIG for kubectl binary") + } + + // curl should NOT get KUBECONFIG + curlEnv := tool.buildEnv("curl") + for _, e := range curlEnv { + if strings.HasPrefix(e, "KUBECONFIG=") { + t.Error("KUBECONFIG should not be set for curl binary") + } + } +} + +func TestBuildEnv_KubectlNoProxy(t *testing.T) { + tmpDir := t.TempDir() + realHome := t.TempDir() + t.Setenv("HOME", realHome) + + // Create kubeconfig with a server address + kubeDir := filepath.Join(realHome, ".kube") + os.MkdirAll(kubeDir, 0o700) + os.WriteFile(filepath.Join(kubeDir, "config"), []byte("apiVersion: v1\nclusters:\n- cluster:\n server: https://my-k8s.example.com:6443\n"), 0o600) + + tool := NewCLIExecuteTool(CLIExecuteConfig{ + AllowedBinaries: []string{"kubectl", "curl"}, + WorkDir: tmpDir, + }) + tool.proxyURL = "http://127.0.0.1:54321" + + // kubectl should get NO_PROXY with the K8s API server host + kubectlEnv := tool.buildEnv("kubectl") + var noProxy string + for _, e := range kubectlEnv { + if strings.HasPrefix(e, "NO_PROXY=") { + noProxy = strings.TrimPrefix(e, "NO_PROXY=") + break + } + } + if noProxy == "" { + t.Fatal("expected NO_PROXY for kubectl binary") + } + if !strings.Contains(noProxy, "my-k8s.example.com") { + t.Errorf("expected NO_PROXY to contain K8s API host, got: %s", noProxy) + } + if !strings.Contains(noProxy, "localhost") { + t.Errorf("expected NO_PROXY to contain localhost, got: %s", noProxy) + } + + // curl should NOT get NO_PROXY + curlEnv := tool.buildEnv("curl") + for _, e := range curlEnv { + if strings.HasPrefix(e, "NO_PROXY=") { + t.Error("NO_PROXY should not be set for curl binary") + } + } +} From 97a86d9b76384389cbeab97af52141c24eb9d58d Mon Sep 17 00:00:00 2001 From: MK Date: Thu, 2 Apr 2026 14:05:26 -0400 Subject: [PATCH 2/4] fix: kubectl auth and cli_execute file attachment behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two fixes: 1. KUBECONFIG/NO_PROXY for kubectl: When HOME is overridden to workDir, kubectl can't find ~/.kube/config. Set KUBECONFIG to the real path (scoped to kubectl/helm only). Also set NO_PROXY with the K8s API server host extracted from kubeconfig so kubectl's mTLS/bearer auth isn't broken by the egress proxy. 2. Skip file part creation for cli_execute: cli_execute is an intermediate tool — the LLM should analyze its output and write a human-readable report. Attaching raw cli_execute JSON as a file causes the LLM to say "see attached" instead of synthesizing a markdown triage report. File parts are still created for file_create and script-backed skill tools. --- forge-core/runtime/loop.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/forge-core/runtime/loop.go b/forge-core/runtime/loop.go index 3359020..58b6410 100644 --- a/forge-core/runtime/loop.go +++ b/forge-core/runtime/loop.go @@ -434,6 +434,10 @@ func (e *LLMExecutor) Execute(ctx context.Context, task *a2a.Task, msg *a2a.Mess // Handle file_create tool: always create a file part. // For other tools with large output, detect content type. + // Skip cli_execute: it's an intermediate tool — the LLM should + // analyze its output and produce a human-readable response, not + // forward raw JSON. Attaching cli_execute output as a file causes + // the LLM to say "see attached" instead of writing a report. if tc.Function.Name == "file_create" { var fc struct { Filename string `json:"filename"` @@ -450,7 +454,7 @@ func (e *LLMExecutor) Execute(ctx context.Context, task *a2a.Task, msg *a2a.Mess }, }) } - } else if len(result) > largeToolOutputThreshold { + } else if tc.Function.Name != "cli_execute" && len(result) > largeToolOutputThreshold { name, mime := detectFileType(result, tc.Function.Name) largeToolOutputs = append(largeToolOutputs, a2a.Part{ Kind: a2a.PartKindFile, From 937d739cc150f478d8bcf5ada5c028af475a8b19 Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 4 Apr 2026 15:01:00 -0400 Subject: [PATCH 3/4] fix: Q&A nudge suppression, UI agent start errors, and chat streaming - Add web_search to no_pii guardrail allow_tools so search results aren't blocked - Suppress continuation nudges for Q&A conversations (no edit/git tools) - Capture stderr and verify PID liveness on agent start for proper error feedback in UI - Extract Error: lines from serve.log instead of showing cobra help text - Add SilenceUsage to run/serve commands to prevent help dump on errors - Load agent .env and encrypted secrets in skill builder provider check - Stream chat text to UI in real-time via status/result SSE events - Show agent start errors in UI card instead of only console.error --- forge-cli/cmd/run.go | 7 +- forge-cli/cmd/serve.go | 10 +- forge-cli/runtime/guardrails_loader.go | 1 + forge-core/forgecore_test.go | 30 ++---- forge-core/runtime/loop.go | 6 ++ forge-core/runtime/loop_test.go | 90 +++++++++++++++++ forge-ui/handlers_skill_builder.go | 12 +++ forge-ui/process.go | 130 ++++++++++++++++++++++++- forge-ui/static/dist/app.js | 28 +++++- 9 files changed, 281 insertions(+), 33 deletions(-) diff --git a/forge-cli/cmd/run.go b/forge-cli/cmd/run.go index d415353..3743b57 100644 --- a/forge-cli/cmd/run.go +++ b/forge-cli/cmd/run.go @@ -34,9 +34,10 @@ var ( ) var runCmd = &cobra.Command{ - Use: "run", - Short: "Run the agent locally with an A2A-compliant dev server", - RunE: runRun, + Use: "run", + Short: "Run the agent locally with an A2A-compliant dev server", + SilenceUsage: true, + RunE: runRun, } func init() { diff --git a/forge-cli/cmd/serve.go b/forge-cli/cmd/serve.go index 1a961b1..9c661ae 100644 --- a/forge-cli/cmd/serve.go +++ b/forge-cli/cmd/serve.go @@ -58,13 +58,15 @@ Examples: forge serve stop # Stop the daemon forge serve status # Show running status forge serve logs # View recent logs`, - RunE: serveStartRun, // bare "forge serve" acts as "forge serve start" + SilenceUsage: true, + RunE: serveStartRun, // bare "forge serve" acts as "forge serve start" } var serveStartCmd = &cobra.Command{ - Use: "start", - Short: "Start the agent daemon", - RunE: serveStartRun, + Use: "start", + Short: "Start the agent daemon", + SilenceUsage: true, + RunE: serveStartRun, } var serveStopCmd = &cobra.Command{ diff --git a/forge-cli/runtime/guardrails_loader.go b/forge-cli/runtime/guardrails_loader.go index de2bf05..2b86ac9 100644 --- a/forge-cli/runtime/guardrails_loader.go +++ b/forge-cli/runtime/guardrails_loader.go @@ -47,6 +47,7 @@ func DefaultPolicyScaffold() *agentspec.PolicyScaffold { "code_agent_write", "code_agent_edit", "cli_execute", + "web_search", }, }, }, diff --git a/forge-core/forgecore_test.go b/forge-core/forgecore_test.go index 1b798af..74c66f6 100644 --- a/forge-core/forgecore_test.go +++ b/forge-core/forgecore_test.go @@ -794,15 +794,6 @@ func TestNewRuntime_WithToolCalling(t *testing.T) { }, FinishReason: "stop", }, - // The agent loop sends a continuation nudge after the first stop. - // Without workflow phases configured, only 1 nudge fires. - { - Message: llm.ChatMessage{ - Role: llm.RoleAssistant, - Content: "I fetched the URL and got: ok", - }, - FinishReason: "stop", - }, }, } @@ -834,9 +825,10 @@ func TestNewRuntime_WithToolCalling(t *testing.T) { t.Errorf("response text = %q, want 'I fetched the URL and got: ok'", resp.Parts[0].Text) } - // Should have made 3 LLM calls (tool call + stop + 1 continuation nudge) - if toolCallClient.callIdx != 3 { - t.Errorf("LLM was called %d times, want 3", toolCallClient.callIdx) + // Should have made 2 LLM calls (tool call + stop). No continuation + // nudge because no workflow phases and no edit/git tools were used. + if toolCallClient.callIdx != 2 { + t.Errorf("LLM was called %d times, want 2", toolCallClient.callIdx) } } @@ -1334,14 +1326,6 @@ func TestIntegration_CompileWithToolCallLoop(t *testing.T) { }, FinishReason: "stop", }, - // Continuation nudge: without workflow phases, only 1 nudge fires. - { - Message: llm.ChatMessage{ - Role: llm.RoleAssistant, - Content: "Found and fetched the result", - }, - FinishReason: "stop", - }, }, } @@ -1371,8 +1355,10 @@ func TestIntegration_CompileWithToolCallLoop(t *testing.T) { if resp.Parts[0].Text != "Found and fetched the result" { t.Errorf("response text = %q", resp.Parts[0].Text) } - if toolCallClient.callIdx != 4 { - t.Errorf("LLM was called %d times, want 4", toolCallClient.callIdx) + // 3 calls: web_search tool call + http_request tool call + stop. + // No continuation nudge because no workflow phases and no edit/git tools. + if toolCallClient.callIdx != 3 { + t.Errorf("LLM was called %d times, want 3", toolCallClient.callIdx) } } diff --git a/forge-core/runtime/loop.go b/forge-core/runtime/loop.go index 58b6410..65dda5f 100644 --- a/forge-core/runtime/loop.go +++ b/forge-core/runtime/loop.go @@ -276,6 +276,12 @@ func (e *LLMExecutor) Execute(ctx context.Context, task *a2a.Task, msg *a2a.Mess maxNudges = 0 // workflow is complete — don't nudge } else if workflowIncomplete && tracker.phaseHasError[phaseGitOps] { maxNudges = 2 + } else if !hasWorkflowRequirements && !tracker.phaseSeen[phaseEdit] && !tracker.phaseSeen[phaseGitOps] { + // Informational / Q&A conversation — agent only used + // explore-phase tools (web_search, file_read, etc.) and + // gave a text response. No code changes were attempted, + // so there's nothing to "continue" with. + maxNudges = 0 } if stopNudgesSent < maxNudges { diff --git a/forge-core/runtime/loop_test.go b/forge-core/runtime/loop_test.go index 4b85688..435d44f 100644 --- a/forge-core/runtime/loop_test.go +++ b/forge-core/runtime/loop_test.go @@ -1190,3 +1190,93 @@ func TestGitNudgeNoVerifyReminderWithoutEditPhase(t *testing.T) { t.Errorf("git nudge should still include git workflow steps, got: %s", msg) } } + +func TestNoStopNudgeForQAConversation(t *testing.T) { + // When no workflow phases are configured and the agent only uses + // explore-phase tools (e.g. web_search), it's a Q&A conversation. + // The stop nudge should NOT fire — the agent's text response is the answer. + makeToolDefs := func(names ...string) []llm.ToolDefinition { + var defs []llm.ToolDefinition + for _, n := range names { + defs = append(defs, llm.ToolDefinition{ + Type: "function", + Function: llm.FunctionSchema{Name: n}, + }) + } + return defs + } + + callIdx := 0 + var capturedMessages []llm.ChatMessage + + client := &mockLLMClient{ + chatFunc: func(ctx context.Context, req *llm.ChatRequest) (*llm.ChatResponse, error) { + callIdx++ + capturedMessages = append([]llm.ChatMessage{}, req.Messages...) + if callIdx == 1 { + // First call: LLM calls web_search + return &llm.ChatResponse{ + Message: llm.ChatMessage{ + Role: llm.RoleAssistant, + ToolCalls: []llm.ToolCall{ + { + ID: "call_1", + Type: "function", + Function: llm.FunctionCall{ + Name: "web_search", + Arguments: `{"query":"top news today"}`, + }, + }, + }, + }, + FinishReason: "tool_calls", + }, nil + } + // Second call: LLM provides answer + return &llm.ChatResponse{ + Message: llm.ChatMessage{Role: llm.RoleAssistant, Content: "Here are the top headlines..."}, + FinishReason: "stop", + }, nil + }, + } + + tools := &mockToolExecutor{ + executeFunc: func(ctx context.Context, name string, arguments json.RawMessage) (string, error) { + return `{"results":[{"title":"News headline"}]}`, nil + }, + toolDefs: makeToolDefs("web_search"), + } + + // No workflow phases — this is a general-purpose agent + executor := NewLLMExecutor(LLMExecutorConfig{ + Client: client, + Tools: tools, + MaxIterations: 20, + }) + + task := &a2a.Task{ID: "qa-no-nudge"} + msg := &a2a.Message{ + Role: a2a.MessageRoleUser, + Parts: []a2a.Part{a2a.NewTextPart("what are the top news?")}, + } + + resp, err := executor.Execute(context.Background(), task, msg) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp == nil { + t.Fatal("expected response") + } + + // Verify no "You stopped" nudge was injected + for _, m := range capturedMessages { + if m.Role == "user" && strings.Contains(m.Content, "You stopped") { + t.Errorf("Q&A conversation should not get stop nudge, but got: %s", m.Content) + } + } + + // Should have exactly 2 LLM calls (tool call + answer), not 3 (+ nudge response) + if callIdx != 2 { + t.Errorf("expected 2 LLM calls for Q&A, got %d", callIdx) + } +} diff --git a/forge-ui/handlers_skill_builder.go b/forge-ui/handlers_skill_builder.go index b1c9cc3..3c20b80 100644 --- a/forge-ui/handlers_skill_builder.go +++ b/forge-ui/handlers_skill_builder.go @@ -8,6 +8,7 @@ import ( "path/filepath" "strings" + "github.com/initializ/forge/forge-cli/runtime" "github.com/initializ/forge/forge-core/types" ) @@ -72,6 +73,17 @@ func (s *UIServer) handleSkillBuilderProvider(w http.ResponseWriter, r *http.Req provider := cfg.Model.Provider model := SkillBuilderCodegenModel(provider, cfg.Model.Name) + // Load the agent's .env and encrypted secrets so we can check for API keys + // that aren't in the UI process's own environment. + envPath := filepath.Join(agentDir, ".env") + envVars, _ := runtime.LoadEnvFile(envPath) + for k, v := range envVars { + if os.Getenv(k) == "" { + _ = os.Setenv(k, v) + } + } + runtime.OverlaySecretsToEnv(cfg, agentDir) + // Check if the provider's API key env var is set hasKey := false switch provider { diff --git a/forge-ui/process.go b/forge-ui/process.go index 86d1839..b322467 100644 --- a/forge-ui/process.go +++ b/forge-ui/process.go @@ -1,11 +1,18 @@ package forgeui import ( + "bytes" + "encoding/json" "fmt" + "net" + "os" "os/exec" + "path/filepath" "strconv" "strings" "sync" + "syscall" + "time" ) // PortAllocator manages port assignment for agent processes. @@ -79,6 +86,10 @@ func (pm *ProcessManager) Start(agentID string, info *AgentInfo, passphrase stri cmd := exec.Command(pm.exePath, args...) cmd.Dir = info.Directory + // Capture stderr so we can surface error details to the UI. + var stderr bytes.Buffer + cmd.Stderr = &stderr + if passphrase != "" { cmd.Env = append(cmd.Environ(), "FORGE_PASSPHRASE="+passphrase) } @@ -87,11 +98,37 @@ func (pm *ProcessManager) Start(agentID string, info *AgentInfo, passphrase stri pm.ports.Release(port) delete(pm.allocated, agentID) + // Build a useful error message from stderr output. + errMsg := strings.TrimSpace(stderr.String()) + if errMsg == "" { + errMsg = err.Error() + } + info.Status = StateErrored - info.Error = err.Error() + info.Error = errMsg pm.broker.Broadcast(SSEEvent{Type: "agent_status", Data: info}) - return fmt.Errorf("forge serve start failed: %w", err) + return fmt.Errorf("forge serve start failed: %s", errMsg) + } + + // Verify the daemon actually started by probing the port. + // forge serve start forks a child process — the child may crash + // immediately after the parent returns success. + if !pm.waitForPort(info.Directory, port) { + pm.ports.Release(port) + delete(pm.allocated, agentID) + + // Read serve.log for diagnostics. + errMsg := pm.readServeLogs(info.Directory) + if errMsg == "" { + errMsg = fmt.Sprintf("agent process exited immediately (port %d never became reachable)", port) + } + + info.Status = StateErrored + info.Error = errMsg + pm.broker.Broadcast(SSEEvent{Type: "agent_status", Data: info}) + + return fmt.Errorf("agent failed to start: %s", errMsg) } info.Status = StateRunning @@ -102,6 +139,95 @@ func (pm *ProcessManager) Start(agentID string, info *AgentInfo, passphrase stri return nil } +// waitForPort polls the TCP port for up to 5 seconds waiting for the +// daemon's HTTP server to become reachable. It also checks PID liveness +// so we fail fast when the child process crashes during startup. +func (pm *ProcessManager) waitForPort(agentDir string, port int) bool { + deadline := time.Now().Add(5 * time.Second) + addr := fmt.Sprintf("127.0.0.1:%d", port) + + // Give the child process a moment to start (or crash). + time.Sleep(500 * time.Millisecond) + + // Read PID from serve.json for liveness checks. + pid := pm.readServePID(agentDir) + + for time.Now().Before(deadline) { + // Fast-fail: if the child process already exited, don't keep polling. + if pid > 0 && !pidAlive(pid) { + return false + } + + conn, err := net.DialTimeout("tcp", addr, 300*time.Millisecond) + if err == nil { + _ = conn.Close() + return true + } + + time.Sleep(300 * time.Millisecond) + } + return false +} + +// readServePID reads the daemon PID from .forge/serve.json. +func (pm *ProcessManager) readServePID(agentDir string) int { + data, err := os.ReadFile(filepath.Join(agentDir, ".forge", "serve.json")) + if err != nil { + return 0 + } + var state struct { + PID int `json:"pid"` + } + if json.Unmarshal(data, &state) != nil { + return 0 + } + return state.PID +} + +// pidAlive checks if a process with the given PID is still running. +func pidAlive(pid int) bool { + proc, err := os.FindProcess(pid) + if err != nil { + return false + } + return proc.Signal(syscall.Signal(0)) == nil +} + +// readServeLogs reads .forge/serve.log and extracts the error message. +// It looks for cobra "Error:" lines first, then falls back to the last +// few non-empty lines. This avoids returning cobra usage/help text that +// gets dumped after the actual error. +func (pm *ProcessManager) readServeLogs(agentDir string) string { + logPath := filepath.Join(agentDir, ".forge", "serve.log") + data, err := os.ReadFile(logPath) + if err != nil { + return "" + } + + lines := strings.Split(strings.TrimSpace(string(data)), "\n") + + // Look for "Error:" lines (cobra format) — return the last one found. + var lastError string + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, "Error:") { + lastError = strings.TrimPrefix(trimmed, "Error: ") + } + } + if lastError != "" { + return lastError + } + + // Fallback: return the last 5 non-empty lines. + var tail []string + for i := len(lines) - 1; i >= 0 && len(tail) < 5; i-- { + if t := strings.TrimSpace(lines[i]); t != "" { + tail = append([]string{t}, tail...) + } + } + return strings.Join(tail, "\n") +} + // Stop stops an agent via `forge serve stop`. func (pm *ProcessManager) Stop(agentID string, agentDir string) error { pm.mu.Lock() diff --git a/forge-ui/static/dist/app.js b/forge-ui/static/dist/app.js index 6f85fb3..a9510a5 100644 --- a/forge-ui/static/dist/app.js +++ b/forge-ui/static/dist/app.js @@ -556,6 +556,16 @@ function useChatStream(agentId) { } } } + // Update messages in real-time to show text as it arrives + setMessages(prev => { + const last = prev[prev.length - 1]; + if (last && last.role === 'agent' && last.isStreaming) { + const updated = [...prev]; + updated[updated.length - 1] = { ...last, content: agentText, tools: [...currentTools] }; + return updated; + } + return [...prev, { role: 'agent', content: agentText, tools: [...currentTools], isStreaming: true }]; + }); } else if (eventType === 'progress') { // Tool execution progress const status = parsed.status || parsed; @@ -574,10 +584,10 @@ function useChatStream(agentId) { const last = prev[prev.length - 1]; if (last && last.role === 'agent' && last.isStreaming) { const updated = [...prev]; - updated[updated.length - 1] = { ...last, tools: [...currentTools] }; + updated[updated.length - 1] = { ...last, content: agentText, tools: [...currentTools] }; return updated; } - return [...prev, { role: 'agent', content: '', tools: [...currentTools], isStreaming: true }]; + return [...prev, { role: 'agent', content: agentText, tools: [...currentTools], isStreaming: true }]; }); } else if (eventType === 'result') { // Final result @@ -590,6 +600,16 @@ function useChatStream(agentId) { } } } + // Show final text immediately + setMessages(prev => { + const last = prev[prev.length - 1]; + if (last && last.role === 'agent' && last.isStreaming) { + const updated = [...prev]; + updated[updated.length - 1] = { ...last, content: agentText, tools: [...currentTools] }; + return updated; + } + return [...prev, { role: 'agent', content: agentText, tools: [...currentTools], isStreaming: true }]; + }); } else if (eventType === 'done') { if (parsed.session_id) { receivedSessionId = parsed.session_id; @@ -2445,6 +2465,10 @@ function App() { return; } console.error('Failed to start agent:', err); + // Update agent state so the error is visible in the UI card. + setAgents(prev => prev.map(a => + a.id === id ? { ...a, status: 'errored', error: err.message } : a + )); } }, [agents]); From dd634b8471298460c6482fbb8e6430900b0f5c3a Mon Sep 17 00:00:00 2001 From: MK Date: Sat, 4 Apr 2026 15:07:31 -0400 Subject: [PATCH 4/4] fix: check error returns in cli_execute_test.go (errcheck lint) --- forge-cli/tools/cli_execute_test.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/forge-cli/tools/cli_execute_test.go b/forge-cli/tools/cli_execute_test.go index 0b877ac..eee9d60 100644 --- a/forge-cli/tools/cli_execute_test.go +++ b/forge-cli/tools/cli_execute_test.go @@ -611,8 +611,12 @@ func TestBuildEnv_KubeconfigScopedToKubectl(t *testing.T) { // Create a fake kubeconfig kubeDir := filepath.Join(realHome, ".kube") - os.MkdirAll(kubeDir, 0o700) - os.WriteFile(filepath.Join(kubeDir, "config"), []byte("apiVersion: v1\nclusters:\n- cluster:\n server: https://192.168.1.100:6443\n"), 0o600) + if err := os.MkdirAll(kubeDir, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(kubeDir, "config"), []byte("apiVersion: v1\nclusters:\n- cluster:\n server: https://192.168.1.100:6443\n"), 0o600); err != nil { + t.Fatal(err) + } tool := NewCLIExecuteTool(CLIExecuteConfig{ AllowedBinaries: []string{"env", "kubectl", "curl"}, @@ -651,8 +655,12 @@ func TestBuildEnv_KubectlNoProxy(t *testing.T) { // Create kubeconfig with a server address kubeDir := filepath.Join(realHome, ".kube") - os.MkdirAll(kubeDir, 0o700) - os.WriteFile(filepath.Join(kubeDir, "config"), []byte("apiVersion: v1\nclusters:\n- cluster:\n server: https://my-k8s.example.com:6443\n"), 0o600) + if err := os.MkdirAll(kubeDir, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(kubeDir, "config"), []byte("apiVersion: v1\nclusters:\n- cluster:\n server: https://my-k8s.example.com:6443\n"), 0o600); err != nil { + t.Fatal(err) + } tool := NewCLIExecuteTool(CLIExecuteConfig{ AllowedBinaries: []string{"kubectl", "curl"},