diff --git a/AGENTS.md b/AGENTS.md index f91a5fd680..625eae239e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -172,6 +172,18 @@ JSON-RPC 2.0 over stdin/stdout between agentctl and agent process. ## Frontend Architecture +### UI Components + +**Shadcn Components:** Import from `@kandev/ui` package: +```typescript +import { Badge } from '@kandev/ui/badge'; +import { Button } from '@kandev/ui/button'; +import { Dialog } from '@kandev/ui/dialog'; +// etc... +``` + +**Do NOT** import from `@/components/ui/*` - always use `@kandev/ui` package. + ### Data Flow Pattern (Critical) ``` @@ -222,6 +234,7 @@ lib/api/domains/{kanban,session,workspace,settings,process}-api.ts # API client ### Frontend - **Data:** SSR fetch → hydrate → read store. Never fetch in components +- **UI Components:** Import shadcn components from `@kandev/ui`, NOT `@/components/ui/*` - **Components:** <200 lines, extract to domain components, composition over props - **Hooks:** Domain-organized in `hooks/domains/`, encapsulate subscription + selection - **WS:** Use subscription hooks only; client auto-deduplicates diff --git a/apps/backend/cmd/kandev/worktree.go b/apps/backend/cmd/kandev/worktree.go index a42fc43848..3096d91629 100644 --- a/apps/backend/cmd/kandev/worktree.go +++ b/apps/backend/cmd/kandev/worktree.go @@ -1,15 +1,42 @@ package main import ( + "context" "database/sql" "github.com/kandev/kandev/internal/agent/lifecycle" - "github.com/kandev/kandev/internal/worktree" "github.com/kandev/kandev/internal/common/config" + "github.com/kandev/kandev/internal/common/constants" "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/task/models" taskservice "github.com/kandev/kandev/internal/task/service" + "github.com/kandev/kandev/internal/worktree" ) +// taskServiceAdapter adapts the task service to the worktree.TaskService interface. +type taskServiceAdapter struct { + svc *taskservice.Service +} + +func (a *taskServiceAdapter) CreateMessage(ctx context.Context, req *worktree.CreateMessageRequest) (*models.Message, error) { + // Convert worktree.CreateMessageRequest to taskservice.CreateMessageRequest + return a.svc.CreateMessage(ctx, &taskservice.CreateMessageRequest{ + TaskSessionID: req.TaskSessionID, + TaskID: req.TaskID, + TurnID: req.TurnID, + Content: req.Content, + AuthorType: req.AuthorType, + AuthorID: req.AuthorID, + RequestsInput: req.RequestsInput, + Type: req.Type, + Metadata: req.Metadata, + }) +} + +func (a *taskServiceAdapter) UpdateMessage(ctx context.Context, message *models.Message) error { + return a.svc.UpdateMessage(ctx, message) +} + func provideWorktreeManager(dbConn *sql.DB, cfg *config.Config, log *logger.Logger, lifecycleMgr *lifecycle.Manager, taskSvc *taskservice.Service) (*worktree.Manager, func() error, error) { manager, cleanup, err := worktree.Provide(dbConn, cfg, log) if err != nil { @@ -19,5 +46,19 @@ func provideWorktreeManager(dbConn *sql.DB, cfg *config.Config, log *logger.Logg lifecycleMgr.SetWorktreeManager(manager) } taskSvc.SetWorktreeCleanup(manager) + + // Wire script message handler with adapters + taskSvcAdapter := &taskServiceAdapter{svc: taskSvc} + + scriptHandler := worktree.NewDefaultScriptMessageHandler( + log, + taskSvcAdapter, + constants.SetupScriptTimeout, + ) + repoAdapter := worktree.NewRepositoryAdapter(taskSvc) + + manager.SetScriptMessageHandler(scriptHandler) + manager.SetRepositoryProvider(repoAdapter) + return manager, cleanup, nil } diff --git a/apps/backend/internal/agent/lifecycle/manager.go b/apps/backend/internal/agent/lifecycle/manager.go index 6ce5e1f581..76431f74e3 100644 --- a/apps/backend/internal/agent/lifecycle/manager.go +++ b/apps/backend/internal/agent/lifecycle/manager.go @@ -995,9 +995,12 @@ func (m *Manager) handleAgentEvent(execution *AgentExecution, event agentctl.Age // Flush any accumulated content and clear streaming state on error m.flushMessageBuffer(execution) + // Log all available error information for debugging m.logger.Error("agent error", zap.String("execution_id", execution.ID), - zap.String("error", event.Error)) + zap.String("error", event.Error), + zap.String("text", event.Text), + zap.Any("data", event.Data)) case "complete": m.logger.Info("agent turn complete", diff --git a/apps/backend/internal/agentctl/server/adapter/adapter.go b/apps/backend/internal/agentctl/server/adapter/adapter.go index cd93662dcc..886d8e2168 100644 --- a/apps/backend/internal/agentctl/server/adapter/adapter.go +++ b/apps/backend/internal/agentctl/server/adapter/adapter.go @@ -39,6 +39,14 @@ const ( EventTypeContextWindow = streams.EventTypeContextWindow ) +// StderrProvider provides access to recent stderr output for error context. +// This is used by adapters to include stderr in error events when the agent +// reports an error without a detailed message (e.g., rate limit errors). +type StderrProvider interface { + // GetRecentStderr returns the most recent stderr lines from the agent process. + GetRecentStderr() []string +} + // AgentInfo contains information about the connected agent. type AgentInfo struct { Name string `json:"name"` diff --git a/apps/backend/internal/agentctl/server/adapter/codex_adapter.go b/apps/backend/internal/agentctl/server/adapter/codex_adapter.go index a120b6487c..7e3f582b21 100644 --- a/apps/backend/internal/agentctl/server/adapter/codex_adapter.go +++ b/apps/backend/internal/agentctl/server/adapter/codex_adapter.go @@ -47,6 +47,9 @@ type CodexAdapter struct { // Permission handler permissionHandler PermissionHandler + // Stderr provider for error context + stderrProvider StderrProvider + // Accumulators for streaming content messageBuffer string reasoningBuffer string @@ -541,6 +544,15 @@ func (a *CodexAdapter) SetPermissionHandler(handler PermissionHandler) { a.permissionHandler = handler } +// SetStderrProvider sets the provider for recent stderr output. +// This is used to include stderr in error events when the agent +// reports an error without a detailed message. +func (a *CodexAdapter) SetStderrProvider(provider StderrProvider) { + a.mu.Lock() + defer a.mu.Unlock() + a.stderrProvider = provider +} + // Close releases resources held by the adapter. func (a *CodexAdapter) Close() error { a.mu.Lock() @@ -656,14 +668,26 @@ func (a *CodexAdapter) handleNotification(method string, params json.RawMessage) } } - // Send error event if the turn failed (for UI notification) - if !p.Success && p.Error != "" { - a.sendUpdate(AgentEvent{ - Type: EventTypeError, - SessionID: threadID, - OperationID: p.TurnID, - Error: p.Error, - }) + // Send error event if the turn failed WITH an explicit error message. + // Note: We don't send error events here based on stderr alone, because + // NotifyError will handle error notifications (prevents duplicate messages). + if !p.Success { + a.logger.Debug("turn completed with failure", + zap.String("thread_id", threadID), + zap.String("turn_id", p.TurnID), + zap.Bool("success", p.Success), + zap.String("error", p.Error)) + + // Only send error event if there's an explicit error message + // (NotifyError handles the case when error details come separately) + if p.Error != "" { + a.sendUpdate(AgentEvent{ + Type: EventTypeError, + SessionID: threadID, + OperationID: p.TurnID, + Error: p.Error, + }) + } } case codex.NotifyTurnDiffUpdated: @@ -705,10 +729,57 @@ func (a *CodexAdapter) handleNotification(method string, params json.RawMessage) a.logger.Warn("failed to parse error notification", zap.Error(err)) return } + + // Get recent stderr for error context + var stderrLines []string + a.mu.RLock() + if a.stderrProvider != nil { + stderrLines = a.stderrProvider.GetRecentStderr() + } + a.mu.RUnlock() + + // Try to parse stderr into structured error info + // This handles Codex-specific error formats (e.g., rate limits) + parsedError := parseCodexStderrLines(stderrLines) + + var parsedMsg string + if parsedError != nil { + parsedMsg = parsedError.Message + } + + a.logger.Debug("received error notification from agent", + zap.String("thread_id", threadID), + zap.Int("code", p.Code), + zap.String("message", p.Message), + zap.String("parsed_message", parsedMsg), + zap.Any("data", p.Data), + zap.Int("stderr_lines", len(stderrLines))) + + // Build error data with all available context + errorData := map[string]interface{}{ + "code": p.Code, + "data": p.Data, + "stderr": stderrLines, + } + + // Include parsed error details if available + if parsedError != nil { + parsed := map[string]any{ + "http_error": parsedError.HTTPError, + } + // Include the full raw JSON (captures all fields from any error type) + if parsedError.RawJSON != nil { + parsed["error_json"] = parsedError.RawJSON + } + errorData["parsed"] = parsed + } + a.sendUpdate(AgentEvent{ Type: EventTypeError, SessionID: threadID, Error: p.Message, + Text: parsedMsg, // User-friendly parsed message + Data: errorData, }) case codex.NotifyItemStarted: diff --git a/apps/backend/internal/agentctl/server/adapter/codex_errors.go b/apps/backend/internal/agentctl/server/adapter/codex_errors.go new file mode 100644 index 0000000000..291f8f1bda --- /dev/null +++ b/apps/backend/internal/agentctl/server/adapter/codex_errors.go @@ -0,0 +1,135 @@ +package adapter + +import ( + "encoding/json" + "fmt" + "regexp" + "strings" + "time" +) + +// codexErrorRegex matches Codex error log format: +// TIMESTAMP ERROR module: error=HTTP_ERROR: Some("JSON") +var codexErrorRegex = regexp.MustCompile(`error=(.+?):\s*Some\("(.+)"\)\s*$`) + +// CodexParsedError contains the parsed error information from Codex stderr. +type CodexParsedError struct { + // Message is the user-friendly error message + Message string + // HTTPError is the HTTP error string (e.g., "http 429 Too Many Requests") + HTTPError string + // RawJSON contains all fields from the error JSON (generic, captures any structure) + RawJSON map[string]any + // ErrorType is the error type from the JSON (e.g., "usage_limit_reached") - may be empty + ErrorType string + // ResetsInSeconds is the number of seconds until the limit resets - may be 0 + ResetsInSeconds int64 +} + +// parseCodexStderrError attempts to parse a Codex stderr error line and extract +// error information. Returns nil if parsing fails. +// +// Example input: +// +// 2026-01-23T22:57:08.953223Z ERROR codex_api::endpoint::responses: error=http 429 Too Many Requests: Some("{\"error\":{...}}") +func parseCodexStderrError(line string) *CodexParsedError { + matches := codexErrorRegex.FindStringSubmatch(line) + if len(matches) < 3 { + return nil + } + + httpError := strings.TrimSpace(matches[1]) + jsonStr := matches[2] + + // Unescape the JSON string (it's double-escaped in the log) + unescaped := strings.ReplaceAll(jsonStr, `\"`, `"`) + unescaped = strings.ReplaceAll(unescaped, `\\`, `\`) + + result := &CodexParsedError{ + HTTPError: httpError, + } + + // Try to parse the JSON into a generic map first (captures all fields) + var rawData map[string]any + if err := json.Unmarshal([]byte(unescaped), &rawData); err != nil { + // Couldn't parse JSON at all, just return HTTP error + result.Message = httpError + return result + } + + result.RawJSON = rawData + + // Extract common fields - check both nested "error" object and top-level fields + var errorType, errorMessage string + var resetsInSeconds int64 + + // First, check for nested error object (standard format) + if errObj, ok := rawData["error"].(map[string]any); ok { + if t, ok := errObj["type"].(string); ok { + errorType = t + } + if m, ok := errObj["message"].(string); ok { + errorMessage = m + } + // Handle both int and float64 (JSON numbers are float64 in Go) + if r, ok := errObj["resets_in_seconds"].(float64); ok { + resetsInSeconds = int64(r) + } + } + + // Fall back to top-level fields if nested fields not found (flat error format) + if errorMessage == "" { + if m, ok := rawData["message"].(string); ok { + errorMessage = m + } + } + if errorType == "" { + if t, ok := rawData["type"].(string); ok { + errorType = t + } + } + + result.ErrorType = errorType + result.ResetsInSeconds = resetsInSeconds + + // Build a user-friendly message from available fields + var msg string + + if errorMessage != "" { + // We have a specific error message from the API + msg = errorMessage + // Add reset time info if available + if resetsInSeconds > 0 { + duration := time.Duration(resetsInSeconds) * time.Second + if duration.Hours() >= 1 { + msg = fmt.Sprintf("%s (resets in %.0f hours)", msg, duration.Hours()) + } else if duration.Minutes() >= 1 { + msg = fmt.Sprintf("%s (resets in %.0f minutes)", msg, duration.Minutes()) + } else { + msg = fmt.Sprintf("%s (resets in %d seconds)", msg, int(duration.Seconds())) + } + } + } else if errorType != "" { + // We have an error type but no message + msg = fmt.Sprintf("Error: %s", errorType) + } else { + // Unknown error format - show HTTP error and include JSON body + jsonBytes, _ := json.MarshalIndent(rawData, "", " ") + msg = fmt.Sprintf("%s\n\n%s", httpError, string(jsonBytes)) + } + + result.Message = msg + return result +} + +// parseCodexStderrLines attempts to parse Codex stderr lines and extract +// error information. Searches from the end (most recent) first. +// Returns nil if no parseable error is found. +func parseCodexStderrLines(lines []string) *CodexParsedError { + for i := len(lines) - 1; i >= 0; i-- { + if parsed := parseCodexStderrError(lines[i]); parsed != nil { + return parsed + } + } + return nil +} diff --git a/apps/backend/internal/agentctl/server/adapter/codex_errors_test.go b/apps/backend/internal/agentctl/server/adapter/codex_errors_test.go new file mode 100644 index 0000000000..44491951b1 --- /dev/null +++ b/apps/backend/internal/agentctl/server/adapter/codex_errors_test.go @@ -0,0 +1,245 @@ +package adapter + +import ( + "strings" + "testing" +) + +func TestParseCodexStderrError(t *testing.T) { + tests := []struct { + name string + input string + wantNil bool + wantHTTPError string + wantErrorType string + wantMsgContains string + wantRawJSON bool + }{ + { + name: "no match - empty string", + input: "", + wantNil: true, + }, + { + name: "no match - regular log line", + input: "2026-01-23T22:57:08.953223Z INFO some_module: doing something", + wantNil: true, + }, + { + name: "no match - error without Some pattern", + input: "2026-01-23T22:57:08.953223Z ERROR some_module: error=something went wrong", + wantNil: true, + }, + { + name: "rate limit error - full format", + input: `2026-01-23T22:57:08.953223Z ERROR codex_api::endpoint::responses: error=http 429 Too Many Requests: Some("{\"error\":{\"type\":\"usage_limit_reached\",\"message\":\"The usage limit has been reached\",\"resets_in_seconds\":57600}}")`, + wantNil: false, + wantHTTPError: "http 429 Too Many Requests", + wantErrorType: "usage_limit_reached", + wantMsgContains: "The usage limit has been reached", + wantRawJSON: true, + }, + { + name: "rate limit error - with reset time in hours", + input: `error=http 429 Too Many Requests: Some("{\"error\":{\"type\":\"usage_limit_reached\",\"message\":\"Limit reached\",\"resets_in_seconds\":7200}}")`, + wantNil: false, + wantHTTPError: "http 429 Too Many Requests", + wantErrorType: "usage_limit_reached", + wantMsgContains: "resets in 2 hours", + wantRawJSON: true, + }, + { + name: "rate limit error - with reset time in minutes", + input: `error=http 429 Too Many Requests: Some("{\"error\":{\"type\":\"usage_limit_reached\",\"message\":\"Limit reached\",\"resets_in_seconds\":300}}")`, + wantNil: false, + wantHTTPError: "http 429 Too Many Requests", + wantMsgContains: "resets in 5 minutes", + wantRawJSON: true, + }, + { + name: "rate limit error - with reset time in seconds", + input: `error=http 429 Too Many Requests: Some("{\"error\":{\"type\":\"usage_limit_reached\",\"message\":\"Limit reached\",\"resets_in_seconds\":45}}")`, + wantNil: false, + wantHTTPError: "http 429 Too Many Requests", + wantMsgContains: "resets in 45 seconds", + wantRawJSON: true, + }, + { + name: "auth error", + input: `error=http 401 Unauthorized: Some("{\"error\":{\"type\":\"invalid_api_key\",\"message\":\"Invalid API key provided\"}}")`, + wantNil: false, + wantHTTPError: "http 401 Unauthorized", + wantErrorType: "invalid_api_key", + wantMsgContains: "Invalid API key provided", + wantRawJSON: true, + }, + { + name: "error with type but no message", + input: `error=http 400 Bad Request: Some("{\"error\":{\"type\":\"invalid_request\"}}")`, + wantNil: false, + wantHTTPError: "http 400 Bad Request", + wantErrorType: "invalid_request", + wantMsgContains: "Error: invalid_request", + wantRawJSON: true, + }, + { + name: "flat error structure - message at top level", + input: `error=http 500 Internal Server Error: Some("{\"message\":\"Something went wrong\",\"code\":500}")`, + wantNil: false, + wantHTTPError: "http 500 Internal Server Error", + wantMsgContains: "Something went wrong", + wantRawJSON: true, + }, + { + name: "unknown JSON structure", + input: `error=http 502 Bad Gateway: Some("{\"status\":\"error\",\"details\":{\"reason\":\"upstream timeout\"}}")`, + wantNil: false, + wantHTTPError: "http 502 Bad Gateway", + wantMsgContains: "http 502 Bad Gateway", + wantRawJSON: true, + }, + { + name: "invalid JSON - still returns HTTP error", + input: `error=http 500 Internal Server Error: Some("not valid json")`, + wantNil: false, + wantHTTPError: "http 500 Internal Server Error", + wantMsgContains: "http 500 Internal Server Error", + wantRawJSON: false, + }, + { + name: "empty JSON object", + input: `error=http 500 Internal Server Error: Some("{}")`, + wantNil: false, + wantHTTPError: "http 500 Internal Server Error", + wantMsgContains: "http 500 Internal Server Error", + wantRawJSON: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseCodexStderrError(tt.input) + + if tt.wantNil { + if result != nil { + t.Errorf("expected nil, got %+v", result) + } + return + } + + if result == nil { + t.Fatal("expected non-nil result, got nil") + } + + if result.HTTPError != tt.wantHTTPError { + t.Errorf("HTTPError = %q, want %q", result.HTTPError, tt.wantHTTPError) + } + + if tt.wantErrorType != "" && result.ErrorType != tt.wantErrorType { + t.Errorf("ErrorType = %q, want %q", result.ErrorType, tt.wantErrorType) + } + + if !strings.Contains(result.Message, tt.wantMsgContains) { + t.Errorf("Message = %q, want to contain %q", result.Message, tt.wantMsgContains) + } + + if tt.wantRawJSON && result.RawJSON == nil { + t.Error("expected RawJSON to be non-nil") + } + if !tt.wantRawJSON && result.RawJSON != nil { + t.Error("expected RawJSON to be nil") + } + }) + } +} + +func TestParseCodexStderrLines(t *testing.T) { + tests := []struct { + name string + lines []string + wantNil bool + wantMsgContains string + }{ + { + name: "empty lines", + lines: []string{}, + wantNil: true, + }, + { + name: "nil lines", + lines: nil, + wantNil: true, + }, + { + name: "no error lines", + lines: []string{ + "2026-01-23T22:57:08Z INFO starting up", + "2026-01-23T22:57:09Z DEBUG processing request", + }, + wantNil: true, + }, + { + name: "error at end - returns most recent", + lines: []string{ + "2026-01-23T22:57:08Z INFO starting up", + `error=http 429 Too Many Requests: Some("{\"error\":{\"message\":\"Rate limited\"}}")`, + }, + wantNil: false, + wantMsgContains: "Rate limited", + }, + { + name: "error in middle - returns it", + lines: []string{ + `error=http 500 Server Error: Some("{\"error\":{\"message\":\"Server error\"}}")`, + "2026-01-23T22:57:09Z INFO recovered", + }, + wantNil: false, + wantMsgContains: "Server error", + }, + { + name: "multiple errors - returns most recent (last)", + lines: []string{ + `error=http 500 Server Error: Some("{\"error\":{\"message\":\"First error\"}}")`, + "2026-01-23T22:57:09Z INFO retrying", + `error=http 429 Rate Limited: Some("{\"error\":{\"message\":\"Second error\"}}")`, + }, + wantNil: false, + wantMsgContains: "Second error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseCodexStderrLines(tt.lines) + + if tt.wantNil { + if result != nil { + t.Errorf("expected nil, got %+v", result) + } + return + } + + if result == nil { + t.Fatal("expected non-nil result, got nil") + } + + if !strings.Contains(result.Message, tt.wantMsgContains) { + t.Errorf("Message = %q, want to contain %q", result.Message, tt.wantMsgContains) + } + }) + } +} + +func TestParseCodexStderrError_ResetsInSeconds(t *testing.T) { + // Test that ResetsInSeconds is correctly extracted + input := `error=http 429 Too Many Requests: Some("{\"error\":{\"type\":\"usage_limit_reached\",\"resets_in_seconds\":3600}}")` + + result := parseCodexStderrError(input) + if result == nil { + t.Fatal("expected non-nil result") + } + + if result.ResetsInSeconds != 3600 { + t.Errorf("ResetsInSeconds = %d, want 3600", result.ResetsInSeconds) + } +} diff --git a/apps/backend/internal/agentctl/server/process/manager.go b/apps/backend/internal/agentctl/server/process/manager.go index ff563997a9..82c6ac9dd0 100644 --- a/apps/backend/internal/agentctl/server/process/manager.go +++ b/apps/backend/internal/agentctl/server/process/manager.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os/exec" + "regexp" "strings" "sync" "sync/atomic" @@ -58,6 +59,9 @@ type PermissionNotification struct { CreatedAt time.Time `json:"created_at"` } +// defaultStderrBufferSize is the number of recent stderr lines to keep for error context +const defaultStderrBufferSize = 50 + // Manager manages the agent subprocess type Manager struct { cfg *config.InstanceConfig @@ -72,6 +76,10 @@ type Manager struct { exitCode atomic.Int32 exitErr atomic.Value // error + // Stderr buffering for error context + stderrBuffer []string + stderrMu sync.RWMutex + // Workspace tracker for git status and file changes workspaceTracker *WorkspaceTracker @@ -386,7 +394,10 @@ func (m *Manager) createAdapter() error { case agent.ProtocolACP: m.adapter = adapter.NewACPAdapter(m.adapterCfg, m.logger) case agent.ProtocolCodex: - m.adapter = adapter.NewCodexAdapter(m.adapterCfg, m.logger) + codexAdapter := adapter.NewCodexAdapter(m.adapterCfg, m.logger) + // Set stderr provider so errors can include recent stderr output + codexAdapter.SetStderrProvider(m) + m.adapter = codexAdapter default: return fmt.Errorf("unsupported protocol: %s", protocol) } @@ -528,6 +539,9 @@ func (m *Manager) readStderr() { for scanner.Scan() { line := scanner.Text() m.logger.Debug("agent stderr", zap.String("line", line)) + + // Buffer the line for error context + m.appendStderr(line) } if err := scanner.Err(); err != nil { @@ -535,6 +549,46 @@ func (m *Manager) readStderr() { } } +// ansiEscapeRegex matches ANSI escape sequences +var ansiEscapeRegex = regexp.MustCompile(`\x1b\[[0-9;]*[a-zA-Z]`) + +// stripANSI removes ANSI escape codes from a string +func stripANSI(s string) string { + return ansiEscapeRegex.ReplaceAllString(s, "") +} + +// appendStderr adds a line to the stderr ring buffer +func (m *Manager) appendStderr(line string) { + m.stderrMu.Lock() + defer m.stderrMu.Unlock() + + // Strip ANSI escape codes for cleaner display + cleanLine := stripANSI(line) + + if len(m.stderrBuffer) >= defaultStderrBufferSize { + // Ring buffer: drop oldest line + m.stderrBuffer = m.stderrBuffer[1:] + } + m.stderrBuffer = append(m.stderrBuffer, cleanLine) +} + +// GetRecentStderr returns a copy of the recent stderr lines +func (m *Manager) GetRecentStderr() []string { + m.stderrMu.RLock() + defer m.stderrMu.RUnlock() + + result := make([]string, len(m.stderrBuffer)) + copy(result, m.stderrBuffer) + return result +} + +// ClearStderrBuffer clears the stderr buffer (e.g., after successful operation) +func (m *Manager) ClearStderrBuffer() { + m.stderrMu.Lock() + defer m.stderrMu.Unlock() + m.stderrBuffer = nil +} + // waitForExit waits for the process to exit func (m *Manager) waitForExit() { defer m.wg.Done() diff --git a/apps/backend/internal/agentctl/server/process/runner_test.go b/apps/backend/internal/agentctl/server/process/runner_test.go index f2053891e3..08dc286169 100644 --- a/apps/backend/internal/agentctl/server/process/runner_test.go +++ b/apps/backend/internal/agentctl/server/process/runner_test.go @@ -76,3 +76,76 @@ func TestProcessRunnerCapturesOutput(t *testing.T) { } t.Fatal("process output not captured in time") } + +func TestStripANSI(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "no ANSI codes", + input: "hello world", + expected: "hello world", + }, + { + name: "empty string", + input: "", + expected: "", + }, + { + name: "simple color code", + input: "\x1b[31mred text\x1b[0m", + expected: "red text", + }, + { + name: "bold text", + input: "\x1b[1mbold\x1b[0m", + expected: "bold", + }, + { + name: "multiple colors", + input: "\x1b[31mred\x1b[32mgreen\x1b[34mblue\x1b[0m", + expected: "redgreenblue", + }, + { + name: "256 color code", + input: "\x1b[38;5;196mcolored\x1b[0m", + expected: "colored", + }, + { + name: "RGB color code", + input: "\x1b[38;2;255;0;0mred\x1b[0m", + expected: "red", + }, + { + name: "cursor movement", + input: "\x1b[2Amove up\x1b[3Bmove down", + expected: "move upmove down", + }, + { + name: "clear line", + input: "text\x1b[2Kcleared", + expected: "textcleared", + }, + { + name: "real world npm output", + input: "\x1b[32m✓\x1b[39m \x1b[90mCompiled successfully\x1b[39m", + expected: "✓ Compiled successfully", + }, + { + name: "mixed with newlines", + input: "\x1b[31mError:\x1b[0m\nSomething failed\n\x1b[33mWarning:\x1b[0m check logs", + expected: "Error:\nSomething failed\nWarning: check logs", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := stripANSI(tt.input) + if result != tt.expected { + t.Errorf("stripANSI(%q) = %q, want %q", tt.input, result, tt.expected) + } + }) + } +} diff --git a/apps/backend/internal/common/constants/timeouts.go b/apps/backend/internal/common/constants/timeouts.go new file mode 100644 index 0000000000..833cadd612 --- /dev/null +++ b/apps/backend/internal/common/constants/timeouts.go @@ -0,0 +1,21 @@ +// Package constants provides application-wide constants and timeouts. +package constants + +import "time" + +// Timeouts for various operations. +const ( + // AgentLaunchTimeout is the maximum time to wait for agent launch, + // including worktree creation and setup script execution. + AgentLaunchTimeout = 6 * time.Minute + + // SetupScriptTimeout is the maximum time to wait for a setup script to complete. + SetupScriptTimeout = 5 * time.Minute + + // CleanupScriptTimeout is the maximum time to wait for a cleanup script to complete. + CleanupScriptTimeout = 5 * time.Minute + + // TaskDeleteTimeout is the maximum time to wait for task deletion, + // including cleanup scripts and worktree removal. + TaskDeleteTimeout = 2 * time.Minute +) diff --git a/apps/backend/internal/orchestrator/event_handlers.go b/apps/backend/internal/orchestrator/event_handlers.go index acfd067763..2cc2d2f786 100644 --- a/apps/backend/internal/orchestrator/event_handlers.go +++ b/apps/backend/internal/orchestrator/event_handlers.go @@ -150,18 +150,36 @@ func (s *Service) handleAgentStreamEvent(ctx context.Context, payload *lifecycle case "error": // Handle error events + // Note: Agent-specific error parsing is done in the adapter layer. + // The adapter provides a user-friendly message in the Text field, + // with raw details in the Data field for debugging. if sessionID != "" && s.messageCreator != nil { + // Build error message - adapters should provide parsed message in Text field + errorMsg := payload.Data.Error + if errorMsg == "" { + errorMsg = payload.Data.Text + } + if errorMsg == "" { + errorMsg = "An error occurred while processing your request" + } + + // Build metadata with all available error context + metadata := map[string]interface{}{ + "provider": "agent", + "provider_agent": payload.AgentID, + } + if payload.Data.Data != nil { + metadata["error_data"] = payload.Data.Data + } + if err := s.messageCreator.CreateSessionMessage( ctx, taskID, - payload.Data.Error, + errorMsg, sessionID, string(v1.MessageTypeError), s.getActiveTurnID(sessionID), - map[string]interface{}{ - "provider": "agent", - "provider_agent": payload.AgentID, - }, + metadata, false, ); err != nil { s.logger.Error("failed to create error message", diff --git a/apps/backend/internal/task/handlers/process_handlers.go b/apps/backend/internal/task/handlers/process_handlers.go index 73593fd3d9..02d7dfe181 100644 --- a/apps/backend/internal/task/handlers/process_handlers.go +++ b/apps/backend/internal/task/handlers/process_handlers.go @@ -364,8 +364,15 @@ func (h *ProcessHandlers) httpListProcesses(c *gin.Context) { procs, err := h.lifecycleMgr.ListProcesses(listCtx, sessionID) if err != nil { var netErr net.Error - if errors.Is(err, context.DeadlineExceeded) || errors.As(err, &netErr) || strings.Contains(err.Error(), "connection refused") { - h.logger.Warn("process list unavailable", zap.String("session_id", sessionID), zap.Error(err)) + // Handle expected "no processes" conditions gracefully: + // - no execution found: agent hasn't started yet (async launch) + // - connection refused: agent not running + // - deadline exceeded: agent not responding + if errors.Is(err, context.DeadlineExceeded) || + errors.As(err, &netErr) || + strings.Contains(err.Error(), "connection refused") || + strings.Contains(err.Error(), "no execution found") { + h.logger.Debug("process list unavailable (agent not ready)", zap.String("session_id", sessionID), zap.Error(err)) c.JSON(http.StatusOK, []agentctlclient.ProcessInfo{}) return } diff --git a/apps/backend/internal/task/handlers/task_handlers.go b/apps/backend/internal/task/handlers/task_handlers.go index f1308b99ec..7982d47b94 100644 --- a/apps/backend/internal/task/handlers/task_handlers.go +++ b/apps/backend/internal/task/handlers/task_handlers.go @@ -3,10 +3,10 @@ package handlers import ( "context" "net/http" - "time" "github.com/gin-gonic/gin" + "github.com/kandev/kandev/internal/common/constants" "github.com/kandev/kandev/internal/common/logger" "github.com/kandev/kandev/internal/orchestrator/executor" "github.com/kandev/kandev/internal/task/controller" @@ -184,17 +184,24 @@ func (h *TaskHandlers) httpCreateTask(c *gin.Context) { response := createTaskResponse{TaskDTO: resp} if body.StartAgent && body.AgentProfileID != "" && h.orchestrator != nil { - startCtx, cancel := context.WithTimeout(context.Background(), 15*time.Second) - defer cancel() - // Use task description as the initial prompt - execution, err := h.orchestrator.StartTask(startCtx, resp.ID, body.AgentProfileID, body.Priority, resp.Description) - if err != nil { - h.logger.Error("failed to start agent for task", zap.Error(err)) - c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to start agent for task"}) - return - } - response.TaskSessionID = execution.SessionID - response.AgentExecutionID = execution.AgentExecutionID + // Launch agent asynchronously so the HTTP request can return immediately. + // The frontend will receive WebSocket updates when the agent actually starts. + // This allows the "new task" dialog to close right away while setup scripts run. + go func() { + // Use a longer timeout to accommodate setup scripts (which can take minutes for npm install, etc.) + startCtx, cancel := context.WithTimeout(context.Background(), constants.AgentLaunchTimeout) + defer cancel() + // Use task description as the initial prompt + execution, err := h.orchestrator.StartTask(startCtx, resp.ID, body.AgentProfileID, body.Priority, resp.Description) + if err != nil { + h.logger.Error("failed to start agent for task (async)", zap.Error(err), zap.String("task_id", resp.ID)) + return + } + h.logger.Info("agent started for task (async)", + zap.String("task_id", resp.ID), + zap.String("session_id", execution.SessionID), + zap.String("execution_id", execution.AgentExecutionID)) + }() } c.JSON(http.StatusOK, response) @@ -278,7 +285,7 @@ func (h *TaskHandlers) httpMoveTask(c *gin.Context) { } func (h *TaskHandlers) httpDeleteTask(c *gin.Context) { - deleteCtx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + deleteCtx, cancel := context.WithTimeout(context.Background(), constants.TaskDeleteTimeout) defer cancel() resp, err := h.controller.DeleteTask(deleteCtx, dto.DeleteTaskRequest{ID: c.Param("id")}) if err != nil { diff --git a/apps/backend/internal/task/models/models.go b/apps/backend/internal/task/models/models.go index 8bc7ffaff5..5f37afce18 100644 --- a/apps/backend/internal/task/models/models.go +++ b/apps/backend/internal/task/models/models.go @@ -106,6 +106,8 @@ const ( MessageTypeStatus MessageType = "status" // MessageTypePermissionRequest is for agent permission requests MessageTypePermissionRequest MessageType = "permission_request" + // MessageTypeScriptExecution is for setup/cleanup script execution messages + MessageTypeScriptExecution MessageType = "script_execution" ) // Message represents a message in a task session diff --git a/apps/backend/internal/task/service/service.go b/apps/backend/internal/task/service/service.go index e494b68077..2dcc9be0b2 100644 --- a/apps/backend/internal/task/service/service.go +++ b/apps/backend/internal/task/service/service.go @@ -1755,6 +1755,21 @@ func (s *Service) DeleteMessage(ctx context.Context, id string) error { return nil } +// UpdateMessage updates an existing message and publishes an event. +func (s *Service) UpdateMessage(ctx context.Context, message *models.Message) error { + if err := s.repo.UpdateMessage(ctx, message); err != nil { + s.logger.Error("failed to update message", + zap.String("message_id", message.ID), + zap.Error(err)) + return err + } + + // Publish message.updated event for real-time streaming + s.publishMessageEvent(ctx, events.MessageUpdated, message) + + return nil +} + // AppendMessageContent appends additional content to an existing message. // This is used for streaming agent responses where content arrives incrementally. func (s *Service) AppendMessageContent(ctx context.Context, messageID, additionalContent string) error { diff --git a/apps/backend/internal/worktree/manager.go b/apps/backend/internal/worktree/manager.go index e56ef6c80b..d1d2ace085 100644 --- a/apps/backend/internal/worktree/manager.go +++ b/apps/backend/internal/worktree/manager.go @@ -25,6 +25,16 @@ type Manager struct { mu sync.RWMutex // Protects worktrees map repoLocks map[string]*sync.Mutex repoLockMu sync.Mutex + + // Optional dependencies for script execution + repoProvider RepositoryProvider + scriptMsgHandler ScriptMessageHandler +} + +// ScriptMessageHandler provides script execution and message streaming. +type ScriptMessageHandler interface { + ExecuteSetupScript(ctx context.Context, req ScriptExecutionRequest) error + ExecuteCleanupScript(ctx context.Context, req ScriptExecutionRequest) error } // Store is the interface for worktree persistence. @@ -75,6 +85,16 @@ func NewManager(cfg Config, store Store, log *logger.Logger) (*Manager, error) { }, nil } +// SetRepositoryProvider sets the repository provider for fetching repository information. +func (m *Manager) SetRepositoryProvider(provider RepositoryProvider) { + m.repoProvider = provider +} + +// SetScriptMessageHandler sets the script message handler for executing setup/cleanup scripts. +func (m *Manager) SetScriptMessageHandler(handler ScriptMessageHandler) { + m.scriptMsgHandler = handler +} + // getRepoLock returns a mutex for the given repository path. func (m *Manager) getRepoLock(repoPath string) *sync.Mutex { m.repoLockMu.Lock() @@ -263,6 +283,66 @@ func (m *Manager) createWorktree(ctx context.Context, req CreateRequest) (*Workt m.mu.Unlock() } + // Execute setup script if configured + if m.scriptMsgHandler != nil && m.repoProvider != nil { + repo, err := m.repoProvider.GetRepository(ctx, wt.RepositoryID) + if err != nil { + m.logger.Warn("failed to fetch repository for setup script", + zap.String("repository_id", wt.RepositoryID), + zap.Error(err)) + } else if strings.TrimSpace(repo.SetupScript) != "" { + m.logger.Info("executing setup script for worktree", + zap.String("worktree_id", wt.ID), + zap.String("repository_id", wt.RepositoryID)) + + scriptReq := ScriptExecutionRequest{ + SessionID: wt.SessionID, + TaskID: wt.TaskID, + RepositoryID: wt.RepositoryID, + Script: repo.SetupScript, + WorkingDir: wt.Path, + ScriptType: "setup", + } + + if err := m.scriptMsgHandler.ExecuteSetupScript(ctx, scriptReq); err != nil { + // Setup failed - cleanup worktree + m.logger.Error("setup script failed, cleaning up worktree", + zap.String("worktree_id", wt.ID), + zap.Error(err)) + + // Remove from cache + if wt.SessionID != "" { + m.mu.Lock() + delete(m.worktrees, wt.SessionID) + m.mu.Unlock() + } + + // Remove directory + if cleanupErr := m.removeWorktreeDir(ctx, wt.Path, req.RepositoryPath); cleanupErr != nil { + m.logger.Warn("failed to cleanup worktree after setup failure", + zap.Error(cleanupErr)) + } + + // Mark as deleted in database + if m.store != nil { + now := time.Now() + wt.Status = StatusDeleted + wt.DeletedAt = &now + wt.UpdatedAt = now + if updateErr := m.store.UpdateWorktree(ctx, wt); updateErr != nil { + m.logger.Warn("failed to update worktree status", + zap.Error(updateErr)) + } + } + + return nil, fmt.Errorf("setup script failed: %w", err) + } + + m.logger.Info("setup script completed successfully", + zap.String("worktree_id", wt.ID)) + } + } + m.logger.Info("created worktree", zap.String("session_id", req.SessionID), zap.String("task_id", req.TaskID), @@ -363,6 +443,39 @@ func (m *Manager) removeWorktree(ctx context.Context, wt *Worktree, removeBranch repoLock.Lock() defer repoLock.Unlock() + // Execute cleanup script BEFORE removing directory + if m.scriptMsgHandler != nil && m.repoProvider != nil { + repo, err := m.repoProvider.GetRepository(ctx, wt.RepositoryID) + if err != nil { + m.logger.Warn("failed to fetch repository for cleanup script", + zap.String("repository_id", wt.RepositoryID), + zap.Error(err)) + } else if strings.TrimSpace(repo.CleanupScript) != "" { + m.logger.Info("executing cleanup script for worktree", + zap.String("worktree_id", wt.ID), + zap.String("repository_id", wt.RepositoryID)) + + scriptReq := ScriptExecutionRequest{ + SessionID: wt.SessionID, + TaskID: wt.TaskID, + RepositoryID: wt.RepositoryID, + Script: repo.CleanupScript, + WorkingDir: wt.Path, + ScriptType: "cleanup", + } + + if err := m.scriptMsgHandler.ExecuteCleanupScript(ctx, scriptReq); err != nil { + // Log error but continue with deletion + m.logger.Warn("cleanup script failed, proceeding with deletion", + zap.String("worktree_id", wt.ID), + zap.Error(err)) + } else { + m.logger.Info("cleanup script completed successfully", + zap.String("worktree_id", wt.ID)) + } + } + } + // Remove worktree directory if err := m.removeWorktreeDir(ctx, wt.Path, wt.RepositoryPath); err != nil { m.logger.Warn("failed to remove worktree directory", diff --git a/apps/backend/internal/worktree/repository_adapter.go b/apps/backend/internal/worktree/repository_adapter.go new file mode 100644 index 0000000000..e34e6733ad --- /dev/null +++ b/apps/backend/internal/worktree/repository_adapter.go @@ -0,0 +1,50 @@ +package worktree + +import ( + "context" + + "github.com/kandev/kandev/internal/task/models" +) + +// Repository contains repository information needed for script execution. +type Repository struct { + ID string + SetupScript string + CleanupScript string +} + +// RepositoryProvider provides access to repository information. +type RepositoryProvider interface { + GetRepository(ctx context.Context, repositoryID string) (*Repository, error) +} + +// RepositoryService interface for task service repository operations. +type RepositoryService interface { + GetRepository(ctx context.Context, id string) (*models.Repository, error) +} + +// RepositoryAdapter adapts the task service repository interface to the worktree manager's needs. +type RepositoryAdapter struct { + repoService RepositoryService +} + +// NewRepositoryAdapter creates a new RepositoryAdapter. +func NewRepositoryAdapter(repoService RepositoryService) *RepositoryAdapter { + return &RepositoryAdapter{ + repoService: repoService, + } +} + +// GetRepository fetches repository information from the task service. +func (a *RepositoryAdapter) GetRepository(ctx context.Context, repositoryID string) (*Repository, error) { + repo, err := a.repoService.GetRepository(ctx, repositoryID) + if err != nil { + return nil, err + } + + return &Repository{ + ID: repo.ID, + SetupScript: repo.SetupScript, + CleanupScript: repo.CleanupScript, + }, nil +} diff --git a/apps/backend/internal/worktree/script_message_handler.go b/apps/backend/internal/worktree/script_message_handler.go new file mode 100644 index 0000000000..263c6f03d1 --- /dev/null +++ b/apps/backend/internal/worktree/script_message_handler.go @@ -0,0 +1,360 @@ +package worktree + +import ( + "bytes" + "context" + "fmt" + "io" + "os/exec" + "sync" + "time" + + "go.uber.org/zap" + + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/task/models" +) + +// ScriptExecutionRequest contains parameters for executing a setup or cleanup script. +type ScriptExecutionRequest struct { + SessionID string + TaskID string + RepositoryID string + Script string + WorkingDir string + ScriptType string // "setup" or "cleanup" +} + +// DefaultScriptMessageHandler manages script execution and streams output to session messages. +type DefaultScriptMessageHandler struct { + logger *logger.Logger + taskService TaskService + timeout time.Duration +} + +// TaskService interface for creating and updating messages. +type TaskService interface { + CreateMessage(ctx context.Context, req *CreateMessageRequest) (*models.Message, error) + UpdateMessage(ctx context.Context, message *models.Message) error +} + +// CreateMessageRequest contains parameters for creating a message. +type CreateMessageRequest struct { + TaskSessionID string + TaskID string + TurnID string + Content string + AuthorType string + AuthorID string + RequestsInput bool + Type string + Metadata map[string]interface{} +} + +// NewDefaultScriptMessageHandler creates a new DefaultScriptMessageHandler. +func NewDefaultScriptMessageHandler( + log *logger.Logger, + taskSvc TaskService, + timeout time.Duration, +) *DefaultScriptMessageHandler { + return &DefaultScriptMessageHandler{ + logger: log.WithFields(zap.String("component", "script-message-handler")), + taskService: taskSvc, + timeout: timeout, + } +} + +// ExecuteSetupScript executes a setup script and streams output to a session message. +// Returns an error if the script fails (non-zero exit code or timeout). +func (h *DefaultScriptMessageHandler) ExecuteSetupScript(ctx context.Context, req ScriptExecutionRequest) error { + return h.executeScript(ctx, req, true) +} + +// ExecuteCleanupScript executes a cleanup script and streams output to a session message. +// Returns nil even if the script fails (best-effort cleanup). +func (h *DefaultScriptMessageHandler) ExecuteCleanupScript(ctx context.Context, req ScriptExecutionRequest) error { + err := h.executeScript(ctx, req, false) + if err != nil { + h.logger.Warn("cleanup script failed, continuing with deletion", + zap.String("session_id", req.SessionID), + zap.Error(err)) + return nil + } + return nil +} + +// executeScript is the core implementation for script execution. +// Note: The parent context is intentionally not used - we create a detached context +// to prevent HTTP request timeouts from cancelling long-running scripts. +func (h *DefaultScriptMessageHandler) executeScript(_ context.Context, req ScriptExecutionRequest, failOnError bool) error { + if h.taskService == nil { + h.logger.Debug("script handler not fully configured, skipping", + zap.String("script_type", req.ScriptType)) + return nil + } + + // Create a detached context for script execution with its own timeout. + // This prevents the HTTP request context from cancelling the script. + scriptCtx, cancel := context.WithTimeout(context.Background(), h.timeout) + defer cancel() + + // Create initial message (best-effort - session may not exist during cleanup) + msg, err := h.createScriptMessage(scriptCtx, req) + if err != nil { + // For cleanup scripts, if session doesn't exist, run the script anyway without a message + if req.ScriptType == "cleanup" { + h.logger.Warn("failed to create cleanup script message, running script without message tracking", + zap.String("session_id", req.SessionID), + zap.Error(err)) + // Run script directly without message tracking + return h.runScriptWithoutMessage(scriptCtx, req, failOnError) + } + // For setup scripts, this is a hard error + return fmt.Errorf("failed to create script message: %w", err) + } + + h.logger.Info("created script execution message", + zap.String("message_id", msg.ID), + zap.String("session_id", req.SessionID), + zap.String("script_type", req.ScriptType)) + + // Update message status to running + msg.Metadata["status"] = "running" + if err := h.taskService.UpdateMessage(scriptCtx, msg); err != nil { + h.logger.Warn("failed to update message with running status", + zap.String("message_id", msg.ID), + zap.Error(err)) + } + + // Execute the script and capture output + exitCode, scriptErr := h.runScriptWithOutput(scriptCtx, req, msg) + + // Update final status + if scriptErr != nil { + msg.Metadata["status"] = "failed" + msg.Metadata["error"] = scriptErr.Error() + if msg.Content == "" { + msg.Content = fmt.Sprintf("Script execution failed: %s", scriptErr.Error()) + } else { + msg.Content += fmt.Sprintf("\n\nScript execution failed: %s", scriptErr.Error()) + } + } else if exitCode == 0 { + msg.Metadata["status"] = "exited" + msg.Metadata["exit_code"] = exitCode + if msg.Content == "" { + msg.Content = "Script completed successfully" + } + } else { + msg.Metadata["status"] = "failed" + msg.Metadata["exit_code"] = exitCode + if msg.Content == "" { + msg.Content = fmt.Sprintf("Script failed with exit code: %d", exitCode) + } else { + msg.Content += fmt.Sprintf("\n\nScript failed with exit code: %d", exitCode) + } + } + + msg.Metadata["completed_at"] = time.Now().UTC().Format(time.RFC3339Nano) + if updateErr := h.taskService.UpdateMessage(scriptCtx, msg); updateErr != nil { + h.logger.Warn("failed to update message with final status", + zap.String("message_id", msg.ID), + zap.Error(updateErr)) + } + + h.logger.Info("script execution completed", + zap.String("message_id", msg.ID), + zap.Int("exit_code", exitCode), + zap.Bool("success", exitCode == 0)) + + // Return error if setup script failed + if failOnError && (scriptErr != nil || exitCode != 0) { + if scriptErr != nil { + return scriptErr + } + return fmt.Errorf("script exited with code %d", exitCode) + } + + return nil +} + +// createScriptMessage creates the initial script execution message. +func (h *DefaultScriptMessageHandler) createScriptMessage(ctx context.Context, req ScriptExecutionRequest) (*models.Message, error) { + metadata := map[string]interface{}{ + "script_type": req.ScriptType, + "command": req.Script, + "status": "starting", + "started_at": time.Now().UTC().Format(time.RFC3339Nano), + } + + createReq := &CreateMessageRequest{ + TaskSessionID: req.SessionID, + TaskID: req.TaskID, + Content: "", // Will be populated with output + AuthorType: "agent", + Type: "script_execution", + Metadata: metadata, + } + + return h.taskService.CreateMessage(ctx, createReq) +} + +// runScriptWithOutput runs the script and captures output, streaming it to the message. +// The passed context should already have an appropriate timeout set. +func (h *DefaultScriptMessageHandler) runScriptWithOutput(ctx context.Context, req ScriptExecutionRequest, msg *models.Message) (int, error) { + // Run script with sh -c to support complex commands + cmd := exec.CommandContext(ctx, "sh", "-c", req.Script) + cmd.Dir = req.WorkingDir + + // Create pipes for stdout and stderr + stdoutPipe, err := cmd.StdoutPipe() + if err != nil { + return -1, fmt.Errorf("failed to create stdout pipe: %w", err) + } + stderrPipe, err := cmd.StderrPipe() + if err != nil { + return -1, fmt.Errorf("failed to create stderr pipe: %w", err) + } + + // Start the command + if err := cmd.Start(); err != nil { + return -1, fmt.Errorf("failed to start script: %w", err) + } + + h.logger.Info("script process started", + zap.String("message_id", msg.ID), + zap.String("command", req.Script)) + + // Stream output + var outputBuf bytes.Buffer + var wg sync.WaitGroup + var mu sync.Mutex + + // Stream stdout + wg.Add(1) + go func() { + defer wg.Done() + buf := make([]byte, 1024) + for { + n, err := stdoutPipe.Read(buf) + if n > 0 { + mu.Lock() + outputBuf.Write(buf[:n]) + msg.Content = outputBuf.String() + mu.Unlock() + + // Update message with incremental output (best-effort) + if updateErr := h.taskService.UpdateMessage(context.Background(), msg); updateErr != nil { + h.logger.Debug("failed to update message with output", + zap.String("message_id", msg.ID), + zap.Error(updateErr)) + } + } + if err != nil { + if err != io.EOF { + h.logger.Debug("error reading stdout", + zap.Error(err)) + } + break + } + } + }() + + // Stream stderr + wg.Add(1) + go func() { + defer wg.Done() + buf := make([]byte, 1024) + for { + n, err := stderrPipe.Read(buf) + if n > 0 { + mu.Lock() + outputBuf.Write(buf[:n]) + msg.Content = outputBuf.String() + mu.Unlock() + + // Update message with incremental output (best-effort) + if updateErr := h.taskService.UpdateMessage(context.Background(), msg); updateErr != nil { + h.logger.Debug("failed to update message with output", + zap.String("message_id", msg.ID), + zap.Error(updateErr)) + } + } + if err != nil { + if err != io.EOF { + h.logger.Debug("error reading stderr", + zap.Error(err)) + } + break + } + } + }() + + // Wait for output streaming to finish + wg.Wait() + + // Wait for command to complete + err = cmd.Wait() + + // Get exit code + exitCode := 0 + if err != nil { + if exitErr, ok := err.(*exec.ExitError); ok { + exitCode = exitErr.ExitCode() + } else { + // Other error (e.g., timeout, command not found) + return -1, err + } + } + + return exitCode, nil +} + +// runScriptWithoutMessage runs a script without message tracking (used when session is deleted). +// The passed context should already have an appropriate timeout set. +func (h *DefaultScriptMessageHandler) runScriptWithoutMessage(ctx context.Context, req ScriptExecutionRequest, failOnError bool) error { + h.logger.Info("executing script without message tracking", + zap.String("script_type", req.ScriptType), + zap.String("command", req.Script)) + + // Run script with sh -c + cmd := exec.CommandContext(ctx, "sh", "-c", req.Script) + cmd.Dir = req.WorkingDir + + // Capture output for logging + output, err := cmd.CombinedOutput() + + // Get exit code + exitCode := 0 + if err != nil { + if exitErr, ok := err.(*exec.ExitError); ok { + exitCode = exitErr.ExitCode() + } else { + h.logger.Error("script execution failed", + zap.String("script_type", req.ScriptType), + zap.Error(err), + zap.String("output", string(output))) + if failOnError { + return err + } + return nil + } + } + + // Log the result + if exitCode == 0 { + h.logger.Info("script completed successfully", + zap.String("script_type", req.ScriptType), + zap.Int("exit_code", exitCode), + zap.String("output", string(output))) + } else { + h.logger.Warn("script failed", + zap.String("script_type", req.ScriptType), + zap.Int("exit_code", exitCode), + zap.String("output", string(output))) + if failOnError { + return fmt.Errorf("script exited with code %d", exitCode) + } + } + + return nil +} diff --git a/apps/web/components/task/chat/message-renderer.tsx b/apps/web/components/task/chat/message-renderer.tsx index f5aa9e76f5..7caf504d3b 100644 --- a/apps/web/components/task/chat/message-renderer.tsx +++ b/apps/web/components/task/chat/message-renderer.tsx @@ -8,6 +8,7 @@ import { StatusMessage } from '@/components/task/chat/messages/status-message'; import { ToolCallMessage } from '@/components/task/chat/messages/tool-call-message'; import { ThinkingMessage } from '@/components/task/chat/messages/thinking-message'; import { TodoMessage } from '@/components/task/chat/messages/todo-message'; +import { ScriptExecutionMessage } from '@/components/task/chat/messages/script-execution-message'; type AdapterContext = { isTaskDescription: boolean; @@ -46,6 +47,10 @@ const adapters: MessageAdapter[] = [ matches: (comment) => comment.type === 'permission_request', render: (comment) => , }, + { + matches: (comment) => comment.type === 'script_execution', + render: (comment) => , + }, { matches: () => true, render: (comment, ctx) => { diff --git a/apps/web/components/task/chat/messages/script-execution-message.tsx b/apps/web/components/task/chat/messages/script-execution-message.tsx new file mode 100644 index 0000000000..a9b4993f40 --- /dev/null +++ b/apps/web/components/task/chat/messages/script-execution-message.tsx @@ -0,0 +1,155 @@ +'use client'; + +import { IconCheck, IconX, IconLoader2, IconTerminal } from '@tabler/icons-react'; +import { cn } from '@/lib/utils'; +import type { Message } from '@/lib/types/http'; +import { Badge } from '@kandev/ui/badge'; + +interface ScriptExecutionMetadata { + script_type: 'setup' | 'cleanup'; + command: string; + status: 'starting' | 'running' | 'exited' | 'failed'; + exit_code?: number; + started_at?: string; + completed_at?: string; + process_id?: string; + error?: string; +} + +export function ScriptExecutionMessage({ comment }: { comment: Message }) { + const metadata = comment.metadata as ScriptExecutionMetadata | undefined; + + // Show fallback if metadata is missing or incomplete + if (!metadata || !metadata.script_type) { + return ( +
+
+ + Script Execution (metadata unavailable) +
+ {comment.content && ( + 
+            {comment.content}
+
+ )} +
+ ); + } + + const { script_type, command, status, exit_code, error } = metadata; + const isSetup = script_type === 'setup'; + const isRunning = status === 'starting' || status === 'running'; + const isSuccess = status === 'exited' && exit_code === 0; + + // Status icon and styling + const StatusIcon = isRunning + ? IconLoader2 + : isSuccess + ? IconCheck + : IconX; + + const statusText = isRunning + ? 'Running...' + : isSuccess + ? 'Completed' + : 'Failed'; + + const statusColor = isRunning + ? 'text-blue-500' + : isSuccess + ? 'text-green-500' + : 'text-red-500'; + + const borderColor = isRunning + ? 'border-blue-500/40' + : isSuccess + ? 'border-green-500/40' + : 'border-red-500/40'; + + const bgColor = isRunning + ? 'bg-blue-500/5' + : isSuccess + ? 'bg-green-500/5' + : 'bg-red-500/5'; + + return ( +
+ {/* Header */} +
+
+ + + {isSetup ? 'Setup Script' : 'Cleanup Script'} + +
+ + {statusText} +
+
+
+ + {/* Command */} +
+ $ {command} +
+ + {/* Output */} + {comment.content && comment.content.trim() !== '' && ( +
+
+ Output +
+ 
+            {comment.content}
+
+
+ )} + + {/* Error message */} + {error && ( +
+ {error} +
+ )} + + {/* Footer with exit code */} + {!isRunning && exit_code !== undefined && ( +
+ Exit code: {exit_code} + {metadata.started_at && metadata.completed_at && ( + + Duration: {calculateDuration(metadata.started_at, metadata.completed_at)} + + )} +
+ )} +
+ ); +} + +// Helper function to calculate duration +function calculateDuration(startedAt: string, completedAt: string): string { + try { + const start = new Date(startedAt).getTime(); + const end = new Date(completedAt).getTime(); + const durationMs = end - start; + + if (durationMs < 1000) { + return `${durationMs}ms`; + } else if (durationMs < 60000) { + return `${(durationMs / 1000).toFixed(1)}s`; + } else { + const minutes = Math.floor(durationMs / 60000); + const seconds = Math.floor((durationMs % 60000) / 1000); + return `${minutes}m ${seconds}s`; + } + } catch { + return 'N/A'; + } +} diff --git a/apps/web/components/task/chat/messages/status-message.tsx b/apps/web/components/task/chat/messages/status-message.tsx index f302ee332e..a867fd349b 100644 --- a/apps/web/components/task/chat/messages/status-message.tsx +++ b/apps/web/components/task/chat/messages/status-message.tsx @@ -1,12 +1,23 @@ 'use client'; -import { IconAlertTriangle, IconInfoCircle, IconPointFilled, IconHandStop } from '@tabler/icons-react'; +import { useState } from 'react'; +import { IconAlertTriangle, IconInfoCircle, IconPointFilled, IconHandStop, IconChevronDown, IconChevronRight } from '@tabler/icons-react'; import { cn } from '@/lib/utils'; import type { Message } from '@/lib/types/http'; import type { StatusMetadata } from '@/components/task/chat/types'; +interface ErrorMetadata extends StatusMetadata { + error?: string; + text?: string; + error_data?: Record; + stderr?: string[]; + provider?: string; + provider_agent?: string; +} + export function StatusMessage({ comment }: { comment: Message }) { - const metadata = comment.metadata as StatusMetadata | undefined; + const [isExpanded, setIsExpanded] = useState(false); + const metadata = comment.metadata as ErrorMetadata | undefined; const progress = typeof metadata?.progress === 'number' ? Math.min(Math.max(metadata.progress, 0), 100) : null; const statusLine = metadata?.stage || metadata?.status; @@ -14,6 +25,9 @@ export function StatusMessage({ comment }: { comment: Message }) { const isError = comment.type === 'error' || metadata?.variant === 'error'; const isWarning = metadata?.variant === 'warning' || metadata?.cancelled === true; + // Check if there are error details to show + const hasErrorDetails = isError && (metadata?.error_data || metadata?.error || metadata?.text || metadata?.stderr); + // Simple system message: no metadata, no progress, just a short message const isSimpleStatus = !isError && !isWarning && progress === null && !statusLine && !metadata?.message; @@ -44,6 +58,33 @@ export function StatusMessage({ comment }: { comment: Message }) { const iconClass = isError ? 'text-red-500' : isWarning ? 'text-amber-500' : 'text-muted-foreground'; const label = isError ? 'Error' : isWarning ? 'Warning' : 'Status'; + // Format error details for display + const formatErrorDetails = () => { + const details: { label: string; value: string }[] = []; + + if (metadata?.stderr && metadata.stderr.length > 0) { + details.push({ label: 'Agent Output', value: metadata.stderr.join('\n') }); + } + if (metadata?.error) { + details.push({ label: 'Error', value: metadata.error }); + } + if (metadata?.text) { + details.push({ label: 'Details', value: metadata.text }); + } + if (metadata?.error_data) { + // Don't show stderr again in error_data since we already show it above + const filteredData = { ...metadata.error_data }; + delete filteredData.stderr; + if (Object.keys(filteredData).length > 0) { + details.push({ label: 'Error Data', value: JSON.stringify(filteredData, null, 2) }); + } + } + + return details; + }; + + const errorDetails = hasErrorDetails ? formatErrorDetails() : []; + return (
{label}
-
{message}
+
{message || 'An error occurred'}
+ + {/* Expandable error details */} + {hasErrorDetails && errorDetails.length > 0 && ( +
+ + + {isExpanded && ( +
+ {errorDetails.map((detail, index) => ( +
+
+ {detail.label} +
+ 
+                    {detail.value}
+
+
+ ))} +
+ )} +
+ )} + {progress !== null && (
diff --git a/apps/web/hooks/use-processed-messages.ts b/apps/web/hooks/use-processed-messages.ts index ac56a684c5..271b185767 100644 --- a/apps/web/hooks/use-processed-messages.ts +++ b/apps/web/hooks/use-processed-messages.ts @@ -50,7 +50,8 @@ export function useProcessedMessages( message.type === 'status' || message.type === 'error' || message.type === 'thinking' || - message.type === 'todo' + message.type === 'todo' || + message.type === 'script_execution' ) { return true; } diff --git a/apps/web/lib/types/http.ts b/apps/web/lib/types/http.ts index d130799c60..96e5bf97b6 100644 --- a/apps/web/lib/types/http.ts +++ b/apps/web/lib/types/http.ts @@ -358,7 +358,8 @@ export type MessageType = | 'status' | 'thinking' | 'todo' - | 'permission_request'; + | 'permission_request' + | 'script_execution'; export type Message = { id: string;