refactor: semantic function clustering — dedup, shared helpers, and generics consolidation

docs/adr/38776-consolidate-duplicated-logic-into-shared-utilities.md

@@ -0,0 +1,39 @@
+# ADR-38776: Consolidate Duplicated Logic into Shared Utilities and Generics
+
+**Date**: 2026-06-12
+**Status**: Draft
+
+## Context
+
+A semantic function-clustering analysis (#38748) identified several groups of structurally identical code scattered across the `pkg/cli`, `pkg/console`, `pkg/parser`, and `pkg/workflow` packages: four inline string-truncation snippets, two near-identical `abbreviateNumber`/`formatTokens` implementations, a hand-rolled dedup loop duplicating `sliceutil.Deduplicate`, two identical inline-section extraction loops, two validation functions sharing the same "feature set → capability supported → error" shape, and inline GitHub remote-URL parsing that duplicated the GHE-aware `parseGitHubRepoSlugFromURL` helper. This duplication increases maintenance cost and risks the copies drifting apart (for example, the inline URL parser hardcoded `github.com` and was not GitHub Enterprise aware). No non-negotiable behavioral change is required; the goal is to reduce duplication without altering observable output.
+
+## Decision
+
+We decided to eliminate the identified duplication by consolidating each cluster into a single shared implementation: route inline truncations through `stringutil.Truncate`, replace both token formatters with one exported `console.FormatTokens`, replace the manual dedup loop with `sliceutil.Deduplicate`, extract a generic `extractInlineSections[T any]` helper used by both `ExtractInlineSkills` and `ExtractInlineSubAgents`, extract a shared `validateCapabilitySupport` helper, and delegate git-remote parsing to the existing GHE-aware `parseGitHubRepoSlugFromURL`. The driver is maintainability and a single source of truth; generics are introduced only where the extraction loops are structurally identical.
+
+## Alternatives Considered
+
+### Alternative 1: Leave the duplication in place (status quo)
+Each copy is short and already works, so we could defer consolidation indefinitely. Rejected because the duplication was actively causing divergence (the inline URL parser was not GHE-aware while the shared helper was), and the analysis in #38748 had already surfaced the cost.
+
+### Alternative 2: Detect duplication with tooling instead of manual consolidation
+We could add a linter or `dupl`-style check to flag clones rather than refactoring them now. Rejected as insufficient on its own — it would report the duplication but not remove it, and the clusters were already enumerated and small enough to consolidate directly. Tooling remains a complementary option for preventing future regressions.
+
+## Consequences
+
+### Positive
+- Single source of truth for truncation, token formatting, deduplication, inline-section extraction, capability validation, and GitHub URL parsing.
+- The git-remote fallback path is now GitHub Enterprise aware, fixing a latent inconsistency with hardcoded `github.com`.
+- Reduced net code and lower future maintenance cost; behavior is preserved (existing tests, e.g. `TestFormatTokens`, were re-pointed at the shared helper).
+
+### Negative
+- Introduces cross-package coupling: callers in `pkg/cli` now depend on `pkg/console`, `pkg/stringutil`, and `pkg/sliceutil` where they previously had local copies.
+- The generic `extractInlineSections[T]` adds a small amount of cognitive overhead (a `makeItem` factory callback) compared with two explicit loops.
+
+### Neutral
+- The rename `parseEventsJSONLFile` → `parseEventsJSONLMetrics` clarifies intent and avoids collision with `parseEventsJSONL`, but requires updating all callers and test names.
+- A pre-existing `perfsprint` lint warning in `action_resolver.go` was fixed in passing; unrelated to the consolidation theme.
+
+---
+
+*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/27420151317) workflow. The PR author must review, complete, and finalize this document before the PR can merge.*

pkg/cli/audit_cross_run_render.go

