Skip to content

Consolidate shell escaping utilities into shell.go - #12074

Merged
pelikhan merged 6 commits into
mainfrom
copilot/refactor-shell-utilities
Jan 27, 2026
Merged

Consolidate shell escaping utilities into shell.go#12074
pelikhan merged 6 commits into
mainfrom
copilot/refactor-shell-utilities

Conversation

CopilotAI commented Jan 27, 2026

Copy link
Copy Markdown
Contributor

Shell Utility Consolidation - Complete ✅

This PR consolidates duplicate shell utility functions as identified in the semantic function clustering analysis (issue #11995).

Changes Completed

  • Move buildDockerCommandWithExpandableVars from mcp_utilities.go to shell.go
  • Replace shellQuote calls with shellEscapeArg in mcp_setup_generator.go
  • Move tests from mcp_utilities_test.go to shell_test.go
  • Delete duplicate functions from mcp_utilities.go
  • Delete empty mcp_utilities.go and mcp_utilities_test.go files
  • Verify all shell tests pass (13 tests, 100% passing)
  • Verify formatting and linting pass
  • Verify build succeeds
  • Merge with main branch (commits 9526e1d, bc6aed9, c689053)
  • Recompile all workflows

Summary

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

  • Deleted: pkg/workflow/mcp_utilities.go (44 lines removed)
  • Deleted: pkg/workflow/mcp_utilities_test.go (189 lines removed)
  • Modified: pkg/workflow/shell.go (+32 lines - added buildDockerCommandWithExpandableVars)
  • Modified: pkg/workflow/shell_test.go (+108 lines - added comprehensive tests)
  • Modified: pkg/workflow/mcp_setup_generator.go (updated to use shellEscapeArg)

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:

  • 📊 450 non-test Go files analyzed across 18 packages
  • Strong semantic clustering through consistent function prefixes (build*, parse*, validate*, extract*, generate*)
  • ⚠️3 duplicate shell utility functions found across 2 files
  • ⚠️6+ scattered YAML writing functions could be consolidated
  • ℹ️ 199 validate functions* across 77 files show good validation architecture
  • ℹ️ 30+ parse*Config functions demonstrate consistent parsing patterns

Function Inventory

By Package

PackageFilesPrimary PurposeOrganization Quality
workflow245Core workflow compilation, safe outputs, MCP, engines✅ Excellent domain grouping
cli158CLI commands, codemods, log processing✅ Feature-oriented structure
parser29YAML/frontmatter parsing, schema validation✅ Clear functional boundaries
campaign13Campaign orchestration✅ Well-contained
console11Terminal UI/formatting✅ Focused utility package
Utilities13stringutil, sliceutil, mathutil, etc.✅ Single-purpose packages

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:

PrefixCountPurposeExamples
build*53Job/step/config buildingbuildJobs, buildSafeOutputsJobs, buildMainJob
parse*45Configuration parsingparseIssuesConfig, parseSafeJobsConfig, parseDispatchWorkflowConfig
generate*37YAML/script generationgeneratePrompt, generateMCPSetup, generateEngineExecutionSteps
extract*35Data extractionextractNetworkPermissions, extractToolsTimeout, extractFeatures
validate*26Validation logicvalidateEngine, validateStrictMode, validateRuntimePackages
add*16Config/env additionsaddSafeOutputEnvVars, addScheduleWarning
process*5Processing operationsprocessToolsAndMarkdown, processManualApprovalConfiguration
merge*5Config mergingMergeSafeOutputs, MergeTools, MergeNetworkPermissions

File Naming Patterns

Creation Pattern (create_*.go - 11 files):

  • create_issue.go, create_pull_request.go, create_discussion.go, create_project.go
  • Status: ✅ Well-organized, one creation type per file

Update Pattern (update_*.go - 8 files):

  • update_issue.go, update_pull_request.go, update_project.go, update_entity_helpers.go
  • Status: ✅ Consistent naming, clear purpose

Validation Pattern (*_validation.go - 27 files):

  • Domain-specific: docker_validation.go, npm_validation.go, sandbox_validation.go
  • Security: dangerous_permissions_validation.go, firewall_validation.go, template_injection_validation.go
  • Architecture: engine_validation.go, runtime_validation.go, agent_validation.go
  • Status: ✅ Excellent domain-focused validation architecture

Helper Pattern (*_helpers.go - 12 files):

  • config_helpers.go (284 lines) - Configuration parsing utilities
  • error_helpers.go (58 lines) - Error formatting
  • map_helpers.go (69 lines) - Map manipulation (underutilized)
  • validation_helpers.go (186 lines) - Reusable validation patterns
  • Status: ⚠️ Some functions scattered outside helper files

Compiler Subsystems (compiler_*.go - 18 files):

  • compiler.go (669 lines - main compiler)
  • compiler_safe_outputs*.go (8 files) - Safe outputs compilation
  • compiler_yaml*.go (4 files) - YAML generation
  • compiler_orchestrator*.go (4 files) - Orchestration logic
  • Status: ✅ Clear subsystem boundaries

Identified Issues

1. Duplicate Shell Utility Functions

Issue: Shell escaping/quoting functions duplicated across 2 files

Duplicate #1: Shell Argument Escaping

Occurrences:

  1. pkg/workflow/shell.go:26 - shellEscapeArg(arg string)
  2. pkg/workflow/mcp_utilities.go:8 - shellQuote(s string)

Comparison:

// sh...</details><!--STARTCOPILOTCODINGAGENTSUFFIX-->-Fixesgithubnext/gh-aw#12041<!--STARTCOPILOTCODINGAGENTTIPS-->---
💬 We'd loveyourinput!ShareyourthoughtsonCopilotcodingagentinour [2minutesurvey](https://gh.io/copilot-coding-agent-survey).---
## Changeset-**Type**: patch-**Description**: Consolidatedshellescapingutilitiesinto`shell.go`, removedtheduplicatehelpers, andalignedthegenerator/testswiththeunifiedhelper.
>Ahoy!Thistreasurewascraftedby [🏴‍☠️ ChangesetGenerator](https://github.com/githubnext/gh-aw/actions/runs/21410404899)

…ll.go
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
CopilotAI changed the title [WIP] Refactor shell utilities and consolidate helper functionsConsolidate shell escaping utilities into shell.goJan 27, 2026
CopilotAI requested a review from pelikhanJanuary 27, 2026 18:19
@pelikhan
pelikhan marked this pull request as ready for review January 27, 2026 18:28
@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot merge main and recompile

CopilotAI commented Jan 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot merge main and recompile

Done! The branch has already merged main (4126801) and recompilation completed successfully - all 140 workflows compiled with no changes needed.

@pelikhan

Copy link
Copy Markdown
Collaborator

@copilot merge main and recompile

CopilotAIand others added 2 commits January 27, 2026 18:55
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>

CopilotAI commented Jan 27, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot merge main and recompile

Done! Merged main (commits 9526e1d, bc6aed9, c689053) and recompiled all workflows. The security-guard workflow was updated with environment variable mirroring. Commit: 9da8f7e

@github-actions

github-actionsBot commented Jan 27, 2026

Copy link
Copy Markdown
Contributor

✨ The prophecy is fulfilled... Smoke Codex has completed its mystical journey. The stars align. 🌟

@github-actions

github-actionsBot commented Jan 27, 2026

Copy link
Copy Markdown
Contributor

🎉 Yo ho ho! Changeset Generator found the treasure and completed successfully! ⚓💰

@github-actions

Copy link
Copy Markdown
Contributor

📰 BREAKING: Smoke Copilot is now investigating this pull request. Sources say the story is developing...

@github-actions

github-actionsBot commented Jan 27, 2026

Copy link
Copy Markdown
Contributor

🎬 THE ENDSmoke ClaudeMISSION: ACCOMPLISHED! The hero saves the day! ✨

@github-actions

Copy link
Copy Markdown
Contributor

GitHub MCP merged PRs: ✅ Add project field with campaign orchestration support to workflows; Add environment variable mirroring from runner to agent container
safeinputs-gh pr list: ✅ [WIP] Add noop safe output type as a built-in tool; [WIP] Migrate deprecated timeout constants to time.Duration types
Serena activate: ✅
Playwright title contains GitHub: ✅
Tavily search: ✅
File write: ✅
Bash cat: ✅
Discussion query + comment: ✅
Overall: PASS

AI generated by Smoke Codex

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test: Copilot - PASS ✅

PRs from GitHub MCP:

Test Results:

  • ✅ GitHub MCP
  • ✅ Safe Inputs GH CLI
  • ✅ Serena MCP
  • ✅ Playwright
  • ✅ File Writing
  • ✅ Bash Tool
  • ✅ Discussion Interaction

Overall: PASS ✅

§21410404909

cc @pelikhan@Copilot

AI generated by Smoke Copilot

@github-actions

Copy link
Copy Markdown
Contributor

📰 VERDICT: Smoke Copilot has concluded. All systems operational. This is a developing story. 🎤

@github-actions

Copy link
Copy Markdown
Contributor

Smoke Test Results: Claude

Status: ✅ PASS

TestResult
GitHub MCP
Safe Inputs GH
Serena MCP
Make Build
Playwright
Tavily Search
File Writing
Bash Tool
Discussion Comment

Run: §21410404937

AI generated by Smoke Claude

@pelikhan
pelikhan merged commit 1a13f11 into mainJan 27, 2026
90 checks passed
@pelikhan
pelikhan deleted the copilot/refactor-shell-utilities branch January 27, 2026 19:15
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@pelikhan