-
Notifications
You must be signed in to change notification settings - Fork 37
Consolidate shell escaping utilities into shell.go #12074
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ll.go Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot merge main and recompile |
|
@copilot merge main and recompile |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟 |
|
🎉 Yo ho ho! Changeset Generator found the treasure and completed successfully! ⚓💰 |
|
📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing... |
|
🎬 THE END — Smoke Claude MISSION: ACCOMPLISHED! The hero saves the day! ✨ |
|
GitHub MCP merged PRs: ✅ Add project field with campaign orchestration support to workflows; Add environment variable mirroring from runner to agent container
|
Smoke Test: Copilot - PASS ✅PRs from GitHub MCP:
Test Results:
Overall: PASS ✅ cc @pelikhan
|
|
📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤 |
Smoke Test Results: ClaudeStatus: ✅ PASS
Run: §21410404937
|
Shell Utility Consolidation - Complete ✅
This PR consolidates duplicate shell utility functions as identified in the semantic function clustering analysis (issue #11995).
Changes Completed
buildDockerCommandWithExpandableVarsfrommcp_utilities.gotoshell.goshellQuotecalls withshellEscapeArginmcp_setup_generator.gomcp_utilities_test.gotoshell_test.gomcp_utilities.gomcp_utilities.goandmcp_utilities_test.gofilesSummary
Successfully consolidated shell utility functions into
shell.go, eliminating code duplication and establishing a single source of truth for shell escaping operations.Files Changed: 5 files
pkg/workflow/mcp_utilities.go(44 lines removed)pkg/workflow/mcp_utilities_test.go(189 lines removed)pkg/workflow/shell.go(+32 lines - addedbuildDockerCommandWithExpandableVars)pkg/workflow/shell_test.go(+108 lines - added comprehensive tests)pkg/workflow/mcp_setup_generator.go(updated to useshellEscapeArg)Net Change: -95 lines of code (237 removed, 142 added)
Benefits
✅ Single source of truth for shell utilities
✅ Consistent shell escaping behavior across codebase
✅ Easier to maintain and test
✅ Reduced code duplication by ~30 lines
✅ All tests pass successfully
✅ Up to date with main branch
Original prompt
This section details on the original issue you should resolve
<issue_title>[refactor] Semantic Function Clustering Analysis: Shell Utilities and Helper Function Consolidation</issue_title>
<issue_description>Analysis of repository: githubnext/gh-aw
Analysis Date: 2026-01-27
Executive Summary
Comprehensive semantic function clustering analysis of 450 Go source files across the pkg/ directory reveals generally excellent code organization with clear naming conventions and domain-focused architecture. However, several high-impact consolidation opportunities were identified that could reduce code duplication and improve maintainability.
Key Findings:
Function Inventory
By Package
Clustering Results
Functions cluster strongly by semantic purpose across the codebase:
View Detailed Function Clustering Patterns
Compiler Method Distribution (pkg/workflow)
Analysis of 245 Compiler methods reveals clear semantic organization:
buildJobs,buildSafeOutputsJobs,buildMainJobparseIssuesConfig,parseSafeJobsConfig,parseDispatchWorkflowConfiggeneratePrompt,generateMCPSetup,generateEngineExecutionStepsextractNetworkPermissions,extractToolsTimeout,extractFeaturesvalidateEngine,validateStrictMode,validateRuntimePackagesaddSafeOutputEnvVars,addScheduleWarningprocessToolsAndMarkdown,processManualApprovalConfigurationMergeSafeOutputs,MergeTools,MergeNetworkPermissionsFile Naming Patterns
Creation Pattern (
create_*.go- 11 files):create_issue.go,create_pull_request.go,create_discussion.go,create_project.goUpdate Pattern (
update_*.go- 8 files):update_issue.go,update_pull_request.go,update_project.go,update_entity_helpers.goValidation Pattern (
*_validation.go- 27 files):docker_validation.go,npm_validation.go,sandbox_validation.godangerous_permissions_validation.go,firewall_validation.go,template_injection_validation.goengine_validation.go,runtime_validation.go,agent_validation.goHelper Pattern (
*_helpers.go- 12 files):config_helpers.go(284 lines) - Configuration parsing utilitieserror_helpers.go(58 lines) - Error formattingmap_helpers.go(69 lines) - Map manipulation (underutilized)validation_helpers.go(186 lines) - Reusable validation patternsCompiler Subsystems (
compiler_*.go- 18 files):compiler.go(669 lines - main compiler)compiler_safe_outputs*.go(8 files) - Safe outputs compilationcompiler_yaml*.go(4 files) - YAML generationcompiler_orchestrator*.go(4 files) - Orchestration logicIdentified Issues
1. Duplicate Shell Utility Functions
Issue: Shell escaping/quoting functions duplicated across 2 files
Duplicate #1: Shell Argument Escaping
Occurrences:
shellEscapeArg(arg string)shellQuote(s string)Comparison: