Refactor script registry pattern - eliminate 27 sync.Once patterns

pkg/workflow/js.go

@@ -4,7 +4,6 @@ import (
 	_ "embed"
 	"fmt"
 	"strings"
-	"sync"
 
 	"github.com/githubnext/gh-aw/pkg/logger"
 )
@@ -23,28 +22,17 @@ var addReactionAndEditCommentScript string
 //go:embed js/check_membership.cjs
 var checkMembershipScriptSource string
 
-var (
-	checkMembershipScript     string
-	checkMembershipScriptOnce sync.Once
-)
+// init registers scripts from js.go with the DefaultScriptRegistry
+func init() {
+	DefaultScriptRegistry.Register("check_membership", checkMembershipScriptSource)
+	DefaultScriptRegistry.Register("safe_outputs_mcp_server", safeOutputsMCPServerScriptSource)
+	DefaultScriptRegistry.Register("update_project", updateProjectScriptSource)
+	DefaultScriptRegistry.Register("interpolate_prompt", interpolatePromptScript)
+}
 
 // getCheckMembershipScript returns the bundled check_membership script
-// Bundling is performed on first access and cached for subsequent calls
 func getCheckMembershipScript() string {
-	checkMembershipScriptOnce.Do(func() {
-		jsLog.Print("Bundling check_membership script")
-		sources := GetJavaScriptSources()
-		bundled, err := BundleJavaScriptFromSources(checkMembershipScriptSource, sources, "")
-		if err != nil {
-			jsLog.Printf("Failed to bundle check_membership script, using source as-is: %v", err)
-			// If bundling fails, use the source as-is
-			checkMembershipScript = checkMembershipScriptSource
-		} else {
-			jsLog.Printf("Successfully bundled check_membership script: %d bytes", len(bundled))
-			checkMembershipScript = bundled
-		}
-	})
-	return checkMembershipScript
+	return DefaultScriptRegistry.Get("check_membership")
 }
 
 //go:embed js/check_stop_time.cjs
@@ -71,28 +59,9 @@ var missingToolScript string
 //go:embed js/safe_outputs_mcp_server.cjs
 var safeOutputsMCPServerScriptSource string
 
-var (
-	safeOutputsMCPServerScript     string
-	safeOutputsMCPServerScriptOnce sync.Once
-)
-
 // getSafeOutputsMCPServerScript returns the bundled safe_outputs_mcp_server script
-// Bundling is performed on first access and cached for subsequent calls
 func getSafeOutputsMCPServerScript() string {
-	safeOutputsMCPServerScriptOnce.Do(func() {
-		jsLog.Print("Bundling safe_outputs_mcp_server script")
-		sources := GetJavaScriptSources()
-		bundled, err := BundleJavaScriptFromSources(safeOutputsMCPServerScriptSource, sources, "")
-		if err != nil {
-			jsLog.Printf("Failed to bundle safe_outputs_mcp_server script, using source as-is: %v", err)
-			// If bundling fails, use the source as-is
-			safeOutputsMCPServerScript = safeOutputsMCPServerScriptSource
-		} else {
-			jsLog.Printf("Successfully bundled safe_outputs_mcp_server script: %d bytes", len(bundled))
-			safeOutputsMCPServerScript = bundled
-		}
-	})
-	return safeOutputsMCPServerScript
+	return DefaultScriptRegistry.Get("safe_outputs_mcp_server")
 }
 
 //go:embed js/safe_outputs_tools.json
@@ -140,28 +109,9 @@ var updateActivationCommentScript string
 //go:embed js/update_project.cjs
 var updateProjectScriptSource string
 
-var (
-	updateProjectScript     string
-	updateProjectScriptOnce sync.Once
-)
-
 // getUpdateProjectScript returns the bundled update_project script
-// Bundling is performed on first access and cached for subsequent calls
 func getUpdateProjectScript() string {
-	updateProjectScriptOnce.Do(func() {
-		jsLog.Print("Bundling update_project script")
-		sources := GetJavaScriptSources()
-		bundled, err := BundleJavaScriptFromSources(updateProjectScriptSource, sources, "")
-		if err != nil {
-			jsLog.Printf("Failed to bundle update_project script, using source as-is: %v", err)
-			// If bundling fails, use the source as-is
-			updateProjectScript = updateProjectScriptSource
-		} else {
-			jsLog.Printf("Successfully bundled update_project script: %d bytes", len(bundled))
-			updateProjectScript = bundled
-		}
-	})
-	return updateProjectScript
+	return DefaultScriptRegistry.Get("update_project")
 }
 
 //go:embed js/generate_footer.cjs

pkg/workflow/script_registry.go