@@ -185,7 +185,7 @@ func markdownPerRunFields(run PerRunFirewallBreakdown) (string, string, string,
 	}
 	tokenStr := "—"
 	if run.Tokens > 0 {
-		tokenStr = formatTokens(run.Tokens)
+		tokenStr = console.FormatTokens(run.Tokens)
 		if run.TokenSpike {
 			tokenStr += " ⚠"
 		}
@@ -246,8 +246,8 @@ func renderPrettyTokenTrend(mt MetricsTrendData) {
 		return
 	}
 	fmt.Fprintf(os.Stderr, "  Tokens:   total=%s  avg=%s/run  min=%s  max=%s\n%s",
-		formatTokens(mt.TotalTokens), formatTokens(mt.AvgTokens),
-		formatTokens(mt.MinTokens), formatTokens(mt.MaxTokens), prettySpikeNote("Token", mt.TokenSpikes))
+		console.FormatTokens(mt.TotalTokens), console.FormatTokens(mt.AvgTokens),
+		console.FormatTokens(mt.MinTokens), console.FormatTokens(mt.MaxTokens), prettySpikeNote("Token", mt.TokenSpikes))
 }
 
 func renderPrettyTurnTrend(mt MetricsTrendData) {
@@ -376,7 +376,7 @@ func prettyPerRunOptionalFields(run PerRunFirewallBreakdown) string {
 	}
 	if run.Tokens > 0 {
 		parts.WriteString("  tokens=")
-		parts.WriteString(formatTokens(run.Tokens))
+		parts.WriteString(console.FormatTokens(run.Tokens))
 		if run.TokenSpike {
 			parts.WriteString("⚠")
 		}

pkg/cli/audit_expanded.go

@@ -172,7 +172,7 @@ func inferFallbackLogMetrics(logsPath string) (LogMetrics, string) {
 	}
 
 	if eventsJSONLPath := findEventsJSONLFile(logsPath); eventsJSONLPath != "" {
-		if metrics, err := parseEventsJSONLFile(eventsJSONLPath, false); err == nil && hasUsefulFallbackMetrics(metrics) {
+		if metrics, err := parseEventsJSONLMetrics(eventsJSONLPath, false); err == nil && hasUsefulFallbackMetrics(metrics) {
 			return metrics, "copilot"
 		}
 	}

pkg/cli/completions.go

@@ -9,6 +9,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/github/gh-aw/pkg/workflow"
 	"github.com/spf13/cobra"
 )
@@ -44,21 +45,14 @@ func getWorkflowDescription(filePath string) string {
 	// Look for description field
 	if desc, ok := result.Frontmatter["description"]; ok {
 		if descStr, ok := desc.(string); ok {
-			// Truncate to 60 characters for better display
-			if len(descStr) > 60 {
-				return descStr[:57] + "..."
-			}
-			return descStr
+			return stringutil.Truncate(descStr, 60)
 		}
 	}
 
 	// Fallback to name field if description not found
 	if name, ok := result.Frontmatter["name"]; ok {
 		if nameStr, ok := name.(string); ok {
-			if len(nameStr) > 60 {
-				return nameStr[:57] + "..."
-			}
-			return nameStr
+			return stringutil.Truncate(nameStr, 60)
 		}
 	}
 

pkg/cli/copilot_events_jsonl.go

@@ -159,7 +159,7 @@ func findFileInDir(dir, name string) string {
 	return found
 }
 
-// parseEventsJSONLFile parses a Copilot events.jsonl file and extracts log metrics.
+// parseEventsJSONLMetrics parses a Copilot events.jsonl file and extracts log metrics.
 //
 // events.jsonl provides precise, structured data about a Copilot CLI session:
 //   - "session.start":          session metadata (sessionId, copilotVersion)
@@ -170,7 +170,7 @@ func findFileInDir(dir, name string) string {
 //
 // Returns the extracted metrics and nil on success, or empty metrics and an
 // error if the file cannot be read or contains no recognizable events.
-func parseEventsJSONLFile(path string, verbose bool) (workflow.LogMetrics, error) {
+func parseEventsJSONLMetrics(path string, verbose bool) (workflow.LogMetrics, error) {
 	copilotEventsJSONLLog.Printf("Parsing events.jsonl from: %s", path)
 
 	var metrics workflow.LogMetrics

pkg/cli/copilot_events_jsonl_test.go

@@ -79,7 +79,7 @@ func realFormatEventsLine(eventType string, dataJSON string) string {
 	return `{"type":"` + eventType + `","data":` + dataJSON + `,"id":"test-id","timestamp":"2026-04-02T04:00:00.000Z"}`
 }
 
-// TestParseEventsJSONLFile verifies that parseEventsJSONLFile correctly extracts
+// TestParseEventsJSONLFile verifies that parseEventsJSONLMetrics correctly extracts
 // turns, tool calls, tool sequences, and token counts from events.jsonl using
 // the real Copilot CLI format (nested data object, tool.execution_start events).
 func TestParseEventsJSONLFile(t *testing.T) {
@@ -156,7 +156,7 @@ func TestParseEventsJSONLFile(t *testing.T) {
 			eventsPath := filepath.Join(dir, "events.jsonl")
 			require.NoError(t, os.WriteFile(eventsPath, []byte(tt.content), 0644))
 
-			metrics, err := parseEventsJSONLFile(eventsPath, false)
+			metrics, err := parseEventsJSONLMetrics(eventsPath, false)
 
 			if tt.wantErr {
 				assert.Error(t, err, "expected an error")
@@ -206,7 +206,7 @@ func TestParseEventsJSONLFile_RealArtifact(t *testing.T) {
 	eventsPath := filepath.Join(dir, "events.jsonl")
 	require.NoError(t, os.WriteFile(eventsPath, []byte(content), 0644))
 
-	metrics, err := parseEventsJSONLFile(eventsPath, false)
+	metrics, err := parseEventsJSONLMetrics(eventsPath, false)
 	require.NoError(t, err, "should parse without error")
 
 	assert.Equal(t, 2, metrics.Turns, "should detect 2 turns")
@@ -306,7 +306,7 @@ func TestParseEventsJSONLFile_TBT(t *testing.T) {
 		eventsPath := filepath.Join(dir, "events.jsonl")
 		require.NoError(t, os.WriteFile(eventsPath, []byte(content), 0644))
 
-		metrics, err := parseEventsJSONLFile(eventsPath, false)
+		metrics, err := parseEventsJSONLMetrics(eventsPath, false)
 		require.NoError(t, err, "should parse without error")
 
 		assert.Equal(t, 3, metrics.Turns, "should detect 3 turns")
@@ -322,7 +322,7 @@ func TestParseEventsJSONLFile_TBT(t *testing.T) {
 		eventsPath := filepath.Join(dir, "events.jsonl")
 		require.NoError(t, os.WriteFile(eventsPath, []byte(content), 0644))
 
-		metrics, err := parseEventsJSONLFile(eventsPath, false)
+		metrics, err := parseEventsJSONLMetrics(eventsPath, false)
 		require.NoError(t, err, "should parse without error")
 
 		assert.Equal(t, 1, metrics.Turns, "should detect 1 turn")
@@ -340,7 +340,7 @@ func TestParseEventsJSONLFile_TBT(t *testing.T) {
 		eventsPath := filepath.Join(dir, "events.jsonl")
 		require.NoError(t, os.WriteFile(eventsPath, []byte(content), 0644))
 
-		metrics, err := parseEventsJSONLFile(eventsPath, false)
+		metrics, err := parseEventsJSONLMetrics(eventsPath, false)
 		require.NoError(t, err, "should parse without error")
 
 		assert.Equal(t, 2, metrics.Turns, "should detect 2 turns")

pkg/cli/git.go

@@ -53,8 +53,9 @@ func findGitRootForPath(path string) (string, error) {
 }
 
 // parseGitHubRepoSlugFromURL extracts owner/repo from a GitHub URL
-// Supports both HTTPS (https://github.com/owner/repo) and SSH (git@github.com:owner/repo) formats
-// Also supports GitHub Enterprise URLs
+// Supports HTTPS (https://github.com/owner/repo), SCP-style SSH (git@github.com:owner/repo),
+// and SSH URL scheme (ssh://git@github.com/owner/repo) formats.
+// Also supports GitHub Enterprise URLs.
 func parseGitHubRepoSlugFromURL(url string) string {
 	gitLog.Printf("Parsing GitHub repo slug from URL: %s", url)
 
@@ -71,14 +72,26 @@ func parseGitHubRepoSlugFromURL(url string) string {
 		return slug
 	}
 
-	// Handle SSH URLs: git@github.com:owner/repo or git@enterprise.github.com:owner/repo
+	// Handle SCP-style SSH URLs: git@github.com:owner/repo or git@enterprise.github.com:owner/repo
 	sshPrefix := "git@" + githubHostWithoutScheme + ":"
 	if after, ok := strings.CutPrefix(url, sshPrefix); ok {
 		slug := after
 		gitLog.Printf("Extracted slug from SSH URL: %s", slug)
 		return slug
 	}
 
+	// Handle SSH URL scheme: ssh://git@github.com/owner/repo or ssh://github.com/owner/repo
+	if after, ok := strings.CutPrefix(url, "ssh://"); ok {
+		// Strip optional user info (e.g. "git@")
+		if _, userStripped, hasAt := strings.Cut(after, "@"); hasAt {
+			after = userStripped
+		}
+		if slug, ok := strings.CutPrefix(after, githubHostWithoutScheme+"/"); ok {
+			gitLog.Printf("Extracted slug from SSH URL scheme: %s", slug)
+			return slug
+		}
+	}
+
 	gitLog.Print("Could not extract slug from URL")
 	return ""
 }

pkg/cli/git_helpers_test.go

@@ -32,6 +32,21 @@ func TestParseGitHubRepoSlugFromURL(t *testing.T) {
 			url:      "git@github.com:github/gh-aw",
 			expected: "github/gh-aw",
 		},
+		{
+			name:     "SSH URL scheme with .git",
+			url:      "ssh://git@github.com/github/gh-aw.git",
+			expected: "github/gh-aw",
+		},
+		{
+			name:     "SSH URL scheme without .git",
+			url:      "ssh://git@github.com/github/gh-aw",
+			expected: "github/gh-aw",
+		},
+		{
+			name:     "SSH URL scheme without user info",
+			url:      "ssh://github.com/github/gh-aw.git",
+			expected: "github/gh-aw",
+		},
 		{
 			name:     "Invalid URL",
 			url:      "not-a-github-url",

pkg/cli/health_metrics.go

@@ -2,9 +2,9 @@ package cli
 
 import (
 	"fmt"
-	"strconv"
 	"time"
 
+	"github.com/github/gh-aw/pkg/console"
 	"github.com/github/gh-aw/pkg/logger"
 	"github.com/github/gh-aw/pkg/stats"
 	"github.com/github/gh-aw/pkg/timeutil"
@@ -107,7 +107,7 @@ func CalculateWorkflowHealth(workflowName string, runs []WorkflowRun, threshold
 	// Format display values
 	displayRate := fmt.Sprintf("%.0f%%  (%d/%d)", successRate, successCount, totalRuns)
 	displayDur := timeutil.FormatDuration(avgDuration)
-	displayTokens := formatTokens(avgTokens)
+	displayTokens := console.FormatTokens(avgTokens)
 
 	belowThreshold := successRate < threshold
 
@@ -215,17 +215,3 @@ func GroupRunsByWorkflow(runs []WorkflowRun) map[string][]WorkflowRun {
 	}
 	return grouped
 }
-
-// formatTokens formats token count in a human-readable format
-func formatTokens(tokens int) string {
-	if tokens == 0 {
-		return "-"
-	}
-	if tokens < 1000 {
-		return strconv.Itoa(tokens)
-	}
-	if tokens < 1000000 {
-		return fmt.Sprintf("%.1fK", float64(tokens)/1000)
-	}
-	return fmt.Sprintf("%.1fM", float64(tokens)/1000000)
-}

pkg/cli/health_metrics_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/github/gh-aw/pkg/console"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -251,7 +252,7 @@ func TestFormatTokens(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			result := formatTokens(tt.tokens)
+			result := console.FormatTokens(tt.tokens)
 			assert.Equal(t, tt.expected, result, "Formatted tokens should match")
 		})
 	}