perf: fix 278.8% regression in BenchmarkCompileMCPWorkflow

pkg/workflow/compiler_cache_regression_test.go

@@ -0,0 +1,90 @@
+package workflow
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/github/gh-aw/pkg/testutil"
+	"github.com/stretchr/testify/require"
+)
+
+func TestComputeAllowedDomainsForSanitizationCacheReplacesByPath(t *testing.T) {
+	compiler := NewCompiler()
+	compiler.markdownPath = "/tmp/workflow-a.md"
+
+	data := &WorkflowData{
+		FrontmatterHash: "hash-1",
+		AI:              "copilot",
+	}
+
+	first, err := compiler.computeAllowedDomainsForSanitization(data)
+	require.NoError(t, err)
+	require.NotEmpty(t, first)
+	require.Len(t, compiler.allowedDomainsCache, 1)
+	require.Equal(t, "hash-1", compiler.allowedDomainsCache["/tmp/workflow-a.md"].frontmatterHash)
+
+	data2 := &WorkflowData{
+		FrontmatterHash: "hash-2",
+		AI:              "copilot",
+	}
+	second, err := compiler.computeAllowedDomainsForSanitization(data2)
+	require.NoError(t, err)
+	require.NotEmpty(t, second)
+	require.Len(t, compiler.allowedDomainsCache, 1)
+	require.Equal(t, "hash-2", compiler.allowedDomainsCache["/tmp/workflow-a.md"].frontmatterHash)
+	require.Equal(t, second, compiler.allowedDomainsCache["/tmp/workflow-a.md"].domains)
+
+	compiler.markdownPath = "/tmp/workflow-b.md"
+	data3 := &WorkflowData{
+		FrontmatterHash: "hash-3",
+		AI:              "copilot",
+	}
+	_, err = compiler.computeAllowedDomainsForSanitization(data3)
+	require.NoError(t, err)
+	require.Len(t, compiler.allowedDomainsCache, 2)
+}
+
+func TestPermissionWarningsCountAcrossCompilations(t *testing.T) {
+	tmpDir := testutil.TempDir(t, "permission-warning-hash")
+	testFile := filepath.Join(tmpDir, "workflow.md")
+
+	content1 := `---
+on: push
+permissions:
+  contents: read
+tools:
+  github:
+    toolsets: [issues]
+---
+
+# Test workflow
+`
+
+	content2 := `---
+on: push
+permissions:
+  contents: read
+tools:
+  github:
+    toolsets: [pull_requests]
+---
+
+# Test workflow
+`
+
+	require.NoError(t, os.WriteFile(testFile, []byte(content1), 0o644))
+
+	compiler := NewCompiler()
+	require.NoError(t, compiler.CompileWorkflow(testFile))
+	require.Positive(t, compiler.GetWarningCount())
+
+	compiler.ResetWarningCount()
+	require.NoError(t, compiler.CompileWorkflow(testFile))
+	require.Positive(t, compiler.GetWarningCount())
+
+	require.NoError(t, os.WriteFile(testFile, []byte(content2), 0o644))
+	compiler.ResetWarningCount()
+	require.NoError(t, compiler.CompileWorkflow(testFile))
+	require.Positive(t, compiler.GetWarningCount())
+}

pkg/workflow/compiler_performance_benchmark_test.go

@@ -130,6 +130,7 @@ permissions:
   pull-requests: read
   actions: read
   issues: read
+  discussions: read
 engine: copilot
 tools:
   github:

pkg/workflow/compiler_types.go