@@ -0,0 +1,190 @@
+// Package workflow provides a ScriptRegistry for managing JavaScript script bundling.
+//
+// # Script Registry Pattern
+//
+// The ScriptRegistry eliminates the repetitive sync.Once pattern found throughout
+// the codebase for lazy script bundling. Instead of declaring separate variables
+// and getter functions for each script, register scripts once and retrieve them
+// by name.
+//
+// # Before (repetitive pattern):
+//
+//	var (
+//	    createIssueScript     string
+//	    createIssueScriptOnce sync.Once
+//	)
+//
+//	func getCreateIssueScript() string {
+//	    createIssueScriptOnce.Do(func() {
+//	        sources := GetJavaScriptSources()
+//	        bundled, err := BundleJavaScriptFromSources(createIssueScriptSource, sources, "")
+//	        if err != nil {
+//	            createIssueScript = createIssueScriptSource
+//	        } else {
+//	            createIssueScript = bundled
+//	        }
+//	    })
+//	    return createIssueScript
+//	}
+//
+// # After (using registry):
+//
+//	// Registration at package init
+//	DefaultScriptRegistry.Register("create_issue", createIssueScriptSource)
+//
+//	// Usage anywhere
+//	script := DefaultScriptRegistry.Get("create_issue")
+//
+// # Benefits
+//
+//   - Eliminates ~15 lines of boilerplate per script (variable pair + getter function)
+//   - Centralizes bundling logic
+//   - Consistent error handling
+//   - Thread-safe lazy initialization
+//   - Easy to add new scripts
+package workflow
+
+import (
+	"sync"
+
+	"github.com/githubnext/gh-aw/pkg/logger"
+)
+
+var registryLog = logger.New("workflow:script_registry")
+
+// scriptEntry holds the source and bundled versions of a script
+type scriptEntry struct {
+	source  string
+	bundled string
+	once    sync.Once
+}
+
+// ScriptRegistry manages lazy bundling of JavaScript scripts.
+// It provides a centralized place to register source scripts and retrieve
+// bundled versions on-demand with caching.
+//
+// Thread-safe: All operations use internal synchronization.
+//
+// Usage:
+//
+//	registry := NewScriptRegistry()
+//	registry.Register("my_script", myScriptSource)
+//	bundled := registry.Get("my_script")
+type ScriptRegistry struct {
+	mu      sync.RWMutex
+	scripts map[string]*scriptEntry
+}
+
+// NewScriptRegistry creates a new empty script registry.
+func NewScriptRegistry() *ScriptRegistry {
+	return &ScriptRegistry{
+		scripts: make(map[string]*scriptEntry),
+	}
+}
+
+// Register adds a script source to the registry.
+// The script will be bundled lazily on first access via Get().
+//
+// Parameters:
+//   - name: Unique identifier for the script (e.g., "create_issue", "add_comment")
+//   - source: The raw JavaScript source code (typically from go:embed)
+//
+// If a script with the same name already exists, it will be overwritten.
+// This is useful for testing but should be avoided in production.
+func (r *ScriptRegistry) Register(name string, source string) {
+	r.mu.Lock()
+	defer r.mu.Unlock()
+
+	if registryLog.Enabled() {
+		registryLog.Printf("Registering script: %s (%d bytes)", name, len(source))
+	}
+
+	r.scripts[name] = &scriptEntry{
+		source: source,
+	}
+}
+
+// Get retrieves a bundled script by name.
+// Bundling is performed lazily on first access and cached for subsequent calls.
+//
+// If bundling fails, the original source is returned as a fallback.
+// If the script is not registered, an empty string is returned.
+//
+// Thread-safe: Multiple goroutines can call Get concurrently.
+func (r *ScriptRegistry) Get(name string) string {
+	r.mu.RLock()
+	entry, exists := r.scripts[name]
+	r.mu.RUnlock()
+
+	if !exists {
+		if registryLog.Enabled() {
+			registryLog.Printf("Script not found: %s", name)
+		}
+		return ""
+	}
+
+	entry.once.Do(func() {
+		if registryLog.Enabled() {
+			registryLog.Printf("Bundling script: %s", name)
+		}
+
+		sources := GetJavaScriptSources()
+		bundled, err := BundleJavaScriptFromSources(entry.source, sources, "")
+		if err != nil {
+			registryLog.Printf("Bundling failed for %s, using source as-is: %v", name, err)
+			entry.bundled = entry.source
+		} else {
+			if registryLog.Enabled() {
+				registryLog.Printf("Successfully bundled %s: %d bytes", name, len(bundled))
+			}
+			entry.bundled = bundled
+		}
+	})
+
+	return entry.bundled
+}
+
+// GetSource retrieves the original (unbundled) source for a script.
+// Useful for testing or when bundling is not needed.
+func (r *ScriptRegistry) GetSource(name string) string {
+	r.mu.RLock()
+	defer r.mu.RUnlock()
+
+	entry, exists := r.scripts[name]
+	if !exists {
+		return ""
+	}
+	return entry.source
+}
+
+// Has checks if a script is registered in the registry.
+func (r *ScriptRegistry) Has(name string) bool {
+	r.mu.RLock()
+	defer r.mu.RUnlock()
+
+	_, exists := r.scripts[name]
+	return exists
+}
+
+// Names returns a list of all registered script names.
+// Useful for debugging and testing.
+func (r *ScriptRegistry) Names() []string {
+	r.mu.RLock()
+	defer r.mu.RUnlock()
+
+	names := make([]string, 0, len(r.scripts))
+	for name := range r.scripts {
+		names = append(names, name)
+	}
+	return names
+}
+
+// DefaultScriptRegistry is the global script registry used by the workflow package.
+// Scripts are registered during package initialization via init() functions.
+var DefaultScriptRegistry = NewScriptRegistry()
+
+// GetScript retrieves a bundled script from the default registry.
+// This is a convenience function equivalent to DefaultScriptRegistry.Get(name).
+func GetScript(name string) string {
+	return DefaultScriptRegistry.Get(name)
+}