From 0a1700de1deec2a686fb7e4718125cac0da4b529 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Jun 2026 03:27:42 +0000 Subject: [PATCH 1/5] Initial plan From 75c2ed5ef93b8745288f60b787a9464a3c703230 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Jun 2026 03:36:45 +0000 Subject: [PATCH 2/5] P1.1 & P1.2: Replace inline percentage calcs and truncations with helpers - Replaced 10 inline percentage calculations with safePercent() - Fixed missing zero-guard at health_metrics.go:178 - Replaced 9 inline truncations with stringutil.Truncate() - All changes build successfully Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/audit_expanded.go | 9 ++------ pkg/cli/audit_report.go | 13 ++++-------- pkg/cli/audit_report_analysis.go | 6 ++---- pkg/cli/deps_report.go | 26 ++++++----------------- pkg/cli/experiments_analyze_statistics.go | 5 +---- pkg/cli/health_metrics.go | 7 ++---- pkg/cli/mcp_inspect_mcp.go | 6 ++---- pkg/cli/mcp_list.go | 13 ++++-------- pkg/cli/mcp_registry_list.go | 6 ++---- 9 files changed, 25 insertions(+), 66 deletions(-) diff --git a/pkg/cli/audit_expanded.go b/pkg/cli/audit_expanded.go index 0fdbe388b19..5b424612557 100644 --- a/pkg/cli/audit_expanded.go +++ b/pkg/cli/audit_expanded.go @@ -485,10 +485,7 @@ func buildMCPServerHealth(mcpToolUsage *MCPToolUsageData, mcpFailures []MCPFailu health.TotalRequests += server.RequestCount health.TotalErrors += server.ErrorCount - errorRate := 0.0 - if server.RequestCount > 0 { - errorRate = float64(server.ErrorCount) / float64(server.RequestCount) * 100 - } + errorRate := safePercent(server.ErrorCount, server.RequestCount) status := "✅ healthy" if _, isFailed := failedServers[server.ServerName]; isFailed { @@ -543,9 +540,7 @@ func buildMCPServerHealth(mcpToolUsage *MCPToolUsageData, mcpFailures []MCPFailu health.HealthySvrs = health.TotalServers - health.FailedSvrs - health.DegradedSvrs // Calculate overall error rate - if health.TotalRequests > 0 { - health.ErrorRate = float64(health.TotalErrors) / float64(health.TotalRequests) * 100 - } + health.ErrorRate = safePercent(health.TotalErrors, health.TotalRequests) // Sort servers by request count (highest first) slices.SortFunc(health.Servers, func(a, b MCPServerHealthDetail) int { diff --git a/pkg/cli/audit_report.go b/pkg/cli/audit_report.go index e1e07b1221b..475b53280ea 100644 --- a/pkg/cli/audit_report.go +++ b/pkg/cli/audit_report.go @@ -14,6 +14,7 @@ import ( "github.com/github/gh-aw/pkg/github" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/sliceutil" + "github.com/github/gh-aw/pkg/stringutil" "github.com/github/gh-aw/pkg/timeutil" ) @@ -687,9 +688,7 @@ func extractPreAgentStepErrors(logsPath string) []ErrorInfo { if len(errorLines) > 0 { message := strings.Join(errorLines, "\n") - if len(message) > maxMessageLen { - message = message[:maxMessageLen] + "..." - } + message = stringutil.Truncate(message, maxMessageLen) auditReportLog.Printf("Extracted ##[error] annotations from flat job log %s (job %d)", jobName, num) errorAnnotations = append(errorAnnotations, ErrorInfo{ Type: "step_failure", @@ -743,9 +742,7 @@ func extractPreAgentStepErrors(logsPath string) []ErrorInfo { if len(errorLines) > 0 { message := strings.Join(errorLines, "\n") - if len(message) > maxMessageLen { - message = message[:maxMessageLen] + "..." - } + message = stringutil.Truncate(message, maxMessageLen) auditReportLog.Printf("Extracted ##[error] annotations from %s (step %d)", stepKey, num) errorAnnotations = append(errorAnnotations, ErrorInfo{ Type: "step_failure", @@ -778,9 +775,7 @@ func extractPreAgentStepErrors(logsPath string) []ErrorInfo { return nil } - if len(message) > maxMessageLen { - message = message[:maxMessageLen] + "..." - } + message = stringutil.Truncate(message, maxMessageLen) auditReportLog.Printf("Extracted pre-agent step error from %s (step %d) as fallback", lastStep.stepKey, lastStep.num) return []ErrorInfo{{ diff --git a/pkg/cli/audit_report_analysis.go b/pkg/cli/audit_report_analysis.go index 88606323a8b..41d07a896b7 100644 --- a/pkg/cli/audit_report_analysis.go +++ b/pkg/cli/audit_report_analysis.go @@ -7,6 +7,7 @@ import ( "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" ) @@ -52,10 +53,7 @@ func generateFindings(processedRun ProcessedRun, metrics MetricsData, errors []E // Append a truncated first error message to help quickly identify the root cause. // Keep descriptions short enough to be useful in a key findings summary. const maxErrMsgLen = 200 - msg := errors[0].Message - if len(msg) > maxErrMsgLen { - msg = msg[:maxErrMsgLen] + "..." - } + msg := stringutil.Truncate(errors[0].Message, maxErrMsgLen) desc += ": " + msg } } diff --git a/pkg/cli/deps_report.go b/pkg/cli/deps_report.go index 08696bd3344..a1699e02fee 100644 --- a/pkg/cli/deps_report.go +++ b/pkg/cli/deps_report.go @@ -119,10 +119,7 @@ func DisplayDependencyReport(report *DependencyReport) { fmt.Fprintf(os.Stderr, "Outdated: %d (%.0f%%)\n", len(report.Outdated), outdatedPercentage) fmt.Fprintf(os.Stderr, "Security advisories: %d\n", len(report.Advisories)) - v0Percentage := 0.0 - if report.TotalDeps > 0 { - v0Percentage = float64(report.V0Count) / float64(report.TotalDeps) * 100 - } + v0Percentage := safePercent(report.V0Count, report.TotalDeps) fmt.Fprintf(os.Stderr, "v0.x dependencies: %d (%.0f%%)", report.V0Count, v0Percentage) if v0Percentage > 30 { fmt.Fprintf(os.Stderr, " ⚠️") @@ -157,16 +154,10 @@ func DisplayDependencyReport(report *DependencyReport) { } fmt.Fprintln(os.Stderr, "") - v1Percentage := 0.0 - if report.TotalDeps > 0 { - v1Percentage = float64(report.V1PlusCount) / float64(report.TotalDeps) * 100 - } + v1Percentage := safePercent(report.V1PlusCount, report.TotalDeps) fmt.Fprintf(os.Stderr, "v1.x (stable): %d (%.0f%%)\n", report.V1PlusCount, v1Percentage) - v2Percentage := 0.0 - if report.TotalDeps > 0 { - v2Percentage = float64(report.V2PlusCount) / float64(report.TotalDeps) * 100 - } + v2Percentage := safePercent(report.V2PlusCount, report.TotalDeps) fmt.Fprintf(os.Stderr, "v2+ (mature): %d (%.0f%%)\n", report.V2PlusCount, v2Percentage) fmt.Fprintln(os.Stderr, "") @@ -201,14 +192,9 @@ func DisplayDependencyReportJSON(report *DependencyReport) error { outdatedPercentage = float64(len(report.Outdated)) / float64(report.DirectDeps) * 100 } - v0Percentage := 0.0 - v1Percentage := 0.0 - v2Percentage := 0.0 - if report.TotalDeps > 0 { - v0Percentage = float64(report.V0Count) / float64(report.TotalDeps) * 100 - v1Percentage = float64(report.V1PlusCount) / float64(report.TotalDeps) * 100 - v2Percentage = float64(report.V2PlusCount) / float64(report.TotalDeps) * 100 - } + v0Percentage := safePercent(report.V0Count, report.TotalDeps) + v1Percentage := safePercent(report.V1PlusCount, report.TotalDeps) + v2Percentage := safePercent(report.V2PlusCount, report.TotalDeps) // Build JSON-friendly output structure output := map[string]any{ diff --git a/pkg/cli/experiments_analyze_statistics.go b/pkg/cli/experiments_analyze_statistics.go index e23fd600dd3..1e659dc0fa1 100644 --- a/pkg/cli/experiments_analyze_statistics.go +++ b/pkg/cli/experiments_analyze_statistics.go @@ -142,10 +142,7 @@ func computeExperimentAnalysis(exp ExperimentVariantStats, cfg *workflow.Experim // Populate per-variant entries. for i, name := range variantNames { count := exp.Variants[name] - obsPct := 0.0 - if exp.Total > 0 { - obsPct = float64(count) / float64(exp.Total) * 100 - } + obsPct := safePercent(count, exp.Total) a.Variants = append(a.Variants, VariantAnalysis{ Name: name, Count: count, diff --git a/pkg/cli/health_metrics.go b/pkg/cli/health_metrics.go index acf2d1d2bea..c3c15d8125c 100644 --- a/pkg/cli/health_metrics.go +++ b/pkg/cli/health_metrics.go @@ -93,10 +93,7 @@ func CalculateWorkflowHealth(workflowName string, runs []WorkflowRun, threshold } totalRuns := len(runs) - successRate := 0.0 - if totalRuns > 0 { - successRate = float64(successCount) / float64(totalRuns) * 100 - } + successRate := safePercent(successCount, totalRuns) avgDuration := time.Duration(durationStats.Mean()) avgTokens := int(tokenStats.Mean()) @@ -175,7 +172,7 @@ func calculateSuccessRate(runs []WorkflowRun) float64 { } } - return float64(successCount) / float64(len(runs)) * 100 + return safePercent(successCount, len(runs)) } // CalculateHealthSummary calculates aggregated health metrics across all workflows diff --git a/pkg/cli/mcp_inspect_mcp.go b/pkg/cli/mcp_inspect_mcp.go index 31b574fda88..12419f211f9 100644 --- a/pkg/cli/mcp_inspect_mcp.go +++ b/pkg/cli/mcp_inspect_mcp.go @@ -14,6 +14,7 @@ import ( "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/parser" + "github.com/github/gh-aw/pkg/stringutil" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -386,10 +387,7 @@ func displayServerCapabilities(info *parser.MCPServerInfo, toolFilter string) { rows := make([][]string, 0, len(info.Resources)) for _, resource := range info.Resources { - description := resource.Description - if len(description) > 40 { - description = description[:37] + "..." - } + description := stringutil.Truncate(resource.Description, 40) mimeType := resource.MIMEType if mimeType == "" { diff --git a/pkg/cli/mcp_list.go b/pkg/cli/mcp_list.go index 850e1798b68..631db772316 100644 --- a/pkg/cli/mcp_list.go +++ b/pkg/cli/mcp_list.go @@ -13,6 +13,7 @@ import ( "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/parser" "github.com/github/gh-aw/pkg/sliceutil" + "github.com/github/gh-aw/pkg/stringutil" "github.com/spf13/cobra" ) @@ -76,9 +77,7 @@ func ListWorkflowMCP(workflowFile string, verbose bool) error { commandOrURL = config.Container } // Truncate if too long - if len(commandOrURL) > 40 { - commandOrURL = commandOrURL[:37] + "..." - } + commandOrURL = stringutil.Truncate(commandOrURL, 40) rows = append(rows, []string{ config.Name, @@ -185,9 +184,7 @@ func listWorkflowsWithMCPServers(workflowsDir string, verbose bool) error { for _, workflow := range workflowData { serverList := strings.Join(workflow.serverNames, ", ") // Truncate if too long - if len(serverList) > 50 { - serverList = serverList[:47] + "..." - } + serverList = stringutil.Truncate(serverList, 50) rows = append(rows, []string{ workflow.name, @@ -241,9 +238,7 @@ func showInteractiveMCPWorkflowSelection(workflows []struct { for i, wf := range workflows { description := fmt.Sprintf("%d server(s): %s", wf.serverCount, strings.Join(wf.serverNames, ", ")) // Truncate description if too long - if len(description) > 80 { - description = description[:77] + "..." - } + description = stringutil.Truncate(description, 80) items[i] = console.NewListItem(wf.name, description, wf.name) } diff --git a/pkg/cli/mcp_registry_list.go b/pkg/cli/mcp_registry_list.go index f29c62e32fa..47d2b132a1f 100644 --- a/pkg/cli/mcp_registry_list.go +++ b/pkg/cli/mcp_registry_list.go @@ -7,6 +7,7 @@ import ( "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/stringutil" ) var mcpRegistryListLog = logger.New("cli:mcp_registry_list") @@ -55,10 +56,7 @@ func listAvailableServers(ctx context.Context, registryURL string, verbose bool) } // Truncate long descriptions for table display - description := server.Description - if len(description) > 80 { - description = description[:77] + "..." - } + description := stringutil.Truncate(server.Description, 80) if description == "" { description = "-" } From 41893baa777e922ed6fd8decfe93aeaa82f19954 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Jun 2026 03:38:50 +0000 Subject: [PATCH 3/5] P1.3: Route local helpers through sliceutil - Removed appendIfMissing from claude_tools.go, replaced with sliceutil.MergeUnique (11 sites) - Removed addUniqueWorkflow from logs_report_errors.go, replaced with sliceutil.MergeUnique (3 sites) - Removed uniqueSorted from central_slash_command_workflow.go, replaced with sliceutil.Deduplicate + sort (2 sites) - All changes build successfully Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/logs_report_errors.go | 14 +++------ pkg/cli/logs_report_mcp.go | 3 +- .../central_slash_command_workflow.go | 20 ++++--------- pkg/workflow/claude_tools.go | 29 +++++++------------ 4 files changed, 22 insertions(+), 44 deletions(-) diff --git a/pkg/cli/logs_report_errors.go b/pkg/cli/logs_report_errors.go index 2dcbf17d70b..c02f348c931 100644 --- a/pkg/cli/logs_report_errors.go +++ b/pkg/cli/logs_report_errors.go @@ -4,6 +4,8 @@ import ( "cmp" "slices" "strings" + + "github.com/github/gh-aw/pkg/sliceutil" ) // ErrorSummary contains aggregated error/warning statistics @@ -18,14 +20,6 @@ type ErrorSummary struct { PatternID string `json:"pattern_id,omitempty" console:"-"` } -// addUniqueWorkflow adds a workflow to the list if it's not already present -func addUniqueWorkflow(workflows []string, workflow string) []string { - if slices.Contains(workflows, workflow) { - return workflows - } - return append(workflows, workflow) -} - // aggregateSummaryItems is a generic helper that aggregates items from processed runs into summaries // It handles the common pattern of grouping by key, counting occurrences, tracking unique workflows, and collecting run IDs func aggregateSummaryItems[TItem any, TSummary any]( @@ -93,7 +87,7 @@ func buildMissingToolsSummary(processedRuns []ProcessedRun) []MissingToolSummary // updateSummary: update existing summary with new occurrence func(summary *MissingToolSummary, tool MissingToolReport) { summary.Count++ - summary.Workflows = addUniqueWorkflow(summary.Workflows, tool.WorkflowName) + summary.Workflows = sliceutil.MergeUnique(summary.Workflows, tool.WorkflowName) summary.RunIDs = append(summary.RunIDs, tool.RunID) }, // finalizeSummary: populate display fields for console rendering @@ -137,7 +131,7 @@ func buildMissingDataSummary(processedRuns []ProcessedRun) []MissingDataSummary // updateSummary: update existing summary with new occurrence func(summary *MissingDataSummary, data MissingDataReport) { summary.Count++ - summary.Workflows = addUniqueWorkflow(summary.Workflows, data.WorkflowName) + summary.Workflows = sliceutil.MergeUnique(summary.Workflows, data.WorkflowName) summary.RunIDs = append(summary.RunIDs, data.RunID) }, // finalizeSummary: populate display fields for console rendering diff --git a/pkg/cli/logs_report_mcp.go b/pkg/cli/logs_report_mcp.go index 55808b3afbd..d443c38ba41 100644 --- a/pkg/cli/logs_report_mcp.go +++ b/pkg/cli/logs_report_mcp.go @@ -6,6 +6,7 @@ import ( "strings" "time" + "github.com/github/gh-aw/pkg/sliceutil" "github.com/github/gh-aw/pkg/timeutil" ) @@ -34,7 +35,7 @@ func buildMCPFailuresSummary(processedRuns []ProcessedRun) []MCPFailureSummary { // updateSummary: update existing summary with new occurrence func(summary *MCPFailureSummary, failure MCPFailureReport) { summary.Count++ - summary.Workflows = addUniqueWorkflow(summary.Workflows, failure.WorkflowName) + summary.Workflows = sliceutil.MergeUnique(summary.Workflows, failure.WorkflowName) summary.RunIDs = append(summary.RunIDs, failure.RunID) }, // finalizeSummary: populate display fields for console rendering diff --git a/pkg/workflow/central_slash_command_workflow.go b/pkg/workflow/central_slash_command_workflow.go index 36d93def314..93a42eabbaa 100644 --- a/pkg/workflow/central_slash_command_workflow.go +++ b/pkg/workflow/central_slash_command_workflow.go @@ -12,6 +12,7 @@ import ( "github.com/github/gh-aw/pkg/constants" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/sliceutil" ) var centralSlashCommandWorkflowLog = logger.New("workflow:central_slash_command_workflow") @@ -121,7 +122,8 @@ func collectCentralSlashCommandRoutes(workflowDataList []*WorkflowData) (map[str } routeEvents := GetCommentEventNames(filteredEvents) - routeEvents = uniqueSorted(routeEvents) + routeEvents = sliceutil.Deduplicate(routeEvents) + sort.Strings(routeEvents) if len(routeEvents) == 0 { continue } @@ -187,7 +189,8 @@ func collectCentralLabelCommandRoutes(workflowDataList []*WorkflowData, mergedEv } filteredEvents := FilterLabelCommandEvents(wd.LabelCommandEvents) - routeEvents := uniqueSorted(filteredEvents) + routeEvents := sliceutil.Deduplicate(filteredEvents) + sort.Strings(routeEvents) if len(routeEvents) == 0 { continue } @@ -551,16 +554,3 @@ func writeCentralSlashEventsYAML(b *strings.Builder, mergedEvents map[string]map b.WriteString(" types: [" + strings.Join(types, ", ") + "]\n") } } - -func uniqueSorted(values []string) []string { - seen := make(map[string]bool, len(values)) - for _, v := range values { - seen[v] = true - } - result := make([]string, 0, len(seen)) - for v := range seen { - result = append(result, v) - } - sort.Strings(result) - return result -} diff --git a/pkg/workflow/claude_tools.go b/pkg/workflow/claude_tools.go index e40e7259608..7f00c5620a8 100644 --- a/pkg/workflow/claude_tools.go +++ b/pkg/workflow/claude_tools.go @@ -261,10 +261,10 @@ func appendCacheMemoryTools(allowedTools []string, cacheMemoryConfig *CacheMemor for _, cache := range cacheMemoryConfig.Caches { cacheDir := cacheMemoryDirFor(cache.ID) cacheDirPattern := cacheDir + "/*" - allowedTools = appendIfMissing(allowedTools, fmt.Sprintf("Read(%s)", cacheDirPattern)) - allowedTools = appendIfMissing(allowedTools, fmt.Sprintf("Write(%s)", cacheDirPattern)) - allowedTools = appendIfMissing(allowedTools, fmt.Sprintf("Edit(%s)", cacheDirPattern)) - allowedTools = appendIfMissing(allowedTools, fmt.Sprintf("MultiEdit(%s)", cacheDirPattern)) + allowedTools = sliceutil.MergeUnique(allowedTools, fmt.Sprintf("Read(%s)", cacheDirPattern)) + allowedTools = sliceutil.MergeUnique(allowedTools, fmt.Sprintf("Write(%s)", cacheDirPattern)) + allowedTools = sliceutil.MergeUnique(allowedTools, fmt.Sprintf("Edit(%s)", cacheDirPattern)) + allowedTools = sliceutil.MergeUnique(allowedTools, fmt.Sprintf("MultiEdit(%s)", cacheDirPattern)) allowedTools = appendCacheMemoryBashTools(allowedTools, cacheDir) } return allowedTools @@ -282,10 +282,10 @@ func appendCacheMemoryBashTools(allowedTools []string, cacheDir string) []string fmt.Sprintf("Bash(mv %s)", cacheDirSlash), } for _, bashTool := range bashCacheTools { - allowedTools = appendIfMissing(allowedTools, bashTool) + allowedTools = sliceutil.MergeUnique(allowedTools, bashTool) } - allowedTools = appendIfMissing(allowedTools, "BashOutput") - allowedTools = appendIfMissing(allowedTools, "KillBash") + allowedTools = sliceutil.MergeUnique(allowedTools, "BashOutput") + allowedTools = sliceutil.MergeUnique(allowedTools, "KillBash") return allowedTools } @@ -371,10 +371,10 @@ func appendSandboxWritableTools(allowedTools []string, sandboxConfig *SandboxCon continue } seenPatterns[pattern] = struct{}{} - allowedTools = appendIfMissing(allowedTools, fmt.Sprintf("Read(%s)", pattern)) - allowedTools = appendIfMissing(allowedTools, fmt.Sprintf("Write(%s)", pattern)) - allowedTools = appendIfMissing(allowedTools, fmt.Sprintf("Edit(%s)", pattern)) - allowedTools = appendIfMissing(allowedTools, fmt.Sprintf("MultiEdit(%s)", pattern)) + allowedTools = sliceutil.MergeUnique(allowedTools, fmt.Sprintf("Read(%s)", pattern)) + allowedTools = sliceutil.MergeUnique(allowedTools, fmt.Sprintf("Write(%s)", pattern)) + allowedTools = sliceutil.MergeUnique(allowedTools, fmt.Sprintf("Edit(%s)", pattern)) + allowedTools = sliceutil.MergeUnique(allowedTools, fmt.Sprintf("MultiEdit(%s)", pattern)) } return allowedTools } @@ -415,13 +415,6 @@ func dedupeAllowedTools(allowedTools []string) []string { return sliceutil.Deduplicate(allowedTools) } -func appendIfMissing(items []string, item string) []string { - if slices.Contains(items, item) { - return items - } - return append(items, item) -} - // generateAllowedToolsComment generates a multi-line comment showing each allowed tool func (e *ClaudeEngine) generateAllowedToolsComment(allowedToolsStr string, indent string) string { if allowedToolsStr == "" { From 5694e2d6ffa31a9bb4cd05fa17114f9067c7ecd2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Jun 2026 03:41:15 +0000 Subject: [PATCH 4/5] P2.1, P2.2, P2.3: Local duplicate consolidation P2.1: Collapsed byte-identical templatable env-var builders - Extracted buildTemplatableEnvVar from buildTemplatableBoolEnvVar and buildTemplatableIntEnvVar - Kept original function names as thin wrappers for readability P2.2: Extracted anomaly-marker formatting (8 sites) - Added formatAnomalyTag(bool) string to audit_math_helpers.go - Added formatAnomalyNote(bool, string) string to audit_math_helpers.go - Replaced all 8 inline anomaly formatting blocks in audit_diff_render.go P2.3: Used sortedMapKeys at 3 map[string]string sites - safe_outputs_config_helpers.go - safe_scripts.go - safe_outputs_app_config.go All changes build successfully Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/audit_diff_render.go | 40 +++++---------------- pkg/cli/audit_math_helpers.go | 18 ++++++++++ pkg/workflow/safe_outputs_app_config.go | 6 +--- pkg/workflow/safe_outputs_config_helpers.go | 7 +--- pkg/workflow/safe_scripts.go | 7 +--- pkg/workflow/templatables.go | 23 ++++++------ 6 files changed, 41 insertions(+), 60 deletions(-) diff --git a/pkg/cli/audit_diff_render.go b/pkg/cli/audit_diff_render.go index 35bdd977734..ce55627060e 100644 --- a/pkg/cli/audit_diff_render.go +++ b/pkg/cli/audit_diff_render.go @@ -150,10 +150,7 @@ func renderFirewallDiffMarkdownSection(diff *FirewallDiff) { for _, entry := range diff.NewDomains { total := entry.Run2Allowed + entry.Run2Blocked statusIcon := firewallStatusEmoji(entry.Run2Status) - anomalyTag := "" - if entry.IsAnomaly { - anomalyTag = " ⚠️" - } + anomalyTag := formatAnomalyTag(entry.IsAnomaly) fmt.Fprintf(os.Stdout, "- %s `%s` (%d requests, %s)%s\n", statusIcon, entry.Domain, total, entry.Run2Status, anomalyTag) } fmt.Fprintln(os.Stdout) @@ -173,10 +170,7 @@ func renderFirewallDiffMarkdownSection(diff *FirewallDiff) { for _, entry := range diff.StatusChanges { icon1 := firewallStatusEmoji(entry.Run1Status) icon2 := firewallStatusEmoji(entry.Run2Status) - anomalyTag := "" - if entry.IsAnomaly { - anomalyTag = " ⚠️" - } + anomalyTag := formatAnomalyTag(entry.IsAnomaly) fmt.Fprintf(os.Stdout, "- `%s`: %s %s → %s %s%s\n", entry.Domain, icon1, entry.Run1Status, icon2, entry.Run2Status, anomalyTag) } fmt.Fprintln(os.Stdout) @@ -205,10 +199,7 @@ func renderMCPToolsDiffMarkdownSection(diff *MCPToolsDiff) { if len(diff.NewTools) > 0 { fmt.Fprintf(os.Stdout, "**New tools (%d)**\n", len(diff.NewTools)) for _, entry := range diff.NewTools { - anomalyTag := "" - if entry.IsAnomaly { - anomalyTag = " ⚠️" - } + anomalyTag := formatAnomalyTag(entry.IsAnomaly) fmt.Fprintf(os.Stdout, "- `%s/%s` (%d calls)%s\n", entry.ServerName, entry.ToolName, entry.Run2CallCount, anomalyTag) } fmt.Fprintln(os.Stdout) @@ -225,10 +216,7 @@ func renderMCPToolsDiffMarkdownSection(diff *MCPToolsDiff) { if len(diff.ChangedTools) > 0 { fmt.Fprintf(os.Stdout, "**Changed tools (%d)**\n", len(diff.ChangedTools)) for _, entry := range diff.ChangedTools { - anomalyTag := "" - if entry.IsAnomaly { - anomalyTag = " ⚠️" - } + anomalyTag := formatAnomalyTag(entry.IsAnomaly) errInfo := "" if entry.Run1ErrorCount > 0 || entry.Run2ErrorCount > 0 { errInfo = fmt.Sprintf(", errors: %d → %d", entry.Run1ErrorCount, entry.Run2ErrorCount) @@ -327,10 +315,7 @@ func renderFirewallDiffPrettySection(diff *FirewallDiff) { } for _, entry := range diff.NewDomains { total := entry.Run2Allowed + entry.Run2Blocked - anomalyNote := "" - if entry.IsAnomaly { - anomalyNote = "⚠️ " + entry.AnomalyNote - } + anomalyNote := formatAnomalyNote(entry.IsAnomaly, entry.AnomalyNote) config.Rows = append(config.Rows, []string{ entry.Domain, firewallStatusEmoji(entry.Run2Status) + " " + entry.Run2Status, @@ -365,10 +350,7 @@ func renderFirewallDiffPrettySection(diff *FirewallDiff) { Rows: make([][]string, 0, len(diff.StatusChanges)), } for _, entry := range diff.StatusChanges { - anomalyNote := "" - if entry.IsAnomaly { - anomalyNote = "⚠️ " + entry.AnomalyNote - } + anomalyNote := formatAnomalyNote(entry.IsAnomaly, entry.AnomalyNote) config.Rows = append(config.Rows, []string{ entry.Domain, firewallStatusEmoji(entry.Run1Status) + " " + entry.Run1Status, @@ -415,10 +397,7 @@ func renderMCPToolsDiffPrettySection(diff *MCPToolsDiff) { Rows: make([][]string, 0, len(diff.NewTools)), } for _, entry := range diff.NewTools { - anomalyNote := "" - if entry.IsAnomaly { - anomalyNote = "⚠️ " + entry.AnomalyNote - } + anomalyNote := formatAnomalyNote(entry.IsAnomaly, entry.AnomalyNote) config.Rows = append(config.Rows, []string{ entry.ServerName, entry.ToolName, @@ -452,10 +431,7 @@ func renderMCPToolsDiffPrettySection(diff *MCPToolsDiff) { Rows: make([][]string, 0, len(diff.ChangedTools)), } for _, entry := range diff.ChangedTools { - anomalyNote := "" - if entry.IsAnomaly { - anomalyNote = "⚠️ " + entry.AnomalyNote - } + anomalyNote := formatAnomalyNote(entry.IsAnomaly, entry.AnomalyNote) config.Rows = append(config.Rows, []string{ entry.ServerName, entry.ToolName, diff --git a/pkg/cli/audit_math_helpers.go b/pkg/cli/audit_math_helpers.go index d719441a5ba..469e00c70cb 100644 --- a/pkg/cli/audit_math_helpers.go +++ b/pkg/cli/audit_math_helpers.go @@ -57,3 +57,21 @@ func formatFloatDelta(value1, value2 float64) string { } return fmt.Sprintf("%.3f", delta) } + +// formatAnomalyTag returns a warning emoji suffix for markdown rendering +// when isAnomaly is true, otherwise returns an empty string. +func formatAnomalyTag(isAnomaly bool) string { + if isAnomaly { + return " ⚠️" + } + return "" +} + +// formatAnomalyNote returns a formatted anomaly note for table rendering +// with a warning emoji prefix when isAnomaly is true, otherwise returns an empty string. +func formatAnomalyNote(isAnomaly bool, anomalyNote string) string { + if isAnomaly { + return "⚠️ " + anomalyNote + } + return "" +} diff --git a/pkg/workflow/safe_outputs_app_config.go b/pkg/workflow/safe_outputs_app_config.go index 513ab3df1be..b9bbbb008c6 100644 --- a/pkg/workflow/safe_outputs_app_config.go +++ b/pkg/workflow/safe_outputs_app_config.go @@ -293,11 +293,7 @@ func (c *Compiler) buildGitHubAppTokenMintStepWithMeta(app *GitHubAppConfig, per } // Extract and sort keys for deterministic ordering - keys := make([]string, 0, len(permissionFields)) - for key := range permissionFields { - keys = append(keys, key) - } - sort.Strings(keys) + keys := sortedMapKeys(permissionFields) // Add permissions in sorted order for _, key := range keys { diff --git a/pkg/workflow/safe_outputs_config_helpers.go b/pkg/workflow/safe_outputs_config_helpers.go index 48fa5292712..31dff0a84f1 100644 --- a/pkg/workflow/safe_outputs_config_helpers.go +++ b/pkg/workflow/safe_outputs_config_helpers.go @@ -2,7 +2,6 @@ package workflow import ( "encoding/json" - "sort" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/stringutil" @@ -24,11 +23,7 @@ func buildNormalizedSortedJSON(names []string, valueFn func(string) string) (str values[normalizedName] = valueFn(normalizedName) } - keys := make([]string, 0, len(values)) - for k := range values { - keys = append(keys, k) - } - sort.Strings(keys) + keys := sortedMapKeys(values) ordered := make(map[string]string, len(keys)) for _, k := range keys { diff --git a/pkg/workflow/safe_scripts.go b/pkg/workflow/safe_scripts.go index 1d2b0b42724..f4d51c86846 100644 --- a/pkg/workflow/safe_scripts.go +++ b/pkg/workflow/safe_scripts.go @@ -2,7 +2,6 @@ package workflow import ( "encoding/json" - "sort" "strings" "github.com/github/gh-aw/pkg/logger" @@ -105,11 +104,7 @@ func buildCustomSafeOutputScriptsJSON(data *WorkflowData) string { } // Sort keys for deterministic output - keys := make([]string, 0, len(scriptMapping)) - for k := range scriptMapping { - keys = append(keys, k) - } - sort.Strings(keys) + keys := sortedMapKeys(scriptMapping) ordered := make(map[string]string, len(keys)) for _, k := range keys { diff --git a/pkg/workflow/templatables.go b/pkg/workflow/templatables.go index 75485f971bb..eac01fc2d9e 100644 --- a/pkg/workflow/templatables.go +++ b/pkg/workflow/templatables.go @@ -156,11 +156,11 @@ func (t *TemplatableBool) String() string { return string(*t) } -// buildTemplatableBoolEnvVar returns a YAML environment variable entry for a -// templatable boolean field. If value is a GitHub Actions expression it is +// buildTemplatableEnvVar returns a YAML environment variable entry for a +// templatable field. If value is a GitHub Actions expression it is // embedded unquoted so that GitHub Actions can evaluate it at runtime; // otherwise the literal string is quoted. Returns nil if value is nil. -func buildTemplatableBoolEnvVar(envVarName string, value *string) []string { +func buildTemplatableEnvVar(envVarName string, value *string) []string { if value == nil { return nil } @@ -171,6 +171,14 @@ func buildTemplatableBoolEnvVar(envVarName string, value *string) []string { return []string{fmt.Sprintf(" %s: %q\n", envVarName, v)} } +// buildTemplatableBoolEnvVar returns a YAML environment variable entry for a +// templatable boolean field. If value is a GitHub Actions expression it is +// embedded unquoted so that GitHub Actions can evaluate it at runtime; +// otherwise the literal string is quoted. Returns nil if value is nil. +func buildTemplatableBoolEnvVar(envVarName string, value *string) []string { + return buildTemplatableEnvVar(envVarName, value) +} + // AddTemplatableBool adds a templatable boolean field to the handler config. // // The stored JSON value depends on the content of *value: @@ -200,14 +208,7 @@ func (b *handlerConfigBuilder) AddTemplatableBool(key string, value *string) *ha // embedded unquoted so that GitHub Actions can evaluate it at runtime; // otherwise the literal string is quoted. Returns nil if value is nil. func buildTemplatableIntEnvVar(envVarName string, value *string) []string { - if value == nil { - return nil - } - v := *value - if isExpression(v) { - return []string{fmt.Sprintf(" %s: %s\n", envVarName, v)} - } - return []string{fmt.Sprintf(" %s: %q\n", envVarName, v)} + return buildTemplatableEnvVar(envVarName, value) } // AddTemplatableInt adds a templatable integer field to the handler config. From c69d264f5cf971f0eaa5c359b1d759820045acaa Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 17 Jun 2026 03:49:54 +0000 Subject: [PATCH 5/5] Fix tests: Update to use sliceutil.MergeUnique - Updated logs_report_test.go to use sliceutil.MergeUnique instead of removed addUniqueWorkflow - All tests passing - make agent-report-progress passes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/logs_report_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/cli/logs_report_test.go b/pkg/cli/logs_report_test.go index b4cb4b3949a..2a48304bc20 100644 --- a/pkg/cli/logs_report_test.go +++ b/pkg/cli/logs_report_test.go @@ -7,6 +7,8 @@ import ( "path/filepath" "testing" "time" + + "github.com/github/gh-aw/pkg/sliceutil" ) // TestRenderLogsConsoleUnified tests the unified console rendering @@ -222,7 +224,7 @@ func TestAddUniqueWorkflow(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - result := addUniqueWorkflow(tt.workflows, tt.workflow) + result := sliceutil.MergeUnique(tt.workflows, tt.workflow) if len(result) != len(tt.expected) { t.Errorf("Expected length %d, got %d", len(tt.expected), len(result)) } @@ -429,7 +431,7 @@ func TestAggregateSummaryItems(t *testing.T) { }, func(summary *MissingToolSummary, tool MissingToolReport) { summary.Count++ - summary.Workflows = addUniqueWorkflow(summary.Workflows, tool.WorkflowName) + summary.Workflows = sliceutil.MergeUnique(summary.Workflows, tool.WorkflowName) summary.RunIDs = append(summary.RunIDs, tool.RunID) }, func(summary *MissingToolSummary) {