@@ -108,6 +108,8 @@ type Compiler struct {
 	ghesArtifactCompat      bool                     // If true, GHES compatibility mode is enabled; artifact actions still use latest non-v3 pins
 	ownerTypeCache          map[string]string        // Cached GitHub owner type ("User"/"Organization"/"") keyed by owner login; not goroutine-safe (Compiler is used sequentially)
 	copilotRequestsTipShown map[string]bool          // Tracks markdown paths that already emitted the copilot-requests enable tip in this compiler instance
+	permissionWarningShown  map[string]string        // Tracks markdown paths and last warning fingerprint (frontmatter hash when available, otherwise formatted warning text)
+	allowedDomainsCache     map[string]allowedDomain // Cached allowed-domains per markdown path with the frontmatter hash that produced it
 	// modelPricingResolver is an optional callback for resolving per-token pricing of models that
 	// are absent from the embedded models.json catalog. When non-nil it is called during
 	// buildInitialWorkflowData for the workflow's configured model; any returned pricing is merged
@@ -116,6 +118,11 @@ type Compiler struct {
 	modelPricingResolver func(ctx context.Context, provider, model string) (map[string]float64, bool)
 }
 
+type allowedDomain struct {
+	frontmatterHash string
+	domains         string
+}
+
 // NewCompiler creates a new workflow compiler with functional options.
 // By default, it auto-detects the version and action mode.
 // Common options: WithVerbose, WithEngineOverride, WithNoEmit, WithSkipValidation
@@ -142,9 +149,11 @@ func NewCompiler(opts ...CompilerOption) *Compiler {
 		artifactManager:         NewArtifactManager(),
 		actionPinWarnings:       make(map[string]bool), // Initialize warning cache
 		priorManifests:          make(map[string]*GHAWManifest),
-		ownerTypeCache:          make(map[string]string), // Initialize owner-type cache (keyed by owner login)
-		copilotRequestsTipShown: make(map[string]bool),   // Initialize one-time tip tracking (keyed by markdown path)
-		gitRoot:                 gitRoot,                 // Auto-detected git root
+		ownerTypeCache:          make(map[string]string),        // Initialize owner-type cache (keyed by owner login)
+		copilotRequestsTipShown: make(map[string]bool),          // Initialize one-time tip tracking (keyed by markdown path)
+		permissionWarningShown:  make(map[string]string),        // Initialize one-time permission warning tracking (keyed by markdown path)
+		allowedDomainsCache:     make(map[string]allowedDomain), // Initialize allowed-domains cache (keyed by markdown path)
+		gitRoot:                 gitRoot,                        // Auto-detected git root
 	}
 
 	// Apply functional options

pkg/workflow/domains.go

@@ -880,6 +880,9 @@ func mergeAPITargetDomains(domainsStr string, apiTarget string) string {
 // The result is cached in data.CachedAllowedDomainsStr after the first call so that
 // repeated calls (e.g. from the activation job, safe-outputs steps, and agent run step)
 // do not recompute the same domain list.
+// Additionally, results are cached on the Compiler keyed by markdown path with the
+// current FrontmatterHash so repeated compilations of an unchanged workflow skip the
+// full domain computation without unbounded hash-key growth in watch mode.
 // Returns an error if the engine's model is malformed (e.g. a leading slash).
 func (c *Compiler) computeAllowedDomainsForSanitization(data *WorkflowData) (string, error) {
 	// Return cached result if available (engine/network/tools/runtimes do not change during compilation).
@@ -889,6 +892,16 @@ func (c *Compiler) computeAllowedDomainsForSanitization(data *WorkflowData) (str
 		return data.CachedAllowedDomainsStr, nil
 	}
 
+	// Check the Compiler-level cache keyed by markdown path.
+	// A cached entry is reusable only when the current frontmatter hash matches.
+	if c.markdownPath != "" && data.FrontmatterHash != "" {
+		if cached, ok := c.allowedDomainsCache[c.markdownPath]; ok && cached.frontmatterHash == data.FrontmatterHash {
+			data.CachedAllowedDomainsStr = cached.domains
+			data.CachedAllowedDomainsComputed = true
+			return cached.domains, nil
+		}
+	}
+
 	// Determine which engine is being used
 	var engineID string
 	if data.EngineConfig != nil {
@@ -941,6 +954,16 @@ func (c *Compiler) computeAllowedDomainsForSanitization(data *WorkflowData) (str
 	// Set the boolean sentinel first so that an empty result is also treated as cached.
 	data.CachedAllowedDomainsComputed = true
 	data.CachedAllowedDomainsStr = base
+
+	// Populate the Compiler-level cache so subsequent compilations of the same
+	// workflow path and unchanged frontmatter skip this computation entirely.
+	if c.markdownPath != "" && data.FrontmatterHash != "" {
+		c.allowedDomainsCache[c.markdownPath] = allowedDomain{
+			frontmatterHash: data.FrontmatterHash,
+			domains:         base,
+		}
+	}
+
 	return base, nil
 }
 

pkg/workflow/permissions_compiler_validator.go

@@ -143,9 +143,20 @@ func (c *Compiler) validatePermissions(workflowData *WorkflowData, markdownPath
 						message += "\n\n" + missingPermissionsDefaultToolsetWarning
 					}
 
-					// In non-strict mode, missing permissions are warnings.
-					// In strict mode with default-only toolsets, this is intentionally downgraded to warning.
-					fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", message))
+					// Emit to stderr once per markdown path + warning fingerprint.
+					// Prefer frontmatter hash when available; otherwise use the formatted
+					// message as a fallback fingerprint for code paths/tests where the hash
+					// is not set.
+					warningFingerprint := workflowData.FrontmatterHash
+					if warningFingerprint == "" {
+						warningFingerprint = message
+					}
+					if c.permissionWarningShown[markdownPath] != warningFingerprint {
+						// In non-strict mode, missing permissions are warnings.
+						// In strict mode with default-only toolsets, this is intentionally downgraded to warning.
+						fmt.Fprintln(os.Stderr, formatCompilerMessage(markdownPath, "warning", message))
+						c.permissionWarningShown[markdownPath] = warningFingerprint
+					}
 					c.IncrementWarningCount()
 				}
 			}

pkg/workflow/permissions_scope_validation_test.go

@@ -91,3 +91,15 @@ pull-requests: read`,
 		})
 	}
 }
+
+func TestGetAllPermissionScopeNamesReturnsCopy(t *testing.T) {
+	scopes := getAllPermissionScopeNames()
+	require.NotEmpty(t, scopes)
+
+	original := scopes[0]
+	scopes[0] = "mutated-scope"
+
+	fresh := getAllPermissionScopeNames()
+	require.NotEmpty(t, fresh)
+	require.Equal(t, original, fresh[0])
+}

pkg/workflow/permissions_validation.go

@@ -8,6 +8,7 @@ import (
 	"slices"
 	"sort"
 	"strings"
+	"sync"
 
 	"github.com/github/gh-aw/pkg/constants"
 	"github.com/github/gh-aw/pkg/setutil"
@@ -16,6 +17,37 @@ import (
 	"github.com/goccy/go-yaml"
 )
 
+var allPermissionScopeNames = sync.OnceValue(func() []string {
+	ghTokenScopes := GetAllPermissionScopes()
+	appOnlyScopes := GetAllGitHubAppOnlyScopes()
+	// +1 for copilot-requests which is not in GetAllPermissionScopes
+	all := make([]string, 0, safeAllocationCapacity(len(ghTokenScopes), len(appOnlyScopes), 1))
+	for _, scope := range ghTokenScopes {
+		all = append(all, string(scope))
+	}
+	for _, scope := range appOnlyScopes {
+		all = append(all, string(scope))
+	}
+	// copilot-requests is valid even though not in GetAllPermissionScopes
+	all = append(all, string(PermissionCopilotRequests))
+	return all
+})
+
+// getAllPermissionScopeNames returns all valid permission scope names for fuzzy matching.
+// The values are computed once and cached across calls.
+func getAllPermissionScopeNames() []string {
+	return slices.Clone(allPermissionScopeNames())
+}
+
+// validPermissionMetaKeys is the set of meta-keys accepted in permissions shorthand contexts.
+// Defined once at package level to avoid per-call map allocation.
+var validPermissionMetaKeys = map[string]struct{}{
+	"all":       {},
+	"read-all":  {},
+	"write-all": {},
+	"none":      {},
+}
+
 // PermissionsValidationResult contains the result of permissions validation
 type PermissionsValidationResult struct {
 	MissingPermissions    map[PermissionScope]PermissionLevel // Permissions required but not granted
@@ -408,28 +440,6 @@ func ValidatePermissionScopeNames(permissionsYAML string) error {
 
 	permissionsValidationLog.Print("Validating permission scope names")
 
-	// Collect all valid scope names for fuzzy matching
-	ghTokenScopes := GetAllPermissionScopes()
-	appOnlyScopes := GetAllGitHubAppOnlyScopes()
-	// +1 for copilot-requests which is not in GetAllPermissionScopes
-	allScopes := make([]string, 0, safeAllocationCapacity(len(ghTokenScopes), len(appOnlyScopes), 1))
-	for _, scope := range ghTokenScopes {
-		allScopes = append(allScopes, string(scope))
-	}
-	for _, scope := range appOnlyScopes {
-		allScopes = append(allScopes, string(scope))
-	}
-	// copilot-requests is valid even though not in GetAllPermissionScopes
-	allScopes = append(allScopes, string(PermissionCopilotRequests))
-	// "all" is a meta-key that is always valid in shorthand contexts
-	validMeta := map[string]struct {
-	}{
-		"all":       {},
-		"read-all":  {},
-		"write-all": {},
-		"none":      {},
-	}
-
 	// Strip optional "permissions:" prefix so we can parse just the map content
 	content := strings.TrimSpace(permissionsYAML)
 	if strings.HasPrefix(content, "permissions:") {
@@ -450,14 +460,17 @@ func ValidatePermissionScopeNames(permissionsYAML string) error {
 	}
 
 	for scopeKey := range permsMap {
-		if setutil.Contains(validMeta, scopeKey) {
+		if setutil.Contains(validPermissionMetaKeys, scopeKey) {
 			continue
 		}
 		if _, ok := validPermissionScopes[scopeKey]; ok {
 			continue
 		}
 
-		// Unknown scope key — check for a case-only difference first (e.g. "Contents" → "contents")
+		// Unknown scope key — retrieve all valid scopes lazily (only on the error path)
+		allScopes := getAllPermissionScopeNames()
+
+		// Check for a case-only difference first (e.g. "Contents" → "contents")
 		lowerScopeKey := strings.ToLower(scopeKey)
 		if lowerScopeKey != scopeKey {
 			if _, ok := validPermissionScopes[lowerScopeKey]; ok {