From 87faf11e5868c6749f89423cb10437b96721e139 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 14:39:48 +0000 Subject: [PATCH 1/7] Initial plan From 738f1defd63ed5e74a6875a54ce0f8926840a216 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 15:00:57 +0000 Subject: [PATCH 2/7] refactor: split gateway_logs.go into parser, renderer, extractor files Move functions from the 1,332-line gateway_logs.go into three new files: - gateway_logs_parser.go: RPC/gateway log parsing (parseRPCMessages, findRPCMessagesPath, parseGatewayLogs, processGatewayLogEntry, getOrCreateServer, getOrCreateTool, calculateGatewayAggregates, buildToolCallsFromRPCMessages) - gateway_logs_renderer.go: console table rendering (renderGatewayMetricsTable, getSortedServerNames, guardPolicyReasonFromCode, displayAggregatedGatewayMetrics) - gateway_logs_extractor.go: high-level extraction (extractMCPToolUsageData, buildGuardPolicySummary) gateway_logs.go retains only types, constants, logger, and isGuardPolicyErrorCode. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/gateway_logs.go | 1151 +---------------------------- pkg/cli/gateway_logs_extractor.go | 247 +++++++ pkg/cli/gateway_logs_parser.go | 630 ++++++++++++++++ pkg/cli/gateway_logs_renderer.go | 301 ++++++++ 4 files changed, 1188 insertions(+), 1141 deletions(-) create mode 100644 pkg/cli/gateway_logs_extractor.go create mode 100644 pkg/cli/gateway_logs_parser.go create mode 100644 pkg/cli/gateway_logs_renderer.go diff --git a/pkg/cli/gateway_logs.go b/pkg/cli/gateway_logs.go index 3bab3f8a0e9..66cdb4bf516 100644 --- a/pkg/cli/gateway_logs.go +++ b/pkg/cli/gateway_logs.go @@ -1,32 +1,24 @@ // This file provides command-line interface functionality for gh-aw. -// This file (gateway_logs.go) contains functions for parsing and analyzing -// MCP gateway logs from gateway.jsonl or rpc-messages.jsonl files. +// This file (gateway_logs.go) contains types, constants, logger, and core helpers +// for parsing and analyzing MCP gateway logs from gateway.jsonl or rpc-messages.jsonl files. // // Key responsibilities: -// - Parsing gateway.jsonl JSONL format logs (preferred) -// - Parsing rpc-messages.jsonl JSONL format logs (canonical fallback) -// - Extracting server and tool usage metrics -// - Aggregating gateway statistics -// - Rendering gateway metrics tables +// - Type definitions for gateway log entries and metrics +// - Guard policy error code constants +// - Core helper: isGuardPolicyErrorCode +// +// See also: +// - gateway_logs_parser.go — RPC/gateway log parsing functions +// - gateway_logs_renderer.go — console table rendering functions +// - gateway_logs_extractor.go — high-level MCP tool usage extraction package cli import ( - "bufio" "encoding/json" - "errors" - "fmt" - "os" - "path/filepath" - "sort" - "strconv" - "strings" "time" - "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" - "github.com/github/gh-aw/pkg/sliceutil" - "github.com/github/gh-aw/pkg/timeutil" ) var gatewayLogsLog = logger.New("cli:gateway_logs") @@ -107,26 +99,6 @@ func isGuardPolicyErrorCode(code int) bool { return code >= guardPolicyErrorCodeIntegrityBelowMin && code <= guardPolicyErrorCodeAccessDenied } -// guardPolicyReasonFromCode returns a human-readable reason string for a guard policy error code. -func guardPolicyReasonFromCode(code int) string { - switch code { - case guardPolicyErrorCodeAccessDenied: - return "access_denied" - case guardPolicyErrorCodeRepoNotAllowed: - return "repo_not_allowed" - case guardPolicyErrorCodeInsufficientPerms: - return "insufficient_permissions" - case guardPolicyErrorCodePrivateRepoDenied: - return "private_repo_denied" - case guardPolicyErrorCodeBlockedUser: - return "blocked_user" - case guardPolicyErrorCodeIntegrityBelowMin: - return "integrity_below_minimum" - default: - return "unknown" - } -} - // GatewayServerMetrics represents usage metrics for a single MCP server type GatewayServerMetrics struct { ServerName string @@ -227,1106 +199,3 @@ type rpcPendingRequest struct { ToolName string Timestamp time.Time } - -// parseRPCMessages parses a rpc-messages.jsonl file and extracts GatewayMetrics. -// This is the canonical fallback when gateway.jsonl is not available. -func parseRPCMessages(logPath string, verbose bool) (*GatewayMetrics, error) { - gatewayLogsLog.Printf("Parsing rpc-messages.jsonl from: %s", logPath) - - file, err := os.Open(logPath) - if err != nil { - return nil, fmt.Errorf("failed to open rpc-messages.jsonl: %w", err) - } - defer file.Close() - - metrics := &GatewayMetrics{ - Servers: make(map[string]*GatewayServerMetrics), - } - - // Track pending requests by (serverID, id) for duration calculation. - // Key format: "/" - pendingRequests := make(map[string]*rpcPendingRequest) - - scanner := bufio.NewScanner(file) - // Increase scanner buffer for large payloads - buf := make([]byte, maxScannerBufferSize) - scanner.Buffer(buf, maxScannerBufferSize) - lineNum := 0 - - for scanner.Scan() { - lineNum++ - line := strings.TrimSpace(scanner.Text()) - if line == "" { - continue - } - - var entry RPCMessageEntry - if err := json.Unmarshal([]byte(line), &entry); err != nil { - gatewayLogsLog.Printf("Failed to parse rpc-messages.jsonl line %d: %v", lineNum, err) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage( - fmt.Sprintf("Failed to parse rpc-messages.jsonl line %d: %v", lineNum, err))) - } - continue - } - - // Update time range - if entry.Timestamp != "" { - if t, err := time.Parse(time.RFC3339Nano, entry.Timestamp); err == nil { - if metrics.StartTime.IsZero() || t.Before(metrics.StartTime) { - metrics.StartTime = t - } - if metrics.EndTime.IsZero() || t.After(metrics.EndTime) { - metrics.EndTime = t - } - } - } - - if entry.ServerID == "" { - continue - } - - switch { - case entry.Type == "DIFC_FILTERED": - // DIFC integrity/secrecy filter event — not a REQUEST or RESPONSE - metrics.TotalFiltered++ - server := getOrCreateServer(metrics, entry.ServerID) - server.FilteredCount++ - metrics.FilteredEvents = append(metrics.FilteredEvents, DifcFilteredEvent{ - Timestamp: entry.Timestamp, - ServerID: entry.ServerID, - ToolName: entry.ToolName, - Description: entry.Description, - Reason: entry.Reason, - SecrecyTags: entry.SecrecyTags, - IntegrityTags: entry.IntegrityTags, - AuthorAssociation: entry.AuthorAssociation, - AuthorLogin: entry.AuthorLogin, - HTMLURL: entry.HTMLURL, - Number: entry.Number, - }) - - case entry.Direction == "OUT" && entry.Type == "REQUEST": - // Outgoing request from AI engine to MCP server - var req rpcRequestPayload - if err := json.Unmarshal(entry.Payload, &req); err != nil { - continue - } - if req.Method != "tools/call" { - continue - } - - // Extract tool name - var params rpcToolCallParams - if err := json.Unmarshal(req.Params, ¶ms); err != nil || params.Name == "" { - continue - } - - metrics.TotalRequests++ - server := getOrCreateServer(metrics, entry.ServerID) - server.RequestCount++ - metrics.TotalToolCalls++ - server.ToolCallCount++ - - tool := getOrCreateTool(server, params.Name) - tool.CallCount++ - - // Store pending request for duration calculation - if req.ID != nil && entry.Timestamp != "" { - if t, err := time.Parse(time.RFC3339Nano, entry.Timestamp); err == nil { - key := fmt.Sprintf("%s/%v", entry.ServerID, req.ID) - pendingRequests[key] = &rpcPendingRequest{ - ServerID: entry.ServerID, - ToolName: params.Name, - Timestamp: t, - } - } - } - - case entry.Direction == "IN" && entry.Type == "RESPONSE": - // Incoming response from MCP server to AI engine - var resp rpcResponsePayload - if err := json.Unmarshal(entry.Payload, &resp); err != nil { - continue - } - - // Track errors and detect guard policy blocks - if resp.Error != nil { - metrics.TotalErrors++ - server := getOrCreateServer(metrics, entry.ServerID) - server.ErrorCount++ - - // Detect guard policy enforcement errors - if isGuardPolicyErrorCode(resp.Error.Code) { - metrics.TotalGuardBlocked++ - server.GuardPolicyBlocked++ - - // Determine tool name from pending request if available - toolName := "" - if resp.ID != nil { - key := fmt.Sprintf("%s/%v", entry.ServerID, resp.ID) - if pending, ok := pendingRequests[key]; ok { - toolName = pending.ToolName - } - } - - reason := guardPolicyReasonFromCode(resp.Error.Code) - if resp.Error.Data != nil && resp.Error.Data.Reason != "" { - reason = resp.Error.Data.Reason - } - - evt := GuardPolicyEvent{ - Timestamp: entry.Timestamp, - ServerID: entry.ServerID, - ToolName: toolName, - ErrorCode: resp.Error.Code, - Reason: reason, - Message: resp.Error.Message, - } - if resp.Error.Data != nil { - evt.Details = resp.Error.Data.Details - evt.Repository = resp.Error.Data.Repository - } - metrics.GuardPolicyEvents = append(metrics.GuardPolicyEvents, evt) - } - } - - // Calculate duration by matching with pending request - if resp.ID != nil && entry.Timestamp != "" { - key := fmt.Sprintf("%s/%v", entry.ServerID, resp.ID) - if pending, ok := pendingRequests[key]; ok { - delete(pendingRequests, key) - if t, err := time.Parse(time.RFC3339Nano, entry.Timestamp); err == nil { - durationMs := float64(t.Sub(pending.Timestamp).Milliseconds()) - if durationMs >= 0 { - server := getOrCreateServer(metrics, entry.ServerID) - server.TotalDuration += durationMs - metrics.TotalDuration += durationMs - - tool := getOrCreateTool(server, pending.ToolName) - tool.TotalDuration += durationMs - if tool.MaxDuration == 0 || durationMs > tool.MaxDuration { - tool.MaxDuration = durationMs - } - if tool.MinDuration == 0 || durationMs < tool.MinDuration { - tool.MinDuration = durationMs - } - - if resp.Error != nil { - tool.ErrorCount++ - } - } - } - } - } - } - } - - if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("error reading rpc-messages.jsonl: %w", err) - } - - calculateGatewayAggregates(metrics) - - gatewayLogsLog.Printf("Successfully parsed rpc-messages.jsonl: %d servers, %d total requests", - len(metrics.Servers), metrics.TotalRequests) - - return metrics, nil -} - -// findRPCMessagesPath returns the path to rpc-messages.jsonl if it exists, or "" if not found. -func findRPCMessagesPath(logDir string) string { - // Check mcp-logs subdirectory (standard location) - mcpLogsPath := filepath.Join(logDir, "mcp-logs", "rpc-messages.jsonl") - if _, err := os.Stat(mcpLogsPath); err == nil { - return mcpLogsPath - } - // Check root directory as fallback - rootPath := filepath.Join(logDir, "rpc-messages.jsonl") - if _, err := os.Stat(rootPath); err == nil { - return rootPath - } - return "" -} - -// parseGatewayLogs parses a gateway.jsonl file and extracts metrics. -// Falls back to rpc-messages.jsonl (canonical fallback) when gateway.jsonl is not present. -func parseGatewayLogs(logDir string, verbose bool) (*GatewayMetrics, error) { - // Try root directory first (for older logs where gateway.jsonl was in the root) - gatewayLogPath := filepath.Join(logDir, "gateway.jsonl") - - // Check if gateway.jsonl exists in root - if _, err := os.Stat(gatewayLogPath); os.IsNotExist(err) { - // Try mcp-logs subdirectory (new path after artifact download) - // Gateway logs are uploaded from /tmp/gh-aw/mcp-logs/gateway.jsonl and the common parent - // /tmp/gh-aw/ is stripped during artifact upload, resulting in mcp-logs/gateway.jsonl after download - mcpLogsPath := filepath.Join(logDir, "mcp-logs", "gateway.jsonl") - if _, err := os.Stat(mcpLogsPath); os.IsNotExist(err) { - // Fall back to rpc-messages.jsonl (canonical fallback when gateway.jsonl is missing) - rpcPath := findRPCMessagesPath(logDir) - if rpcPath != "" { - gatewayLogsLog.Printf("gateway.jsonl not found; falling back to rpc-messages.jsonl: %s", rpcPath) - return parseRPCMessages(rpcPath, verbose) - } - gatewayLogsLog.Printf("gateway.jsonl not found at: %s or %s", gatewayLogPath, mcpLogsPath) - return nil, errors.New("gateway.jsonl not found") - } - gatewayLogPath = mcpLogsPath - gatewayLogsLog.Printf("Found gateway.jsonl in mcp-logs subdirectory") - } - - gatewayLogsLog.Printf("Parsing gateway.jsonl from: %s", gatewayLogPath) - - file, err := os.Open(gatewayLogPath) - if err != nil { - return nil, fmt.Errorf("failed to open gateway.jsonl: %w", err) - } - defer file.Close() - - metrics := &GatewayMetrics{ - Servers: make(map[string]*GatewayServerMetrics), - } - - scanner := bufio.NewScanner(file) - buf := make([]byte, maxScannerBufferSize) - scanner.Buffer(buf, maxScannerBufferSize) - lineNum := 0 - - for scanner.Scan() { - lineNum++ - line := strings.TrimSpace(scanner.Text()) - - // Skip empty lines - if line == "" { - continue - } - - var entry GatewayLogEntry - if err := json.Unmarshal([]byte(line), &entry); err != nil { - gatewayLogsLog.Printf("Failed to parse line %d: %v", lineNum, err) - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to parse gateway.jsonl line %d: %v", lineNum, err))) - } - continue - } - - // Process the entry based on its type/event - processGatewayLogEntry(&entry, metrics, verbose) - } - - if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("error reading gateway.jsonl: %w", err) - } - - // Calculate aggregate statistics - calculateGatewayAggregates(metrics) - - gatewayLogsLog.Printf("Successfully parsed gateway.jsonl: %d servers, %d total requests", - len(metrics.Servers), metrics.TotalRequests) - - return metrics, nil -} - -// processGatewayLogEntry processes a single log entry and updates metrics -func processGatewayLogEntry(entry *GatewayLogEntry, metrics *GatewayMetrics, verbose bool) { - // Parse timestamp for time range (supports both RFC3339 and RFC3339Nano) - if entry.Timestamp != "" { - t, err := time.Parse(time.RFC3339Nano, entry.Timestamp) - if err != nil { - t, err = time.Parse(time.RFC3339, entry.Timestamp) - } - if err == nil { - if metrics.StartTime.IsZero() || t.Before(metrics.StartTime) { - metrics.StartTime = t - } - if metrics.EndTime.IsZero() || t.After(metrics.EndTime) { - metrics.EndTime = t - } - } - } - - // Handle DIFC_FILTERED events - if entry.Type == "DIFC_FILTERED" { - metrics.TotalFiltered++ - // DIFC_FILTERED events use server_id; fall back to server_name for compatibility - serverKey := entry.ServerID - if serverKey == "" { - serverKey = entry.ServerName - } - if serverKey != "" { - server := getOrCreateServer(metrics, serverKey) - server.FilteredCount++ - } - metrics.FilteredEvents = append(metrics.FilteredEvents, DifcFilteredEvent{ - Timestamp: entry.Timestamp, - ServerID: serverKey, - ToolName: entry.ToolName, - Description: entry.Description, - Reason: entry.Reason, - SecrecyTags: entry.SecrecyTags, - IntegrityTags: entry.IntegrityTags, - AuthorAssociation: entry.AuthorAssociation, - AuthorLogin: entry.AuthorLogin, - HTMLURL: entry.HTMLURL, - Number: entry.Number, - }) - return - } - - // Handle GUARD_POLICY_BLOCKED events from gateway.jsonl - if entry.Type == "GUARD_POLICY_BLOCKED" { - metrics.TotalGuardBlocked++ - serverKey := entry.ServerID - if serverKey == "" { - serverKey = entry.ServerName - } - if serverKey != "" { - server := getOrCreateServer(metrics, serverKey) - server.GuardPolicyBlocked++ - } - metrics.GuardPolicyEvents = append(metrics.GuardPolicyEvents, GuardPolicyEvent{ - Timestamp: entry.Timestamp, - ServerID: serverKey, - ToolName: entry.ToolName, - Reason: entry.Reason, - Message: entry.Message, - Details: entry.Description, - }) - return - } - - // Track errors - if entry.Status == "error" || entry.Error != "" { - metrics.TotalErrors++ - if entry.ServerName != "" { - server := getOrCreateServer(metrics, entry.ServerName) - server.ErrorCount++ - - if entry.ToolName != "" { - tool := getOrCreateTool(server, entry.ToolName) - tool.ErrorCount++ - } - } - } - - // Process based on event type - switch entry.Event { - case "request", "tool_call", "rpc_call": - metrics.TotalRequests++ - - if entry.ServerName != "" { - server := getOrCreateServer(metrics, entry.ServerName) - server.RequestCount++ - - if entry.Duration > 0 { - server.TotalDuration += entry.Duration - metrics.TotalDuration += entry.Duration - } - - // Track tool calls - if entry.ToolName != "" || entry.Method != "" { - toolName := entry.ToolName - if toolName == "" { - toolName = entry.Method - } - - metrics.TotalToolCalls++ - server.ToolCallCount++ - - tool := getOrCreateTool(server, toolName) - tool.CallCount++ - - if entry.Duration > 0 { - tool.TotalDuration += entry.Duration - if tool.MaxDuration == 0 || entry.Duration > tool.MaxDuration { - tool.MaxDuration = entry.Duration - } - if tool.MinDuration == 0 || entry.Duration < tool.MinDuration { - tool.MinDuration = entry.Duration - } - } - - if entry.InputSize > 0 { - tool.TotalInputSize += entry.InputSize - } - if entry.OutputSize > 0 { - tool.TotalOutputSize += entry.OutputSize - } - } - } - } -} - -// getOrCreateServer gets or creates a server metrics entry -func getOrCreateServer(metrics *GatewayMetrics, serverName string) *GatewayServerMetrics { - if server, exists := metrics.Servers[serverName]; exists { - return server - } - - server := &GatewayServerMetrics{ - ServerName: serverName, - Tools: make(map[string]*GatewayToolMetrics), - } - metrics.Servers[serverName] = server - return server -} - -// getOrCreateTool gets or creates a tool metrics entry -func getOrCreateTool(server *GatewayServerMetrics, toolName string) *GatewayToolMetrics { - if tool, exists := server.Tools[toolName]; exists { - return tool - } - - tool := &GatewayToolMetrics{ - ToolName: toolName, - } - server.Tools[toolName] = tool - return tool -} - -// calculateGatewayAggregates calculates aggregate statistics -func calculateGatewayAggregates(metrics *GatewayMetrics) { - for _, server := range metrics.Servers { - for _, tool := range server.Tools { - if tool.CallCount > 0 { - tool.AvgDuration = tool.TotalDuration / float64(tool.CallCount) - } - } - } -} - -// renderGatewayMetricsTable renders gateway metrics as a console table -func renderGatewayMetricsTable(metrics *GatewayMetrics, verbose bool) string { - if metrics == nil || len(metrics.Servers) == 0 { - return "" - } - - var output strings.Builder - - output.WriteString("\n") - output.WriteString(console.FormatInfoMessage("MCP Gateway Metrics")) - output.WriteString("\n\n") - - // Summary statistics - fmt.Fprintf(&output, "Total Requests: %d\n", metrics.TotalRequests) - fmt.Fprintf(&output, "Total Tool Calls: %d\n", metrics.TotalToolCalls) - fmt.Fprintf(&output, "Total Errors: %d\n", metrics.TotalErrors) - if metrics.TotalFiltered > 0 { - fmt.Fprintf(&output, "Total DIFC Filtered: %d\n", metrics.TotalFiltered) - } - if metrics.TotalGuardBlocked > 0 { - fmt.Fprintf(&output, "Total Guard Policy Blocked: %d\n", metrics.TotalGuardBlocked) - } - fmt.Fprintf(&output, "Servers: %d\n", len(metrics.Servers)) - - if !metrics.StartTime.IsZero() && !metrics.EndTime.IsZero() { - duration := metrics.EndTime.Sub(metrics.StartTime) - fmt.Fprintf(&output, "Time Range: %s\n", duration.Round(time.Second)) - } - - output.WriteString("\n") - - // Server metrics table - if len(metrics.Servers) > 0 { - // Sort servers by request count - serverNames := getSortedServerNames(metrics) - - hasFiltered := metrics.TotalFiltered > 0 - hasGuardPolicy := metrics.TotalGuardBlocked > 0 - serverRows := make([][]string, 0, len(serverNames)) - for _, serverName := range serverNames { - server := metrics.Servers[serverName] - avgTime := 0.0 - if server.RequestCount > 0 { - avgTime = server.TotalDuration / float64(server.RequestCount) - } - row := []string{ - serverName, - strconv.Itoa(server.RequestCount), - strconv.Itoa(server.ToolCallCount), - fmt.Sprintf("%.0fms", avgTime), - strconv.Itoa(server.ErrorCount), - } - if hasFiltered { - row = append(row, strconv.Itoa(server.FilteredCount)) - } - if hasGuardPolicy { - row = append(row, strconv.Itoa(server.GuardPolicyBlocked)) - } - serverRows = append(serverRows, row) - } - - headers := []string{"Server", "Requests", "Tool Calls", "Avg Time", "Errors"} - if hasFiltered { - headers = append(headers, "Filtered") - } - if hasGuardPolicy { - headers = append(headers, "Guard Blocked") - } - output.WriteString(console.RenderTable(console.TableConfig{ - Title: "Server Usage", - Headers: headers, - Rows: serverRows, - })) - } - - // DIFC filtered events table - if len(metrics.FilteredEvents) > 0 { - output.WriteString("\n") - filteredRows := make([][]string, 0, len(metrics.FilteredEvents)) - for _, fe := range metrics.FilteredEvents { - reason := fe.Reason - if len(reason) > 80 { - reason = reason[:77] + "..." - } - filteredRows = append(filteredRows, []string{ - fe.ServerID, - fe.ToolName, - fe.AuthorLogin, - reason, - }) - } - output.WriteString(console.RenderTable(console.TableConfig{ - Title: "DIFC Filtered Events", - Headers: []string{"Server", "Tool", "User", "Reason"}, - Rows: filteredRows, - })) - } - - // Guard policy events table - if len(metrics.GuardPolicyEvents) > 0 { - output.WriteString("\n") - guardRows := make([][]string, 0, len(metrics.GuardPolicyEvents)) - for _, gpe := range metrics.GuardPolicyEvents { - message := gpe.Message - if len(message) > 60 { - message = message[:57] + "..." - } - repo := gpe.Repository - if repo == "" { - repo = "-" - } - guardRows = append(guardRows, []string{ - gpe.ServerID, - gpe.ToolName, - gpe.Reason, - message, - repo, - }) - } - output.WriteString(console.RenderTable(console.TableConfig{ - Title: "Guard Policy Blocked Events", - Headers: []string{"Server", "Tool", "Reason", "Message", "Repository"}, - Rows: guardRows, - })) - } - - // Tool metrics table (if verbose) - if verbose { - output.WriteString("\n") - output.WriteString("Tool Usage Details:\n") - - for _, serverName := range getSortedServerNames(metrics) { - server := metrics.Servers[serverName] - if len(server.Tools) == 0 { - continue - } - - // Sort tools by call count - toolNames := sliceutil.MapToSlice(server.Tools) - sort.Slice(toolNames, func(i, j int) bool { - return server.Tools[toolNames[i]].CallCount > server.Tools[toolNames[j]].CallCount - }) - - toolRows := make([][]string, 0, len(toolNames)) - for _, toolName := range toolNames { - tool := server.Tools[toolName] - toolRows = append(toolRows, []string{ - toolName, - strconv.Itoa(tool.CallCount), - fmt.Sprintf("%.0fms", tool.AvgDuration), - fmt.Sprintf("%.0fms", tool.MaxDuration), - strconv.Itoa(tool.ErrorCount), - }) - } - - output.WriteString(console.RenderTable(console.TableConfig{ - Title: serverName, - Headers: []string{"Tool", "Calls", "Avg Time", "Max Time", "Errors"}, - Rows: toolRows, - })) - } - } - - return output.String() -} - -// getSortedServerNames returns server names sorted by request count -func getSortedServerNames(metrics *GatewayMetrics) []string { - names := sliceutil.MapToSlice(metrics.Servers) - sort.Slice(names, func(i, j int) bool { - return metrics.Servers[names[i]].RequestCount > metrics.Servers[names[j]].RequestCount - }) - return names -} - -// buildToolCallsFromRPCMessages reads rpc-messages.jsonl and builds MCPToolCall records. -// Duration is computed by pairing outgoing requests with incoming responses. -// Input/output sizes are not available in rpc-messages.jsonl and will be 0. -func buildToolCallsFromRPCMessages(logPath string) ([]MCPToolCall, error) { - file, err := os.Open(logPath) - if err != nil { - return nil, fmt.Errorf("failed to open rpc-messages.jsonl: %w", err) - } - defer file.Close() - - type pendingCall struct { - serverID string - toolName string - timestamp time.Time - } - pending := make(map[string]*pendingCall) // key: "/" - - // Collect requests first to pair with responses - type rawEntry struct { - entry RPCMessageEntry - req rpcRequestPayload - resp rpcResponsePayload - valid bool - } - var entries []rawEntry - - scanner := bufio.NewScanner(file) - buf := make([]byte, maxScannerBufferSize) - scanner.Buffer(buf, maxScannerBufferSize) - - for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) - if line == "" { - continue - } - var e RPCMessageEntry - if err := json.Unmarshal([]byte(line), &e); err != nil { - continue - } - entries = append(entries, rawEntry{entry: e, valid: true}) - } - if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("error reading rpc-messages.jsonl: %w", err) - } - - // Second pass: build MCPToolCall records. - // Declared before first pass so requests without IDs can be appended immediately. - var toolCalls []MCPToolCall - processedKeys := make(map[string]bool) - - // First pass: index outgoing tool-call requests by (serverID, id) - for i := range entries { - e := &entries[i] - if e.entry.Direction != "OUT" || e.entry.Type != "REQUEST" { - continue - } - if err := json.Unmarshal(e.entry.Payload, &e.req); err != nil || e.req.Method != "tools/call" { - continue - } - var params rpcToolCallParams - if err := json.Unmarshal(e.req.Params, ¶ms); err != nil || params.Name == "" { - continue - } - if e.req.ID == nil { - // Requests without an ID cannot be matched to responses. - // Emit the tool call immediately with "unknown" status so it appears - // in the tool_calls list (same as parseRPCMessages counts it in the summary). - toolCalls = append(toolCalls, MCPToolCall{ - Timestamp: e.entry.Timestamp, - ServerName: e.entry.ServerID, - ToolName: params.Name, - Status: "unknown", - }) - continue - } - t, err := time.Parse(time.RFC3339Nano, e.entry.Timestamp) - if err != nil { - continue - } - key := fmt.Sprintf("%s/%v", e.entry.ServerID, e.req.ID) - pending[key] = &pendingCall{ - serverID: e.entry.ServerID, - toolName: params.Name, - timestamp: t, - } - } - - // Second pass: pair responses with pending requests to compute durations - - for i := range entries { - e := &entries[i] - switch { - case e.entry.Direction == "OUT" && e.entry.Type == "REQUEST": - // Outgoing tool-call request – we'll emit the record when we see the response - // (or after if no response found) - case e.entry.Direction == "IN" && e.entry.Type == "RESPONSE": - if err := json.Unmarshal(e.entry.Payload, &e.resp); err != nil { - continue - } - if e.resp.ID == nil { - continue - } - key := fmt.Sprintf("%s/%v", e.entry.ServerID, e.resp.ID) - p, ok := pending[key] - if !ok { - continue - } - processedKeys[key] = true - - call := MCPToolCall{ - Timestamp: p.timestamp.Format(time.RFC3339Nano), - ServerName: p.serverID, - ToolName: p.toolName, - Status: "success", - } - if e.resp.Error != nil { - call.Status = "error" - call.Error = e.resp.Error.Message - } - if t, err := time.Parse(time.RFC3339Nano, e.entry.Timestamp); err == nil { - d := t.Sub(p.timestamp) - if d >= 0 { - call.Duration = timeutil.FormatDuration(d) - } - } - toolCalls = append(toolCalls, call) - } - } - - // Emit any requests that never received a response - for key, p := range pending { - if !processedKeys[key] { - toolCalls = append(toolCalls, MCPToolCall{ - Timestamp: p.timestamp.Format(time.RFC3339Nano), - ServerName: p.serverID, - ToolName: p.toolName, - Status: "unknown", - }) - } - } - - return toolCalls, nil -} - -// extractMCPToolUsageData creates detailed MCP tool usage data from gateway metrics -func extractMCPToolUsageData(logDir string, verbose bool) (*MCPToolUsageData, error) { - // Parse gateway logs (falls back to rpc-messages.jsonl automatically) - gatewayMetrics, err := parseGatewayLogs(logDir, verbose) - if err != nil { - // Return nil if no log file exists (not an error for workflows without MCP) - if strings.Contains(err.Error(), "not found") { - return nil, nil - } - return nil, fmt.Errorf("failed to parse gateway logs: %w", err) - } - - if gatewayMetrics == nil || len(gatewayMetrics.Servers) == 0 { - return nil, nil - } - - mcpData := &MCPToolUsageData{ - Summary: []MCPToolSummary{}, - ToolCalls: []MCPToolCall{}, - Servers: []MCPServerStats{}, - FilteredEvents: gatewayMetrics.FilteredEvents, - } - - // Build guard policy summary if there are guard policy events - if len(gatewayMetrics.GuardPolicyEvents) > 0 { - mcpData.GuardPolicySummary = buildGuardPolicySummary(gatewayMetrics) - } - - // Read the log file again to get individual tool call records. - // Prefer gateway.jsonl; fall back to rpc-messages.jsonl when not available. - gatewayLogPath := filepath.Join(logDir, "gateway.jsonl") - usingRPCMessages := false - - if _, err := os.Stat(gatewayLogPath); os.IsNotExist(err) { - mcpLogsPath := filepath.Join(logDir, "mcp-logs", "gateway.jsonl") - if _, err := os.Stat(mcpLogsPath); os.IsNotExist(err) { - // Fall back to rpc-messages.jsonl - rpcPath := findRPCMessagesPath(logDir) - if rpcPath == "" { - return nil, errors.New("gateway.jsonl not found") - } - gatewayLogPath = rpcPath - usingRPCMessages = true - } else { - gatewayLogPath = mcpLogsPath - } - } - - if usingRPCMessages { - // Build tool call records from rpc-messages.jsonl - toolCalls, err := buildToolCallsFromRPCMessages(gatewayLogPath) - if err != nil { - return nil, fmt.Errorf("failed to read rpc-messages.jsonl: %w", err) - } - mcpData.ToolCalls = toolCalls - } else { - file, err := os.Open(gatewayLogPath) - if err != nil { - return nil, fmt.Errorf("failed to open gateway.jsonl: %w", err) - } - defer file.Close() - - scanner := bufio.NewScanner(file) - buf := make([]byte, maxScannerBufferSize) - scanner.Buffer(buf, maxScannerBufferSize) - for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) - if line == "" { - continue - } - - var entry GatewayLogEntry - if err := json.Unmarshal([]byte(line), &entry); err != nil { - continue // Skip malformed lines - } - - // Only process tool call events - if entry.Event == "tool_call" || entry.Event == "rpc_call" || entry.Event == "request" { - toolName := entry.ToolName - if toolName == "" { - toolName = entry.Method - } - - // Skip entries without tool information - if entry.ServerName == "" || toolName == "" { - continue - } - - // Create individual tool call record - toolCall := MCPToolCall{ - Timestamp: entry.Timestamp, - ServerName: entry.ServerName, - ToolName: toolName, - Method: entry.Method, - InputSize: entry.InputSize, - OutputSize: entry.OutputSize, - Status: entry.Status, - Error: entry.Error, - } - - if entry.Duration > 0 { - toolCall.Duration = timeutil.FormatDuration(time.Duration(entry.Duration * float64(time.Millisecond))) - } - - mcpData.ToolCalls = append(mcpData.ToolCalls, toolCall) - } - } - - if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("error reading gateway.jsonl: %w", err) - } - } - - // Build summary statistics from aggregated metrics - for serverName, serverMetrics := range gatewayMetrics.Servers { - // Server-level stats - serverStats := MCPServerStats{ - ServerName: serverName, - RequestCount: serverMetrics.RequestCount, - ToolCallCount: serverMetrics.ToolCallCount, - TotalInputSize: 0, - TotalOutputSize: 0, - ErrorCount: serverMetrics.ErrorCount, - } - - if serverMetrics.RequestCount > 0 { - avgDur := serverMetrics.TotalDuration / float64(serverMetrics.RequestCount) - serverStats.AvgDuration = timeutil.FormatDuration(time.Duration(avgDur * float64(time.Millisecond))) - } - - // Tool-level stats - for toolName, toolMetrics := range serverMetrics.Tools { - summary := MCPToolSummary{ - ServerName: serverName, - ToolName: toolName, - CallCount: toolMetrics.CallCount, - TotalInputSize: toolMetrics.TotalInputSize, - TotalOutputSize: toolMetrics.TotalOutputSize, - MaxInputSize: 0, // Will be calculated below - MaxOutputSize: 0, // Will be calculated below - ErrorCount: toolMetrics.ErrorCount, - } - - if toolMetrics.AvgDuration > 0 { - summary.AvgDuration = timeutil.FormatDuration(time.Duration(toolMetrics.AvgDuration * float64(time.Millisecond))) - } - if toolMetrics.MaxDuration > 0 { - summary.MaxDuration = timeutil.FormatDuration(time.Duration(toolMetrics.MaxDuration * float64(time.Millisecond))) - } - - // Calculate max input/output sizes from individual tool calls - for _, tc := range mcpData.ToolCalls { - if tc.ServerName == serverName && tc.ToolName == toolName { - if tc.InputSize > summary.MaxInputSize { - summary.MaxInputSize = tc.InputSize - } - if tc.OutputSize > summary.MaxOutputSize { - summary.MaxOutputSize = tc.OutputSize - } - } - } - - mcpData.Summary = append(mcpData.Summary, summary) - - // Update server totals - serverStats.TotalInputSize += toolMetrics.TotalInputSize - serverStats.TotalOutputSize += toolMetrics.TotalOutputSize - } - - mcpData.Servers = append(mcpData.Servers, serverStats) - } - - // Sort summaries by server name, then tool name - sort.Slice(mcpData.Summary, func(i, j int) bool { - if mcpData.Summary[i].ServerName != mcpData.Summary[j].ServerName { - return mcpData.Summary[i].ServerName < mcpData.Summary[j].ServerName - } - return mcpData.Summary[i].ToolName < mcpData.Summary[j].ToolName - }) - - // Sort servers by name - sort.Slice(mcpData.Servers, func(i, j int) bool { - return mcpData.Servers[i].ServerName < mcpData.Servers[j].ServerName - }) - - return mcpData, nil -} - -// buildGuardPolicySummary creates a GuardPolicySummary from GatewayMetrics. -func buildGuardPolicySummary(metrics *GatewayMetrics) *GuardPolicySummary { - summary := &GuardPolicySummary{ - TotalBlocked: metrics.TotalGuardBlocked, - Events: metrics.GuardPolicyEvents, - BlockedToolCounts: make(map[string]int), - BlockedServerCounts: make(map[string]int), - } - - for _, evt := range metrics.GuardPolicyEvents { - // Categorize by error code - switch evt.ErrorCode { - case guardPolicyErrorCodeIntegrityBelowMin: - summary.IntegrityBlocked++ - case guardPolicyErrorCodeRepoNotAllowed: - summary.RepoScopeBlocked++ - case guardPolicyErrorCodeAccessDenied: - summary.AccessDenied++ - case guardPolicyErrorCodeBlockedUser: - summary.BlockedUserDenied++ - case guardPolicyErrorCodeInsufficientPerms: - summary.PermissionDenied++ - case guardPolicyErrorCodePrivateRepoDenied: - summary.PrivateRepoDenied++ - } - - // Track per-tool blocked counts - if evt.ToolName != "" { - summary.BlockedToolCounts[evt.ToolName]++ - } - - // Track per-server blocked counts - if evt.ServerID != "" { - summary.BlockedServerCounts[evt.ServerID]++ - } - } - - return summary -} - -// displayAggregatedGatewayMetrics aggregates and displays gateway metrics across all processed runs -func displayAggregatedGatewayMetrics(processedRuns []ProcessedRun, outputDir string, verbose bool) { - // Aggregate gateway metrics from all runs - aggregated := &GatewayMetrics{ - Servers: make(map[string]*GatewayServerMetrics), - } - - runCount := 0 - for _, pr := range processedRuns { - runDir := pr.Run.LogsPath - if runDir == "" { - continue - } - - // Try to parse gateway.jsonl from this run - runMetrics, err := parseGatewayLogs(runDir, false) - if err != nil { - // Skip runs without gateway.jsonl (this is normal for runs without MCP gateway) - continue - } - - runCount++ - - // Merge metrics from this run into aggregated metrics - aggregated.TotalRequests += runMetrics.TotalRequests - aggregated.TotalToolCalls += runMetrics.TotalToolCalls - aggregated.TotalErrors += runMetrics.TotalErrors - aggregated.TotalFiltered += runMetrics.TotalFiltered - aggregated.TotalGuardBlocked += runMetrics.TotalGuardBlocked - aggregated.TotalDuration += runMetrics.TotalDuration - aggregated.FilteredEvents = append(aggregated.FilteredEvents, runMetrics.FilteredEvents...) - aggregated.GuardPolicyEvents = append(aggregated.GuardPolicyEvents, runMetrics.GuardPolicyEvents...) - - // Merge server metrics - for serverName, serverMetrics := range runMetrics.Servers { - aggServer := getOrCreateServer(aggregated, serverName) - aggServer.RequestCount += serverMetrics.RequestCount - aggServer.ToolCallCount += serverMetrics.ToolCallCount - aggServer.TotalDuration += serverMetrics.TotalDuration - aggServer.ErrorCount += serverMetrics.ErrorCount - aggServer.FilteredCount += serverMetrics.FilteredCount - aggServer.GuardPolicyBlocked += serverMetrics.GuardPolicyBlocked - - // Merge tool metrics - for toolName, toolMetrics := range serverMetrics.Tools { - aggTool := getOrCreateTool(aggServer, toolName) - aggTool.CallCount += toolMetrics.CallCount - aggTool.TotalDuration += toolMetrics.TotalDuration - aggTool.ErrorCount += toolMetrics.ErrorCount - aggTool.TotalInputSize += toolMetrics.TotalInputSize - aggTool.TotalOutputSize += toolMetrics.TotalOutputSize - - // Update max/min durations - if toolMetrics.MaxDuration > aggTool.MaxDuration { - aggTool.MaxDuration = toolMetrics.MaxDuration - } - if aggTool.MinDuration == 0 || (toolMetrics.MinDuration > 0 && toolMetrics.MinDuration < aggTool.MinDuration) { - aggTool.MinDuration = toolMetrics.MinDuration - } - } - } - - // Update time range - if aggregated.StartTime.IsZero() || (!runMetrics.StartTime.IsZero() && runMetrics.StartTime.Before(aggregated.StartTime)) { - aggregated.StartTime = runMetrics.StartTime - } - if aggregated.EndTime.IsZero() || (!runMetrics.EndTime.IsZero() && runMetrics.EndTime.After(aggregated.EndTime)) { - aggregated.EndTime = runMetrics.EndTime - } - } - - // Only display if we found gateway metrics - if runCount == 0 || len(aggregated.Servers) == 0 { - return - } - - // Recalculate averages for aggregated data - calculateGatewayAggregates(aggregated) - - // Display the aggregated metrics - if metricsOutput := renderGatewayMetricsTable(aggregated, verbose); metricsOutput != "" { - fmt.Fprint(os.Stderr, metricsOutput) - if runCount > 1 { - fmt.Fprintf(os.Stderr, "\n%s\n", - console.FormatInfoMessage(fmt.Sprintf("Gateway metrics aggregated from %d runs", runCount))) - } - } -} diff --git a/pkg/cli/gateway_logs_extractor.go b/pkg/cli/gateway_logs_extractor.go new file mode 100644 index 00000000000..501595d23a3 --- /dev/null +++ b/pkg/cli/gateway_logs_extractor.go @@ -0,0 +1,247 @@ +// This file provides high-level MCP tool usage extraction functions for gh-aw. +// It combines gateway metrics and individual tool call records into structured +// MCPToolUsageData and GuardPolicySummary for reporting and audit output. + +package cli + +import ( + "bufio" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "sort" + "strings" + "time" + + "github.com/github/gh-aw/pkg/timeutil" +) + +// extractMCPToolUsageData creates detailed MCP tool usage data from gateway metrics +func extractMCPToolUsageData(logDir string, verbose bool) (*MCPToolUsageData, error) { + // Parse gateway logs (falls back to rpc-messages.jsonl automatically) + gatewayMetrics, err := parseGatewayLogs(logDir, verbose) + if err != nil { + // Return nil if no log file exists (not an error for workflows without MCP) + if strings.Contains(err.Error(), "not found") { + return nil, nil + } + return nil, fmt.Errorf("failed to parse gateway logs: %w", err) + } + + if gatewayMetrics == nil || len(gatewayMetrics.Servers) == 0 { + return nil, nil + } + + mcpData := &MCPToolUsageData{ + Summary: []MCPToolSummary{}, + ToolCalls: []MCPToolCall{}, + Servers: []MCPServerStats{}, + FilteredEvents: gatewayMetrics.FilteredEvents, + } + + // Build guard policy summary if there are guard policy events + if len(gatewayMetrics.GuardPolicyEvents) > 0 { + mcpData.GuardPolicySummary = buildGuardPolicySummary(gatewayMetrics) + } + + // Read the log file again to get individual tool call records. + // Prefer gateway.jsonl; fall back to rpc-messages.jsonl when not available. + gatewayLogPath := filepath.Join(logDir, "gateway.jsonl") + usingRPCMessages := false + + if _, err := os.Stat(gatewayLogPath); os.IsNotExist(err) { + mcpLogsPath := filepath.Join(logDir, "mcp-logs", "gateway.jsonl") + if _, err := os.Stat(mcpLogsPath); os.IsNotExist(err) { + // Fall back to rpc-messages.jsonl + rpcPath := findRPCMessagesPath(logDir) + if rpcPath == "" { + return nil, errors.New("gateway.jsonl not found") + } + gatewayLogPath = rpcPath + usingRPCMessages = true + } else { + gatewayLogPath = mcpLogsPath + } + } + + if usingRPCMessages { + // Build tool call records from rpc-messages.jsonl + toolCalls, err := buildToolCallsFromRPCMessages(gatewayLogPath) + if err != nil { + return nil, fmt.Errorf("failed to read rpc-messages.jsonl: %w", err) + } + mcpData.ToolCalls = toolCalls + } else { + file, err := os.Open(gatewayLogPath) + if err != nil { + return nil, fmt.Errorf("failed to open gateway.jsonl: %w", err) + } + defer file.Close() + + scanner := bufio.NewScanner(file) + buf := make([]byte, maxScannerBufferSize) + scanner.Buffer(buf, maxScannerBufferSize) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" { + continue + } + + var entry GatewayLogEntry + if err := json.Unmarshal([]byte(line), &entry); err != nil { + continue // Skip malformed lines + } + + // Only process tool call events + if entry.Event == "tool_call" || entry.Event == "rpc_call" || entry.Event == "request" { + toolName := entry.ToolName + if toolName == "" { + toolName = entry.Method + } + + // Skip entries without tool information + if entry.ServerName == "" || toolName == "" { + continue + } + + // Create individual tool call record + toolCall := MCPToolCall{ + Timestamp: entry.Timestamp, + ServerName: entry.ServerName, + ToolName: toolName, + Method: entry.Method, + InputSize: entry.InputSize, + OutputSize: entry.OutputSize, + Status: entry.Status, + Error: entry.Error, + } + + if entry.Duration > 0 { + toolCall.Duration = timeutil.FormatDuration(time.Duration(entry.Duration * float64(time.Millisecond))) + } + + mcpData.ToolCalls = append(mcpData.ToolCalls, toolCall) + } + } + + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("error reading gateway.jsonl: %w", err) + } + } + + // Build summary statistics from aggregated metrics + for serverName, serverMetrics := range gatewayMetrics.Servers { + // Server-level stats + serverStats := MCPServerStats{ + ServerName: serverName, + RequestCount: serverMetrics.RequestCount, + ToolCallCount: serverMetrics.ToolCallCount, + TotalInputSize: 0, + TotalOutputSize: 0, + ErrorCount: serverMetrics.ErrorCount, + } + + if serverMetrics.RequestCount > 0 { + avgDur := serverMetrics.TotalDuration / float64(serverMetrics.RequestCount) + serverStats.AvgDuration = timeutil.FormatDuration(time.Duration(avgDur * float64(time.Millisecond))) + } + + // Tool-level stats + for toolName, toolMetrics := range serverMetrics.Tools { + summary := MCPToolSummary{ + ServerName: serverName, + ToolName: toolName, + CallCount: toolMetrics.CallCount, + TotalInputSize: toolMetrics.TotalInputSize, + TotalOutputSize: toolMetrics.TotalOutputSize, + MaxInputSize: 0, // Will be calculated below + MaxOutputSize: 0, // Will be calculated below + ErrorCount: toolMetrics.ErrorCount, + } + + if toolMetrics.AvgDuration > 0 { + summary.AvgDuration = timeutil.FormatDuration(time.Duration(toolMetrics.AvgDuration * float64(time.Millisecond))) + } + if toolMetrics.MaxDuration > 0 { + summary.MaxDuration = timeutil.FormatDuration(time.Duration(toolMetrics.MaxDuration * float64(time.Millisecond))) + } + + // Calculate max input/output sizes from individual tool calls + for _, tc := range mcpData.ToolCalls { + if tc.ServerName == serverName && tc.ToolName == toolName { + if tc.InputSize > summary.MaxInputSize { + summary.MaxInputSize = tc.InputSize + } + if tc.OutputSize > summary.MaxOutputSize { + summary.MaxOutputSize = tc.OutputSize + } + } + } + + mcpData.Summary = append(mcpData.Summary, summary) + + // Update server totals + serverStats.TotalInputSize += toolMetrics.TotalInputSize + serverStats.TotalOutputSize += toolMetrics.TotalOutputSize + } + + mcpData.Servers = append(mcpData.Servers, serverStats) + } + + // Sort summaries by server name, then tool name + sort.Slice(mcpData.Summary, func(i, j int) bool { + if mcpData.Summary[i].ServerName != mcpData.Summary[j].ServerName { + return mcpData.Summary[i].ServerName < mcpData.Summary[j].ServerName + } + return mcpData.Summary[i].ToolName < mcpData.Summary[j].ToolName + }) + + // Sort servers by name + sort.Slice(mcpData.Servers, func(i, j int) bool { + return mcpData.Servers[i].ServerName < mcpData.Servers[j].ServerName + }) + + return mcpData, nil +} + +// buildGuardPolicySummary creates a GuardPolicySummary from GatewayMetrics. +func buildGuardPolicySummary(metrics *GatewayMetrics) *GuardPolicySummary { + summary := &GuardPolicySummary{ + TotalBlocked: metrics.TotalGuardBlocked, + Events: metrics.GuardPolicyEvents, + BlockedToolCounts: make(map[string]int), + BlockedServerCounts: make(map[string]int), + } + + for _, evt := range metrics.GuardPolicyEvents { + // Categorize by error code + switch evt.ErrorCode { + case guardPolicyErrorCodeIntegrityBelowMin: + summary.IntegrityBlocked++ + case guardPolicyErrorCodeRepoNotAllowed: + summary.RepoScopeBlocked++ + case guardPolicyErrorCodeAccessDenied: + summary.AccessDenied++ + case guardPolicyErrorCodeBlockedUser: + summary.BlockedUserDenied++ + case guardPolicyErrorCodeInsufficientPerms: + summary.PermissionDenied++ + case guardPolicyErrorCodePrivateRepoDenied: + summary.PrivateRepoDenied++ + } + + // Track per-tool blocked counts + if evt.ToolName != "" { + summary.BlockedToolCounts[evt.ToolName]++ + } + + // Track per-server blocked counts + if evt.ServerID != "" { + summary.BlockedServerCounts[evt.ServerID]++ + } + } + + return summary +} diff --git a/pkg/cli/gateway_logs_parser.go b/pkg/cli/gateway_logs_parser.go new file mode 100644 index 00000000000..37a0580cb19 --- /dev/null +++ b/pkg/cli/gateway_logs_parser.go @@ -0,0 +1,630 @@ +// This file provides RPC message and gateway log parsing functions for gh-aw. +// It handles reading gateway.jsonl and rpc-messages.jsonl, building GatewayMetrics +// and MCPToolCall records, and computing aggregate statistics. + +package cli + +import ( + "bufio" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "time" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/timeutil" +) + +// parseRPCMessages parses a rpc-messages.jsonl file and extracts GatewayMetrics. +// This is the canonical fallback when gateway.jsonl is not available. +func parseRPCMessages(logPath string, verbose bool) (*GatewayMetrics, error) { + gatewayLogsLog.Printf("Parsing rpc-messages.jsonl from: %s", logPath) + + file, err := os.Open(logPath) + if err != nil { + return nil, fmt.Errorf("failed to open rpc-messages.jsonl: %w", err) + } + defer file.Close() + + metrics := &GatewayMetrics{ + Servers: make(map[string]*GatewayServerMetrics), + } + + // Track pending requests by (serverID, id) for duration calculation. + // Key format: "/" + pendingRequests := make(map[string]*rpcPendingRequest) + + scanner := bufio.NewScanner(file) + // Increase scanner buffer for large payloads + buf := make([]byte, maxScannerBufferSize) + scanner.Buffer(buf, maxScannerBufferSize) + lineNum := 0 + + for scanner.Scan() { + lineNum++ + line := strings.TrimSpace(scanner.Text()) + if line == "" { + continue + } + + var entry RPCMessageEntry + if err := json.Unmarshal([]byte(line), &entry); err != nil { + gatewayLogsLog.Printf("Failed to parse rpc-messages.jsonl line %d: %v", lineNum, err) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage( + fmt.Sprintf("Failed to parse rpc-messages.jsonl line %d: %v", lineNum, err))) + } + continue + } + + // Update time range + if entry.Timestamp != "" { + if t, err := time.Parse(time.RFC3339Nano, entry.Timestamp); err == nil { + if metrics.StartTime.IsZero() || t.Before(metrics.StartTime) { + metrics.StartTime = t + } + if metrics.EndTime.IsZero() || t.After(metrics.EndTime) { + metrics.EndTime = t + } + } + } + + if entry.ServerID == "" { + continue + } + + switch { + case entry.Type == "DIFC_FILTERED": + // DIFC integrity/secrecy filter event — not a REQUEST or RESPONSE + metrics.TotalFiltered++ + server := getOrCreateServer(metrics, entry.ServerID) + server.FilteredCount++ + metrics.FilteredEvents = append(metrics.FilteredEvents, DifcFilteredEvent{ + Timestamp: entry.Timestamp, + ServerID: entry.ServerID, + ToolName: entry.ToolName, + Description: entry.Description, + Reason: entry.Reason, + SecrecyTags: entry.SecrecyTags, + IntegrityTags: entry.IntegrityTags, + AuthorAssociation: entry.AuthorAssociation, + AuthorLogin: entry.AuthorLogin, + HTMLURL: entry.HTMLURL, + Number: entry.Number, + }) + + case entry.Direction == "OUT" && entry.Type == "REQUEST": + // Outgoing request from AI engine to MCP server + var req rpcRequestPayload + if err := json.Unmarshal(entry.Payload, &req); err != nil { + continue + } + if req.Method != "tools/call" { + continue + } + + // Extract tool name + var params rpcToolCallParams + if err := json.Unmarshal(req.Params, ¶ms); err != nil || params.Name == "" { + continue + } + + metrics.TotalRequests++ + server := getOrCreateServer(metrics, entry.ServerID) + server.RequestCount++ + metrics.TotalToolCalls++ + server.ToolCallCount++ + + tool := getOrCreateTool(server, params.Name) + tool.CallCount++ + + // Store pending request for duration calculation + if req.ID != nil && entry.Timestamp != "" { + if t, err := time.Parse(time.RFC3339Nano, entry.Timestamp); err == nil { + key := fmt.Sprintf("%s/%v", entry.ServerID, req.ID) + pendingRequests[key] = &rpcPendingRequest{ + ServerID: entry.ServerID, + ToolName: params.Name, + Timestamp: t, + } + } + } + + case entry.Direction == "IN" && entry.Type == "RESPONSE": + // Incoming response from MCP server to AI engine + var resp rpcResponsePayload + if err := json.Unmarshal(entry.Payload, &resp); err != nil { + continue + } + + // Track errors and detect guard policy blocks + if resp.Error != nil { + metrics.TotalErrors++ + server := getOrCreateServer(metrics, entry.ServerID) + server.ErrorCount++ + + // Detect guard policy enforcement errors + if isGuardPolicyErrorCode(resp.Error.Code) { + metrics.TotalGuardBlocked++ + server.GuardPolicyBlocked++ + + // Determine tool name from pending request if available + toolName := "" + if resp.ID != nil { + key := fmt.Sprintf("%s/%v", entry.ServerID, resp.ID) + if pending, ok := pendingRequests[key]; ok { + toolName = pending.ToolName + } + } + + reason := guardPolicyReasonFromCode(resp.Error.Code) + if resp.Error.Data != nil && resp.Error.Data.Reason != "" { + reason = resp.Error.Data.Reason + } + + evt := GuardPolicyEvent{ + Timestamp: entry.Timestamp, + ServerID: entry.ServerID, + ToolName: toolName, + ErrorCode: resp.Error.Code, + Reason: reason, + Message: resp.Error.Message, + } + if resp.Error.Data != nil { + evt.Details = resp.Error.Data.Details + evt.Repository = resp.Error.Data.Repository + } + metrics.GuardPolicyEvents = append(metrics.GuardPolicyEvents, evt) + } + } + + // Calculate duration by matching with pending request + if resp.ID != nil && entry.Timestamp != "" { + key := fmt.Sprintf("%s/%v", entry.ServerID, resp.ID) + if pending, ok := pendingRequests[key]; ok { + delete(pendingRequests, key) + if t, err := time.Parse(time.RFC3339Nano, entry.Timestamp); err == nil { + durationMs := float64(t.Sub(pending.Timestamp).Milliseconds()) + if durationMs >= 0 { + server := getOrCreateServer(metrics, entry.ServerID) + server.TotalDuration += durationMs + metrics.TotalDuration += durationMs + + tool := getOrCreateTool(server, pending.ToolName) + tool.TotalDuration += durationMs + if tool.MaxDuration == 0 || durationMs > tool.MaxDuration { + tool.MaxDuration = durationMs + } + if tool.MinDuration == 0 || durationMs < tool.MinDuration { + tool.MinDuration = durationMs + } + + if resp.Error != nil { + tool.ErrorCount++ + } + } + } + } + } + } + } + + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("error reading rpc-messages.jsonl: %w", err) + } + + calculateGatewayAggregates(metrics) + + gatewayLogsLog.Printf("Successfully parsed rpc-messages.jsonl: %d servers, %d total requests", + len(metrics.Servers), metrics.TotalRequests) + + return metrics, nil +} + +// findRPCMessagesPath returns the path to rpc-messages.jsonl if it exists, or "" if not found. +func findRPCMessagesPath(logDir string) string { + // Check mcp-logs subdirectory (standard location) + mcpLogsPath := filepath.Join(logDir, "mcp-logs", "rpc-messages.jsonl") + if _, err := os.Stat(mcpLogsPath); err == nil { + return mcpLogsPath + } + // Check root directory as fallback + rootPath := filepath.Join(logDir, "rpc-messages.jsonl") + if _, err := os.Stat(rootPath); err == nil { + return rootPath + } + return "" +} + +// parseGatewayLogs parses a gateway.jsonl file and extracts metrics. +// Falls back to rpc-messages.jsonl (canonical fallback) when gateway.jsonl is not present. +func parseGatewayLogs(logDir string, verbose bool) (*GatewayMetrics, error) { + // Try root directory first (for older logs where gateway.jsonl was in the root) + gatewayLogPath := filepath.Join(logDir, "gateway.jsonl") + + // Check if gateway.jsonl exists in root + if _, err := os.Stat(gatewayLogPath); os.IsNotExist(err) { + // Try mcp-logs subdirectory (new path after artifact download) + // Gateway logs are uploaded from /tmp/gh-aw/mcp-logs/gateway.jsonl and the common parent + // /tmp/gh-aw/ is stripped during artifact upload, resulting in mcp-logs/gateway.jsonl after download + mcpLogsPath := filepath.Join(logDir, "mcp-logs", "gateway.jsonl") + if _, err := os.Stat(mcpLogsPath); os.IsNotExist(err) { + // Fall back to rpc-messages.jsonl (canonical fallback when gateway.jsonl is missing) + rpcPath := findRPCMessagesPath(logDir) + if rpcPath != "" { + gatewayLogsLog.Printf("gateway.jsonl not found; falling back to rpc-messages.jsonl: %s", rpcPath) + return parseRPCMessages(rpcPath, verbose) + } + gatewayLogsLog.Printf("gateway.jsonl not found at: %s or %s", gatewayLogPath, mcpLogsPath) + return nil, errors.New("gateway.jsonl not found") + } + gatewayLogPath = mcpLogsPath + gatewayLogsLog.Printf("Found gateway.jsonl in mcp-logs subdirectory") + } + + gatewayLogsLog.Printf("Parsing gateway.jsonl from: %s", gatewayLogPath) + + file, err := os.Open(gatewayLogPath) + if err != nil { + return nil, fmt.Errorf("failed to open gateway.jsonl: %w", err) + } + defer file.Close() + + metrics := &GatewayMetrics{ + Servers: make(map[string]*GatewayServerMetrics), + } + + scanner := bufio.NewScanner(file) + buf := make([]byte, maxScannerBufferSize) + scanner.Buffer(buf, maxScannerBufferSize) + lineNum := 0 + + for scanner.Scan() { + lineNum++ + line := strings.TrimSpace(scanner.Text()) + + // Skip empty lines + if line == "" { + continue + } + + var entry GatewayLogEntry + if err := json.Unmarshal([]byte(line), &entry); err != nil { + gatewayLogsLog.Printf("Failed to parse line %d: %v", lineNum, err) + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to parse gateway.jsonl line %d: %v", lineNum, err))) + } + continue + } + + // Process the entry based on its type/event + processGatewayLogEntry(&entry, metrics, verbose) + } + + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("error reading gateway.jsonl: %w", err) + } + + // Calculate aggregate statistics + calculateGatewayAggregates(metrics) + + gatewayLogsLog.Printf("Successfully parsed gateway.jsonl: %d servers, %d total requests", + len(metrics.Servers), metrics.TotalRequests) + + return metrics, nil +} + +// processGatewayLogEntry processes a single log entry and updates metrics +func processGatewayLogEntry(entry *GatewayLogEntry, metrics *GatewayMetrics, verbose bool) { + // Parse timestamp for time range (supports both RFC3339 and RFC3339Nano) + if entry.Timestamp != "" { + t, err := time.Parse(time.RFC3339Nano, entry.Timestamp) + if err != nil { + t, err = time.Parse(time.RFC3339, entry.Timestamp) + } + if err == nil { + if metrics.StartTime.IsZero() || t.Before(metrics.StartTime) { + metrics.StartTime = t + } + if metrics.EndTime.IsZero() || t.After(metrics.EndTime) { + metrics.EndTime = t + } + } + } + + // Handle DIFC_FILTERED events + if entry.Type == "DIFC_FILTERED" { + metrics.TotalFiltered++ + // DIFC_FILTERED events use server_id; fall back to server_name for compatibility + serverKey := entry.ServerID + if serverKey == "" { + serverKey = entry.ServerName + } + if serverKey != "" { + server := getOrCreateServer(metrics, serverKey) + server.FilteredCount++ + } + metrics.FilteredEvents = append(metrics.FilteredEvents, DifcFilteredEvent{ + Timestamp: entry.Timestamp, + ServerID: serverKey, + ToolName: entry.ToolName, + Description: entry.Description, + Reason: entry.Reason, + SecrecyTags: entry.SecrecyTags, + IntegrityTags: entry.IntegrityTags, + AuthorAssociation: entry.AuthorAssociation, + AuthorLogin: entry.AuthorLogin, + HTMLURL: entry.HTMLURL, + Number: entry.Number, + }) + return + } + + // Handle GUARD_POLICY_BLOCKED events from gateway.jsonl + if entry.Type == "GUARD_POLICY_BLOCKED" { + metrics.TotalGuardBlocked++ + serverKey := entry.ServerID + if serverKey == "" { + serverKey = entry.ServerName + } + if serverKey != "" { + server := getOrCreateServer(metrics, serverKey) + server.GuardPolicyBlocked++ + } + metrics.GuardPolicyEvents = append(metrics.GuardPolicyEvents, GuardPolicyEvent{ + Timestamp: entry.Timestamp, + ServerID: serverKey, + ToolName: entry.ToolName, + Reason: entry.Reason, + Message: entry.Message, + Details: entry.Description, + }) + return + } + + // Track errors + if entry.Status == "error" || entry.Error != "" { + metrics.TotalErrors++ + if entry.ServerName != "" { + server := getOrCreateServer(metrics, entry.ServerName) + server.ErrorCount++ + + if entry.ToolName != "" { + tool := getOrCreateTool(server, entry.ToolName) + tool.ErrorCount++ + } + } + } + + // Process based on event type + switch entry.Event { + case "request", "tool_call", "rpc_call": + metrics.TotalRequests++ + + if entry.ServerName != "" { + server := getOrCreateServer(metrics, entry.ServerName) + server.RequestCount++ + + if entry.Duration > 0 { + server.TotalDuration += entry.Duration + metrics.TotalDuration += entry.Duration + } + + // Track tool calls + if entry.ToolName != "" || entry.Method != "" { + toolName := entry.ToolName + if toolName == "" { + toolName = entry.Method + } + + metrics.TotalToolCalls++ + server.ToolCallCount++ + + tool := getOrCreateTool(server, toolName) + tool.CallCount++ + + if entry.Duration > 0 { + tool.TotalDuration += entry.Duration + if tool.MaxDuration == 0 || entry.Duration > tool.MaxDuration { + tool.MaxDuration = entry.Duration + } + if tool.MinDuration == 0 || entry.Duration < tool.MinDuration { + tool.MinDuration = entry.Duration + } + } + + if entry.InputSize > 0 { + tool.TotalInputSize += entry.InputSize + } + if entry.OutputSize > 0 { + tool.TotalOutputSize += entry.OutputSize + } + } + } + } +} + +// getOrCreateServer gets or creates a server metrics entry +func getOrCreateServer(metrics *GatewayMetrics, serverName string) *GatewayServerMetrics { + if server, exists := metrics.Servers[serverName]; exists { + return server + } + + server := &GatewayServerMetrics{ + ServerName: serverName, + Tools: make(map[string]*GatewayToolMetrics), + } + metrics.Servers[serverName] = server + return server +} + +// getOrCreateTool gets or creates a tool metrics entry +func getOrCreateTool(server *GatewayServerMetrics, toolName string) *GatewayToolMetrics { + if tool, exists := server.Tools[toolName]; exists { + return tool + } + + tool := &GatewayToolMetrics{ + ToolName: toolName, + } + server.Tools[toolName] = tool + return tool +} + +// calculateGatewayAggregates calculates aggregate statistics +func calculateGatewayAggregates(metrics *GatewayMetrics) { + for _, server := range metrics.Servers { + for _, tool := range server.Tools { + if tool.CallCount > 0 { + tool.AvgDuration = tool.TotalDuration / float64(tool.CallCount) + } + } + } +} + +// buildToolCallsFromRPCMessages reads rpc-messages.jsonl and builds MCPToolCall records. +// Duration is computed by pairing outgoing requests with incoming responses. +// Input/output sizes are not available in rpc-messages.jsonl and will be 0. +func buildToolCallsFromRPCMessages(logPath string) ([]MCPToolCall, error) { + file, err := os.Open(logPath) + if err != nil { + return nil, fmt.Errorf("failed to open rpc-messages.jsonl: %w", err) + } + defer file.Close() + + type pendingCall struct { + serverID string + toolName string + timestamp time.Time + } + pending := make(map[string]*pendingCall) // key: "/" + + // Collect requests first to pair with responses + type rawEntry struct { + entry RPCMessageEntry + req rpcRequestPayload + resp rpcResponsePayload + valid bool + } + var entries []rawEntry + + scanner := bufio.NewScanner(file) + buf := make([]byte, maxScannerBufferSize) + scanner.Buffer(buf, maxScannerBufferSize) + + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" { + continue + } + var e RPCMessageEntry + if err := json.Unmarshal([]byte(line), &e); err != nil { + continue + } + entries = append(entries, rawEntry{entry: e, valid: true}) + } + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("error reading rpc-messages.jsonl: %w", err) + } + + // Second pass: build MCPToolCall records. + // Declared before first pass so requests without IDs can be appended immediately. + var toolCalls []MCPToolCall + processedKeys := make(map[string]bool) + + // First pass: index outgoing tool-call requests by (serverID, id) + for i := range entries { + e := &entries[i] + if e.entry.Direction != "OUT" || e.entry.Type != "REQUEST" { + continue + } + if err := json.Unmarshal(e.entry.Payload, &e.req); err != nil || e.req.Method != "tools/call" { + continue + } + var params rpcToolCallParams + if err := json.Unmarshal(e.req.Params, ¶ms); err != nil || params.Name == "" { + continue + } + if e.req.ID == nil { + // Requests without an ID cannot be matched to responses. + // Emit the tool call immediately with "unknown" status so it appears + // in the tool_calls list (same as parseRPCMessages counts it in the summary). + toolCalls = append(toolCalls, MCPToolCall{ + Timestamp: e.entry.Timestamp, + ServerName: e.entry.ServerID, + ToolName: params.Name, + Status: "unknown", + }) + continue + } + t, err := time.Parse(time.RFC3339Nano, e.entry.Timestamp) + if err != nil { + continue + } + key := fmt.Sprintf("%s/%v", e.entry.ServerID, e.req.ID) + pending[key] = &pendingCall{ + serverID: e.entry.ServerID, + toolName: params.Name, + timestamp: t, + } + } + + // Second pass: pair responses with pending requests to compute durations + + for i := range entries { + e := &entries[i] + switch { + case e.entry.Direction == "OUT" && e.entry.Type == "REQUEST": + // Outgoing tool-call request – we'll emit the record when we see the response + // (or after if no response found) + case e.entry.Direction == "IN" && e.entry.Type == "RESPONSE": + if err := json.Unmarshal(e.entry.Payload, &e.resp); err != nil { + continue + } + if e.resp.ID == nil { + continue + } + key := fmt.Sprintf("%s/%v", e.entry.ServerID, e.resp.ID) + p, ok := pending[key] + if !ok { + continue + } + processedKeys[key] = true + + call := MCPToolCall{ + Timestamp: p.timestamp.Format(time.RFC3339Nano), + ServerName: p.serverID, + ToolName: p.toolName, + Status: "success", + } + if e.resp.Error != nil { + call.Status = "error" + call.Error = e.resp.Error.Message + } + if t, err := time.Parse(time.RFC3339Nano, e.entry.Timestamp); err == nil { + d := t.Sub(p.timestamp) + if d >= 0 { + call.Duration = timeutil.FormatDuration(d) + } + } + toolCalls = append(toolCalls, call) + } + } + + // Emit any requests that never received a response + for key, p := range pending { + if !processedKeys[key] { + toolCalls = append(toolCalls, MCPToolCall{ + Timestamp: p.timestamp.Format(time.RFC3339Nano), + ServerName: p.serverID, + ToolName: p.toolName, + Status: "unknown", + }) + } + } + + return toolCalls, nil +} diff --git a/pkg/cli/gateway_logs_renderer.go b/pkg/cli/gateway_logs_renderer.go new file mode 100644 index 00000000000..8ab69ac6add --- /dev/null +++ b/pkg/cli/gateway_logs_renderer.go @@ -0,0 +1,301 @@ +// This file provides rendering functions for MCP gateway metrics in gh-aw. +// It formats GatewayMetrics into human-readable console tables and aggregates +// metrics across multiple workflow runs for display. + +package cli + +import ( + "fmt" + "os" + "sort" + "strconv" + "strings" + "time" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/sliceutil" +) + +// guardPolicyReasonFromCode returns a human-readable reason string for a guard policy error code. +func guardPolicyReasonFromCode(code int) string { + switch code { + case guardPolicyErrorCodeAccessDenied: + return "access_denied" + case guardPolicyErrorCodeRepoNotAllowed: + return "repo_not_allowed" + case guardPolicyErrorCodeInsufficientPerms: + return "insufficient_permissions" + case guardPolicyErrorCodePrivateRepoDenied: + return "private_repo_denied" + case guardPolicyErrorCodeBlockedUser: + return "blocked_user" + case guardPolicyErrorCodeIntegrityBelowMin: + return "integrity_below_minimum" + default: + return "unknown" + } +} + +// renderGatewayMetricsTable renders gateway metrics as a console table +func renderGatewayMetricsTable(metrics *GatewayMetrics, verbose bool) string { + if metrics == nil || len(metrics.Servers) == 0 { + return "" + } + + var output strings.Builder + + output.WriteString("\n") + output.WriteString(console.FormatInfoMessage("MCP Gateway Metrics")) + output.WriteString("\n\n") + + // Summary statistics + fmt.Fprintf(&output, "Total Requests: %d\n", metrics.TotalRequests) + fmt.Fprintf(&output, "Total Tool Calls: %d\n", metrics.TotalToolCalls) + fmt.Fprintf(&output, "Total Errors: %d\n", metrics.TotalErrors) + if metrics.TotalFiltered > 0 { + fmt.Fprintf(&output, "Total DIFC Filtered: %d\n", metrics.TotalFiltered) + } + if metrics.TotalGuardBlocked > 0 { + fmt.Fprintf(&output, "Total Guard Policy Blocked: %d\n", metrics.TotalGuardBlocked) + } + fmt.Fprintf(&output, "Servers: %d\n", len(metrics.Servers)) + + if !metrics.StartTime.IsZero() && !metrics.EndTime.IsZero() { + duration := metrics.EndTime.Sub(metrics.StartTime) + fmt.Fprintf(&output, "Time Range: %s\n", duration.Round(time.Second)) + } + + output.WriteString("\n") + + // Server metrics table + if len(metrics.Servers) > 0 { + // Sort servers by request count + serverNames := getSortedServerNames(metrics) + + hasFiltered := metrics.TotalFiltered > 0 + hasGuardPolicy := metrics.TotalGuardBlocked > 0 + serverRows := make([][]string, 0, len(serverNames)) + for _, serverName := range serverNames { + server := metrics.Servers[serverName] + avgTime := 0.0 + if server.RequestCount > 0 { + avgTime = server.TotalDuration / float64(server.RequestCount) + } + row := []string{ + serverName, + strconv.Itoa(server.RequestCount), + strconv.Itoa(server.ToolCallCount), + fmt.Sprintf("%.0fms", avgTime), + strconv.Itoa(server.ErrorCount), + } + if hasFiltered { + row = append(row, strconv.Itoa(server.FilteredCount)) + } + if hasGuardPolicy { + row = append(row, strconv.Itoa(server.GuardPolicyBlocked)) + } + serverRows = append(serverRows, row) + } + + headers := []string{"Server", "Requests", "Tool Calls", "Avg Time", "Errors"} + if hasFiltered { + headers = append(headers, "Filtered") + } + if hasGuardPolicy { + headers = append(headers, "Guard Blocked") + } + output.WriteString(console.RenderTable(console.TableConfig{ + Title: "Server Usage", + Headers: headers, + Rows: serverRows, + })) + } + + // DIFC filtered events table + if len(metrics.FilteredEvents) > 0 { + output.WriteString("\n") + filteredRows := make([][]string, 0, len(metrics.FilteredEvents)) + for _, fe := range metrics.FilteredEvents { + reason := fe.Reason + if len(reason) > 80 { + reason = reason[:77] + "..." + } + filteredRows = append(filteredRows, []string{ + fe.ServerID, + fe.ToolName, + fe.AuthorLogin, + reason, + }) + } + output.WriteString(console.RenderTable(console.TableConfig{ + Title: "DIFC Filtered Events", + Headers: []string{"Server", "Tool", "User", "Reason"}, + Rows: filteredRows, + })) + } + + // Guard policy events table + if len(metrics.GuardPolicyEvents) > 0 { + output.WriteString("\n") + guardRows := make([][]string, 0, len(metrics.GuardPolicyEvents)) + for _, gpe := range metrics.GuardPolicyEvents { + message := gpe.Message + if len(message) > 60 { + message = message[:57] + "..." + } + repo := gpe.Repository + if repo == "" { + repo = "-" + } + guardRows = append(guardRows, []string{ + gpe.ServerID, + gpe.ToolName, + gpe.Reason, + message, + repo, + }) + } + output.WriteString(console.RenderTable(console.TableConfig{ + Title: "Guard Policy Blocked Events", + Headers: []string{"Server", "Tool", "Reason", "Message", "Repository"}, + Rows: guardRows, + })) + } + + // Tool metrics table (if verbose) + if verbose { + output.WriteString("\n") + output.WriteString("Tool Usage Details:\n") + + for _, serverName := range getSortedServerNames(metrics) { + server := metrics.Servers[serverName] + if len(server.Tools) == 0 { + continue + } + + // Sort tools by call count + toolNames := sliceutil.MapToSlice(server.Tools) + sort.Slice(toolNames, func(i, j int) bool { + return server.Tools[toolNames[i]].CallCount > server.Tools[toolNames[j]].CallCount + }) + + toolRows := make([][]string, 0, len(toolNames)) + for _, toolName := range toolNames { + tool := server.Tools[toolName] + toolRows = append(toolRows, []string{ + toolName, + strconv.Itoa(tool.CallCount), + fmt.Sprintf("%.0fms", tool.AvgDuration), + fmt.Sprintf("%.0fms", tool.MaxDuration), + strconv.Itoa(tool.ErrorCount), + }) + } + + output.WriteString(console.RenderTable(console.TableConfig{ + Title: serverName, + Headers: []string{"Tool", "Calls", "Avg Time", "Max Time", "Errors"}, + Rows: toolRows, + })) + } + } + + return output.String() +} + +// getSortedServerNames returns server names sorted by request count +func getSortedServerNames(metrics *GatewayMetrics) []string { + names := sliceutil.MapToSlice(metrics.Servers) + sort.Slice(names, func(i, j int) bool { + return metrics.Servers[names[i]].RequestCount > metrics.Servers[names[j]].RequestCount + }) + return names +} + +// displayAggregatedGatewayMetrics aggregates and displays gateway metrics across all processed runs +func displayAggregatedGatewayMetrics(processedRuns []ProcessedRun, outputDir string, verbose bool) { + // Aggregate gateway metrics from all runs + aggregated := &GatewayMetrics{ + Servers: make(map[string]*GatewayServerMetrics), + } + + runCount := 0 + for _, pr := range processedRuns { + runDir := pr.Run.LogsPath + if runDir == "" { + continue + } + + // Try to parse gateway.jsonl from this run + runMetrics, err := parseGatewayLogs(runDir, false) + if err != nil { + // Skip runs without gateway.jsonl (this is normal for runs without MCP gateway) + continue + } + + runCount++ + + // Merge metrics from this run into aggregated metrics + aggregated.TotalRequests += runMetrics.TotalRequests + aggregated.TotalToolCalls += runMetrics.TotalToolCalls + aggregated.TotalErrors += runMetrics.TotalErrors + aggregated.TotalFiltered += runMetrics.TotalFiltered + aggregated.TotalGuardBlocked += runMetrics.TotalGuardBlocked + aggregated.TotalDuration += runMetrics.TotalDuration + aggregated.FilteredEvents = append(aggregated.FilteredEvents, runMetrics.FilteredEvents...) + aggregated.GuardPolicyEvents = append(aggregated.GuardPolicyEvents, runMetrics.GuardPolicyEvents...) + + // Merge server metrics + for serverName, serverMetrics := range runMetrics.Servers { + aggServer := getOrCreateServer(aggregated, serverName) + aggServer.RequestCount += serverMetrics.RequestCount + aggServer.ToolCallCount += serverMetrics.ToolCallCount + aggServer.TotalDuration += serverMetrics.TotalDuration + aggServer.ErrorCount += serverMetrics.ErrorCount + aggServer.FilteredCount += serverMetrics.FilteredCount + aggServer.GuardPolicyBlocked += serverMetrics.GuardPolicyBlocked + + // Merge tool metrics + for toolName, toolMetrics := range serverMetrics.Tools { + aggTool := getOrCreateTool(aggServer, toolName) + aggTool.CallCount += toolMetrics.CallCount + aggTool.TotalDuration += toolMetrics.TotalDuration + aggTool.ErrorCount += toolMetrics.ErrorCount + aggTool.TotalInputSize += toolMetrics.TotalInputSize + aggTool.TotalOutputSize += toolMetrics.TotalOutputSize + + // Update max/min durations + if toolMetrics.MaxDuration > aggTool.MaxDuration { + aggTool.MaxDuration = toolMetrics.MaxDuration + } + if aggTool.MinDuration == 0 || (toolMetrics.MinDuration > 0 && toolMetrics.MinDuration < aggTool.MinDuration) { + aggTool.MinDuration = toolMetrics.MinDuration + } + } + } + + // Update time range + if aggregated.StartTime.IsZero() || (!runMetrics.StartTime.IsZero() && runMetrics.StartTime.Before(aggregated.StartTime)) { + aggregated.StartTime = runMetrics.StartTime + } + if aggregated.EndTime.IsZero() || (!runMetrics.EndTime.IsZero() && runMetrics.EndTime.After(aggregated.EndTime)) { + aggregated.EndTime = runMetrics.EndTime + } + } + + // Only display if we found gateway metrics + if runCount == 0 || len(aggregated.Servers) == 0 { + return + } + + // Recalculate averages for aggregated data + calculateGatewayAggregates(aggregated) + + // Display the aggregated metrics + if metricsOutput := renderGatewayMetricsTable(aggregated, verbose); metricsOutput != "" { + fmt.Fprint(os.Stderr, metricsOutput) + if runCount > 1 { + fmt.Fprintf(os.Stderr, "\n%s\n", + console.FormatInfoMessage(fmt.Sprintf("Gateway metrics aggregated from %d runs", runCount))) + } + } +} From 7c8967af0b17f47d45c2c61fa13ef5dc3926af08 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 15:11:26 +0000 Subject: [PATCH 3/7] refactor: split audit_report_render.go into domain-specific files Extract policy/firewall rendering to audit_render_policy.go, performance/engine rendering to audit_render_performance.go, and token/rate-limit/MCP-health rendering to audit_render_tokens.go. Reduces audit_report_render.go from 1,139 lines to ~691 lines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/audit_render_performance.go | 118 ++++++++ pkg/cli/audit_render_policy.go | 218 ++++++++++++++ pkg/cli/audit_render_tokens.go | 141 +++++++++ pkg/cli/audit_report_render.go | 448 ---------------------------- 4 files changed, 477 insertions(+), 448 deletions(-) create mode 100644 pkg/cli/audit_render_performance.go create mode 100644 pkg/cli/audit_render_policy.go create mode 100644 pkg/cli/audit_render_tokens.go diff --git a/pkg/cli/audit_render_performance.go b/pkg/cli/audit_render_performance.go new file mode 100644 index 00000000000..83136595b35 --- /dev/null +++ b/pkg/cli/audit_render_performance.go @@ -0,0 +1,118 @@ +package cli + +import ( + "fmt" + "os" + "strings" + + "github.com/github/gh-aw/pkg/console" +) + +// renderPerformanceMetrics renders performance metrics +func renderPerformanceMetrics(metrics *PerformanceMetrics) { + auditReportLog.Printf("Rendering performance metrics: tokens_per_min=%.1f, cost_efficiency=%s, most_used_tool=%s", + metrics.TokensPerMinute, metrics.CostEfficiency, metrics.MostUsedTool) + if metrics.TokensPerMinute > 0 { + fmt.Fprintf(os.Stderr, " Tokens per Minute: %.1f\n", metrics.TokensPerMinute) + } + + if metrics.CostEfficiency != "" { + efficiencyDisplay := metrics.CostEfficiency + switch metrics.CostEfficiency { + case "excellent", "good": + efficiencyDisplay = console.FormatSuccessMessage(metrics.CostEfficiency) + case "moderate": + efficiencyDisplay = console.FormatWarningMessage(metrics.CostEfficiency) + case "poor": + efficiencyDisplay = console.FormatErrorMessage(metrics.CostEfficiency) + } + fmt.Fprintf(os.Stderr, " Cost Efficiency: %s\n", efficiencyDisplay) + } + + if metrics.AvgToolDuration != "" { + fmt.Fprintf(os.Stderr, " Average Tool Duration: %s\n", metrics.AvgToolDuration) + } + + if metrics.MostUsedTool != "" { + fmt.Fprintf(os.Stderr, " Most Used Tool: %s\n", metrics.MostUsedTool) + } + + if metrics.NetworkRequests > 0 { + fmt.Fprintf(os.Stderr, " Network Requests: %d\n", metrics.NetworkRequests) + } + + fmt.Fprintln(os.Stderr) +} + +// renderEngineConfig renders engine configuration details +func renderEngineConfig(config *EngineConfig) { + if config == nil { + return + } + fmt.Fprintf(os.Stderr, " Engine ID: %s\n", config.EngineID) + if config.EngineName != "" { + fmt.Fprintf(os.Stderr, " Engine Name: %s\n", config.EngineName) + } + if config.Model != "" { + fmt.Fprintf(os.Stderr, " Model: %s\n", config.Model) + } + if config.Version != "" { + fmt.Fprintf(os.Stderr, " Version: %s\n", config.Version) + } + if config.CLIVersion != "" { + fmt.Fprintf(os.Stderr, " CLI Version: %s\n", config.CLIVersion) + } + if config.FirewallVersion != "" { + fmt.Fprintf(os.Stderr, " Firewall Version: %s\n", config.FirewallVersion) + } + if config.TriggerEvent != "" { + fmt.Fprintf(os.Stderr, " Trigger Event: %s\n", config.TriggerEvent) + } + if config.Repository != "" { + fmt.Fprintf(os.Stderr, " Repository: %s\n", config.Repository) + } + if len(config.MCPServers) > 0 { + fmt.Fprintf(os.Stderr, " MCP Servers: %s\n", strings.Join(config.MCPServers, ", ")) + } + fmt.Fprintln(os.Stderr) +} + +// renderPromptAnalysis renders prompt analysis metrics +func renderPromptAnalysis(analysis *PromptAnalysis) { + if analysis == nil { + return + } + fmt.Fprintf(os.Stderr, " Prompt Size: %s chars\n", console.FormatNumber(analysis.PromptSize)) + if analysis.PromptFile != "" { + fmt.Fprintf(os.Stderr, " Prompt File: %s\n", analysis.PromptFile) + } + fmt.Fprintln(os.Stderr) +} + +// renderSessionAnalysis renders session and agent performance metrics +func renderSessionAnalysis(session *SessionAnalysis) { + if session == nil { + return + } + if session.WallTime != "" { + fmt.Fprintf(os.Stderr, " Wall Time: %s\n", session.WallTime) + } + if session.TurnCount > 0 { + fmt.Fprintf(os.Stderr, " Turn Count: %d\n", session.TurnCount) + } + if session.AvgTurnDuration != "" { + fmt.Fprintf(os.Stderr, " Avg Turn Duration: %s\n", session.AvgTurnDuration) + } + if session.TokensPerMinute > 0 { + fmt.Fprintf(os.Stderr, " Tokens/Minute: %.1f\n", session.TokensPerMinute) + } + if session.NoopCount > 0 { + fmt.Fprintf(os.Stderr, " Noop Count: %d\n", session.NoopCount) + } + if session.TimeoutDetected { + fmt.Fprintf(os.Stderr, " Timeout Detected: %s\n", console.FormatWarningMessage("Yes")) + } else { + fmt.Fprintf(os.Stderr, " Timeout Detected: %s\n", console.FormatSuccessMessage("No")) + } + fmt.Fprintln(os.Stderr) +} diff --git a/pkg/cli/audit_render_policy.go b/pkg/cli/audit_render_policy.go new file mode 100644 index 00000000000..b82f1f2a37d --- /dev/null +++ b/pkg/cli/audit_render_policy.go @@ -0,0 +1,218 @@ +package cli + +import ( + "fmt" + "math" + "os" + "sort" + "strconv" + "time" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/sliceutil" + "github.com/github/gh-aw/pkg/stringutil" +) + +// renderGuardPolicySummary renders the guard policy enforcement summary +func renderGuardPolicySummary(summary *GuardPolicySummary) { + auditReportLog.Printf("Rendering guard policy summary: %d total blocked", summary.TotalBlocked) + + fmt.Fprintln(os.Stderr) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage( + fmt.Sprintf("Guard Policy: %d tool call(s) blocked", summary.TotalBlocked))) + fmt.Fprintln(os.Stderr) + + // Breakdown by reason + fmt.Fprintln(os.Stderr, " Block Reasons:") + if summary.IntegrityBlocked > 0 { + fmt.Fprintf(os.Stderr, " Integrity below minimum : %d\n", summary.IntegrityBlocked) + } + if summary.RepoScopeBlocked > 0 { + fmt.Fprintf(os.Stderr, " Repository not allowed : %d\n", summary.RepoScopeBlocked) + } + if summary.AccessDenied > 0 { + fmt.Fprintf(os.Stderr, " Access denied : %d\n", summary.AccessDenied) + } + if summary.BlockedUserDenied > 0 { + fmt.Fprintf(os.Stderr, " Blocked user : %d\n", summary.BlockedUserDenied) + } + if summary.PermissionDenied > 0 { + fmt.Fprintf(os.Stderr, " Insufficient permissions: %d\n", summary.PermissionDenied) + } + if summary.PrivateRepoDenied > 0 { + fmt.Fprintf(os.Stderr, " Private repo denied : %d\n", summary.PrivateRepoDenied) + } + fmt.Fprintln(os.Stderr) + + // Most frequently blocked tools + if len(summary.BlockedToolCounts) > 0 { + toolNames := sliceutil.MapToSlice(summary.BlockedToolCounts) + sort.Slice(toolNames, func(i, j int) bool { + return summary.BlockedToolCounts[toolNames[i]] > summary.BlockedToolCounts[toolNames[j]] + }) + + toolRows := make([][]string, 0, len(toolNames)) + for _, name := range toolNames { + toolRows = append(toolRows, []string{name, strconv.Itoa(summary.BlockedToolCounts[name])}) + } + fmt.Fprint(os.Stderr, console.RenderTable(console.TableConfig{ + Title: "Most Blocked Tools", + Headers: []string{"Tool", "Blocked"}, + Rows: toolRows, + })) + } + + // Guard policy event details + if len(summary.Events) > 0 { + fmt.Fprintln(os.Stderr) + eventRows := make([][]string, 0, len(summary.Events)) + for _, evt := range summary.Events { + message := evt.Message + if len(message) > 60 { + message = message[:57] + "..." + } + repo := evt.Repository + if repo == "" { + repo = "-" + } + eventRows = append(eventRows, []string{ + stringutil.Truncate(evt.ServerID, 20), + stringutil.Truncate(evt.ToolName, 25), + evt.Reason, + message, + repo, + }) + } + fmt.Fprint(os.Stderr, console.RenderTable(console.TableConfig{ + Title: "Guard Policy Events", + Headers: []string{"Server", "Tool", "Reason", "Message", "Repository"}, + Rows: eventRows, + })) + } +} + +// renderFirewallAnalysis renders firewall analysis with summary and domain breakdown +func renderFirewallAnalysis(analysis *FirewallAnalysis) { + auditReportLog.Printf("Rendering firewall analysis: total=%d, allowed=%d, blocked=%d, allowed_domains=%d, blocked_domains=%d", + analysis.TotalRequests, analysis.AllowedRequests, analysis.BlockedRequests, len(analysis.AllowedDomains), len(analysis.BlockedDomains)) + // Summary statistics + fmt.Fprintf(os.Stderr, " Total Requests : %d\n", analysis.TotalRequests) + fmt.Fprintf(os.Stderr, " Allowed : %d\n", analysis.AllowedRequests) + fmt.Fprintf(os.Stderr, " Blocked : %d\n", analysis.BlockedRequests) + fmt.Fprintln(os.Stderr) + + // Allowed domains + if len(analysis.AllowedDomains) > 0 { + fmt.Fprintln(os.Stderr, " Allowed Domains:") + for _, domain := range analysis.AllowedDomains { + if stats, ok := analysis.RequestsByDomain[domain]; ok { + fmt.Fprintf(os.Stderr, " ✓ %s (%d requests)\n", domain, stats.Allowed) + } + } + fmt.Fprintln(os.Stderr) + } + + // Blocked domains + if len(analysis.BlockedDomains) > 0 { + fmt.Fprintln(os.Stderr, " Blocked Domains:") + for _, domain := range analysis.BlockedDomains { + if stats, ok := analysis.RequestsByDomain[domain]; ok { + fmt.Fprintf(os.Stderr, " ✗ %s (%d requests)\n", domain, stats.Blocked) + } + } + fmt.Fprintln(os.Stderr) + } +} + +// renderRedactedDomainsAnalysis renders redacted domains analysis +func renderRedactedDomainsAnalysis(analysis *RedactedDomainsAnalysis) { + auditReportLog.Printf("Rendering redacted domains analysis: total_domains=%d", analysis.TotalDomains) + // Summary statistics + fmt.Fprintf(os.Stderr, " Total Domains Redacted: %d\n", analysis.TotalDomains) + fmt.Fprintln(os.Stderr) + + // List domains + if len(analysis.Domains) > 0 { + fmt.Fprintln(os.Stderr, " Redacted Domains:") + for _, domain := range analysis.Domains { + fmt.Fprintf(os.Stderr, " 🔒 %s\n", domain) + } + fmt.Fprintln(os.Stderr) + } +} + +// renderPolicyAnalysis renders the enriched firewall policy analysis with rule attribution +func renderPolicyAnalysis(analysis *PolicyAnalysis) { + auditReportLog.Printf("Rendering policy analysis: rules=%d, denied=%d", len(analysis.RuleHits), analysis.DeniedCount) + + // Policy summary using RenderStruct + display := PolicySummaryDisplay{ + Policy: analysis.PolicySummary, + TotalRequests: analysis.TotalRequests, + Allowed: analysis.AllowedCount, + Denied: analysis.DeniedCount, + UniqueDomains: analysis.UniqueDomains, + } + fmt.Fprint(os.Stderr, console.RenderStruct(display)) + fmt.Fprintln(os.Stderr) + + // Rule hit table + if len(analysis.RuleHits) > 0 { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Policy Rules:")) + fmt.Fprintln(os.Stderr) + + ruleConfig := console.TableConfig{ + Headers: []string{"Rule", "Action", "Description", "Hits"}, + Rows: make([][]string, 0, len(analysis.RuleHits)), + } + + for _, rh := range analysis.RuleHits { + row := []string{ + stringutil.Truncate(rh.Rule.ID, 30), + rh.Rule.Action, + stringutil.Truncate(rh.Rule.Description, 50), + strconv.Itoa(rh.Hits), + } + ruleConfig.Rows = append(ruleConfig.Rows, row) + } + + fmt.Fprint(os.Stderr, console.RenderTable(ruleConfig)) + fmt.Fprintln(os.Stderr) + } + + // Denied requests detail + if len(analysis.DeniedRequests) > 0 { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Denied Requests (%d):", len(analysis.DeniedRequests)))) + fmt.Fprintln(os.Stderr) + + deniedConfig := console.TableConfig{ + Headers: []string{"Time", "Domain", "Rule", "Reason"}, + Rows: make([][]string, 0, len(analysis.DeniedRequests)), + } + + for _, req := range analysis.DeniedRequests { + timeStr := formatUnixTimestamp(req.Timestamp) + row := []string{ + timeStr, + stringutil.Truncate(req.Host, 40), + stringutil.Truncate(req.RuleID, 25), + stringutil.Truncate(req.Reason, 40), + } + deniedConfig.Rows = append(deniedConfig.Rows, row) + } + + fmt.Fprint(os.Stderr, console.RenderTable(deniedConfig)) + fmt.Fprintln(os.Stderr) + } +} + +// formatUnixTimestamp converts a Unix timestamp (float64) to a human-readable time string (HH:MM:SS). +func formatUnixTimestamp(ts float64) string { + if ts <= 0 { + return "-" + } + sec := int64(math.Floor(ts)) + nsec := int64((ts - float64(sec)) * 1e9) + t := time.Unix(sec, nsec).UTC() + return t.Format("15:04:05") +} diff --git a/pkg/cli/audit_render_tokens.go b/pkg/cli/audit_render_tokens.go new file mode 100644 index 00000000000..d35b7679adf --- /dev/null +++ b/pkg/cli/audit_render_tokens.go @@ -0,0 +1,141 @@ +package cli + +import ( + "fmt" + "os" + "strconv" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/timeutil" +) + +// renderMCPServerHealth renders MCP server health summary +func renderMCPServerHealth(health *MCPServerHealth) { + if health == nil { + return + } + fmt.Fprintf(os.Stderr, " %s\n", health.Summary) + if health.TotalRequests > 0 { + fmt.Fprintf(os.Stderr, " Total Requests: %d\n", health.TotalRequests) + fmt.Fprintf(os.Stderr, " Total Errors: %d\n", health.TotalErrors) + fmt.Fprintf(os.Stderr, " Error Rate: %.1f%%\n", health.ErrorRate) + } + fmt.Fprintln(os.Stderr) + + // Server health table + if len(health.Servers) > 0 { + config := console.TableConfig{ + Headers: []string{"Server", "Requests", "Tool Calls", "Errors", "Error Rate", "Avg Latency", "Status"}, + Rows: make([][]string, 0, len(health.Servers)), + } + for _, server := range health.Servers { + row := []string{ + server.ServerName, + strconv.Itoa(server.RequestCount), + strconv.Itoa(server.ToolCalls), + strconv.Itoa(server.ErrorCount), + server.ErrorRateStr, + server.AvgLatency, + server.Status, + } + config.Rows = append(config.Rows, row) + } + fmt.Fprint(os.Stderr, console.RenderTable(config)) + } + + // Slowest tool calls + if len(health.SlowestCalls) > 0 { + fmt.Fprintln(os.Stderr) + fmt.Fprintln(os.Stderr, " Slowest Tool Calls:") + config := console.TableConfig{ + Headers: []string{"Server", "Tool", "Duration"}, + Rows: make([][]string, 0, len(health.SlowestCalls)), + } + for _, call := range health.SlowestCalls { + row := []string{call.ServerName, call.ToolName, call.Duration} + config.Rows = append(config.Rows, row) + } + fmt.Fprint(os.Stderr, console.RenderTable(config)) + } + + fmt.Fprintln(os.Stderr) +} + +// renderTokenUsage displays token usage data from the firewall proxy +func renderTokenUsage(summary *TokenUsageSummary) { + totalTokens := summary.TotalTokens() + cacheTokens := summary.TotalCacheReadTokens + summary.TotalCacheWriteTokens + + fmt.Fprintf(os.Stderr, " Total: %s tokens (%s input, %s output, %s cache)\n", + console.FormatNumber(totalTokens), + console.FormatNumber(summary.TotalInputTokens), + console.FormatNumber(summary.TotalOutputTokens), + console.FormatNumber(cacheTokens)) + fmt.Fprintf(os.Stderr, " Requests: %d (avg %s)\n", + summary.TotalRequests, timeutil.FormatDurationMs(summary.AvgDurationMs())) + if summary.CacheEfficiency > 0 { + fmt.Fprintf(os.Stderr, " Cache hit: %.1f%%\n", summary.CacheEfficiency*100) + } + fmt.Fprintln(os.Stderr) + + rows := summary.ModelRows() + if len(rows) > 0 { + config := console.TableConfig{ + Headers: []string{"Model", "Provider", "Input", "Output", "Cache Read", "Cache Write", "Requests", "Avg Duration"}, + Rows: make([][]string, 0, len(rows)), + } + for _, row := range rows { + config.Rows = append(config.Rows, []string{ + row.Model, + row.Provider, + console.FormatNumber(row.InputTokens), + console.FormatNumber(row.OutputTokens), + console.FormatNumber(row.CacheReadTokens), + console.FormatNumber(row.CacheWriteTokens), + strconv.Itoa(row.Requests), + row.AvgDuration, + }) + } + fmt.Fprint(os.Stderr, console.RenderTable(config)) + fmt.Fprintln(os.Stderr) + } +} + +// renderGitHubRateLimitUsage displays GitHub API quota consumption for the run. +func renderGitHubRateLimitUsage(usage *GitHubRateLimitUsage) { + if usage == nil { + return + } + + // Summary line + summary := "Total GitHub API calls: " + console.FormatNumber(usage.TotalRequestsMade) + if usage.CoreLimit > 0 { + summary += fmt.Sprintf(" | Core quota consumed: %s / %s (remaining: %s)", + console.FormatNumber(usage.CoreConsumed), + console.FormatNumber(usage.CoreLimit), + console.FormatNumber(usage.CoreRemaining), + ) + } + fmt.Fprintf(os.Stderr, " %s\n\n", summary) + + // Per-resource breakdown table (only when there are multiple resources or non-core resources) + rows := usage.ResourceRows() + if len(rows) == 0 { + return + } + cfg := console.TableConfig{ + Headers: []string{"Resource", "API Calls", "Quota Consumed", "Remaining", "Limit"}, + Rows: make([][]string, 0, len(rows)), + } + for _, row := range rows { + cfg.Rows = append(cfg.Rows, []string{ + row.Resource, + console.FormatNumber(row.RequestsMade), + console.FormatNumber(row.QuotaConsumed), + console.FormatNumber(row.FinalRemaining), + console.FormatNumber(row.Limit), + }) + } + fmt.Fprint(os.Stderr, console.RenderTable(cfg)) + fmt.Fprintln(os.Stderr) +} diff --git a/pkg/cli/audit_report_render.go b/pkg/cli/audit_report_render.go index d59cb602cbf..496fc328114 100644 --- a/pkg/cli/audit_report_render.go +++ b/pkg/cli/audit_report_render.go @@ -3,18 +3,14 @@ package cli import ( "encoding/json" "fmt" - "math" "os" "path/filepath" - "sort" "strconv" "strings" - "time" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/sliceutil" "github.com/github/gh-aw/pkg/stringutil" - "github.com/github/gh-aw/pkg/timeutil" ) // renderJSON outputs the audit data as JSON @@ -537,134 +533,6 @@ func renderMCPToolUsageTable(mcpData *MCPToolUsageData) { } } -// renderGuardPolicySummary renders the guard policy enforcement summary -func renderGuardPolicySummary(summary *GuardPolicySummary) { - auditReportLog.Printf("Rendering guard policy summary: %d total blocked", summary.TotalBlocked) - - fmt.Fprintln(os.Stderr) - fmt.Fprintln(os.Stderr, console.FormatWarningMessage( - fmt.Sprintf("Guard Policy: %d tool call(s) blocked", summary.TotalBlocked))) - fmt.Fprintln(os.Stderr) - - // Breakdown by reason - fmt.Fprintln(os.Stderr, " Block Reasons:") - if summary.IntegrityBlocked > 0 { - fmt.Fprintf(os.Stderr, " Integrity below minimum : %d\n", summary.IntegrityBlocked) - } - if summary.RepoScopeBlocked > 0 { - fmt.Fprintf(os.Stderr, " Repository not allowed : %d\n", summary.RepoScopeBlocked) - } - if summary.AccessDenied > 0 { - fmt.Fprintf(os.Stderr, " Access denied : %d\n", summary.AccessDenied) - } - if summary.BlockedUserDenied > 0 { - fmt.Fprintf(os.Stderr, " Blocked user : %d\n", summary.BlockedUserDenied) - } - if summary.PermissionDenied > 0 { - fmt.Fprintf(os.Stderr, " Insufficient permissions: %d\n", summary.PermissionDenied) - } - if summary.PrivateRepoDenied > 0 { - fmt.Fprintf(os.Stderr, " Private repo denied : %d\n", summary.PrivateRepoDenied) - } - fmt.Fprintln(os.Stderr) - - // Most frequently blocked tools - if len(summary.BlockedToolCounts) > 0 { - toolNames := sliceutil.MapToSlice(summary.BlockedToolCounts) - sort.Slice(toolNames, func(i, j int) bool { - return summary.BlockedToolCounts[toolNames[i]] > summary.BlockedToolCounts[toolNames[j]] - }) - - toolRows := make([][]string, 0, len(toolNames)) - for _, name := range toolNames { - toolRows = append(toolRows, []string{name, strconv.Itoa(summary.BlockedToolCounts[name])}) - } - fmt.Fprint(os.Stderr, console.RenderTable(console.TableConfig{ - Title: "Most Blocked Tools", - Headers: []string{"Tool", "Blocked"}, - Rows: toolRows, - })) - } - - // Guard policy event details - if len(summary.Events) > 0 { - fmt.Fprintln(os.Stderr) - eventRows := make([][]string, 0, len(summary.Events)) - for _, evt := range summary.Events { - message := evt.Message - if len(message) > 60 { - message = message[:57] + "..." - } - repo := evt.Repository - if repo == "" { - repo = "-" - } - eventRows = append(eventRows, []string{ - stringutil.Truncate(evt.ServerID, 20), - stringutil.Truncate(evt.ToolName, 25), - evt.Reason, - message, - repo, - }) - } - fmt.Fprint(os.Stderr, console.RenderTable(console.TableConfig{ - Title: "Guard Policy Events", - Headers: []string{"Server", "Tool", "Reason", "Message", "Repository"}, - Rows: eventRows, - })) - } -} - -// renderFirewallAnalysis renders firewall analysis with summary and domain breakdown -func renderFirewallAnalysis(analysis *FirewallAnalysis) { - auditReportLog.Printf("Rendering firewall analysis: total=%d, allowed=%d, blocked=%d, allowed_domains=%d, blocked_domains=%d", - analysis.TotalRequests, analysis.AllowedRequests, analysis.BlockedRequests, len(analysis.AllowedDomains), len(analysis.BlockedDomains)) - // Summary statistics - fmt.Fprintf(os.Stderr, " Total Requests : %d\n", analysis.TotalRequests) - fmt.Fprintf(os.Stderr, " Allowed : %d\n", analysis.AllowedRequests) - fmt.Fprintf(os.Stderr, " Blocked : %d\n", analysis.BlockedRequests) - fmt.Fprintln(os.Stderr) - - // Allowed domains - if len(analysis.AllowedDomains) > 0 { - fmt.Fprintln(os.Stderr, " Allowed Domains:") - for _, domain := range analysis.AllowedDomains { - if stats, ok := analysis.RequestsByDomain[domain]; ok { - fmt.Fprintf(os.Stderr, " ✓ %s (%d requests)\n", domain, stats.Allowed) - } - } - fmt.Fprintln(os.Stderr) - } - - // Blocked domains - if len(analysis.BlockedDomains) > 0 { - fmt.Fprintln(os.Stderr, " Blocked Domains:") - for _, domain := range analysis.BlockedDomains { - if stats, ok := analysis.RequestsByDomain[domain]; ok { - fmt.Fprintf(os.Stderr, " ✗ %s (%d requests)\n", domain, stats.Blocked) - } - } - fmt.Fprintln(os.Stderr) - } -} - -// renderRedactedDomainsAnalysis renders redacted domains analysis -func renderRedactedDomainsAnalysis(analysis *RedactedDomainsAnalysis) { - auditReportLog.Printf("Rendering redacted domains analysis: total_domains=%d", analysis.TotalDomains) - // Summary statistics - fmt.Fprintf(os.Stderr, " Total Domains Redacted: %d\n", analysis.TotalDomains) - fmt.Fprintln(os.Stderr) - - // List domains - if len(analysis.Domains) > 0 { - fmt.Fprintln(os.Stderr, " Redacted Domains:") - for _, domain := range analysis.Domains { - fmt.Fprintf(os.Stderr, " 🔒 %s\n", domain) - } - fmt.Fprintln(os.Stderr) - } -} - // renderCreatedItemsTable renders the list of items created in GitHub by safe output handlers // as a table with clickable URLs for easy auditing. func renderCreatedItemsTable(items []CreatedItemReport) { @@ -798,243 +666,6 @@ func renderRecommendations(recommendations []Recommendation) { } } -// renderPerformanceMetrics renders performance metrics -func renderPerformanceMetrics(metrics *PerformanceMetrics) { - auditReportLog.Printf("Rendering performance metrics: tokens_per_min=%.1f, cost_efficiency=%s, most_used_tool=%s", - metrics.TokensPerMinute, metrics.CostEfficiency, metrics.MostUsedTool) - if metrics.TokensPerMinute > 0 { - fmt.Fprintf(os.Stderr, " Tokens per Minute: %.1f\n", metrics.TokensPerMinute) - } - - if metrics.CostEfficiency != "" { - efficiencyDisplay := metrics.CostEfficiency - switch metrics.CostEfficiency { - case "excellent", "good": - efficiencyDisplay = console.FormatSuccessMessage(metrics.CostEfficiency) - case "moderate": - efficiencyDisplay = console.FormatWarningMessage(metrics.CostEfficiency) - case "poor": - efficiencyDisplay = console.FormatErrorMessage(metrics.CostEfficiency) - } - fmt.Fprintf(os.Stderr, " Cost Efficiency: %s\n", efficiencyDisplay) - } - - if metrics.AvgToolDuration != "" { - fmt.Fprintf(os.Stderr, " Average Tool Duration: %s\n", metrics.AvgToolDuration) - } - - if metrics.MostUsedTool != "" { - fmt.Fprintf(os.Stderr, " Most Used Tool: %s\n", metrics.MostUsedTool) - } - - if metrics.NetworkRequests > 0 { - fmt.Fprintf(os.Stderr, " Network Requests: %d\n", metrics.NetworkRequests) - } - - fmt.Fprintln(os.Stderr) -} - -// renderPolicyAnalysis renders the enriched firewall policy analysis with rule attribution -func renderPolicyAnalysis(analysis *PolicyAnalysis) { - auditReportLog.Printf("Rendering policy analysis: rules=%d, denied=%d", len(analysis.RuleHits), analysis.DeniedCount) - - // Policy summary using RenderStruct - display := PolicySummaryDisplay{ - Policy: analysis.PolicySummary, - TotalRequests: analysis.TotalRequests, - Allowed: analysis.AllowedCount, - Denied: analysis.DeniedCount, - UniqueDomains: analysis.UniqueDomains, - } - fmt.Fprint(os.Stderr, console.RenderStruct(display)) - fmt.Fprintln(os.Stderr) - - // Rule hit table - if len(analysis.RuleHits) > 0 { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Policy Rules:")) - fmt.Fprintln(os.Stderr) - - ruleConfig := console.TableConfig{ - Headers: []string{"Rule", "Action", "Description", "Hits"}, - Rows: make([][]string, 0, len(analysis.RuleHits)), - } - - for _, rh := range analysis.RuleHits { - row := []string{ - stringutil.Truncate(rh.Rule.ID, 30), - rh.Rule.Action, - stringutil.Truncate(rh.Rule.Description, 50), - strconv.Itoa(rh.Hits), - } - ruleConfig.Rows = append(ruleConfig.Rows, row) - } - - fmt.Fprint(os.Stderr, console.RenderTable(ruleConfig)) - fmt.Fprintln(os.Stderr) - } - - // Denied requests detail - if len(analysis.DeniedRequests) > 0 { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Denied Requests (%d):", len(analysis.DeniedRequests)))) - fmt.Fprintln(os.Stderr) - - deniedConfig := console.TableConfig{ - Headers: []string{"Time", "Domain", "Rule", "Reason"}, - Rows: make([][]string, 0, len(analysis.DeniedRequests)), - } - - for _, req := range analysis.DeniedRequests { - timeStr := formatUnixTimestamp(req.Timestamp) - row := []string{ - timeStr, - stringutil.Truncate(req.Host, 40), - stringutil.Truncate(req.RuleID, 25), - stringutil.Truncate(req.Reason, 40), - } - deniedConfig.Rows = append(deniedConfig.Rows, row) - } - - fmt.Fprint(os.Stderr, console.RenderTable(deniedConfig)) - fmt.Fprintln(os.Stderr) - } -} - -// formatUnixTimestamp converts a Unix timestamp (float64) to a human-readable time string (HH:MM:SS). -func formatUnixTimestamp(ts float64) string { - if ts <= 0 { - return "-" - } - sec := int64(math.Floor(ts)) - nsec := int64((ts - float64(sec)) * 1e9) - t := time.Unix(sec, nsec).UTC() - return t.Format("15:04:05") -} - -// renderEngineConfig renders engine configuration details -func renderEngineConfig(config *EngineConfig) { - if config == nil { - return - } - fmt.Fprintf(os.Stderr, " Engine ID: %s\n", config.EngineID) - if config.EngineName != "" { - fmt.Fprintf(os.Stderr, " Engine Name: %s\n", config.EngineName) - } - if config.Model != "" { - fmt.Fprintf(os.Stderr, " Model: %s\n", config.Model) - } - if config.Version != "" { - fmt.Fprintf(os.Stderr, " Version: %s\n", config.Version) - } - if config.CLIVersion != "" { - fmt.Fprintf(os.Stderr, " CLI Version: %s\n", config.CLIVersion) - } - if config.FirewallVersion != "" { - fmt.Fprintf(os.Stderr, " Firewall Version: %s\n", config.FirewallVersion) - } - if config.TriggerEvent != "" { - fmt.Fprintf(os.Stderr, " Trigger Event: %s\n", config.TriggerEvent) - } - if config.Repository != "" { - fmt.Fprintf(os.Stderr, " Repository: %s\n", config.Repository) - } - if len(config.MCPServers) > 0 { - fmt.Fprintf(os.Stderr, " MCP Servers: %s\n", strings.Join(config.MCPServers, ", ")) - } - fmt.Fprintln(os.Stderr) -} - -// renderPromptAnalysis renders prompt analysis metrics -func renderPromptAnalysis(analysis *PromptAnalysis) { - if analysis == nil { - return - } - fmt.Fprintf(os.Stderr, " Prompt Size: %s chars\n", console.FormatNumber(analysis.PromptSize)) - if analysis.PromptFile != "" { - fmt.Fprintf(os.Stderr, " Prompt File: %s\n", analysis.PromptFile) - } - fmt.Fprintln(os.Stderr) -} - -// renderSessionAnalysis renders session and agent performance metrics -func renderSessionAnalysis(session *SessionAnalysis) { - if session == nil { - return - } - if session.WallTime != "" { - fmt.Fprintf(os.Stderr, " Wall Time: %s\n", session.WallTime) - } - if session.TurnCount > 0 { - fmt.Fprintf(os.Stderr, " Turn Count: %d\n", session.TurnCount) - } - if session.AvgTurnDuration != "" { - fmt.Fprintf(os.Stderr, " Avg Turn Duration: %s\n", session.AvgTurnDuration) - } - if session.TokensPerMinute > 0 { - fmt.Fprintf(os.Stderr, " Tokens/Minute: %.1f\n", session.TokensPerMinute) - } - if session.NoopCount > 0 { - fmt.Fprintf(os.Stderr, " Noop Count: %d\n", session.NoopCount) - } - if session.TimeoutDetected { - fmt.Fprintf(os.Stderr, " Timeout Detected: %s\n", console.FormatWarningMessage("Yes")) - } else { - fmt.Fprintf(os.Stderr, " Timeout Detected: %s\n", console.FormatSuccessMessage("No")) - } - fmt.Fprintln(os.Stderr) -} - -// renderMCPServerHealth renders MCP server health summary -func renderMCPServerHealth(health *MCPServerHealth) { - if health == nil { - return - } - fmt.Fprintf(os.Stderr, " %s\n", health.Summary) - if health.TotalRequests > 0 { - fmt.Fprintf(os.Stderr, " Total Requests: %d\n", health.TotalRequests) - fmt.Fprintf(os.Stderr, " Total Errors: %d\n", health.TotalErrors) - fmt.Fprintf(os.Stderr, " Error Rate: %.1f%%\n", health.ErrorRate) - } - fmt.Fprintln(os.Stderr) - - // Server health table - if len(health.Servers) > 0 { - config := console.TableConfig{ - Headers: []string{"Server", "Requests", "Tool Calls", "Errors", "Error Rate", "Avg Latency", "Status"}, - Rows: make([][]string, 0, len(health.Servers)), - } - for _, server := range health.Servers { - row := []string{ - server.ServerName, - strconv.Itoa(server.RequestCount), - strconv.Itoa(server.ToolCalls), - strconv.Itoa(server.ErrorCount), - server.ErrorRateStr, - server.AvgLatency, - server.Status, - } - config.Rows = append(config.Rows, row) - } - fmt.Fprint(os.Stderr, console.RenderTable(config)) - } - - // Slowest tool calls - if len(health.SlowestCalls) > 0 { - fmt.Fprintln(os.Stderr) - fmt.Fprintln(os.Stderr, " Slowest Tool Calls:") - config := console.TableConfig{ - Headers: []string{"Server", "Tool", "Duration"}, - Rows: make([][]string, 0, len(health.SlowestCalls)), - } - for _, call := range health.SlowestCalls { - row := []string{call.ServerName, call.ToolName, call.Duration} - config.Rows = append(config.Rows, row) - } - fmt.Fprint(os.Stderr, console.RenderTable(config)) - } - - fmt.Fprintln(os.Stderr) -} - // renderSafeOutputSummary renders safe output summary with type breakdown func renderSafeOutputSummary(summary *SafeOutputSummary) { if summary == nil { @@ -1058,82 +689,3 @@ func renderSafeOutputSummary(summary *SafeOutputSummary) { fmt.Fprintln(os.Stderr) } } - -// renderTokenUsage displays token usage data from the firewall proxy -func renderTokenUsage(summary *TokenUsageSummary) { - totalTokens := summary.TotalTokens() - cacheTokens := summary.TotalCacheReadTokens + summary.TotalCacheWriteTokens - - fmt.Fprintf(os.Stderr, " Total: %s tokens (%s input, %s output, %s cache)\n", - console.FormatNumber(totalTokens), - console.FormatNumber(summary.TotalInputTokens), - console.FormatNumber(summary.TotalOutputTokens), - console.FormatNumber(cacheTokens)) - fmt.Fprintf(os.Stderr, " Requests: %d (avg %s)\n", - summary.TotalRequests, timeutil.FormatDurationMs(summary.AvgDurationMs())) - if summary.CacheEfficiency > 0 { - fmt.Fprintf(os.Stderr, " Cache hit: %.1f%%\n", summary.CacheEfficiency*100) - } - fmt.Fprintln(os.Stderr) - - rows := summary.ModelRows() - if len(rows) > 0 { - config := console.TableConfig{ - Headers: []string{"Model", "Provider", "Input", "Output", "Cache Read", "Cache Write", "Requests", "Avg Duration"}, - Rows: make([][]string, 0, len(rows)), - } - for _, row := range rows { - config.Rows = append(config.Rows, []string{ - row.Model, - row.Provider, - console.FormatNumber(row.InputTokens), - console.FormatNumber(row.OutputTokens), - console.FormatNumber(row.CacheReadTokens), - console.FormatNumber(row.CacheWriteTokens), - strconv.Itoa(row.Requests), - row.AvgDuration, - }) - } - fmt.Fprint(os.Stderr, console.RenderTable(config)) - fmt.Fprintln(os.Stderr) - } -} - -// renderGitHubRateLimitUsage displays GitHub API quota consumption for the run. -func renderGitHubRateLimitUsage(usage *GitHubRateLimitUsage) { - if usage == nil { - return - } - - // Summary line - summary := "Total GitHub API calls: " + console.FormatNumber(usage.TotalRequestsMade) - if usage.CoreLimit > 0 { - summary += fmt.Sprintf(" | Core quota consumed: %s / %s (remaining: %s)", - console.FormatNumber(usage.CoreConsumed), - console.FormatNumber(usage.CoreLimit), - console.FormatNumber(usage.CoreRemaining), - ) - } - fmt.Fprintf(os.Stderr, " %s\n\n", summary) - - // Per-resource breakdown table (only when there are multiple resources or non-core resources) - rows := usage.ResourceRows() - if len(rows) == 0 { - return - } - cfg := console.TableConfig{ - Headers: []string{"Resource", "API Calls", "Quota Consumed", "Remaining", "Limit"}, - Rows: make([][]string, 0, len(rows)), - } - for _, row := range rows { - cfg.Rows = append(cfg.Rows, []string{ - row.Resource, - console.FormatNumber(row.RequestsMade), - console.FormatNumber(row.QuotaConsumed), - console.FormatNumber(row.FinalRemaining), - console.FormatNumber(row.Limit), - }) - } - fmt.Fprint(os.Stderr, console.RenderTable(cfg)) - fmt.Fprintln(os.Stderr) -} From 2280e7054eecc38a32e5eb01bd8249329559b1a1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 15:23:46 +0000 Subject: [PATCH 4/7] refactor: split handlerRegistry into domain-specific files Move handler builder entries from the monolithic handlerRegistry variable into four domain-specific files using init() functions: - compiler_safe_outputs_handlers_issues.go: create/close/update_issue, link_sub_issue, add/remove_labels, assign_milestone, set_issue_type, assign/unassign_from_user - compiler_safe_outputs_handlers_prs.go: create/push/update/close PR, review comments, submit/reply/resolve review, add_reviewer, mark_as_ready - compiler_safe_outputs_handlers_discussions.go: add_comment, create/close/update_discussion, hide_comment - compiler_safe_outputs_handlers_misc.go: dispatch_workflow/repository, call_workflow, noop, missing_tool/data, report_incomplete, assign_to_agent, upload_asset/artifact, autofix_code_scanning_alert, create/update_project, create_project_status_update, update_release, create_code_scanning_alert, create_agent_session The main file is reduced from 1035 to 263 lines. handlerRegistry starts empty and is populated via init() calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_safe_outputs_config.go | 786 +----------------- ...piler_safe_outputs_handlers_discussions.go | 103 +++ .../compiler_safe_outputs_handlers_issues.go | 205 +++++ .../compiler_safe_outputs_handlers_misc.go | 317 +++++++ .../compiler_safe_outputs_handlers_prs.go | 205 +++++ 5 files changed, 837 insertions(+), 779 deletions(-) create mode 100644 pkg/workflow/compiler_safe_outputs_handlers_discussions.go create mode 100644 pkg/workflow/compiler_safe_outputs_handlers_issues.go create mode 100644 pkg/workflow/compiler_safe_outputs_handlers_misc.go create mode 100644 pkg/workflow/compiler_safe_outputs_handlers_prs.go diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index 12b747fba4a..f3dbf22d23d 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -144,785 +144,13 @@ func (b *handlerConfigBuilder) Build() map[string]any { // handlerBuilder is a function that builds a handler config from SafeOutputsConfig type handlerBuilder func(*SafeOutputsConfig) map[string]any -// handlerRegistry maps handler names to their builder functions -var handlerRegistry = map[string]handlerBuilder{ - "create_issue": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.CreateIssues == nil { - return nil - } - c := cfg.CreateIssues - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("allowed_labels", c.AllowedLabels). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfPositive("expires", c.Expires). - AddStringSlice("labels", c.Labels). - AddIfNotEmpty("title_prefix", c.TitlePrefix). - AddStringSlice("assignees", c.Assignees). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddTemplatableBool("group", c.Group). - AddTemplatableBool("close_older_issues", c.CloseOlderIssues). - AddIfNotEmpty("close_older_key", c.CloseOlderKey). - AddTemplatableBool("group_by_day", c.GroupByDay). - AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "add_comment": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.AddComments == nil { - return nil - } - c := cfg.AddComments - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target). - AddTemplatableBool("hide_older_comments", c.HideOlderComments). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). - AddIfTrue("staged", c.Staged). - Build() - }, - "create_discussion": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.CreateDiscussions == nil { - return nil - } - c := cfg.CreateDiscussions - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("category", c.Category). - AddIfNotEmpty("title_prefix", c.TitlePrefix). - AddStringSlice("labels", c.Labels). - AddStringSlice("allowed_labels", c.AllowedLabels). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddTemplatableBool("close_older_discussions", c.CloseOlderDiscussions). - AddIfNotEmpty("close_older_key", c.CloseOlderKey). - AddIfNotEmpty("required_category", c.RequiredCategory). - AddIfPositive("expires", c.Expires). - AddBoolPtr("fallback_to_issue", c.FallbackToIssue). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "close_issue": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.CloseIssues == nil { - return nil - } - c := cfg.CloseIssues - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target). - AddStringSlice("required_labels", c.RequiredLabels). - AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("state_reason", c.StateReason). - AddIfTrue("staged", c.Staged). - Build() - }, - "close_discussion": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.CloseDiscussions == nil { - return nil - } - c := cfg.CloseDiscussions - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target). - AddStringSlice("required_labels", c.RequiredLabels). - AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfTrue("staged", c.Staged). - Build() - }, - "add_labels": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.AddLabels == nil { - return nil - } - c := cfg.AddLabels - config := newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("allowed", c.Allowed). - AddStringSlice("blocked", c.Blocked). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - // If config is empty, it means add_labels was explicitly configured with no options - // (null config), which means "allow any labels". Return non-nil empty map to - // indicate the handler is enabled. - if len(config) == 0 { - // Return empty map so handler is included in config - return make(map[string]any) - } - return config - }, - "remove_labels": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.RemoveLabels == nil { - return nil - } - c := cfg.RemoveLabels - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("allowed", c.Allowed). - AddStringSlice("blocked", c.Blocked). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "add_reviewer": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.AddReviewer == nil { - return nil - } - c := cfg.AddReviewer - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("allowed", c.Reviewers). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "assign_milestone": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.AssignMilestone == nil { - return nil - } - c := cfg.AssignMilestone - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("allowed", c.Allowed). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - AddIfTrue("auto_create", c.AutoCreate). - Build() - }, - "mark_pull_request_as_ready_for_review": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.MarkPullRequestAsReadyForReview == nil { - return nil - } - c := cfg.MarkPullRequestAsReadyForReview - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target). - AddStringSlice("required_labels", c.RequiredLabels). - AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "create_code_scanning_alert": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.CreateCodeScanningAlerts == nil { - return nil - } - c := cfg.CreateCodeScanningAlerts - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("driver", c.Driver). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "create_agent_session": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.CreateAgentSessions == nil { - return nil - } - c := cfg.CreateAgentSessions - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("base", c.Base). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "update_issue": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.UpdateIssues == nil { - return nil - } - c := cfg.UpdateIssues - builder := newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("title_prefix", c.TitlePrefix) - // Boolean pointer fields indicate which fields can be updated - if c.Status != nil { - builder.AddDefault("allow_status", true) - } - if c.Title != nil { - builder.AddDefault("allow_title", true) - } - // Body uses boolean value mode - add the actual boolean value - builder.AddBoolPtrOrDefault("allow_body", c.Body, true) - return builder. - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). - AddIfTrue("staged", c.Staged). - Build() - }, - "update_discussion": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.UpdateDiscussions == nil { - return nil - } - c := cfg.UpdateDiscussions - builder := newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target) - // Boolean pointer fields indicate which fields can be updated - if c.Title != nil { - builder.AddDefault("allow_title", true) - } - if c.Body != nil { - builder.AddDefault("allow_body", true) - } - if c.Labels != nil { - builder.AddDefault("allow_labels", true) - } - return builder. - AddStringSlice("allowed_labels", c.AllowedLabels). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). - AddIfTrue("staged", c.Staged). - Build() - }, - "link_sub_issue": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.LinkSubIssue == nil { - return nil - } - c := cfg.LinkSubIssue - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("parent_required_labels", c.ParentRequiredLabels). - AddIfNotEmpty("parent_title_prefix", c.ParentTitlePrefix). - AddStringSlice("sub_required_labels", c.SubRequiredLabels). - AddIfNotEmpty("sub_title_prefix", c.SubTitlePrefix). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "update_release": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.UpdateRelease == nil { - return nil - } - c := cfg.UpdateRelease - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("github-token", c.GitHubToken). - AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). - AddIfTrue("staged", c.Staged). - Build() - }, - "create_pull_request_review_comment": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.CreatePullRequestReviewComments == nil { - return nil - } - c := cfg.CreatePullRequestReviewComments - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("side", c.Side). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "submit_pull_request_review": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.SubmitPullRequestReview == nil { - return nil - } - c := cfg.SubmitPullRequestReview - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddStringSlice("allowed_events", c.AllowedEvents). - AddIfNotEmpty("github-token", c.GitHubToken). - AddStringPtr("footer", getEffectiveFooterString(c.Footer, cfg.Footer)). - AddIfTrue("staged", c.Staged). - Build() - }, - "reply_to_pull_request_review_comment": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.ReplyToPullRequestReviewComment == nil { - return nil - } - c := cfg.ReplyToPullRequestReviewComment - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). - AddIfTrue("staged", c.Staged). - Build() - }, - "resolve_pull_request_review_thread": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.ResolvePullRequestReviewThread == nil { - return nil - } - c := cfg.ResolvePullRequestReviewThread - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "create_pull_request": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.CreatePullRequests == nil { - return nil - } - c := cfg.CreatePullRequests - maxPatchSize := 1024 // default 1024 KB - if cfg.MaximumPatchSize > 0 { - maxPatchSize = cfg.MaximumPatchSize - } - builder := newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("title_prefix", c.TitlePrefix). - AddStringSlice("labels", c.Labels). - AddStringSlice("reviewers", c.Reviewers). - AddStringSlice("assignees", c.Assignees). - AddTemplatableBool("draft", c.Draft). - AddIfNotEmpty("if_no_changes", c.IfNoChanges). - AddTemplatableBool("allow_empty", c.AllowEmpty). - AddTemplatableBool("auto_merge", c.AutoMerge). - AddIfPositive("expires", c.Expires). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddDefault("max_patch_size", maxPatchSize). - AddIfNotEmpty("github-token", c.GitHubToken). - AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). - AddBoolPtr("fallback_as_issue", c.FallbackAsIssue). - AddTemplatableBool("auto_close_issue", c.AutoCloseIssue). - AddIfNotEmpty("base_branch", c.BaseBranch). - AddStringPtr("protected_files_policy", c.ManifestFilesPolicy). - AddStringSlice("protected_files", getAllManifestFiles()). - AddStringSlice("protected_path_prefixes", getProtectedPathPrefixes()). - AddStringSlice("allowed_files", c.AllowedFiles). - AddStringSlice("excluded_files", c.ExcludedFiles). - AddIfTrue("preserve_branch_name", c.PreserveBranchName). - AddIfNotEmpty("patch_format", c.PatchFormat). - AddIfTrue("staged", c.Staged) - return builder.Build() - }, - "push_to_pull_request_branch": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.PushToPullRequestBranch == nil { - return nil - } - c := cfg.PushToPullRequestBranch - maxPatchSize := 1024 // default 1024 KB - if cfg.MaximumPatchSize > 0 { - maxPatchSize = cfg.MaximumPatchSize - } - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("title_prefix", c.TitlePrefix). - AddStringSlice("labels", c.Labels). - AddIfNotEmpty("if_no_changes", c.IfNoChanges). - AddIfNotEmpty("commit_title_suffix", c.CommitTitleSuffix). - AddDefault("max_patch_size", maxPatchSize). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - AddStringPtr("protected_files_policy", c.ManifestFilesPolicy). - AddStringSlice("protected_files", getAllManifestFiles()). - AddStringSlice("protected_path_prefixes", getProtectedPathPrefixes()). - AddStringSlice("allowed_files", c.AllowedFiles). - AddStringSlice("excluded_files", c.ExcludedFiles). - AddIfNotEmpty("patch_format", c.PatchFormat). - Build() - }, - "update_pull_request": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.UpdatePullRequests == nil { - return nil - } - c := cfg.UpdatePullRequests - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target). - AddBoolPtrOrDefault("allow_title", c.Title, true). - AddBoolPtrOrDefault("allow_body", c.Body, true). - AddStringPtr("default_operation", c.Operation). - AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "close_pull_request": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.ClosePullRequests == nil { - return nil - } - c := cfg.ClosePullRequests - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target", c.Target). - AddStringSlice("required_labels", c.RequiredLabels). - AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "hide_comment": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.HideComment == nil { - return nil - } - c := cfg.HideComment - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("allowed_reasons", c.AllowedReasons). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "dispatch_workflow": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.DispatchWorkflow == nil { - return nil - } - c := cfg.DispatchWorkflow - builder := newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("workflows", c.Workflows). - AddIfNotEmpty("target-repo", c.TargetRepoSlug) - - // Add workflow_files map if it has entries - if len(c.WorkflowFiles) > 0 { - builder.AddDefault("workflow_files", c.WorkflowFiles) - } - - // Add aw_context_workflows list if it has entries - if len(c.AwContextWorkflows) > 0 { - builder.AddStringSlice("aw_context_workflows", c.AwContextWorkflows) - } - - builder.AddIfNotEmpty("target-ref", c.TargetRef) - builder.AddIfNotEmpty("github-token", c.GitHubToken) - builder.AddIfTrue("staged", c.Staged) - return builder.Build() - }, - "dispatch_repository": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.DispatchRepository == nil || len(cfg.DispatchRepository.Tools) == 0 { - return nil - } - // Serialize each tool as a sub-map - tools := make(map[string]any, len(cfg.DispatchRepository.Tools)) - for toolKey, tool := range cfg.DispatchRepository.Tools { - toolConfig := newHandlerConfigBuilder(). - AddIfNotEmpty("workflow", tool.Workflow). - AddIfNotEmpty("event_type", tool.EventType). - AddIfNotEmpty("repository", tool.Repository). - AddStringSlice("allowed_repositories", tool.AllowedRepositories). - AddTemplatableInt("max", tool.Max). - AddIfNotEmpty("github-token", tool.GitHubToken). - AddIfTrue("staged", tool.Staged). - Build() - tools[toolKey] = toolConfig - } - return map[string]any{"tools": tools} - }, - "call_workflow": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.CallWorkflow == nil { - return nil - } - c := cfg.CallWorkflow - builder := newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("workflows", c.Workflows) - - // Add workflow_files map if it has entries - if len(c.WorkflowFiles) > 0 { - builder.AddDefault("workflow_files", c.WorkflowFiles) - } - - builder.AddIfTrue("staged", c.Staged) - return builder.Build() - }, - "missing_tool": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.MissingTool == nil { - return nil - } - c := cfg.MissingTool - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "missing_data": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.MissingData == nil { - return nil - } - c := cfg.MissingData - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "noop": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.NoOp == nil { - return nil - } - c := cfg.NoOp - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringPtr("report-as-issue", c.ReportAsIssue). - AddIfTrue("staged", c.Staged). - Build() - }, - "report_incomplete": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.ReportIncomplete == nil { - return nil - } - c := cfg.ReportIncomplete - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "create_report_incomplete_issue": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.ReportIncomplete == nil { - return nil - } - c := cfg.ReportIncomplete - if !c.CreateIssue { - return nil - } - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("title-prefix", c.TitlePrefix). - AddStringSlice("labels", c.Labels). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "assign_to_agent": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.AssignToAgent == nil { - return nil - } - c := cfg.AssignToAgent - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("name", c.DefaultAgent). - AddIfNotEmpty("model", c.DefaultModel). - AddIfNotEmpty("custom-agent", c.DefaultCustomAgent). - AddIfNotEmpty("custom-instructions", c.DefaultCustomInstructions). - AddStringSlice("allowed", c.Allowed). - AddIfTrue("ignore-if-error", c.IgnoreIfError). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed-repos", c.AllowedRepos). - AddIfNotEmpty("pull-request-repo", c.PullRequestRepoSlug). - AddStringSlice("allowed-pull-request-repos", c.AllowedPullRequestRepos). - AddIfNotEmpty("base-branch", c.BaseBranch). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "upload_asset": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.UploadAssets == nil { - return nil - } - c := cfg.UploadAssets - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("branch", c.BranchName). - AddIfPositive("max-size", c.MaxSizeKB). - AddStringSlice("allowed-exts", c.AllowedExts). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "upload_artifact": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.UploadArtifact == nil { - return nil - } - c := cfg.UploadArtifact - b := newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfPositive("max-uploads", c.MaxUploads). - AddTemplatableInt("retention-days", c.RetentionDays). - AddTemplatableBool("skip-archive", c.SkipArchive). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged) - if c.MaxSizeBytes > 0 { - b = b.AddDefault("max-size-bytes", c.MaxSizeBytes) - } - if len(c.AllowedPaths) > 0 { - b = b.AddStringSlice("allowed-paths", c.AllowedPaths) - } - if c.Defaults != nil { - if c.Defaults.IfNoFiles != "" { - b = b.AddIfNotEmpty("default-if-no-files", c.Defaults.IfNoFiles) - } - } - if c.Filters != nil { - if len(c.Filters.Include) > 0 { - b = b.AddStringSlice("filters-include", c.Filters.Include) - } - if len(c.Filters.Exclude) > 0 { - b = b.AddStringSlice("filters-exclude", c.Filters.Exclude) - } - } - return b.Build() - }, - "autofix_code_scanning_alert": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.AutofixCodeScanningAlert == nil { - return nil - } - c := cfg.AutofixCodeScanningAlert - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - // Note: create_project, update_project and create_project_status_update are handled by the unified handler, - // not the separate project handler manager, so they are included in this registry. - "create_project": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.CreateProjects == nil { - return nil - } - c := cfg.CreateProjects - builder := newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("target_owner", c.TargetOwner). - AddIfNotEmpty("title_prefix", c.TitlePrefix). - AddIfNotEmpty("github-token", c.GitHubToken) - if len(c.Views) > 0 { - builder.AddDefault("views", c.Views) - } - if len(c.FieldDefinitions) > 0 { - builder.AddDefault("field_definitions", c.FieldDefinitions) - } - builder.AddIfTrue("staged", c.Staged) - return builder.Build() - }, - "update_project": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.UpdateProjects == nil { - return nil - } - c := cfg.UpdateProjects - builder := newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfNotEmpty("project", c.Project). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos) - if len(c.Views) > 0 { - builder.AddDefault("views", c.Views) - } - if len(c.FieldDefinitions) > 0 { - builder.AddDefault("field_definitions", c.FieldDefinitions) - } - builder.AddIfTrue("staged", c.Staged) - return builder.Build() - }, - "assign_to_user": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.AssignToUser == nil { - return nil - } - c := cfg.AssignToUser - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("allowed", c.Allowed). - AddStringSlice("blocked", c.Blocked). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddTemplatableBool("unassign_first", c.UnassignFirst). - AddIfTrue("staged", c.Staged). - Build() - }, - "unassign_from_user": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.UnassignFromUser == nil { - return nil - } - c := cfg.UnassignFromUser - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("allowed", c.Allowed). - AddStringSlice("blocked", c.Blocked). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - }, - "create_project_status_update": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.CreateProjectStatusUpdates == nil { - return nil - } - c := cfg.CreateProjectStatusUpdates - return newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfNotEmpty("project", c.Project). - AddIfTrue("staged", c.Staged). - Build() - }, - "set_issue_type": func(cfg *SafeOutputsConfig) map[string]any { - if cfg.SetIssueType == nil { - return nil - } - c := cfg.SetIssueType - config := newHandlerConfigBuilder(). - AddTemplatableInt("max", c.Max). - AddStringSlice("allowed", c.Allowed). - AddIfNotEmpty("target", c.Target). - AddIfNotEmpty("target-repo", c.TargetRepoSlug). - AddStringSlice("allowed_repos", c.AllowedRepos). - AddIfNotEmpty("github-token", c.GitHubToken). - AddIfTrue("staged", c.Staged). - Build() - // If config is empty, it means set_issue_type was explicitly configured with no options - // (null config), which means "allow any type". Return non-nil empty map to - // indicate the handler is enabled. - if len(config) == 0 { - return make(map[string]any) - } - return config - }, -} +// handlerRegistry maps handler names to their builder functions. +// Entries are registered via init() in domain-specific files: +// - compiler_safe_outputs_handlers_issues.go +// - compiler_safe_outputs_handlers_prs.go +// - compiler_safe_outputs_handlers_discussions.go +// - compiler_safe_outputs_handlers_misc.go +var handlerRegistry = map[string]handlerBuilder{} func (c *Compiler) addHandlerManagerConfigEnvVar(steps *[]string, data *WorkflowData) { if data.SafeOutputs == nil { diff --git a/pkg/workflow/compiler_safe_outputs_handlers_discussions.go b/pkg/workflow/compiler_safe_outputs_handlers_discussions.go new file mode 100644 index 00000000000..4482cef803c --- /dev/null +++ b/pkg/workflow/compiler_safe_outputs_handlers_discussions.go @@ -0,0 +1,103 @@ +package workflow + +func init() { + handlerRegistry["add_comment"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.AddComments == nil { + return nil + } + c := cfg.AddComments + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). + AddTemplatableBool("hide_older_comments", c.HideOlderComments). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["create_discussion"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.CreateDiscussions == nil { + return nil + } + c := cfg.CreateDiscussions + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("category", c.Category). + AddIfNotEmpty("title_prefix", c.TitlePrefix). + AddStringSlice("labels", c.Labels). + AddStringSlice("allowed_labels", c.AllowedLabels). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddTemplatableBool("close_older_discussions", c.CloseOlderDiscussions). + AddIfNotEmpty("close_older_key", c.CloseOlderKey). + AddIfNotEmpty("required_category", c.RequiredCategory). + AddIfPositive("expires", c.Expires). + AddBoolPtr("fallback_to_issue", c.FallbackToIssue). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["close_discussion"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.CloseDiscussions == nil { + return nil + } + c := cfg.CloseDiscussions + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). + AddStringSlice("required_labels", c.RequiredLabels). + AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["update_discussion"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.UpdateDiscussions == nil { + return nil + } + c := cfg.UpdateDiscussions + builder := newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target) + // Boolean pointer fields indicate which fields can be updated + if c.Title != nil { + builder.AddDefault("allow_title", true) + } + if c.Body != nil { + builder.AddDefault("allow_body", true) + } + if c.Labels != nil { + builder.AddDefault("allow_labels", true) + } + return builder. + AddStringSlice("allowed_labels", c.AllowedLabels). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["hide_comment"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.HideComment == nil { + return nil + } + c := cfg.HideComment + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("allowed_reasons", c.AllowedReasons). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } +} diff --git a/pkg/workflow/compiler_safe_outputs_handlers_issues.go b/pkg/workflow/compiler_safe_outputs_handlers_issues.go new file mode 100644 index 00000000000..bff293accba --- /dev/null +++ b/pkg/workflow/compiler_safe_outputs_handlers_issues.go @@ -0,0 +1,205 @@ +package workflow + +func init() { + handlerRegistry["create_issue"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.CreateIssues == nil { + return nil + } + c := cfg.CreateIssues + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("allowed_labels", c.AllowedLabels). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfPositive("expires", c.Expires). + AddStringSlice("labels", c.Labels). + AddIfNotEmpty("title_prefix", c.TitlePrefix). + AddStringSlice("assignees", c.Assignees). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddTemplatableBool("group", c.Group). + AddTemplatableBool("close_older_issues", c.CloseOlderIssues). + AddIfNotEmpty("close_older_key", c.CloseOlderKey). + AddTemplatableBool("group_by_day", c.GroupByDay). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["close_issue"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.CloseIssues == nil { + return nil + } + c := cfg.CloseIssues + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). + AddStringSlice("required_labels", c.RequiredLabels). + AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("state_reason", c.StateReason). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["update_issue"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.UpdateIssues == nil { + return nil + } + c := cfg.UpdateIssues + builder := newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("title_prefix", c.TitlePrefix) + // Boolean pointer fields indicate which fields can be updated + if c.Status != nil { + builder.AddDefault("allow_status", true) + } + if c.Title != nil { + builder.AddDefault("allow_title", true) + } + // Body uses boolean value mode - add the actual boolean value + builder.AddBoolPtrOrDefault("allow_body", c.Body, true) + return builder. + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["link_sub_issue"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.LinkSubIssue == nil { + return nil + } + c := cfg.LinkSubIssue + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("parent_required_labels", c.ParentRequiredLabels). + AddIfNotEmpty("parent_title_prefix", c.ParentTitlePrefix). + AddStringSlice("sub_required_labels", c.SubRequiredLabels). + AddIfNotEmpty("sub_title_prefix", c.SubTitlePrefix). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["add_labels"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.AddLabels == nil { + return nil + } + c := cfg.AddLabels + config := newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("allowed", c.Allowed). + AddStringSlice("blocked", c.Blocked). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + // If config is empty, it means add_labels was explicitly configured with no options + // (null config), which means "allow any labels". Return non-nil empty map to + // indicate the handler is enabled. + if len(config) == 0 { + return make(map[string]any) + } + return config + } + + handlerRegistry["remove_labels"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.RemoveLabels == nil { + return nil + } + c := cfg.RemoveLabels + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("allowed", c.Allowed). + AddStringSlice("blocked", c.Blocked). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["assign_milestone"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.AssignMilestone == nil { + return nil + } + c := cfg.AssignMilestone + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("allowed", c.Allowed). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + AddIfTrue("auto_create", c.AutoCreate). + Build() + } + + handlerRegistry["set_issue_type"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.SetIssueType == nil { + return nil + } + c := cfg.SetIssueType + config := newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("allowed", c.Allowed). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + // If config is empty, it means set_issue_type was explicitly configured with no options + // (null config), which means "allow any type". Return non-nil empty map to + // indicate the handler is enabled. + if len(config) == 0 { + return make(map[string]any) + } + return config + } + + handlerRegistry["assign_to_user"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.AssignToUser == nil { + return nil + } + c := cfg.AssignToUser + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("allowed", c.Allowed). + AddStringSlice("blocked", c.Blocked). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddTemplatableBool("unassign_first", c.UnassignFirst). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["unassign_from_user"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.UnassignFromUser == nil { + return nil + } + c := cfg.UnassignFromUser + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("allowed", c.Allowed). + AddStringSlice("blocked", c.Blocked). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } +} diff --git a/pkg/workflow/compiler_safe_outputs_handlers_misc.go b/pkg/workflow/compiler_safe_outputs_handlers_misc.go new file mode 100644 index 00000000000..d4f53421fe5 --- /dev/null +++ b/pkg/workflow/compiler_safe_outputs_handlers_misc.go @@ -0,0 +1,317 @@ +package workflow + +func init() { + handlerRegistry["dispatch_workflow"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.DispatchWorkflow == nil { + return nil + } + c := cfg.DispatchWorkflow + builder := newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("workflows", c.Workflows). + AddIfNotEmpty("target-repo", c.TargetRepoSlug) + + // Add workflow_files map if it has entries + if len(c.WorkflowFiles) > 0 { + builder.AddDefault("workflow_files", c.WorkflowFiles) + } + + // Add aw_context_workflows list if it has entries + if len(c.AwContextWorkflows) > 0 { + builder.AddStringSlice("aw_context_workflows", c.AwContextWorkflows) + } + + builder.AddIfNotEmpty("target-ref", c.TargetRef) + builder.AddIfNotEmpty("github-token", c.GitHubToken) + builder.AddIfTrue("staged", c.Staged) + return builder.Build() + } + + handlerRegistry["dispatch_repository"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.DispatchRepository == nil || len(cfg.DispatchRepository.Tools) == 0 { + return nil + } + // Serialize each tool as a sub-map + tools := make(map[string]any, len(cfg.DispatchRepository.Tools)) + for toolKey, tool := range cfg.DispatchRepository.Tools { + toolConfig := newHandlerConfigBuilder(). + AddIfNotEmpty("workflow", tool.Workflow). + AddIfNotEmpty("event_type", tool.EventType). + AddIfNotEmpty("repository", tool.Repository). + AddStringSlice("allowed_repositories", tool.AllowedRepositories). + AddTemplatableInt("max", tool.Max). + AddIfNotEmpty("github-token", tool.GitHubToken). + AddIfTrue("staged", tool.Staged). + Build() + tools[toolKey] = toolConfig + } + return map[string]any{"tools": tools} + } + + handlerRegistry["call_workflow"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.CallWorkflow == nil { + return nil + } + c := cfg.CallWorkflow + builder := newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("workflows", c.Workflows) + + // Add workflow_files map if it has entries + if len(c.WorkflowFiles) > 0 { + builder.AddDefault("workflow_files", c.WorkflowFiles) + } + + builder.AddIfTrue("staged", c.Staged) + return builder.Build() + } + + handlerRegistry["noop"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.NoOp == nil { + return nil + } + c := cfg.NoOp + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringPtr("report-as-issue", c.ReportAsIssue). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["missing_tool"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.MissingTool == nil { + return nil + } + c := cfg.MissingTool + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["missing_data"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.MissingData == nil { + return nil + } + c := cfg.MissingData + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["report_incomplete"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.ReportIncomplete == nil { + return nil + } + c := cfg.ReportIncomplete + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["create_report_incomplete_issue"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.ReportIncomplete == nil { + return nil + } + c := cfg.ReportIncomplete + if !c.CreateIssue { + return nil + } + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("title-prefix", c.TitlePrefix). + AddStringSlice("labels", c.Labels). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["assign_to_agent"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.AssignToAgent == nil { + return nil + } + c := cfg.AssignToAgent + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("name", c.DefaultAgent). + AddIfNotEmpty("model", c.DefaultModel). + AddIfNotEmpty("custom-agent", c.DefaultCustomAgent). + AddIfNotEmpty("custom-instructions", c.DefaultCustomInstructions). + AddStringSlice("allowed", c.Allowed). + AddIfTrue("ignore-if-error", c.IgnoreIfError). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed-repos", c.AllowedRepos). + AddIfNotEmpty("pull-request-repo", c.PullRequestRepoSlug). + AddStringSlice("allowed-pull-request-repos", c.AllowedPullRequestRepos). + AddIfNotEmpty("base-branch", c.BaseBranch). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["upload_asset"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.UploadAssets == nil { + return nil + } + c := cfg.UploadAssets + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("branch", c.BranchName). + AddIfPositive("max-size", c.MaxSizeKB). + AddStringSlice("allowed-exts", c.AllowedExts). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["upload_artifact"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.UploadArtifact == nil { + return nil + } + c := cfg.UploadArtifact + b := newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfPositive("max-uploads", c.MaxUploads). + AddTemplatableInt("retention-days", c.RetentionDays). + AddTemplatableBool("skip-archive", c.SkipArchive). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged) + if c.MaxSizeBytes > 0 { + b = b.AddDefault("max-size-bytes", c.MaxSizeBytes) + } + if len(c.AllowedPaths) > 0 { + b = b.AddStringSlice("allowed-paths", c.AllowedPaths) + } + if c.Defaults != nil { + if c.Defaults.IfNoFiles != "" { + b = b.AddIfNotEmpty("default-if-no-files", c.Defaults.IfNoFiles) + } + } + if c.Filters != nil { + if len(c.Filters.Include) > 0 { + b = b.AddStringSlice("filters-include", c.Filters.Include) + } + if len(c.Filters.Exclude) > 0 { + b = b.AddStringSlice("filters-exclude", c.Filters.Exclude) + } + } + return b.Build() + } + + handlerRegistry["autofix_code_scanning_alert"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.AutofixCodeScanningAlert == nil { + return nil + } + c := cfg.AutofixCodeScanningAlert + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + // Note: create_project, update_project and create_project_status_update are handled by the unified handler, + // not the separate project handler manager, so they are included in this registry. + handlerRegistry["create_project"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.CreateProjects == nil { + return nil + } + c := cfg.CreateProjects + builder := newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target_owner", c.TargetOwner). + AddIfNotEmpty("title_prefix", c.TitlePrefix). + AddIfNotEmpty("github-token", c.GitHubToken) + if len(c.Views) > 0 { + builder.AddDefault("views", c.Views) + } + if len(c.FieldDefinitions) > 0 { + builder.AddDefault("field_definitions", c.FieldDefinitions) + } + builder.AddIfTrue("staged", c.Staged) + return builder.Build() + } + + handlerRegistry["update_project"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.UpdateProjects == nil { + return nil + } + c := cfg.UpdateProjects + builder := newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfNotEmpty("project", c.Project). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos) + if len(c.Views) > 0 { + builder.AddDefault("views", c.Views) + } + if len(c.FieldDefinitions) > 0 { + builder.AddDefault("field_definitions", c.FieldDefinitions) + } + builder.AddIfTrue("staged", c.Staged) + return builder.Build() + } + + handlerRegistry["create_project_status_update"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.CreateProjectStatusUpdates == nil { + return nil + } + c := cfg.CreateProjectStatusUpdates + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfNotEmpty("project", c.Project). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["update_release"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.UpdateRelease == nil { + return nil + } + c := cfg.UpdateRelease + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("github-token", c.GitHubToken). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["create_code_scanning_alert"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.CreateCodeScanningAlerts == nil { + return nil + } + c := cfg.CreateCodeScanningAlerts + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("driver", c.Driver). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["create_agent_session"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.CreateAgentSessions == nil { + return nil + } + c := cfg.CreateAgentSessions + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("base", c.Base). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } +} diff --git a/pkg/workflow/compiler_safe_outputs_handlers_prs.go b/pkg/workflow/compiler_safe_outputs_handlers_prs.go new file mode 100644 index 00000000000..24de6a3b9f3 --- /dev/null +++ b/pkg/workflow/compiler_safe_outputs_handlers_prs.go @@ -0,0 +1,205 @@ +package workflow + +func init() { + handlerRegistry["create_pull_request"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.CreatePullRequests == nil { + return nil + } + c := cfg.CreatePullRequests + maxPatchSize := 1024 // default 1024 KB + if cfg.MaximumPatchSize > 0 { + maxPatchSize = cfg.MaximumPatchSize + } + builder := newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("title_prefix", c.TitlePrefix). + AddStringSlice("labels", c.Labels). + AddStringSlice("reviewers", c.Reviewers). + AddStringSlice("assignees", c.Assignees). + AddTemplatableBool("draft", c.Draft). + AddIfNotEmpty("if_no_changes", c.IfNoChanges). + AddTemplatableBool("allow_empty", c.AllowEmpty). + AddTemplatableBool("auto_merge", c.AutoMerge). + AddIfPositive("expires", c.Expires). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddDefault("max_patch_size", maxPatchSize). + AddIfNotEmpty("github-token", c.GitHubToken). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). + AddBoolPtr("fallback_as_issue", c.FallbackAsIssue). + AddTemplatableBool("auto_close_issue", c.AutoCloseIssue). + AddIfNotEmpty("base_branch", c.BaseBranch). + AddStringPtr("protected_files_policy", c.ManifestFilesPolicy). + AddStringSlice("protected_files", getAllManifestFiles()). + AddStringSlice("protected_path_prefixes", getProtectedPathPrefixes()). + AddStringSlice("allowed_files", c.AllowedFiles). + AddStringSlice("excluded_files", c.ExcludedFiles). + AddIfTrue("preserve_branch_name", c.PreserveBranchName). + AddIfNotEmpty("patch_format", c.PatchFormat). + AddIfTrue("staged", c.Staged) + return builder.Build() + } + + handlerRegistry["push_to_pull_request_branch"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.PushToPullRequestBranch == nil { + return nil + } + c := cfg.PushToPullRequestBranch + maxPatchSize := 1024 // default 1024 KB + if cfg.MaximumPatchSize > 0 { + maxPatchSize = cfg.MaximumPatchSize + } + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("title_prefix", c.TitlePrefix). + AddStringSlice("labels", c.Labels). + AddIfNotEmpty("if_no_changes", c.IfNoChanges). + AddIfNotEmpty("commit_title_suffix", c.CommitTitleSuffix). + AddDefault("max_patch_size", maxPatchSize). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + AddStringPtr("protected_files_policy", c.ManifestFilesPolicy). + AddStringSlice("protected_files", getAllManifestFiles()). + AddStringSlice("protected_path_prefixes", getProtectedPathPrefixes()). + AddStringSlice("allowed_files", c.AllowedFiles). + AddStringSlice("excluded_files", c.ExcludedFiles). + AddIfNotEmpty("patch_format", c.PatchFormat). + Build() + } + + handlerRegistry["update_pull_request"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.UpdatePullRequests == nil { + return nil + } + c := cfg.UpdatePullRequests + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). + AddBoolPtrOrDefault("allow_title", c.Title, true). + AddBoolPtrOrDefault("allow_body", c.Body, true). + AddStringPtr("default_operation", c.Operation). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["close_pull_request"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.ClosePullRequests == nil { + return nil + } + c := cfg.ClosePullRequests + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). + AddStringSlice("required_labels", c.RequiredLabels). + AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["create_pull_request_review_comment"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.CreatePullRequestReviewComments == nil { + return nil + } + c := cfg.CreatePullRequestReviewComments + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("side", c.Side). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["submit_pull_request_review"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.SubmitPullRequestReview == nil { + return nil + } + c := cfg.SubmitPullRequestReview + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddStringSlice("allowed_events", c.AllowedEvents). + AddIfNotEmpty("github-token", c.GitHubToken). + AddStringPtr("footer", getEffectiveFooterString(c.Footer, cfg.Footer)). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["reply_to_pull_request_review_comment"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.ReplyToPullRequestReviewComment == nil { + return nil + } + c := cfg.ReplyToPullRequestReviewComment + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddTemplatableBool("footer", getEffectiveFooterForTemplatable(c.Footer, cfg.Footer)). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["resolve_pull_request_review_thread"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.ResolvePullRequestReviewThread == nil { + return nil + } + c := cfg.ResolvePullRequestReviewThread + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["add_reviewer"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.AddReviewer == nil { + return nil + } + c := cfg.AddReviewer + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddStringSlice("allowed", c.Reviewers). + AddIfNotEmpty("target", c.Target). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } + + handlerRegistry["mark_pull_request_as_ready_for_review"] = func(cfg *SafeOutputsConfig) map[string]any { + if cfg.MarkPullRequestAsReadyForReview == nil { + return nil + } + c := cfg.MarkPullRequestAsReadyForReview + return newHandlerConfigBuilder(). + AddTemplatableInt("max", c.Max). + AddIfNotEmpty("target", c.Target). + AddStringSlice("required_labels", c.RequiredLabels). + AddIfNotEmpty("required_title_prefix", c.RequiredTitlePrefix). + AddIfNotEmpty("target-repo", c.TargetRepoSlug). + AddStringSlice("allowed_repos", c.AllowedRepos). + AddIfNotEmpty("github-token", c.GitHubToken). + AddIfTrue("staged", c.Staged). + Build() + } +} From 71257d51f00731566f2c37b980b8c6924247f21b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 15:28:33 +0000 Subject: [PATCH 5/7] refactor: extract step/service/filter helpers from compiler_orchestrator_workflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move processAndMergeSteps, processAndMergePreSteps, processAndMergePostSteps, processAndMergeServices, mergeJobsFromYAMLImports, and processOnSectionAndFilters into a new compiler_orchestrator_helpers.go file. compiler_orchestrator_workflow.go: 1,062 → 674 lines compiler_orchestrator_helpers.go: 403 lines (new) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/compiler_orchestrator_helpers.go | 403 ++++++++++++++++++ .../compiler_orchestrator_workflow.go | 392 ----------------- 2 files changed, 403 insertions(+), 392 deletions(-) create mode 100644 pkg/workflow/compiler_orchestrator_helpers.go diff --git a/pkg/workflow/compiler_orchestrator_helpers.go b/pkg/workflow/compiler_orchestrator_helpers.go new file mode 100644 index 00000000000..fae907e56c3 --- /dev/null +++ b/pkg/workflow/compiler_orchestrator_helpers.go @@ -0,0 +1,403 @@ +//go:build !integration + +package workflow + +import ( + "encoding/json" + "fmt" + "maps" + "os" + "strings" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/parser" + "github.com/goccy/go-yaml" +) + +// processAndMergeSteps handles the merging of imported steps with main workflow steps +func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { + orchestratorWorkflowLog.Print("Processing and merging custom steps") + + workflowData.CustomSteps = c.extractTopLevelYAMLSection(frontmatter, "steps") + + // Parse copilot-setup-steps if present (these go at the start) + var copilotSetupSteps []any + if importsResult.CopilotSetupSteps != "" { + if err := yaml.Unmarshal([]byte(importsResult.CopilotSetupSteps), &copilotSetupSteps); err != nil { + orchestratorWorkflowLog.Printf("Failed to unmarshal copilot-setup steps: %v", err) + } else { + // Convert to typed steps for action pinning + typedCopilotSteps, err := SliceToSteps(copilotSetupSteps) + if err != nil { + orchestratorWorkflowLog.Printf("Failed to convert copilot-setup steps to typed steps: %v", err) + } else { + // Apply action pinning to copilot-setup steps + typedCopilotSteps = ApplyActionPinsToTypedSteps(typedCopilotSteps, workflowData) + // Convert back to []any for YAML marshaling + copilotSetupSteps = StepsToSlice(typedCopilotSteps) + } + } + } + + // Parse other imported steps if present (these go after copilot-setup but before main steps) + var otherImportedSteps []any + if importsResult.MergedSteps != "" { + if err := yaml.Unmarshal([]byte(importsResult.MergedSteps), &otherImportedSteps); err == nil { + // Convert to typed steps for action pinning + typedOtherSteps, err := SliceToSteps(otherImportedSteps) + if err != nil { + orchestratorWorkflowLog.Printf("Failed to convert other imported steps to typed steps: %v", err) + } else { + // Apply action pinning to other imported steps + typedOtherSteps = ApplyActionPinsToTypedSteps(typedOtherSteps, workflowData) + // Convert back to []any for YAML marshaling + otherImportedSteps = StepsToSlice(typedOtherSteps) + } + } + } + + // If there are main workflow steps, parse them + var mainSteps []any + if workflowData.CustomSteps != "" { + var mainStepsWrapper map[string]any + if err := yaml.Unmarshal([]byte(workflowData.CustomSteps), &mainStepsWrapper); err == nil { + if mainStepsVal, hasSteps := mainStepsWrapper["steps"]; hasSteps { + if steps, ok := mainStepsVal.([]any); ok { + mainSteps = steps + // Convert to typed steps for action pinning + typedMainSteps, err := SliceToSteps(mainSteps) + if err != nil { + orchestratorWorkflowLog.Printf("Failed to convert main steps to typed steps: %v", err) + } else { + // Apply action pinning to main steps + typedMainSteps = ApplyActionPinsToTypedSteps(typedMainSteps, workflowData) + // Convert back to []any for YAML marshaling + mainSteps = StepsToSlice(typedMainSteps) + } + } + } + } + } + + // Merge steps in the correct order: + // 1. copilot-setup-steps (at start) + // 2. other imported steps (after copilot-setup) + // 3. main frontmatter steps (last) + var allSteps []any + if len(copilotSetupSteps) > 0 || len(mainSteps) > 0 || len(otherImportedSteps) > 0 { + allSteps = append(allSteps, copilotSetupSteps...) + allSteps = append(allSteps, otherImportedSteps...) + allSteps = append(allSteps, mainSteps...) + + // Convert back to YAML with "steps:" wrapper + stepsWrapper := map[string]any{"steps": allSteps} + stepsYAML, err := yaml.Marshal(stepsWrapper) + if err == nil { + // Remove quotes from uses values with version comments + workflowData.CustomSteps = unquoteUsesWithComments(string(stepsYAML)) + } + } +} + +// processAndMergePreSteps handles the processing and merging of pre-steps with action pinning. +// Pre-steps run at the very beginning of the agent job, before checkout and the subsequent +// built-in steps, allowing users to mint tokens or perform other setup that must happen +// before the repository is checked out. Imported pre-steps are merged before the main +// workflow's pre-steps so that the main workflow can override or extend the imports. +func (c *Compiler) processAndMergePreSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { + orchestratorWorkflowLog.Print("Processing and merging pre-steps") + + mainPreStepsYAML := c.extractTopLevelYAMLSection(frontmatter, "pre-steps") + + // Parse imported pre-steps if present (these go before the main workflow's pre-steps) + var importedPreSteps []any + if importsResult.MergedPreSteps != "" { + if err := yaml.Unmarshal([]byte(importsResult.MergedPreSteps), &importedPreSteps); err != nil { + orchestratorWorkflowLog.Printf("Failed to unmarshal imported pre-steps: %v", err) + } else { + typedImported, err := SliceToSteps(importedPreSteps) + if err != nil { + orchestratorWorkflowLog.Printf("Failed to convert imported pre-steps to typed steps: %v", err) + } else { + typedImported = ApplyActionPinsToTypedSteps(typedImported, workflowData) + importedPreSteps = StepsToSlice(typedImported) + } + } + } + + // Parse main workflow pre-steps if present + var mainPreSteps []any + if mainPreStepsYAML != "" { + var mainWrapper map[string]any + if err := yaml.Unmarshal([]byte(mainPreStepsYAML), &mainWrapper); err == nil { + if mainVal, ok := mainWrapper["pre-steps"]; ok { + if steps, ok := mainVal.([]any); ok { + mainPreSteps = steps + typedMain, err := SliceToSteps(mainPreSteps) + if err != nil { + orchestratorWorkflowLog.Printf("Failed to convert main pre-steps to typed steps: %v", err) + } else { + typedMain = ApplyActionPinsToTypedSteps(typedMain, workflowData) + mainPreSteps = StepsToSlice(typedMain) + } + } + } + } + } + + // Merge in order: imported pre-steps first, then main workflow's pre-steps + var allPreSteps []any + if len(importedPreSteps) > 0 || len(mainPreSteps) > 0 { + allPreSteps = append(allPreSteps, importedPreSteps...) + allPreSteps = append(allPreSteps, mainPreSteps...) + + stepsWrapper := map[string]any{"pre-steps": allPreSteps} + stepsYAML, err := yaml.Marshal(stepsWrapper) + if err == nil { + workflowData.PreSteps = unquoteUsesWithComments(string(stepsYAML)) + } + } +} + +// processAndMergePostSteps handles the processing and merging of post-steps with action pinning. +// Imported post-steps are appended after the main workflow's post-steps. +func (c *Compiler) processAndMergePostSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { + orchestratorWorkflowLog.Print("Processing and merging post-steps") + + mainPostStepsYAML := c.extractTopLevelYAMLSection(frontmatter, "post-steps") + + // Parse imported post-steps if present (these go after the main workflow's post-steps) + var importedPostSteps []any + if importsResult.MergedPostSteps != "" { + if err := yaml.Unmarshal([]byte(importsResult.MergedPostSteps), &importedPostSteps); err != nil { + orchestratorWorkflowLog.Printf("Failed to unmarshal imported post-steps: %v", err) + } else { + typedImported, err := SliceToSteps(importedPostSteps) + if err != nil { + orchestratorWorkflowLog.Printf("Failed to convert imported post-steps to typed steps: %v", err) + } else { + typedImported = ApplyActionPinsToTypedSteps(typedImported, workflowData) + importedPostSteps = StepsToSlice(typedImported) + } + } + } + + // Parse main workflow post-steps if present + var mainPostSteps []any + if mainPostStepsYAML != "" { + var mainWrapper map[string]any + if err := yaml.Unmarshal([]byte(mainPostStepsYAML), &mainWrapper); err == nil { + if mainVal, ok := mainWrapper["post-steps"]; ok { + if steps, ok := mainVal.([]any); ok { + mainPostSteps = steps + typedMain, err := SliceToSteps(mainPostSteps) + if err != nil { + orchestratorWorkflowLog.Printf("Failed to convert main post-steps to typed steps: %v", err) + } else { + typedMain = ApplyActionPinsToTypedSteps(typedMain, workflowData) + mainPostSteps = StepsToSlice(typedMain) + } + } + } + } + } + + // Merge in order: main workflow's post-steps first, then imported post-steps + var allPostSteps []any + if len(mainPostSteps) > 0 || len(importedPostSteps) > 0 { + allPostSteps = append(allPostSteps, mainPostSteps...) + allPostSteps = append(allPostSteps, importedPostSteps...) + + stepsWrapper := map[string]any{"post-steps": allPostSteps} + stepsYAML, err := yaml.Marshal(stepsWrapper) + if err == nil { + workflowData.PostSteps = unquoteUsesWithComments(string(stepsYAML)) + } + } +} + +// processAndMergeServices handles the merging of imported services with main workflow services +func (c *Compiler) processAndMergeServices(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { + orchestratorWorkflowLog.Print("Processing and merging services") + + workflowData.Services = c.extractTopLevelYAMLSection(frontmatter, "services") + + // Merge imported services if any + if importsResult.MergedServices != "" { + // Parse imported services from YAML + var importedServices map[string]any + if err := yaml.Unmarshal([]byte(importsResult.MergedServices), &importedServices); err == nil { + // If there are main workflow services, parse and merge them + if workflowData.Services != "" { + // Parse main workflow services + var mainServicesWrapper map[string]any + if err := yaml.Unmarshal([]byte(workflowData.Services), &mainServicesWrapper); err == nil { + if mainServices, ok := mainServicesWrapper["services"].(map[string]any); ok { + // Merge: main workflow services take precedence over imported + for key, value := range importedServices { + if _, exists := mainServices[key]; !exists { + mainServices[key] = value + } + } + // Convert back to YAML with "services:" wrapper + servicesWrapper := map[string]any{"services": mainServices} + servicesYAML, err := yaml.Marshal(servicesWrapper) + if err == nil { + workflowData.Services = string(servicesYAML) + } + } + } + } else { + // Only imported services exist, wrap in "services:" format + servicesWrapper := map[string]any{"services": importedServices} + servicesYAML, err := yaml.Marshal(servicesWrapper) + if err == nil { + workflowData.Services = string(servicesYAML) + } + } + } + } + + // Extract service port expressions for AWF --allow-host-service-ports + if workflowData.Services != "" { + expressions, warnings := ExtractServicePortExpressions(workflowData.Services) + workflowData.ServicePortExpressions = expressions + for _, w := range warnings { + orchestratorWorkflowLog.Printf("Warning: %s", w) + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(w)) + c.IncrementWarningCount() + } + if expressions != "" { + orchestratorWorkflowLog.Printf("Extracted service port expressions: %s", expressions) + } + } +} + +// mergeJobsFromYAMLImports merges jobs from imported YAML workflows with main workflow jobs. +// Main workflow jobs take precedence over imported jobs (override behavior). +func (c *Compiler) mergeJobsFromYAMLImports(mainJobs map[string]any, mergedJobsJSON string) map[string]any { + orchestratorWorkflowLog.Print("Merging jobs from imported YAML workflows") + + if mergedJobsJSON == "" || mergedJobsJSON == "{}" { + orchestratorWorkflowLog.Print("No imported jobs to merge") + return mainJobs + } + + // Initialize result with main jobs or create empty map + result := make(map[string]any) + maps.Copy(result, mainJobs) + + // Split by newlines to handle multiple JSON objects from different imports + lines := strings.Split(mergedJobsJSON, "\n") + orchestratorWorkflowLog.Printf("Processing %d job definition lines", len(lines)) + + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" || line == "{}" { + continue + } + + // Parse JSON line to map + var importedJobs map[string]any + if err := json.Unmarshal([]byte(line), &importedJobs); err != nil { + orchestratorWorkflowLog.Printf("Skipping malformed job entry: %v", err) + continue + } + + // Merge jobs - main workflow jobs take precedence (don't override) + for jobName, jobConfig := range importedJobs { + if _, exists := result[jobName]; !exists { + orchestratorWorkflowLog.Printf("Adding imported job: %s", jobName) + result[jobName] = jobConfig + } else { + orchestratorWorkflowLog.Printf("Skipping imported job %s (already defined in main workflow)", jobName) + } + } + } + + orchestratorWorkflowLog.Printf("Successfully merged jobs: total=%d, imported=%d", len(result), len(result)-len(mainJobs)) + return result +} + +// processOnSectionAndFilters processes the on section configuration and applies various filters +func (c *Compiler) processOnSectionAndFilters( + frontmatter map[string]any, + workflowData *WorkflowData, + cleanPath string, +) error { + orchestratorWorkflowLog.Print("Processing on section and filters") + + // Process stop-after configuration from the on: section + if err := c.processStopAfterConfiguration(frontmatter, workflowData, cleanPath); err != nil { + return err + } + + // Process skip-if-match configuration from the on: section + if err := c.processSkipIfMatchConfiguration(frontmatter, workflowData); err != nil { + return err + } + + // Process skip-if-no-match configuration from the on: section + if err := c.processSkipIfNoMatchConfiguration(frontmatter, workflowData); err != nil { + return err + } + + // Process skip-if-check-failing configuration from the on: section + if err := c.processSkipIfCheckFailingConfiguration(frontmatter, workflowData); err != nil { + return err + } + + // Process manual-approval configuration from the on: section + if err := c.processManualApprovalConfiguration(frontmatter, workflowData); err != nil { + return err + } + + // Parse the "on" section for command triggers, reactions, and other events + if err := c.parseOnSection(frontmatter, workflowData, cleanPath); err != nil { + return err + } + + // Apply defaults + if err := c.applyDefaults(workflowData, cleanPath); err != nil { + return err + } + + // Apply pull request draft filter if specified + c.applyPullRequestDraftFilter(workflowData, frontmatter) + + // Apply pull request fork filter if specified + c.applyPullRequestForkFilter(workflowData, frontmatter) + + // Apply label filter if specified + c.applyLabelFilter(workflowData, frontmatter) + + // Extract on.steps for pre-activation step injection + onSteps, err := extractOnSteps(frontmatter) + if err != nil { + return err + } + + // Apply action pinning to on.steps + if len(onSteps) > 0 { + anySteps := make([]any, len(onSteps)) + for i, s := range onSteps { + anySteps[i] = s + } + typedSteps, convErr := SliceToSteps(anySteps) + if convErr == nil { + typedSteps = ApplyActionPinsToTypedSteps(typedSteps, workflowData) + for i, s := range typedSteps { + onSteps[i] = s.ToMap() + } + } else { + orchestratorWorkflowLog.Printf("Failed to convert on.steps to typed steps for action pinning: %v", convErr) + } + } + + workflowData.OnSteps = onSteps + + // Extract on.permissions for pre-activation job permissions + workflowData.OnPermissions = extractOnPermissions(frontmatter) + + return nil +} diff --git a/pkg/workflow/compiler_orchestrator_workflow.go b/pkg/workflow/compiler_orchestrator_workflow.go index 4630e2d294b..d198e61cce1 100644 --- a/pkg/workflow/compiler_orchestrator_workflow.go +++ b/pkg/workflow/compiler_orchestrator_workflow.go @@ -3,14 +3,10 @@ package workflow import ( "encoding/json" "fmt" - "maps" - "os" "strings" - "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/parser" - "github.com/goccy/go-yaml" ) var orchestratorWorkflowLog = logger.New("workflow:compiler_orchestrator_workflow") @@ -411,311 +407,6 @@ func extractDispatchItemNumber(frontmatter map[string]any) bool { return ok } -// processAndMergeSteps handles the merging of imported steps with main workflow steps -func (c *Compiler) processAndMergeSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { - orchestratorWorkflowLog.Print("Processing and merging custom steps") - - workflowData.CustomSteps = c.extractTopLevelYAMLSection(frontmatter, "steps") - - // Parse copilot-setup-steps if present (these go at the start) - var copilotSetupSteps []any - if importsResult.CopilotSetupSteps != "" { - if err := yaml.Unmarshal([]byte(importsResult.CopilotSetupSteps), &copilotSetupSteps); err != nil { - orchestratorWorkflowLog.Printf("Failed to unmarshal copilot-setup steps: %v", err) - } else { - // Convert to typed steps for action pinning - typedCopilotSteps, err := SliceToSteps(copilotSetupSteps) - if err != nil { - orchestratorWorkflowLog.Printf("Failed to convert copilot-setup steps to typed steps: %v", err) - } else { - // Apply action pinning to copilot-setup steps - typedCopilotSteps = ApplyActionPinsToTypedSteps(typedCopilotSteps, workflowData) - // Convert back to []any for YAML marshaling - copilotSetupSteps = StepsToSlice(typedCopilotSteps) - } - } - } - - // Parse other imported steps if present (these go after copilot-setup but before main steps) - var otherImportedSteps []any - if importsResult.MergedSteps != "" { - if err := yaml.Unmarshal([]byte(importsResult.MergedSteps), &otherImportedSteps); err == nil { - // Convert to typed steps for action pinning - typedOtherSteps, err := SliceToSteps(otherImportedSteps) - if err != nil { - orchestratorWorkflowLog.Printf("Failed to convert other imported steps to typed steps: %v", err) - } else { - // Apply action pinning to other imported steps - typedOtherSteps = ApplyActionPinsToTypedSteps(typedOtherSteps, workflowData) - // Convert back to []any for YAML marshaling - otherImportedSteps = StepsToSlice(typedOtherSteps) - } - } - } - - // If there are main workflow steps, parse them - var mainSteps []any - if workflowData.CustomSteps != "" { - var mainStepsWrapper map[string]any - if err := yaml.Unmarshal([]byte(workflowData.CustomSteps), &mainStepsWrapper); err == nil { - if mainStepsVal, hasSteps := mainStepsWrapper["steps"]; hasSteps { - if steps, ok := mainStepsVal.([]any); ok { - mainSteps = steps - // Convert to typed steps for action pinning - typedMainSteps, err := SliceToSteps(mainSteps) - if err != nil { - orchestratorWorkflowLog.Printf("Failed to convert main steps to typed steps: %v", err) - } else { - // Apply action pinning to main steps - typedMainSteps = ApplyActionPinsToTypedSteps(typedMainSteps, workflowData) - // Convert back to []any for YAML marshaling - mainSteps = StepsToSlice(typedMainSteps) - } - } - } - } - } - - // Merge steps in the correct order: - // 1. copilot-setup-steps (at start) - // 2. other imported steps (after copilot-setup) - // 3. main frontmatter steps (last) - var allSteps []any - if len(copilotSetupSteps) > 0 || len(mainSteps) > 0 || len(otherImportedSteps) > 0 { - allSteps = append(allSteps, copilotSetupSteps...) - allSteps = append(allSteps, otherImportedSteps...) - allSteps = append(allSteps, mainSteps...) - - // Convert back to YAML with "steps:" wrapper - stepsWrapper := map[string]any{"steps": allSteps} - stepsYAML, err := yaml.Marshal(stepsWrapper) - if err == nil { - // Remove quotes from uses values with version comments - workflowData.CustomSteps = unquoteUsesWithComments(string(stepsYAML)) - } - } -} - -// processAndMergePreSteps handles the processing and merging of pre-steps with action pinning. -// Pre-steps run at the very beginning of the agent job, before checkout and the subsequent -// built-in steps, allowing users to mint tokens or perform other setup that must happen -// before the repository is checked out. Imported pre-steps are merged before the main -// workflow's pre-steps so that the main workflow can override or extend the imports. -func (c *Compiler) processAndMergePreSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { - orchestratorWorkflowLog.Print("Processing and merging pre-steps") - - mainPreStepsYAML := c.extractTopLevelYAMLSection(frontmatter, "pre-steps") - - // Parse imported pre-steps if present (these go before the main workflow's pre-steps) - var importedPreSteps []any - if importsResult.MergedPreSteps != "" { - if err := yaml.Unmarshal([]byte(importsResult.MergedPreSteps), &importedPreSteps); err != nil { - orchestratorWorkflowLog.Printf("Failed to unmarshal imported pre-steps: %v", err) - } else { - typedImported, err := SliceToSteps(importedPreSteps) - if err != nil { - orchestratorWorkflowLog.Printf("Failed to convert imported pre-steps to typed steps: %v", err) - } else { - typedImported = ApplyActionPinsToTypedSteps(typedImported, workflowData) - importedPreSteps = StepsToSlice(typedImported) - } - } - } - - // Parse main workflow pre-steps if present - var mainPreSteps []any - if mainPreStepsYAML != "" { - var mainWrapper map[string]any - if err := yaml.Unmarshal([]byte(mainPreStepsYAML), &mainWrapper); err == nil { - if mainVal, ok := mainWrapper["pre-steps"]; ok { - if steps, ok := mainVal.([]any); ok { - mainPreSteps = steps - typedMain, err := SliceToSteps(mainPreSteps) - if err != nil { - orchestratorWorkflowLog.Printf("Failed to convert main pre-steps to typed steps: %v", err) - } else { - typedMain = ApplyActionPinsToTypedSteps(typedMain, workflowData) - mainPreSteps = StepsToSlice(typedMain) - } - } - } - } - } - - // Merge in order: imported pre-steps first, then main workflow's pre-steps - var allPreSteps []any - if len(importedPreSteps) > 0 || len(mainPreSteps) > 0 { - allPreSteps = append(allPreSteps, importedPreSteps...) - allPreSteps = append(allPreSteps, mainPreSteps...) - - stepsWrapper := map[string]any{"pre-steps": allPreSteps} - stepsYAML, err := yaml.Marshal(stepsWrapper) - if err == nil { - workflowData.PreSteps = unquoteUsesWithComments(string(stepsYAML)) - } - } -} - -// processAndMergePostSteps handles the processing and merging of post-steps with action pinning. -// Imported post-steps are appended after the main workflow's post-steps. -func (c *Compiler) processAndMergePostSteps(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { - orchestratorWorkflowLog.Print("Processing and merging post-steps") - - mainPostStepsYAML := c.extractTopLevelYAMLSection(frontmatter, "post-steps") - - // Parse imported post-steps if present (these go after the main workflow's post-steps) - var importedPostSteps []any - if importsResult.MergedPostSteps != "" { - if err := yaml.Unmarshal([]byte(importsResult.MergedPostSteps), &importedPostSteps); err != nil { - orchestratorWorkflowLog.Printf("Failed to unmarshal imported post-steps: %v", err) - } else { - typedImported, err := SliceToSteps(importedPostSteps) - if err != nil { - orchestratorWorkflowLog.Printf("Failed to convert imported post-steps to typed steps: %v", err) - } else { - typedImported = ApplyActionPinsToTypedSteps(typedImported, workflowData) - importedPostSteps = StepsToSlice(typedImported) - } - } - } - - // Parse main workflow post-steps if present - var mainPostSteps []any - if mainPostStepsYAML != "" { - var mainWrapper map[string]any - if err := yaml.Unmarshal([]byte(mainPostStepsYAML), &mainWrapper); err == nil { - if mainVal, ok := mainWrapper["post-steps"]; ok { - if steps, ok := mainVal.([]any); ok { - mainPostSteps = steps - typedMain, err := SliceToSteps(mainPostSteps) - if err != nil { - orchestratorWorkflowLog.Printf("Failed to convert main post-steps to typed steps: %v", err) - } else { - typedMain = ApplyActionPinsToTypedSteps(typedMain, workflowData) - mainPostSteps = StepsToSlice(typedMain) - } - } - } - } - } - - // Merge in order: main workflow's post-steps first, then imported post-steps - var allPostSteps []any - if len(mainPostSteps) > 0 || len(importedPostSteps) > 0 { - allPostSteps = append(allPostSteps, mainPostSteps...) - allPostSteps = append(allPostSteps, importedPostSteps...) - - stepsWrapper := map[string]any{"post-steps": allPostSteps} - stepsYAML, err := yaml.Marshal(stepsWrapper) - if err == nil { - workflowData.PostSteps = unquoteUsesWithComments(string(stepsYAML)) - } - } -} - -// processAndMergeServices handles the merging of imported services with main workflow services -func (c *Compiler) processAndMergeServices(frontmatter map[string]any, workflowData *WorkflowData, importsResult *parser.ImportsResult) { - orchestratorWorkflowLog.Print("Processing and merging services") - - workflowData.Services = c.extractTopLevelYAMLSection(frontmatter, "services") - - // Merge imported services if any - if importsResult.MergedServices != "" { - // Parse imported services from YAML - var importedServices map[string]any - if err := yaml.Unmarshal([]byte(importsResult.MergedServices), &importedServices); err == nil { - // If there are main workflow services, parse and merge them - if workflowData.Services != "" { - // Parse main workflow services - var mainServicesWrapper map[string]any - if err := yaml.Unmarshal([]byte(workflowData.Services), &mainServicesWrapper); err == nil { - if mainServices, ok := mainServicesWrapper["services"].(map[string]any); ok { - // Merge: main workflow services take precedence over imported - for key, value := range importedServices { - if _, exists := mainServices[key]; !exists { - mainServices[key] = value - } - } - // Convert back to YAML with "services:" wrapper - servicesWrapper := map[string]any{"services": mainServices} - servicesYAML, err := yaml.Marshal(servicesWrapper) - if err == nil { - workflowData.Services = string(servicesYAML) - } - } - } - } else { - // Only imported services exist, wrap in "services:" format - servicesWrapper := map[string]any{"services": importedServices} - servicesYAML, err := yaml.Marshal(servicesWrapper) - if err == nil { - workflowData.Services = string(servicesYAML) - } - } - } - } - - // Extract service port expressions for AWF --allow-host-service-ports - if workflowData.Services != "" { - expressions, warnings := ExtractServicePortExpressions(workflowData.Services) - workflowData.ServicePortExpressions = expressions - for _, w := range warnings { - orchestratorWorkflowLog.Printf("Warning: %s", w) - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(w)) - c.IncrementWarningCount() - } - if expressions != "" { - orchestratorWorkflowLog.Printf("Extracted service port expressions: %s", expressions) - } - } -} - -// mergeJobsFromYAMLImports merges jobs from imported YAML workflows with main workflow jobs -// Main workflow jobs take precedence over imported jobs (override behavior) -func (c *Compiler) mergeJobsFromYAMLImports(mainJobs map[string]any, mergedJobsJSON string) map[string]any { - orchestratorWorkflowLog.Print("Merging jobs from imported YAML workflows") - - if mergedJobsJSON == "" || mergedJobsJSON == "{}" { - orchestratorWorkflowLog.Print("No imported jobs to merge") - return mainJobs - } - - // Initialize result with main jobs or create empty map - result := make(map[string]any) - maps.Copy(result, mainJobs) - - // Split by newlines to handle multiple JSON objects from different imports - lines := strings.Split(mergedJobsJSON, "\n") - orchestratorWorkflowLog.Printf("Processing %d job definition lines", len(lines)) - - for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" || line == "{}" { - continue - } - - // Parse JSON line to map - var importedJobs map[string]any - if err := json.Unmarshal([]byte(line), &importedJobs); err != nil { - orchestratorWorkflowLog.Printf("Skipping malformed job entry: %v", err) - continue - } - - // Merge jobs - main workflow jobs take precedence (don't override) - for jobName, jobConfig := range importedJobs { - if _, exists := result[jobName]; !exists { - orchestratorWorkflowLog.Printf("Adding imported job: %s", jobName) - result[jobName] = jobConfig - } else { - orchestratorWorkflowLog.Printf("Skipping imported job %s (already defined in main workflow)", jobName) - } - } - } - - orchestratorWorkflowLog.Printf("Successfully merged jobs: total=%d, imported=%d", len(result), len(result)-len(mainJobs)) - return result -} - // extractTopLevelGitHubApp extracts the 'github-app' field from the top-level frontmatter. // This provides a single GitHub App configuration that serves as a fallback for all nested // github-app token minting operations (on, safe-outputs, checkout, tools.github, dependencies). @@ -977,86 +668,3 @@ func (c *Compiler) extractAdditionalConfigurations( return nil } - -// processOnSectionAndFilters processes the on section configuration and applies various filters -func (c *Compiler) processOnSectionAndFilters( - frontmatter map[string]any, - workflowData *WorkflowData, - cleanPath string, -) error { - orchestratorWorkflowLog.Print("Processing on section and filters") - - // Process stop-after configuration from the on: section - if err := c.processStopAfterConfiguration(frontmatter, workflowData, cleanPath); err != nil { - return err - } - - // Process skip-if-match configuration from the on: section - if err := c.processSkipIfMatchConfiguration(frontmatter, workflowData); err != nil { - return err - } - - // Process skip-if-no-match configuration from the on: section - if err := c.processSkipIfNoMatchConfiguration(frontmatter, workflowData); err != nil { - return err - } - - // Process skip-if-check-failing configuration from the on: section - if err := c.processSkipIfCheckFailingConfiguration(frontmatter, workflowData); err != nil { - return err - } - - // Process manual-approval configuration from the on: section - if err := c.processManualApprovalConfiguration(frontmatter, workflowData); err != nil { - return err - } - - // Parse the "on" section for command triggers, reactions, and other events - if err := c.parseOnSection(frontmatter, workflowData, cleanPath); err != nil { - return err - } - - // Apply defaults - if err := c.applyDefaults(workflowData, cleanPath); err != nil { - return err - } - - // Apply pull request draft filter if specified - c.applyPullRequestDraftFilter(workflowData, frontmatter) - - // Apply pull request fork filter if specified - c.applyPullRequestForkFilter(workflowData, frontmatter) - - // Apply label filter if specified - c.applyLabelFilter(workflowData, frontmatter) - - // Extract on.steps for pre-activation step injection - onSteps, err := extractOnSteps(frontmatter) - if err != nil { - return err - } - - // Apply action pinning to on.steps - if len(onSteps) > 0 { - anySteps := make([]any, len(onSteps)) - for i, s := range onSteps { - anySteps[i] = s - } - typedSteps, convErr := SliceToSteps(anySteps) - if convErr == nil { - typedSteps = ApplyActionPinsToTypedSteps(typedSteps, workflowData) - for i, s := range typedSteps { - onSteps[i] = s.ToMap() - } - } else { - orchestratorWorkflowLog.Printf("Failed to convert on.steps to typed steps for action pinning: %v", convErr) - } - } - - workflowData.OnSteps = onSteps - - // Extract on.permissions for pre-activation job permissions - workflowData.OnPermissions = extractOnPermissions(frontmatter) - - return nil -} From 9c7959e9ca318a590830642597dd3e8336955c24 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 10 Apr 2026 15:37:14 +0000 Subject: [PATCH 6/7] refactor: split large Go files to address architecture BLOCKER violations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Split pkg/cli/gateway_logs.go (1332→201 lines) into: gateway_logs_parser.go, gateway_logs_renderer.go, gateway_logs_extractor.go - Split pkg/cli/audit_report_render.go (1139→691 lines) into: audit_render_policy.go, audit_render_performance.go, audit_render_tokens.go - Split pkg/workflow/compiler_safe_outputs_config.go (1035→263 lines): extracted handler registry into 4 domain init() files - Split pkg/workflow/compiler_orchestrator_workflow.go (1062→670 lines): extracted helpers into compiler_orchestrator_helpers.go - Split pkg/cli/logs_orchestrator.go (1074→600 lines): extracted concurrency functions into logs_orchestrator_concurrency.go - Split pkg/cli/logs_report.go (1065→896 lines): extracted network analysis into logs_report_network.go" Agent-Logs-Url: https://github.com/github/gh-aw/sessions/ec5a1c29-2e00-4c3b-98c8-15bb82259626 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/logs_orchestrator.go | 475 ---------------------- pkg/cli/logs_orchestrator_concurrency.go | 494 +++++++++++++++++++++++ pkg/cli/logs_report.go | 169 -------- pkg/cli/logs_report_network.go | 177 ++++++++ 4 files changed, 671 insertions(+), 644 deletions(-) create mode 100644 pkg/cli/logs_orchestrator_concurrency.go create mode 100644 pkg/cli/logs_report_network.go diff --git a/pkg/cli/logs_orchestrator.go b/pkg/cli/logs_orchestrator.go index e29caa8c2fe..0a76f4d1a12 100644 --- a/pkg/cli/logs_orchestrator.go +++ b/pkg/cli/logs_orchestrator.go @@ -13,23 +13,18 @@ package cli import ( "context" - "encoding/json" - "errors" "fmt" "math" "os" "path/filepath" "strings" - "sync/atomic" "time" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/envutil" "github.com/github/gh-aw/pkg/logger" - "github.com/github/gh-aw/pkg/stringutil" "github.com/github/gh-aw/pkg/workflow" - "github.com/sourcegraph/conc/pool" ) var logsOrchestratorLog = logger.New("cli:logs_orchestrator") @@ -602,473 +597,3 @@ func DownloadWorkflowLogs(ctx context.Context, workflowName string, count int, s return nil } - -// downloadRunArtifactsConcurrent downloads artifacts for multiple workflow runs concurrently -func downloadRunArtifactsConcurrent(ctx context.Context, runs []WorkflowRun, outputDir string, verbose bool, maxRuns int, repoOverride string, artifactFilter []string) []DownloadResult { - logsOrchestratorLog.Printf("Starting concurrent artifact download: runs=%d, outputDir=%s, maxRuns=%d", len(runs), outputDir, maxRuns) - if len(runs) == 0 { - return []DownloadResult{} - } - - // Process all runs in the batch to account for caching and filtering - // The maxRuns parameter indicates how many successful results we need, but we may need to - // process more runs to account for: - // 1. Cached runs that may fail filters (engine, firewall, etc.) - // 2. Runs that may be skipped due to errors - // 3. Runs without artifacts - // - // By processing all runs in the batch, we ensure that the count parameter correctly - // reflects the number of matching logs (both downloaded and cached), not just attempts. - actualRuns := runs - - totalRuns := len(actualRuns) - - if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Processing %d runs in parallel...", totalRuns))) - } - - // Create progress bar for tracking run processing (only in non-verbose, non-CI mode) - // In CI environments \r is treated as a newline, producing excessive output for each update. - var progressBar *console.ProgressBar - if !verbose && !IsRunningInCI() { - progressBar = console.NewProgressBar(int64(totalRuns)) - fmt.Fprintf(os.Stderr, "Processing runs: %s\r", progressBar.Update(0)) - } - - // Use atomic counter for thread-safe progress tracking - var completedCount int64 - - // Get configured max concurrent downloads (default or from environment variable) - maxConcurrent := getMaxConcurrentDownloads() - - // Parse repoOverride into owner/repo once for cross-repo artifact download - var dlOwner, dlRepo string - if repoOverride != "" { - parts := strings.SplitN(repoOverride, "/", 2) - if len(parts) == 2 { - dlOwner = parts[0] - dlRepo = parts[1] - } - } - - // Configure concurrent download pool with bounded parallelism and context cancellation. - // The conc pool automatically handles panic recovery and prevents goroutine leaks. - // WithContext enables graceful cancellation via Ctrl+C. - p := pool.NewWithResults[DownloadResult](). - WithContext(ctx). - WithMaxGoroutines(maxConcurrent) - - // Each download task runs concurrently with context awareness. - // Context cancellation (e.g., via Ctrl+C) will stop all in-flight downloads gracefully. - // Panics are automatically recovered by the pool and re-raised with full stack traces - // after all tasks complete. This ensures one failing download doesn't break others. - for _, run := range actualRuns { - p.Go(func(ctx context.Context) (DownloadResult, error) { - // Check for context cancellation before starting download - select { - case <-ctx.Done(): - return DownloadResult{ - Run: run, - Skipped: true, - Error: ctx.Err(), - }, nil - default: - } - if verbose { - fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Processing run %d (%s)...", run.DatabaseID, run.Status))) - } - - // Download artifacts and logs for this run - runOutputDir := filepath.Join(outputDir, fmt.Sprintf("run-%d", run.DatabaseID)) - - // Try to load cached summary first - if summary, ok := loadRunSummary(runOutputDir, verbose); ok { - // Valid cached summary exists, use it directly - result := DownloadResult{ - Run: summary.Run, - Metrics: summary.Metrics, - AwContext: summary.AwContext, - TaskDomain: summary.TaskDomain, - BehaviorFingerprint: summary.BehaviorFingerprint, - AgenticAssessments: summary.AgenticAssessments, - AccessAnalysis: summary.AccessAnalysis, - FirewallAnalysis: summary.FirewallAnalysis, - RedactedDomainsAnalysis: summary.RedactedDomainsAnalysis, - MissingTools: summary.MissingTools, - MissingData: summary.MissingData, - Noops: summary.Noops, - MCPFailures: summary.MCPFailures, - MCPToolUsage: summary.MCPToolUsage, - TokenUsage: summary.TokenUsage, - GitHubRateLimitUsage: summary.GitHubRateLimitUsage, - JobDetails: summary.JobDetails, - LogsPath: runOutputDir, - Cached: true, // Mark as cached - } - // Update progress counter - completed := atomic.AddInt64(&completedCount, 1) - if progressBar != nil { - fmt.Fprintf(os.Stderr, "Processing runs: %s\r", progressBar.Update(completed)) - } - return result, nil - } - - // No cached summary or version mismatch - download and process - err := downloadRunArtifacts(run.DatabaseID, runOutputDir, verbose, dlOwner, dlRepo, "", artifactFilter) - - result := DownloadResult{ - Run: run, - LogsPath: runOutputDir, - } - - if err != nil { - // Check if this is a "no artifacts" case - if errors.Is(err, ErrNoArtifacts) { - // For runs with important conclusions (timed_out, failure, cancelled), - // still process them even without artifacts to show the failure in reports - if isFailureConclusion(run.Conclusion) { - // Don't skip - we want these to appear in the report - // Just use empty metrics - result.Metrics = LogMetrics{} - - // Try to fetch job details to get error count - if failedJobCount, jobErr := fetchJobStatuses(run.DatabaseID, verbose); jobErr == nil { - run.ErrorCount = failedJobCount - } - } else { - // For other runs (success, neutral, etc.) without artifacts, skip them - result.Skipped = true - result.Error = err - } - } else { - result.Error = err - } - } else { - // Extract metrics from logs - metrics, metricsErr := extractLogMetrics(runOutputDir, verbose) - if metricsErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract metrics for run %d: %v", run.DatabaseID, metricsErr))) - } - // Don't fail the whole download for metrics errors - metrics = LogMetrics{} - } - result.Metrics = metrics - - // Update run with metrics so fingerprint computation uses the same data - // as the audit tool, which also derives these fields from extracted log metrics. - result.Run.TokenUsage = metrics.TokenUsage - result.Run.EstimatedCost = metrics.EstimatedCost - result.Run.Turns = metrics.Turns - result.Run.LogsPath = runOutputDir - - // Calculate duration and billable minutes from GitHub API timestamps. - // This mirrors the identical computation in audit.go so that - // processedRun.Run.Duration is consistent across both tools. - if !result.Run.StartedAt.IsZero() && !result.Run.UpdatedAt.IsZero() { - result.Run.Duration = result.Run.UpdatedAt.Sub(result.Run.StartedAt) - result.Run.ActionMinutes = math.Ceil(result.Run.Duration.Minutes()) - } - - // Analyze access logs if available - accessAnalysis, accessErr := analyzeAccessLogs(runOutputDir, verbose) - if accessErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze access logs for run %d: %v", run.DatabaseID, accessErr))) - } - } - result.AccessAnalysis = accessAnalysis - - // Analyze firewall/gateway data only when firewall-audit-logs was downloaded. - // Skip silently when the artifact was intentionally excluded from the filter to - // avoid spurious "not found" warnings in verbose mode. - hasFirewallArtifact := artifactMatchesFilter(constants.FirewallAuditArtifactName, artifactFilter) - - // Analyze firewall logs if available - var firewallAnalysis *FirewallAnalysis - if hasFirewallArtifact { - var firewallErr error - firewallAnalysis, firewallErr = analyzeFirewallLogs(runOutputDir, verbose) - if firewallErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze firewall logs for run %d: %v", run.DatabaseID, firewallErr))) - } - } - } - result.FirewallAnalysis = firewallAnalysis - - // Analyze redacted domains if available - redactedDomainsAnalysis, redactedErr := analyzeRedactedDomains(runOutputDir, verbose) - if redactedErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze redacted domains for run %d: %v", run.DatabaseID, redactedErr))) - } - } - result.RedactedDomainsAnalysis = redactedDomainsAnalysis - - // Extract missing tools if available - missingTools, missingErr := extractMissingToolsFromRun(runOutputDir, run, verbose) - if missingErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract missing tools for run %d: %v", run.DatabaseID, missingErr))) - } - } - result.MissingTools = missingTools - - // Extract missing data if available - missingData, missingDataErr := extractMissingDataFromRun(runOutputDir, run, verbose) - if missingDataErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract missing data for run %d: %v", run.DatabaseID, missingDataErr))) - } - } - result.MissingData = missingData - - // Extract noops if available - noops, noopErr := extractNoopsFromRun(runOutputDir, run, verbose) - if noopErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract noops for run %d: %v", run.DatabaseID, noopErr))) - } - } - result.Noops = noops - - // Extract MCP failures if available - mcpFailures, mcpErr := extractMCPFailuresFromRun(runOutputDir, run, verbose) - if mcpErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract MCP failures for run %d: %v", run.DatabaseID, mcpErr))) - } - } - result.MCPFailures = mcpFailures - - // Extract MCP tool usage data from gateway logs if available. - // Gated on hasFirewallArtifact since gateway.jsonl lives in firewall-audit-logs. - var mcpToolUsage *MCPToolUsageData - if hasFirewallArtifact { - var mcpToolErr error - mcpToolUsage, mcpToolErr = extractMCPToolUsageData(runOutputDir, verbose) - if mcpToolErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract MCP tool usage for run %d: %v", run.DatabaseID, mcpToolErr))) - } - } - } - result.MCPToolUsage = mcpToolUsage - - // Analyze token usage from firewall proxy logs. - // Gated on hasFirewallArtifact since token-usage.jsonl lives in firewall-audit-logs. - var tokenUsage *TokenUsageSummary - if hasFirewallArtifact { - var tokenErr error - tokenUsage, tokenErr = analyzeTokenUsage(runOutputDir, verbose) - if tokenErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze token usage for run %d: %v", run.DatabaseID, tokenErr))) - } - } - } - result.TokenUsage = tokenUsage - - // Propagate effective tokens from the firewall proxy summary when available - if tokenUsage != nil && tokenUsage.TotalEffectiveTokens > 0 { - result.Run.EffectiveTokens = tokenUsage.TotalEffectiveTokens - } - - // Analyze GitHub API rate limit consumption from github_rate_limits.jsonl - rateLimitUsage, rlErr := analyzeGitHubRateLimits(runOutputDir, verbose) - if rlErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze GitHub rate limit usage for run %d: %v", run.DatabaseID, rlErr))) - } - } - result.GitHubRateLimitUsage = rateLimitUsage - - // Count safe output items created in GitHub (from manifest artifact) - result.Run.SafeItemsCount = len(extractCreatedItemsFromManifest(runOutputDir)) - - // Fetch job details for the summary - jobDetails, jobErr := fetchJobDetails(run.DatabaseID, verbose) - if jobErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch job details for run %d: %v", run.DatabaseID, jobErr))) - } - } - - // List all artifacts - artifacts, listErr := listArtifacts(runOutputDir) - if listErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to list artifacts for run %d: %v", run.DatabaseID, listErr))) - } - } - - processedRun := ProcessedRun{ - Run: result.Run, - AccessAnalysis: accessAnalysis, - FirewallAnalysis: firewallAnalysis, - RedactedDomainsAnalysis: redactedDomainsAnalysis, - MissingTools: missingTools, - MissingData: missingData, - Noops: noops, - MCPFailures: mcpFailures, - MCPToolUsage: mcpToolUsage, - TokenUsage: tokenUsage, - GitHubRateLimitUsage: rateLimitUsage, - JobDetails: jobDetails, - } - awContext, _, _, taskDomain, behaviorFingerprint, agenticAssessments := deriveRunAgenticAnalysis(processedRun, metrics) - result.AwContext = awContext - result.TaskDomain = taskDomain - result.BehaviorFingerprint = behaviorFingerprint - result.AgenticAssessments = agenticAssessments - - // Create and save run summary - summary := &RunSummary{ - CLIVersion: GetVersion(), - RunID: run.DatabaseID, - ProcessedAt: time.Now(), - Run: result.Run, - Metrics: metrics, - AwContext: result.AwContext, - TaskDomain: result.TaskDomain, - BehaviorFingerprint: result.BehaviorFingerprint, - AgenticAssessments: result.AgenticAssessments, - AccessAnalysis: accessAnalysis, - FirewallAnalysis: firewallAnalysis, - RedactedDomainsAnalysis: redactedDomainsAnalysis, - MissingTools: missingTools, - MissingData: missingData, - Noops: noops, - MCPFailures: mcpFailures, - MCPToolUsage: mcpToolUsage, - TokenUsage: tokenUsage, - GitHubRateLimitUsage: rateLimitUsage, - ArtifactsList: artifacts, - JobDetails: jobDetails, - } - - if saveErr := saveRunSummary(runOutputDir, summary, verbose); saveErr != nil { - if verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to save run summary for run %d: %v", run.DatabaseID, saveErr))) - } - } - } - - // Update progress counter for completed downloads - completed := atomic.AddInt64(&completedCount, 1) - if progressBar != nil { - fmt.Fprintf(os.Stderr, "Processing runs: %s\r", progressBar.Update(completed)) - } - - return result, nil - }) - } - - // Wait blocks until all downloads complete, context is cancelled, or panic occurs. - // With context support, the pool guarantees: - // - All goroutines finish gracefully on cancellation (no leaks) - // - Panics are propagated with stack traces - // - Partial results are returned when context is cancelled - // - Results are collected in submission order - results, err := p.Wait() - - // Handle context cancellation - if err != nil && verbose { - fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Download interrupted: %v", err))) - } - - // Clear progress bar silently - detailed summary shown at the end - if progressBar != nil { - console.ClearLine() // Clear the line - } - - if verbose { - successCount := 0 - for _, result := range results { - if result.Error == nil && !result.Skipped { - successCount++ - } - } - fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Completed parallel processing: %d successful, %d total", successCount, len(results)))) - } - - return results -} - -// runContainsSafeOutputType checks if a run's agent_output.json contains a specific safe output type -func runContainsSafeOutputType(runDir string, safeOutputType string, verbose bool) (bool, error) { - logsOrchestratorLog.Printf("Checking run for safe output type: dir=%s, type=%s", runDir, safeOutputType) - // Normalize the type for comparison (convert dashes to underscores) - normalizedType := stringutil.NormalizeSafeOutputIdentifier(safeOutputType) - - // Look for agent_output.json in the run directory - agentOutputPath := filepath.Join(runDir, constants.AgentOutputFilename) - - // Support both new flattened form and old directory form - if stat, err := os.Stat(agentOutputPath); err != nil || stat.IsDir() { - // Try old structure - oldPath := filepath.Join(runDir, constants.AgentOutputArtifactName, constants.AgentOutputArtifactName) - if _, err := os.Stat(oldPath); err == nil { - agentOutputPath = oldPath - } else { - // No agent_output.json found - return false, nil - } - } - - // Read the file - content, err := os.ReadFile(agentOutputPath) - if err != nil { - // File doesn't exist or can't be read - return false, nil - } - - // Parse the JSON - var safeOutput struct { - Items []json.RawMessage `json:"items"` - } - - if err := json.Unmarshal(content, &safeOutput); err != nil { - return false, fmt.Errorf("failed to parse agent_output.json: %w", err) - } - - // Check each item for the specified type - for _, itemRaw := range safeOutput.Items { - var item struct { - Type string `json:"type"` - } - - if err := json.Unmarshal(itemRaw, &item); err != nil { - continue // Skip malformed items - } - - // Normalize the item type for comparison - normalizedItemType := stringutil.NormalizeSafeOutputIdentifier(item.Type) - - if normalizedItemType == normalizedType { - return true, nil - } - } - - return false, nil -} - -// runHasDifcFilteredItems checks if a run's gateway logs contain any DIFC_FILTERED events. -// It parses the gateway logs (falling back to rpc-messages.jsonl when gateway.jsonl is absent) -// and returns true when at least one DIFC integrity- or secrecy-filtered event is present. -func runHasDifcFilteredItems(runDir string, verbose bool) (bool, error) { - logsOrchestratorLog.Printf("Checking run for DIFC filtered items: dir=%s", runDir) - - gatewayMetrics, err := parseGatewayLogs(runDir, verbose) - if err != nil { - // No gateway log file present — not an error for workflows without MCP - return false, nil - } - - if gatewayMetrics == nil { - return false, nil - } - - return gatewayMetrics.TotalFiltered > 0, nil -} diff --git a/pkg/cli/logs_orchestrator_concurrency.go b/pkg/cli/logs_orchestrator_concurrency.go new file mode 100644 index 00000000000..c0dfb703613 --- /dev/null +++ b/pkg/cli/logs_orchestrator_concurrency.go @@ -0,0 +1,494 @@ +//go:build !integration + +// This file contains the concurrent artifact download logic and safe-output/DIFC filter helpers +// extracted from logs_orchestrator.go to keep that file under 1,000 lines. + +package cli + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "math" + "os" + "path/filepath" + "strings" + "sync/atomic" + "time" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/constants" + "github.com/github/gh-aw/pkg/stringutil" + "github.com/sourcegraph/conc/pool" +) + +// downloadRunArtifactsConcurrent downloads artifacts for multiple workflow runs concurrently +func downloadRunArtifactsConcurrent(ctx context.Context, runs []WorkflowRun, outputDir string, verbose bool, maxRuns int, repoOverride string, artifactFilter []string) []DownloadResult { + logsOrchestratorLog.Printf("Starting concurrent artifact download: runs=%d, outputDir=%s, maxRuns=%d", len(runs), outputDir, maxRuns) + if len(runs) == 0 { + return []DownloadResult{} + } + + // Process all runs in the batch to account for caching and filtering + // The maxRuns parameter indicates how many successful results we need, but we may need to + // process more runs to account for: + // 1. Cached runs that may fail filters (engine, firewall, etc.) + // 2. Runs that may be skipped due to errors + // 3. Runs without artifacts + // + // By processing all runs in the batch, we ensure that the count parameter correctly + // reflects the number of matching logs (both downloaded and cached), not just attempts. + actualRuns := runs + + totalRuns := len(actualRuns) + + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Processing %d runs in parallel...", totalRuns))) + } + + // Create progress bar for tracking run processing (only in non-verbose, non-CI mode) + // In CI environments \r is treated as a newline, producing excessive output for each update. + var progressBar *console.ProgressBar + if !verbose && !IsRunningInCI() { + progressBar = console.NewProgressBar(int64(totalRuns)) + fmt.Fprintf(os.Stderr, "Processing runs: %s\r", progressBar.Update(0)) + } + + // Use atomic counter for thread-safe progress tracking + var completedCount int64 + + // Get configured max concurrent downloads (default or from environment variable) + maxConcurrent := getMaxConcurrentDownloads() + + // Parse repoOverride into owner/repo once for cross-repo artifact download + var dlOwner, dlRepo string + if repoOverride != "" { + parts := strings.SplitN(repoOverride, "/", 2) + if len(parts) == 2 { + dlOwner = parts[0] + dlRepo = parts[1] + } + } + + // Configure concurrent download pool with bounded parallelism and context cancellation. + // The conc pool automatically handles panic recovery and prevents goroutine leaks. + // WithContext enables graceful cancellation via Ctrl+C. + p := pool.NewWithResults[DownloadResult](). + WithContext(ctx). + WithMaxGoroutines(maxConcurrent) + + // Each download task runs concurrently with context awareness. + // Context cancellation (e.g., via Ctrl+C) will stop all in-flight downloads gracefully. + // Panics are automatically recovered by the pool and re-raised with full stack traces + // after all tasks complete. This ensures one failing download doesn't break others. + for _, run := range actualRuns { + p.Go(func(ctx context.Context) (DownloadResult, error) { + // Check for context cancellation before starting download + select { + case <-ctx.Done(): + return DownloadResult{ + Run: run, + Skipped: true, + Error: ctx.Err(), + }, nil + default: + } + if verbose { + fmt.Fprintln(os.Stderr, console.FormatInfoMessage(fmt.Sprintf("Processing run %d (%s)...", run.DatabaseID, run.Status))) + } + + // Download artifacts and logs for this run + runOutputDir := filepath.Join(outputDir, fmt.Sprintf("run-%d", run.DatabaseID)) + + // Try to load cached summary first + if summary, ok := loadRunSummary(runOutputDir, verbose); ok { + // Valid cached summary exists, use it directly + result := DownloadResult{ + Run: summary.Run, + Metrics: summary.Metrics, + AwContext: summary.AwContext, + TaskDomain: summary.TaskDomain, + BehaviorFingerprint: summary.BehaviorFingerprint, + AgenticAssessments: summary.AgenticAssessments, + AccessAnalysis: summary.AccessAnalysis, + FirewallAnalysis: summary.FirewallAnalysis, + RedactedDomainsAnalysis: summary.RedactedDomainsAnalysis, + MissingTools: summary.MissingTools, + MissingData: summary.MissingData, + Noops: summary.Noops, + MCPFailures: summary.MCPFailures, + MCPToolUsage: summary.MCPToolUsage, + TokenUsage: summary.TokenUsage, + GitHubRateLimitUsage: summary.GitHubRateLimitUsage, + JobDetails: summary.JobDetails, + LogsPath: runOutputDir, + Cached: true, // Mark as cached + } + // Update progress counter + completed := atomic.AddInt64(&completedCount, 1) + if progressBar != nil { + fmt.Fprintf(os.Stderr, "Processing runs: %s\r", progressBar.Update(completed)) + } + return result, nil + } + + // No cached summary or version mismatch - download and process + err := downloadRunArtifacts(run.DatabaseID, runOutputDir, verbose, dlOwner, dlRepo, "", artifactFilter) + + result := DownloadResult{ + Run: run, + LogsPath: runOutputDir, + } + + if err != nil { + // Check if this is a "no artifacts" case + if errors.Is(err, ErrNoArtifacts) { + // For runs with important conclusions (timed_out, failure, cancelled), + // still process them even without artifacts to show the failure in reports + if isFailureConclusion(run.Conclusion) { + // Don't skip - we want these to appear in the report + // Just use empty metrics + result.Metrics = LogMetrics{} + + // Try to fetch job details to get error count + if failedJobCount, jobErr := fetchJobStatuses(run.DatabaseID, verbose); jobErr == nil { + run.ErrorCount = failedJobCount + } + } else { + // For other runs (success, neutral, etc.) without artifacts, skip them + result.Skipped = true + result.Error = err + } + } else { + result.Error = err + } + } else { + // Extract metrics from logs + metrics, metricsErr := extractLogMetrics(runOutputDir, verbose) + if metricsErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract metrics for run %d: %v", run.DatabaseID, metricsErr))) + } + // Don't fail the whole download for metrics errors + metrics = LogMetrics{} + } + result.Metrics = metrics + + // Update run with metrics so fingerprint computation uses the same data + // as the audit tool, which also derives these fields from extracted log metrics. + result.Run.TokenUsage = metrics.TokenUsage + result.Run.EstimatedCost = metrics.EstimatedCost + result.Run.Turns = metrics.Turns + result.Run.LogsPath = runOutputDir + + // Calculate duration and billable minutes from GitHub API timestamps. + // This mirrors the identical computation in audit.go so that + // processedRun.Run.Duration is consistent across both tools. + if !result.Run.StartedAt.IsZero() && !result.Run.UpdatedAt.IsZero() { + result.Run.Duration = result.Run.UpdatedAt.Sub(result.Run.StartedAt) + result.Run.ActionMinutes = math.Ceil(result.Run.Duration.Minutes()) + } + + // Analyze access logs if available + accessAnalysis, accessErr := analyzeAccessLogs(runOutputDir, verbose) + if accessErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze access logs for run %d: %v", run.DatabaseID, accessErr))) + } + } + result.AccessAnalysis = accessAnalysis + + // Analyze firewall/gateway data only when firewall-audit-logs was downloaded. + // Skip silently when the artifact was intentionally excluded from the filter to + // avoid spurious "not found" warnings in verbose mode. + hasFirewallArtifact := artifactMatchesFilter(constants.FirewallAuditArtifactName, artifactFilter) + + // Analyze firewall logs if available + var firewallAnalysis *FirewallAnalysis + if hasFirewallArtifact { + var firewallErr error + firewallAnalysis, firewallErr = analyzeFirewallLogs(runOutputDir, verbose) + if firewallErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze firewall logs for run %d: %v", run.DatabaseID, firewallErr))) + } + } + } + result.FirewallAnalysis = firewallAnalysis + + // Analyze redacted domains if available + redactedDomainsAnalysis, redactedErr := analyzeRedactedDomains(runOutputDir, verbose) + if redactedErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze redacted domains for run %d: %v", run.DatabaseID, redactedErr))) + } + } + result.RedactedDomainsAnalysis = redactedDomainsAnalysis + + // Extract missing tools if available + missingTools, missingErr := extractMissingToolsFromRun(runOutputDir, run, verbose) + if missingErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract missing tools for run %d: %v", run.DatabaseID, missingErr))) + } + } + result.MissingTools = missingTools + + // Extract missing data if available + missingData, missingDataErr := extractMissingDataFromRun(runOutputDir, run, verbose) + if missingDataErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract missing data for run %d: %v", run.DatabaseID, missingDataErr))) + } + } + result.MissingData = missingData + + // Extract noops if available + noops, noopErr := extractNoopsFromRun(runOutputDir, run, verbose) + if noopErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract noops for run %d: %v", run.DatabaseID, noopErr))) + } + } + result.Noops = noops + + // Extract MCP failures if available + mcpFailures, mcpErr := extractMCPFailuresFromRun(runOutputDir, run, verbose) + if mcpErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract MCP failures for run %d: %v", run.DatabaseID, mcpErr))) + } + } + result.MCPFailures = mcpFailures + + // Extract MCP tool usage data from gateway logs if available. + // Gated on hasFirewallArtifact since gateway.jsonl lives in firewall-audit-logs. + var mcpToolUsage *MCPToolUsageData + if hasFirewallArtifact { + var mcpToolErr error + mcpToolUsage, mcpToolErr = extractMCPToolUsageData(runOutputDir, verbose) + if mcpToolErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to extract MCP tool usage for run %d: %v", run.DatabaseID, mcpToolErr))) + } + } + } + result.MCPToolUsage = mcpToolUsage + + // Analyze token usage from firewall proxy logs. + // Gated on hasFirewallArtifact since token-usage.jsonl lives in firewall-audit-logs. + var tokenUsage *TokenUsageSummary + if hasFirewallArtifact { + var tokenErr error + tokenUsage, tokenErr = analyzeTokenUsage(runOutputDir, verbose) + if tokenErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze token usage for run %d: %v", run.DatabaseID, tokenErr))) + } + } + } + result.TokenUsage = tokenUsage + + // Propagate effective tokens from the firewall proxy summary when available + if tokenUsage != nil && tokenUsage.TotalEffectiveTokens > 0 { + result.Run.EffectiveTokens = tokenUsage.TotalEffectiveTokens + } + + // Analyze GitHub API rate limit consumption from github_rate_limits.jsonl + rateLimitUsage, rlErr := analyzeGitHubRateLimits(runOutputDir, verbose) + if rlErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to analyze GitHub rate limit usage for run %d: %v", run.DatabaseID, rlErr))) + } + } + result.GitHubRateLimitUsage = rateLimitUsage + + // Count safe output items created in GitHub (from manifest artifact) + result.Run.SafeItemsCount = len(extractCreatedItemsFromManifest(runOutputDir)) + + // Fetch job details for the summary + jobDetails, jobErr := fetchJobDetails(run.DatabaseID, verbose) + if jobErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to fetch job details for run %d: %v", run.DatabaseID, jobErr))) + } + } + + // List all artifacts + artifacts, listErr := listArtifacts(runOutputDir) + if listErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to list artifacts for run %d: %v", run.DatabaseID, listErr))) + } + } + + processedRun := ProcessedRun{ + Run: result.Run, + AccessAnalysis: accessAnalysis, + FirewallAnalysis: firewallAnalysis, + RedactedDomainsAnalysis: redactedDomainsAnalysis, + MissingTools: missingTools, + MissingData: missingData, + Noops: noops, + MCPFailures: mcpFailures, + MCPToolUsage: mcpToolUsage, + TokenUsage: tokenUsage, + GitHubRateLimitUsage: rateLimitUsage, + JobDetails: jobDetails, + } + awContext, _, _, taskDomain, behaviorFingerprint, agenticAssessments := deriveRunAgenticAnalysis(processedRun, metrics) + result.AwContext = awContext + result.TaskDomain = taskDomain + result.BehaviorFingerprint = behaviorFingerprint + result.AgenticAssessments = agenticAssessments + + // Create and save run summary + summary := &RunSummary{ + CLIVersion: GetVersion(), + RunID: run.DatabaseID, + ProcessedAt: time.Now(), + Run: result.Run, + Metrics: metrics, + AwContext: result.AwContext, + TaskDomain: result.TaskDomain, + BehaviorFingerprint: result.BehaviorFingerprint, + AgenticAssessments: result.AgenticAssessments, + AccessAnalysis: accessAnalysis, + FirewallAnalysis: firewallAnalysis, + RedactedDomainsAnalysis: redactedDomainsAnalysis, + MissingTools: missingTools, + MissingData: missingData, + Noops: noops, + MCPFailures: mcpFailures, + MCPToolUsage: mcpToolUsage, + TokenUsage: tokenUsage, + GitHubRateLimitUsage: rateLimitUsage, + ArtifactsList: artifacts, + JobDetails: jobDetails, + } + + if saveErr := saveRunSummary(runOutputDir, summary, verbose); saveErr != nil { + if verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Failed to save run summary for run %d: %v", run.DatabaseID, saveErr))) + } + } + } + + // Update progress counter for completed downloads + completed := atomic.AddInt64(&completedCount, 1) + if progressBar != nil { + fmt.Fprintf(os.Stderr, "Processing runs: %s\r", progressBar.Update(completed)) + } + + return result, nil + }) + } + + // Wait blocks until all downloads complete, context is cancelled, or panic occurs. + // With context support, the pool guarantees: + // - All goroutines finish gracefully on cancellation (no leaks) + // - Panics are propagated with stack traces + // - Partial results are returned when context is cancelled + // - Results are collected in submission order + results, err := p.Wait() + + // Handle context cancellation + if err != nil && verbose { + fmt.Fprintln(os.Stderr, console.FormatWarningMessage(fmt.Sprintf("Download interrupted: %v", err))) + } + + // Clear progress bar silently - detailed summary shown at the end + if progressBar != nil { + console.ClearLine() // Clear the line + } + + if verbose { + successCount := 0 + for _, result := range results { + if result.Error == nil && !result.Skipped { + successCount++ + } + } + fmt.Fprintln(os.Stderr, console.FormatSuccessMessage(fmt.Sprintf("Completed parallel processing: %d successful, %d total", successCount, len(results)))) + } + + return results +} + +// runContainsSafeOutputType checks if a run's agent_output.json contains a specific safe output type +func runContainsSafeOutputType(runDir string, safeOutputType string, verbose bool) (bool, error) { + logsOrchestratorLog.Printf("Checking run for safe output type: dir=%s, type=%s", runDir, safeOutputType) + // Normalize the type for comparison (convert dashes to underscores) + normalizedType := stringutil.NormalizeSafeOutputIdentifier(safeOutputType) + + // Look for agent_output.json in the run directory + agentOutputPath := filepath.Join(runDir, constants.AgentOutputFilename) + + // Support both new flattened form and old directory form + if stat, err := os.Stat(agentOutputPath); err != nil || stat.IsDir() { + // Try old structure + oldPath := filepath.Join(runDir, constants.AgentOutputArtifactName, constants.AgentOutputArtifactName) + if _, err := os.Stat(oldPath); err == nil { + agentOutputPath = oldPath + } else { + // No agent_output.json found + return false, nil + } + } + + // Read the file + content, err := os.ReadFile(agentOutputPath) + if err != nil { + // File doesn't exist or can't be read + return false, nil + } + + // Parse the JSON + var safeOutput struct { + Items []json.RawMessage `json:"items"` + } + + if err := json.Unmarshal(content, &safeOutput); err != nil { + return false, fmt.Errorf("failed to parse agent_output.json: %w", err) + } + + // Check each item for the specified type + for _, itemRaw := range safeOutput.Items { + var item struct { + Type string `json:"type"` + } + + if err := json.Unmarshal(itemRaw, &item); err != nil { + continue // Skip malformed items + } + + // Normalize the item type for comparison + normalizedItemType := stringutil.NormalizeSafeOutputIdentifier(item.Type) + + if normalizedItemType == normalizedType { + return true, nil + } + } + + return false, nil +} + +// runHasDifcFilteredItems checks if a run's gateway logs contain any DIFC_FILTERED events. +// It parses the gateway logs (falling back to rpc-messages.jsonl when gateway.jsonl is absent) +// and returns true when at least one DIFC integrity- or secrecy-filtered event is present. +func runHasDifcFilteredItems(runDir string, verbose bool) (bool, error) { + logsOrchestratorLog.Printf("Checking run for DIFC filtered items: dir=%s", runDir) + + gatewayMetrics, err := parseGatewayLogs(runDir, verbose) + if err != nil { + // No gateway log file present — not an error for workflows without MCP + return false, nil + } + + if gatewayMetrics == nil { + return false, nil + } + + return gatewayMetrics.TotalFiltered > 0, nil +} diff --git a/pkg/cli/logs_report.go b/pkg/cli/logs_report.go index ebf5084554f..822a6490dbb 100644 --- a/pkg/cli/logs_report.go +++ b/pkg/cli/logs_report.go @@ -665,175 +665,6 @@ func buildMCPFailuresSummary(processedRuns []ProcessedRun) []MCPFailureSummary { return result } -// domainAggregation holds the result of aggregating domain statistics -type domainAggregation struct { - allAllowedDomains map[string]bool - allBlockedDomains map[string]bool - totalRequests int - allowedCount int - blockedCount int -} - -// aggregateDomainStats aggregates domain statistics across runs -// This is a shared helper for both access log and firewall log summaries -func aggregateDomainStats(processedRuns []ProcessedRun, getAnalysis func(*ProcessedRun) (allowedDomains, blockedDomains []string, totalRequests, allowedCount, blockedCount int, exists bool)) *domainAggregation { - agg := &domainAggregation{ - allAllowedDomains: make(map[string]bool), - allBlockedDomains: make(map[string]bool), - } - - for _, pr := range processedRuns { - allowedDomains, blockedDomains, totalRequests, allowedCount, blockedCount, exists := getAnalysis(&pr) - if !exists { - continue - } - - agg.totalRequests += totalRequests - agg.allowedCount += allowedCount - agg.blockedCount += blockedCount - - for _, domain := range allowedDomains { - agg.allAllowedDomains[domain] = true - } - for _, domain := range blockedDomains { - agg.allBlockedDomains[domain] = true - } - } - - return agg -} - -// convertDomainsToSortedSlices converts domain maps to sorted slices -func convertDomainsToSortedSlices(allowedMap, blockedMap map[string]bool) (allowed, blocked []string) { - for domain := range allowedMap { - allowed = append(allowed, domain) - } - slices.Sort(allowed) - - for domain := range blockedMap { - blocked = append(blocked, domain) - } - slices.Sort(blocked) - - return allowed, blocked -} - -// buildAccessLogSummary aggregates access log data across all runs -func buildAccessLogSummary(processedRuns []ProcessedRun) *AccessLogSummary { - byWorkflow := make(map[string]*DomainAnalysis) - - // Use shared aggregation helper - agg := aggregateDomainStats(processedRuns, func(pr *ProcessedRun) ([]string, []string, int, int, int, bool) { - if pr.AccessAnalysis == nil { - return nil, nil, 0, 0, 0, false - } - byWorkflow[pr.Run.WorkflowName] = pr.AccessAnalysis - return pr.AccessAnalysis.AllowedDomains, - pr.AccessAnalysis.BlockedDomains, - pr.AccessAnalysis.TotalRequests, - pr.AccessAnalysis.AllowedCount, - pr.AccessAnalysis.BlockedCount, - true - }) - - if agg.totalRequests == 0 { - return nil - } - - allowedDomains, blockedDomains := convertDomainsToSortedSlices(agg.allAllowedDomains, agg.allBlockedDomains) - - return &AccessLogSummary{ - TotalRequests: agg.totalRequests, - AllowedCount: agg.allowedCount, - BlockedCount: agg.blockedCount, - AllowedDomains: allowedDomains, - BlockedDomains: blockedDomains, - ByWorkflow: byWorkflow, - } -} - -// buildFirewallLogSummary aggregates firewall log data across all runs -func buildFirewallLogSummary(processedRuns []ProcessedRun) *FirewallLogSummary { - allRequestsByDomain := make(map[string]DomainRequestStats) - byWorkflow := make(map[string]*FirewallAnalysis) - - // Use shared aggregation helper - agg := aggregateDomainStats(processedRuns, func(pr *ProcessedRun) ([]string, []string, int, int, int, bool) { - if pr.FirewallAnalysis == nil { - return nil, nil, 0, 0, 0, false - } - byWorkflow[pr.Run.WorkflowName] = pr.FirewallAnalysis - - // Aggregate request stats by domain (firewall-specific) - for domain, stats := range pr.FirewallAnalysis.RequestsByDomain { - existing := allRequestsByDomain[domain] - existing.Allowed += stats.Allowed - existing.Blocked += stats.Blocked - allRequestsByDomain[domain] = existing - } - - return pr.FirewallAnalysis.AllowedDomains, - pr.FirewallAnalysis.BlockedDomains, - pr.FirewallAnalysis.TotalRequests, - pr.FirewallAnalysis.AllowedRequests, - pr.FirewallAnalysis.BlockedRequests, - true - }) - - if agg.totalRequests == 0 { - return nil - } - - allowedDomains, blockedDomains := convertDomainsToSortedSlices(agg.allAllowedDomains, agg.allBlockedDomains) - - return &FirewallLogSummary{ - TotalRequests: agg.totalRequests, - AllowedRequests: agg.allowedCount, - BlockedRequests: agg.blockedCount, - AllowedDomains: allowedDomains, - BlockedDomains: blockedDomains, - RequestsByDomain: allRequestsByDomain, - ByWorkflow: byWorkflow, - } -} - -// buildRedactedDomainsSummary aggregates redacted domains data across all runs -func buildRedactedDomainsSummary(processedRuns []ProcessedRun) *RedactedDomainsLogSummary { - allDomainsSet := make(map[string]bool) - byWorkflow := make(map[string]*RedactedDomainsAnalysis) - hasData := false - - for _, pr := range processedRuns { - if pr.RedactedDomainsAnalysis == nil { - continue - } - hasData = true - byWorkflow[pr.Run.WorkflowName] = pr.RedactedDomainsAnalysis - - // Collect all unique domains - for _, domain := range pr.RedactedDomainsAnalysis.Domains { - allDomainsSet[domain] = true - } - } - - if !hasData { - return nil - } - - // Convert set to sorted slice - var allDomains []string - for domain := range allDomainsSet { - allDomains = append(allDomains, domain) - } - slices.Sort(allDomains) - - return &RedactedDomainsLogSummary{ - TotalDomains: len(allDomains), - Domains: allDomains, - ByWorkflow: byWorkflow, - } -} - // buildMCPToolUsageSummary aggregates MCP tool usage data across all runs func buildMCPToolUsageSummary(processedRuns []ProcessedRun) *MCPToolUsageSummary { reportLog.Printf("Building MCP tool usage summary from %d processed runs", len(processedRuns)) diff --git a/pkg/cli/logs_report_network.go b/pkg/cli/logs_report_network.go new file mode 100644 index 00000000000..ec9be546b2f --- /dev/null +++ b/pkg/cli/logs_report_network.go @@ -0,0 +1,177 @@ +//go:build !integration + +// This file contains network/domain analysis functions extracted from logs_report.go +// to keep that file under 1,000 lines. + +package cli + +import "slices" + +// domainAggregation holds the result of aggregating domain statistics +type domainAggregation struct { + allAllowedDomains map[string]bool + allBlockedDomains map[string]bool + totalRequests int + allowedCount int + blockedCount int +} + +// aggregateDomainStats aggregates domain statistics across runs +// This is a shared helper for both access log and firewall log summaries +func aggregateDomainStats(processedRuns []ProcessedRun, getAnalysis func(*ProcessedRun) (allowedDomains, blockedDomains []string, totalRequests, allowedCount, blockedCount int, exists bool)) *domainAggregation { + agg := &domainAggregation{ + allAllowedDomains: make(map[string]bool), + allBlockedDomains: make(map[string]bool), + } + + for _, pr := range processedRuns { + allowedDomains, blockedDomains, totalRequests, allowedCount, blockedCount, exists := getAnalysis(&pr) + if !exists { + continue + } + + agg.totalRequests += totalRequests + agg.allowedCount += allowedCount + agg.blockedCount += blockedCount + + for _, domain := range allowedDomains { + agg.allAllowedDomains[domain] = true + } + for _, domain := range blockedDomains { + agg.allBlockedDomains[domain] = true + } + } + + return agg +} + +// convertDomainsToSortedSlices converts domain maps to sorted slices +func convertDomainsToSortedSlices(allowedMap, blockedMap map[string]bool) (allowed, blocked []string) { + for domain := range allowedMap { + allowed = append(allowed, domain) + } + slices.Sort(allowed) + + for domain := range blockedMap { + blocked = append(blocked, domain) + } + slices.Sort(blocked) + + return allowed, blocked +} + +// buildAccessLogSummary aggregates access log data across all runs +func buildAccessLogSummary(processedRuns []ProcessedRun) *AccessLogSummary { + byWorkflow := make(map[string]*DomainAnalysis) + + // Use shared aggregation helper + agg := aggregateDomainStats(processedRuns, func(pr *ProcessedRun) ([]string, []string, int, int, int, bool) { + if pr.AccessAnalysis == nil { + return nil, nil, 0, 0, 0, false + } + byWorkflow[pr.Run.WorkflowName] = pr.AccessAnalysis + return pr.AccessAnalysis.AllowedDomains, + pr.AccessAnalysis.BlockedDomains, + pr.AccessAnalysis.TotalRequests, + pr.AccessAnalysis.AllowedCount, + pr.AccessAnalysis.BlockedCount, + true + }) + + if agg.totalRequests == 0 { + return nil + } + + allowedDomains, blockedDomains := convertDomainsToSortedSlices(agg.allAllowedDomains, agg.allBlockedDomains) + + return &AccessLogSummary{ + TotalRequests: agg.totalRequests, + AllowedCount: agg.allowedCount, + BlockedCount: agg.blockedCount, + AllowedDomains: allowedDomains, + BlockedDomains: blockedDomains, + ByWorkflow: byWorkflow, + } +} + +// buildFirewallLogSummary aggregates firewall log data across all runs +func buildFirewallLogSummary(processedRuns []ProcessedRun) *FirewallLogSummary { + allRequestsByDomain := make(map[string]DomainRequestStats) + byWorkflow := make(map[string]*FirewallAnalysis) + + // Use shared aggregation helper + agg := aggregateDomainStats(processedRuns, func(pr *ProcessedRun) ([]string, []string, int, int, int, bool) { + if pr.FirewallAnalysis == nil { + return nil, nil, 0, 0, 0, false + } + byWorkflow[pr.Run.WorkflowName] = pr.FirewallAnalysis + + // Aggregate request stats by domain (firewall-specific) + for domain, stats := range pr.FirewallAnalysis.RequestsByDomain { + existing := allRequestsByDomain[domain] + existing.Allowed += stats.Allowed + existing.Blocked += stats.Blocked + allRequestsByDomain[domain] = existing + } + + return pr.FirewallAnalysis.AllowedDomains, + pr.FirewallAnalysis.BlockedDomains, + pr.FirewallAnalysis.TotalRequests, + pr.FirewallAnalysis.AllowedRequests, + pr.FirewallAnalysis.BlockedRequests, + true + }) + + if agg.totalRequests == 0 { + return nil + } + + allowedDomains, blockedDomains := convertDomainsToSortedSlices(agg.allAllowedDomains, agg.allBlockedDomains) + + return &FirewallLogSummary{ + TotalRequests: agg.totalRequests, + AllowedRequests: agg.allowedCount, + BlockedRequests: agg.blockedCount, + AllowedDomains: allowedDomains, + BlockedDomains: blockedDomains, + RequestsByDomain: allRequestsByDomain, + ByWorkflow: byWorkflow, + } +} + +// buildRedactedDomainsSummary aggregates redacted domains data across all runs +func buildRedactedDomainsSummary(processedRuns []ProcessedRun) *RedactedDomainsLogSummary { + allDomainsSet := make(map[string]bool) + byWorkflow := make(map[string]*RedactedDomainsAnalysis) + hasData := false + + for _, pr := range processedRuns { + if pr.RedactedDomainsAnalysis == nil { + continue + } + hasData = true + byWorkflow[pr.Run.WorkflowName] = pr.RedactedDomainsAnalysis + + // Collect all unique domains + for _, domain := range pr.RedactedDomainsAnalysis.Domains { + allDomainsSet[domain] = true + } + } + + if !hasData { + return nil + } + + // Convert set to sorted slice + var allDomains []string + for domain := range allDomainsSet { + allDomains = append(allDomains, domain) + } + slices.Sort(allDomains) + + return &RedactedDomainsLogSummary{ + TotalDomains: len(allDomains), + Domains: allDomains, + ByWorkflow: byWorkflow, + } +} From d5dd74eea838a226e8707c966152d2a9f29ee873 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Fri, 10 Apr 2026 16:12:46 +0000 Subject: [PATCH 7/7] docs(adr): add draft ADR-0001 for file-size enforcement and domain-based decomposition Generated by the Design Decision Gate workflow to document the architectural decision to cap Go source files at 1,000 lines and split violations into domain-specific sibling files using the init()-registry pattern. Co-Authored-By: Claude Sonnet 4.6 --- ...ze-limit-via-domain-based-decomposition.md | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 docs/adr/0001-enforce-file-size-limit-via-domain-based-decomposition.md diff --git a/docs/adr/0001-enforce-file-size-limit-via-domain-based-decomposition.md b/docs/adr/0001-enforce-file-size-limit-via-domain-based-decomposition.md new file mode 100644 index 00000000000..d5aeb3d8e39 --- /dev/null +++ b/docs/adr/0001-enforce-file-size-limit-via-domain-based-decomposition.md @@ -0,0 +1,90 @@ +# ADR-0001: Enforce File Size Limit via Domain-Based Decomposition + +**Date**: 2026-04-10 +**Status**: Draft +**Deciders**: pelikhan, Copilot (SWE agent) + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +The `gh-aw` codebase uses an automated architecture linter that flags Go source files exceeding 1,000 lines as **BLOCKER** violations on the grounds that large files mix unrelated concerns, reduce discoverability, and slow code review. Six files in `pkg/cli/` and `pkg/workflow/` had grown well beyond this threshold—some to over 1,300 lines—because feature additions were accumulated in existing files rather than being distributed across new, focused ones. The project needed a clear, repeatable strategy for how to break such files apart and where each extracted function should land. + +### Decision + +We will enforce a hard 1,000-line cap on Go source files in this repository and resolve violations by splitting the offending file into multiple **domain-specific sibling files**. Each sibling file is named after its parent with a suffix that describes the concern it owns (e.g., `gateway_logs_parser.go`, `gateway_logs_renderer.go`, `gateway_logs_extractor.go`). When a large file contains a monolithic registry variable (such as `handlerRegistry`), we will initialize the registry as an empty map in the primary file and use Go's `init()` functions in domain-specific files to register entries, one domain per file. No logic changes are permitted during a decomposition commit; the split is purely structural. + +### Alternatives Considered + +#### Alternative 1: Increase or Remove the Line Limit + +Accept files longer than 1,000 lines and raise the threshold (or remove it entirely). This was rejected because the existing violations were already causing demonstrable pain: PR diffs against a 1,300-line file are hard to review, `git blame` across such files is noisy, and the underlying mixing of parsing, rendering, and extraction logic in a single file made isolated unit testing difficult. Removing the limit would institutionalize that pain. + +#### Alternative 2: Extract to Separate Packages Instead of Sibling Files + +Move extracted functions into wholly new sub-packages (e.g., `pkg/cli/gateway/parser`, `pkg/cli/gateway/renderer`). This is a cleaner long-term architecture but was rejected for this iteration because it would require updating all import paths and call sites across the codebase—a larger, riskier change that mixes structural refactoring with the goal of simply clearing BLOCKER violations. A future ADR may revisit this as the package surface stabilises. + +#### Alternative 3: Inline the Extracted Code via Interface Dispatch + +Introduce interfaces and replace the large functions with smaller implementations spread across types. This was rejected because it would constitute a logic change (not merely a structural split), complicating verification that no behaviour was altered. Keeping the refactor purely mechanical—move functions, change no logic—was a non-negotiable constraint for this PR. + +### Consequences + +#### Positive +- All six BLOCKER files are now under 1,000 lines; the architecture linter gate passes. +- Each sibling file has a single-concern scope (parsing, rendering, extraction, concurrency, etc.), making it significantly easier to locate and review related code. +- The `init()`-based registry pattern allows future handler domains to be added in their own files without touching the primary config file, reducing merge conflicts. +- Diffs are smaller and more focused per file, improving pull-request reviewability going forward. + +#### Negative +- The total file count in `pkg/cli/` and `pkg/workflow/` increases, which adds a small cognitive overhead when navigating the directory listing. +- Sibling files share the same Go package, so there is no compile-time enforcement of the "one concern per file" boundary; discipline is required to prevent concern-creep back into files over time. +- The `init()` registration pattern for registries introduces implicit ordering dependency on the Go runtime's package initialisation order; it is correct but non-obvious to developers unfamiliar with the pattern. +- JavaScript BLOCKER files (`log_parser_shared.cjs` at 1,703 lines, `create_pull_request.cjs` at 1,666 lines, and others) remain above threshold and are out of scope for this PR, creating an inconsistent enforcement state between Go and JavaScript. + +#### Neutral +- This refactor requires updating the architecture linter's allowlist (if any) to reflect the new file structure. +- Future contributors adding functionality to these packages should default to creating new domain-specific files rather than extending existing ones past the threshold. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### File Size Constraint + +1. Go source files in this repository **MUST NOT** exceed 1,000 lines (including blank lines and comments). +2. A file that exceeds 1,000 lines **MUST** be decomposed before or as part of the pull request that causes it to exceed the threshold; it **MUST NOT** be merged in a violating state. +3. JavaScript source files **SHOULD** also respect the 1,000-line limit; enforcement for JavaScript is deferred to a follow-up initiative [TODO: verify timeline]. + +### Decomposition Strategy + +1. When a Go file must be split, the primary file **MUST** retain its original name and **MUST** keep the package-level declarations (types, constants, package-scope variables, and the primary entry points) that give the file its identity. +2. Extracted functions **MUST** be moved to sibling files within the same directory and package; they **MUST NOT** be relocated to a different package unless a separate architectural decision records that refactoring. +3. Sibling files **MUST** be named using the pattern `{original_basename}_{concern}.go`, where `{concern}` is a short, lowercase, single-word or hyphenated descriptor of the responsibility the file owns (e.g., `_parser`, `_renderer`, `_extractor`, `_helpers`, `_concurrency`, `_network`). +4. A decomposition commit **MUST NOT** alter function signatures, logic, or behaviour; it **MUST** be a purely mechanical relocation of code. + +### Registry Splitting via `init()` + +1. When a large registry variable (a `map` populated with inline closures) must be split, the registry variable **MUST** be declared and initialised as an empty map in the primary file. +2. Each domain group of registry entries **MUST** be registered in a dedicated `init()` function in its own domain-specific sibling file. +3. Each domain sibling file **MUST** register only entries belonging to its declared domain; it **MUST NOT** register entries from another domain's file. +4. Registry sibling files **SHOULD** be named `{primary_file}_{domain}.go` (e.g., `compiler_safe_outputs_handlers_issues.go`). + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Specifically: + +- Every Go source file in the repository is ≤ 1,000 lines, OR a decomposition is in progress under an open pull request. +- All decompositions use sibling files in the same package with the prescribed naming pattern. +- No decomposition commit introduces logic changes. +- Registry splits declare an empty map in the primary file and use `init()` functions in domain siblings. + +Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*ADR created by [adr-writer agent](https://github.com/github/gh-aw/actions/runs/24252159284). Review and finalize before changing status from Draft to Accepted.*