Skip to content

Commit 8fa628b

Browse files
authored
Refactor script registry pattern - eliminate 27 sync.Once patterns (#4768)
1 parent 7c246d0 commit 8fa628b

5 files changed

Lines changed: 435 additions & 520 deletions

File tree

.changeset/patch-add-changeset-automation.md

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pkg/workflow/js.go

Lines changed: 10 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import (
44
_ "embed"
55
"fmt"
66
"strings"
7-
"sync"
87

98
"github.com/githubnext/gh-aw/pkg/logger"
109
)
@@ -23,28 +22,17 @@ var addReactionAndEditCommentScript string
2322
//go:embed js/check_membership.cjs
2423
var checkMembershipScriptSource string
2524

26-
var (
27-
checkMembershipScript string
28-
checkMembershipScriptOnce sync.Once
29-
)
25+
// init registers scripts from js.go with the DefaultScriptRegistry
26+
func init() {
27+
DefaultScriptRegistry.Register("check_membership", checkMembershipScriptSource)
28+
DefaultScriptRegistry.Register("safe_outputs_mcp_server", safeOutputsMCPServerScriptSource)
29+
DefaultScriptRegistry.Register("update_project", updateProjectScriptSource)
30+
DefaultScriptRegistry.Register("interpolate_prompt", interpolatePromptScript)
31+
}
3032

3133
// getCheckMembershipScript returns the bundled check_membership script
32-
// Bundling is performed on first access and cached for subsequent calls
3334
func getCheckMembershipScript() string {
34-
checkMembershipScriptOnce.Do(func() {
35-
jsLog.Print("Bundling check_membership script")
36-
sources := GetJavaScriptSources()
37-
bundled, err := BundleJavaScriptFromSources(checkMembershipScriptSource, sources, "")
38-
if err != nil {
39-
jsLog.Printf("Failed to bundle check_membership script, using source as-is: %v", err)
40-
// If bundling fails, use the source as-is
41-
checkMembershipScript = checkMembershipScriptSource
42-
} else {
43-
jsLog.Printf("Successfully bundled check_membership script: %d bytes", len(bundled))
44-
checkMembershipScript = bundled
45-
}
46-
})
47-
return checkMembershipScript
35+
return DefaultScriptRegistry.Get("check_membership")
4836
}
4937

5038
//go:embed js/check_stop_time.cjs
@@ -71,28 +59,9 @@ var missingToolScript string
7159
//go:embed js/safe_outputs_mcp_server.cjs
7260
var safeOutputsMCPServerScriptSource string
7361

74-
var (
75-
safeOutputsMCPServerScript string
76-
safeOutputsMCPServerScriptOnce sync.Once
77-
)
78-
7962
// getSafeOutputsMCPServerScript returns the bundled safe_outputs_mcp_server script
80-
// Bundling is performed on first access and cached for subsequent calls
8163
func getSafeOutputsMCPServerScript() string {
82-
safeOutputsMCPServerScriptOnce.Do(func() {
83-
jsLog.Print("Bundling safe_outputs_mcp_server script")
84-
sources := GetJavaScriptSources()
85-
bundled, err := BundleJavaScriptFromSources(safeOutputsMCPServerScriptSource, sources, "")
86-
if err != nil {
87-
jsLog.Printf("Failed to bundle safe_outputs_mcp_server script, using source as-is: %v", err)
88-
// If bundling fails, use the source as-is
89-
safeOutputsMCPServerScript = safeOutputsMCPServerScriptSource
90-
} else {
91-
jsLog.Printf("Successfully bundled safe_outputs_mcp_server script: %d bytes", len(bundled))
92-
safeOutputsMCPServerScript = bundled
93-
}
94-
})
95-
return safeOutputsMCPServerScript
64+
return DefaultScriptRegistry.Get("safe_outputs_mcp_server")
9665
}
9766

9867
//go:embed js/safe_outputs_tools.json
@@ -140,28 +109,9 @@ var updateActivationCommentScript string
140109
//go:embed js/update_project.cjs
141110
var updateProjectScriptSource string
142111

143-
var (
144-
updateProjectScript string
145-
updateProjectScriptOnce sync.Once
146-
)
147-
148112
// getUpdateProjectScript returns the bundled update_project script
149-
// Bundling is performed on first access and cached for subsequent calls
150113
func getUpdateProjectScript() string {
151-
updateProjectScriptOnce.Do(func() {
152-
jsLog.Print("Bundling update_project script")
153-
sources := GetJavaScriptSources()
154-
bundled, err := BundleJavaScriptFromSources(updateProjectScriptSource, sources, "")
155-
if err != nil {
156-
jsLog.Printf("Failed to bundle update_project script, using source as-is: %v", err)
157-
// If bundling fails, use the source as-is
158-
updateProjectScript = updateProjectScriptSource
159-
} else {
160-
jsLog.Printf("Successfully bundled update_project script: %d bytes", len(bundled))
161-
updateProjectScript = bundled
162-
}
163-
})
164-
return updateProjectScript
114+
return DefaultScriptRegistry.Get("update_project")
165115
}
166116

167117
//go:embed js/generate_footer.cjs

pkg/workflow/script_registry.go

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
// Package workflow provides a ScriptRegistry for managing JavaScript script bundling.
2+
//
3+
// # Script Registry Pattern
4+
//
5+
// The ScriptRegistry eliminates the repetitive sync.Once pattern found throughout
6+
// the codebase for lazy script bundling. Instead of declaring separate variables
7+
// and getter functions for each script, register scripts once and retrieve them
8+
// by name.
9+
//
10+
// # Before (repetitive pattern):
11+
//
12+
// var (
13+
// createIssueScript string
14+
// createIssueScriptOnce sync.Once
15+
// )
16+
//
17+
// func getCreateIssueScript() string {
18+
// createIssueScriptOnce.Do(func() {
19+
// sources := GetJavaScriptSources()
20+
// bundled, err := BundleJavaScriptFromSources(createIssueScriptSource, sources, "")
21+
// if err != nil {
22+
// createIssueScript = createIssueScriptSource
23+
// } else {
24+
// createIssueScript = bundled
25+
// }
26+
// })
27+
// return createIssueScript
28+
// }
29+
//
30+
// # After (using registry):
31+
//
32+
// // Registration at package init
33+
// DefaultScriptRegistry.Register("create_issue", createIssueScriptSource)
34+
//
35+
// // Usage anywhere
36+
// script := DefaultScriptRegistry.Get("create_issue")
37+
//
38+
// # Benefits
39+
//
40+
// - Eliminates ~15 lines of boilerplate per script (variable pair + getter function)
41+
// - Centralizes bundling logic
42+
// - Consistent error handling
43+
// - Thread-safe lazy initialization
44+
// - Easy to add new scripts
45+
package workflow
46+
47+
import (
48+
"sync"
49+
50+
"github.com/githubnext/gh-aw/pkg/logger"
51+
)
52+
53+
var registryLog = logger.New("workflow:script_registry")
54+
55+
// scriptEntry holds the source and bundled versions of a script
56+
type scriptEntry struct {
57+
source string
58+
bundled string
59+
once sync.Once
60+
}
61+
62+
// ScriptRegistry manages lazy bundling of JavaScript scripts.
63+
// It provides a centralized place to register source scripts and retrieve
64+
// bundled versions on-demand with caching.
65+
//
66+
// Thread-safe: All operations use internal synchronization.
67+
//
68+
// Usage:
69+
//
70+
// registry := NewScriptRegistry()
71+
// registry.Register("my_script", myScriptSource)
72+
// bundled := registry.Get("my_script")
73+
type ScriptRegistry struct {
74+
mu sync.RWMutex
75+
scripts map[string]*scriptEntry
76+
}
77+
78+
// NewScriptRegistry creates a new empty script registry.
79+
func NewScriptRegistry() *ScriptRegistry {
80+
return &ScriptRegistry{
81+
scripts: make(map[string]*scriptEntry),
82+
}
83+
}
84+
85+
// Register adds a script source to the registry.
86+
// The script will be bundled lazily on first access via Get().
87+
//
88+
// Parameters:
89+
// - name: Unique identifier for the script (e.g., "create_issue", "add_comment")
90+
// - source: The raw JavaScript source code (typically from go:embed)
91+
//
92+
// If a script with the same name already exists, it will be overwritten.
93+
// This is useful for testing but should be avoided in production.
94+
func (r *ScriptRegistry) Register(name string, source string) {
95+
r.mu.Lock()
96+
defer r.mu.Unlock()
97+
98+
if registryLog.Enabled() {
99+
registryLog.Printf("Registering script: %s (%d bytes)", name, len(source))
100+
}
101+
102+
r.scripts[name] = &scriptEntry{
103+
source: source,
104+
}
105+
}
106+
107+
// Get retrieves a bundled script by name.
108+
// Bundling is performed lazily on first access and cached for subsequent calls.
109+
//
110+
// If bundling fails, the original source is returned as a fallback.
111+
// If the script is not registered, an empty string is returned.
112+
//
113+
// Thread-safe: Multiple goroutines can call Get concurrently.
114+
func (r *ScriptRegistry) Get(name string) string {
115+
r.mu.RLock()
116+
entry, exists := r.scripts[name]
117+
r.mu.RUnlock()
118+
119+
if !exists {
120+
if registryLog.Enabled() {
121+
registryLog.Printf("Script not found: %s", name)
122+
}
123+
return ""
124+
}
125+
126+
entry.once.Do(func() {
127+
if registryLog.Enabled() {
128+
registryLog.Printf("Bundling script: %s", name)
129+
}
130+
131+
sources := GetJavaScriptSources()
132+
bundled, err := BundleJavaScriptFromSources(entry.source, sources, "")
133+
if err != nil {
134+
registryLog.Printf("Bundling failed for %s, using source as-is: %v", name, err)
135+
entry.bundled = entry.source
136+
} else {
137+
if registryLog.Enabled() {
138+
registryLog.Printf("Successfully bundled %s: %d bytes", name, len(bundled))
139+
}
140+
entry.bundled = bundled
141+
}
142+
})
143+
144+
return entry.bundled
145+
}
146+
147+
// GetSource retrieves the original (unbundled) source for a script.
148+
// Useful for testing or when bundling is not needed.
149+
func (r *ScriptRegistry) GetSource(name string) string {
150+
r.mu.RLock()
151+
defer r.mu.RUnlock()
152+
153+
entry, exists := r.scripts[name]
154+
if !exists {
155+
return ""
156+
}
157+
return entry.source
158+
}
159+
160+
// Has checks if a script is registered in the registry.
161+
func (r *ScriptRegistry) Has(name string) bool {
162+
r.mu.RLock()
163+
defer r.mu.RUnlock()
164+
165+
_, exists := r.scripts[name]
166+
return exists
167+
}
168+
169+
// Names returns a list of all registered script names.
170+
// Useful for debugging and testing.
171+
func (r *ScriptRegistry) Names() []string {
172+
r.mu.RLock()
173+
defer r.mu.RUnlock()
174+
175+
names := make([]string, 0, len(r.scripts))
176+
for name := range r.scripts {
177+
names = append(names, name)
178+
}
179+
return names
180+
}
181+
182+
// DefaultScriptRegistry is the global script registry used by the workflow package.
183+
// Scripts are registered during package initialization via init() functions.
184+
var DefaultScriptRegistry = NewScriptRegistry()
185+
186+
// GetScript retrieves a bundled script from the default registry.
187+
// This is a convenience function equivalent to DefaultScriptRegistry.Get(name).
188+
func GetScript(name string) string {
189+
return DefaultScriptRegistry.Get(name)
190+
}

0 commit comments

Comments
 (0)