Refactor script registry pattern - eliminate 27 sync.Once patterns#4768
Merged
Refactor script registry pattern - eliminate 27 sync.Once patterns#4768
Conversation
- Create ScriptRegistry abstraction in script_registry.go (190 LOC) - Reduce scripts.go from 657 LOC to 270 LOC (-59%) - Reduce js.go by ~40 LOC of boilerplate - Net reduction: ~330 lines of duplicated code - All 27 getter functions now use registry.Get() - Lazy bundling with caching maintained - Full backward compatibility with existing API Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
- Remove cross-file implementation detail comment - Fix documentation accuracy (15 LOC per script, not 20) Co-authored-by: mnkiefer <8320933+mnkiefer@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor semantic function clustering analysis results
Refactor script registry pattern - eliminate 27 sync.Once patterns
Nov 25, 2025
Contributor
There was a problem hiding this comment.
Pull request overview
This PR successfully eliminates 27 repetitive sync.Once patterns by introducing a centralized ScriptRegistry abstraction for lazy JavaScript script bundling. The refactoring reduces code by ~330 lines while maintaining full backward compatibility and improving maintainability.
Key Changes:
- Introduces
ScriptRegistrypattern to centralize lazy bundling logic with thread-safe caching - Refactors
scripts.gofrom 657 to 270 lines (-59%) by replacing individualsync.Oncepatterns with registry registration - Refactors
js.goby migrating 3 additional script patterns to the same registry system
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/workflow/script_registry.go | New abstraction providing thread-safe lazy bundling with sync.Once semantics internally; includes Register() and Get() API with fallback error handling |
| pkg/workflow/script_registry_test.go | Comprehensive test coverage including concurrent access, overwrite behavior, and edge cases |
| pkg/workflow/scripts.go | Refactored to use ScriptRegistry; init() registers 26 scripts, getter functions reduced to one-line registry calls |
| pkg/workflow/js.go | Refactored to use ScriptRegistry; init() registers 4 scripts (check_membership, safe_outputs_mcp_server, update_project, interpolate_prompt) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
Adds TestScriptRegistry_Overwrite_AfterGet to verify that: 1. Get() triggers bundling on initial registration 2. Overwriting with new source replaces the entry 3. Get() returns bundled version of new source after overwrite Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
|
✅ Agentic Changeset Generator completed successfully. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
package_extraction.go(used by npm.go, pip.go, dependabot.go)BuildStandardNpmEngineInstallStepsfor engine installation (used by Claude, Codex, Copilot)buildStandardSafeOutputEnvVarsfor safe output jobs (used by 16+ files)buildSafeOutputJobfor safe output job scaffolding (used by 22+ files)ScriptRegistryabstraction inscript_registry.go(190 LOC)scripts.gofrom 657 LOC to 270 LOC (-59% reduction)js.goto use registry, removing ~40 LOC of boilerplatesync.OncepatternsOriginal prompt
This section details on the original issue you should resolve
<issue_title>[refactor] 🔧 Semantic Function Clustering Analysis - November 2025</issue_title>
<issue_description># 🔧 Semantic Function Clustering Analysis
Analysis Date: November 25, 2025
Repository: githubnext/gh-aw
Workflow Run: §19662572763
Executive Summary
Analyzed 238 non-test Go source files (65,911 lines of code) across the gh-aw repository using semantic analysis. The codebase demonstrates strong architectural patterns in validation files (14 dedicated validation files), configuration helpers, and engine organization. Key findings reveal high-value consolidation opportunities in safe output builders, package extraction patterns, and script loading mechanisms.
Key Findings:
pkg/workflow(142 files, 60%),pkg/cli(75 files, 31%)Full Refactoring Analysis Report
Repository Structure
Package Distribution
pkg/workflowpkg/clipkg/parserpkg/consoleTotal: 238 non-test Go files, 65,911 lines of code
Well-Organized Patterns ✅
1. Validation Files (Exemplary Organization)
14 dedicated validation files demonstrate best-in-class separation of concerns:
✅ Each validation domain has its own file
✅ Clear, predictable naming convention
✅ Easy to locate and extend
✅ Follows "one file per feature" principle
2. Configuration Helpers (Model Pattern)
The file
config_helpers.gois an exemplar of consolidation done right:Impact: These helpers eliminate ~500 lines of duplication across the codebase.
✅ This is the model pattern that should be extended to other areas
3. Engine Implementation Files (Clean Architecture)
5 engine files with consistent structure:
Supporting files:
engine_helpers.go- Shared utilitiesengine_validation.go- Engine validationengine_output.go- Output handlingengine_network_hooks.go- Network hooksengine_firewall_support.go- Firewall support✅ One file per engine type
✅ Clear base interface pattern (BaseEngine)
✅ Appropriate helper file usage
✅ Good separation of concerns
4. Safe Output Files Pattern
19 safe output files follow consistent
*_*.gopattern:Create operations (6 files):
Close operations (3 files):
Other operations (10 files):
✅ One file per safe output type
✅ Predictable structure within each file
✅ Easy to locate functionality
5. CLI Command Structure (Excellent)
CLI commands follow clear
*_command.gopattern: