Skip to content

refactor(workflow): resolve 8 semantic function clustering issues in pkg/workflow#18435

Closed
Copilot wants to merge 4 commits intomainfrom
copilot/refactor-semantic-function-clustering-36dd6f72-4e4a-4f91-b42e-0d519359b1aa
Closed

refactor(workflow): resolve 8 semantic function clustering issues in pkg/workflow#18435
Copilot wants to merge 4 commits intomainfrom
copilot/refactor-semantic-function-clustering-36dd6f72-4e4a-4f91-b42e-0d519359b1aa

Conversation

Copy link
Contributor

Copilot AI commented Feb 26, 2026

Automated semantic analysis of pkg/workflow identified misplaced functions, near-duplicate patterns, and missing helpers causing repetition across 15+ call sites. This PR addresses all 8 actionable findings.

Function Relocations

  • isValidFullSHA + shaRegex: features_validation.govalidation_helpers.go (general-purpose utility; was also called from unrelated action_pins.go)
  • collectPackagesFromWorkflow: runtime_validation.gopackage_extraction.go (pure extraction helper with no validation logic)
  • validateTargetRepoSlug: validation_helpers.gosafe_outputs_target_validation.go; return type changed from bool to error to match the rest of the file; 6 callers updated

New Shared Helpers

  • emitWarning(msg string) on *Compiler (compiler.go): eliminates 10+ repeated two-liner fmt.Fprintln(os.Stderr, console.FormatWarningMessage(msg)) + c.IncrementWarningCount() across 5 files
  • findWriteScopesForPolicy(permissions *Permissions, scopes []PermissionScope) []PermissionScope in permissions_operations.go: shared by findWritePermissions (dangerous_permissions) and validateStrictPermissions (strict_mode), which previously used two divergent implementations for the same semantic query
  • validateMountStrings(mounts []string, docsURL string) []string in validation_helpers.go: shared core loop between validateMCPMountsSyntax and validateMountsSyntax (both iterate mounts, call validateMountStringFormat, and distinguish format vs mode errors via the same sentinel)

Error Aggregation Consistency

validateNpxPackages, validateContainerImages, and validateRuntimePackages migrated from manual var errors []string / append / strings.Join / NewValidationError to NewErrorCollector, matching the pattern already used in dispatch_workflow_validation.go and strict_mode_validation.go.

File Consolidation

network_firewall_validation.go (single function: validateNetworkFirewallConfig) merged into firewall_validation.go; the two files covered the same NetworkPermissions/Firewall struct domain.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/graphql
    • Triggering command: /usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw (http block)
    • Triggering command: /usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw GOMOD GOMODCACHE go env -json GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh /usr/bin/gh api graphql -f query=query($owner: String!, $name: String!) { repository(owner: $owner, name: $name) { hasDiscussionsEnabled } } -f owner=github -f name=gh-aw GO111MODULE /home/REDACTED/go/bin/bash node js/f�� i/install.sh bash /opt/hostedtoolcache/node/24.13.1/x64/bin/node --noprofile GOPROXY /usr/bin/git node (http block)
  • https://api.github.com/repos/actions/ai-inference/git/ref/tags/v1
    • Triggering command: /usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha --noprofile cfg 64/pkg/tool/linux_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/11bd71901bbe5b1630ceea73d27597364c9af683
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/11bd71901bbe5b1630ceea73d27597364c9af683 --jq .object.sha -json GO111MODULE $name) { hasDiscussionsEnabled } } GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE sh (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v3
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq .object.sha --noprofile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq .object.sha --show-toplevel x_amd64/vet /usr/bin/git k/gh-aw/gh-aw/pkgit k/gh-aw/gh-aw/pkrev-parse x_amd64/vet git rev-�� --show-toplevel x_amd64/vet /usr/bin/git o actions/setup-git --others 64/pkg/tool/linu--show-toplevel git (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE sh (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v4 --jq .object.sha copilot/refactor-semantic-functi-s (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v4 --jq .object.sha git status --porcelain --ignore-submodules | head -n 10 (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v4 --jq .object.sha -unreachable=false /tmp/go-build836097165/b047/vet.cfg 097165/b299/vet.cfg dd6f72-4e4a-4f91git (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v5
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha --noprofile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha e=false (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha -unreachable=false /tmp/go-build836097165/b096/vet.cfg /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/vet --get-regexp --local ache/go/1.25.0/x--show-toplevel /opt/hostedtoolcache/go/1.25.0/x228885d3daf8178a3795af6ad60d1fc7d4f3fd65..HEAD -ato�� -bool -buildtags /opt/hostedtoolcache/node/24.13.1/x64/bin/bash -errorsas -ifaceassert -nilfunc bash (http block)
  • https://api.github.com/repos/actions/checkout/git/ref/tags/v6
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha e=false GO111MODULE 64/bin/go GOINSECURE %H %ct %D GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE sh (http block)
  • https://api.github.com/repos/actions/github-script/git/ref/tags/v7
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v7 --jq .object.sha GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD GOMODCACHE go ache�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD ode-gyp-bin/node-json go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v7 --jq .object.sha GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD FFiles,SFiles,Sw-json go ache�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v7 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node /opt�� prettier --check 64/bin/go **/*.ts **/*.json --ignore-path go (http block)
  • https://api.github.com/repos/actions/github-script/git/ref/tags/v8
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha git status --porcelain --ignore-submodules | head -n 10 (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha git status --porcelain --ignore-submodules | head -n 10 rty 64/pkg/tool/linux_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha -unreachable=false /tmp/go-build836097165/b043/vet.cfg /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/vet d -n 10 (http block)
  • https://api.github.com/repos/actions/setup-go/git/ref/tags/4dc6199c7b1a012772edbd06daecab0f50c9053c
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/4dc6199c7b1a012772edbd06daecab0f50c9053c --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node /opt�� prettier --check r: $owner, name: $name) { hasDiscussionsEnabled } } **/*.ts **/*.json --ignore-path go (http block)
  • https://api.github.com/repos/actions/setup-go/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v4 --jq .object.sha on k/gh-aw/gh-aw/pkg/workflow/add_reviewer.go 64/pkg/tool/linux_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v4 --jq .object.sha --show-toplevel 64/pkg/tool/linux_amd64/vet /usr/bin/tr --noprofile cfg 64/pkg/tool/linu--show-toplevel tr [:up�� [:lower:] 64/pkg/tool/linux_amd64/vet 1/x64/bin/node --noprofile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v4 --jq .object.sha -json GOMOD 64/bin/go tierignore GO111MODULE 64/bin/go go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/actions/setup-go/git/ref/tags/v5
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v5 --jq .object.sha 32b091ecc75c3eb4GOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE ache/go/1.25.0/xGOPROXY env 110033/b402/_pkgGOSUMDB GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v5 --jq .object.sha d7ab9ec12dbe903eGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE ache/go/1.25.0/xGOPROXY env 110033/b408/_pkgGOSUMDB GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/actions/setup-go/git/ref/tags/v6
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v6 --jq .object.sha GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v6 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD erignore go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v6 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go ache�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE erignore (http block)
  • https://api.github.com/repos/actions/setup-node/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v4 --jq .object.sha from .github/aw to pkg/workflow/data/action_pins.json..." (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v4 --jq .object.sha tags/v5 64/pkg/tool/linux_amd64/vet /usr/bin/git --noprofile cfg 64/pkg/tool/linu--show-toplevel git diff�� --stat 228885d3daf8178a3795af6ad60d1fc7d4f3fd65..HEAD 1/x64/bin/node --noprofile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v4 --jq .object.sha GOPATH go 64/bin/go tierignore GO111MODULE 64/bin/go go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/actions/setup-node/git/ref/tags/v6
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v6 --jq .object.sha GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v6 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env heck '**/*.cjs' GOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v6 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node /hom�� --check **/*.cjs 64/bin/go **/*.json --ignore-path ../../../.prettiinspect go (http block)
  • https://api.github.com/repos/astral-sh/setup-uv/git/ref/tags/eac588ad8def6316056a12d4907a9d4d84ff7a3b
    • Triggering command: /usr/bin/gh gh api /repos/astral-sh/setup-uv/git/ref/tags/eac588ad8def6316056a12d4907a9d4d84ff7a3b --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go ache�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/docker/build-push-action/git/ref/tags/v6
    • Triggering command: /usr/bin/gh gh api /repos/docker/build-push-action/git/ref/tags/v6 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node /opt�� prettier --check 64/bin/go **/*.ts **/*.json --ignore-path node (http block)
    • Triggering command: /usr/bin/gh gh api /repos/docker/build-push-action/git/ref/tags/v6 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node /opt�� prettier --check r: $owner, name: $name) { hasDiscussionsEnabled } } **/*.ts **/*.json --ignore-path git (http block)
  • https://api.github.com/repos/docker/login-action/git/ref/tags/v3
    • Triggering command: /usr/bin/gh gh api /repos/docker/login-action/git/ref/tags/v3 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node /opt�� prettier --check r: $owner, name: $name) { hasDiscussionsEnabled } } **/*.ts **/*.json --ignore-path node (http block)
    • Triggering command: /usr/bin/gh gh api /repos/docker/login-action/git/ref/tags/v3 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node /opt�� prettier --check r: $owner, name: $name) { hasDiscussionsEnabled } } **/*.ts **/*.json --ignore-path golangci-lint (http block)
  • https://api.github.com/repos/docker/metadata-action/git/ref/tags/v5
    • Triggering command: /usr/bin/gh gh api /repos/docker/metadata-action/git/ref/tags/v5 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node /opt�� prettier --check 64/bin/go **/*.ts **/*.json --ignore-path /bin/sh (http block)
    • Triggering command: /usr/bin/gh gh api /repos/docker/metadata-action/git/ref/tags/v5 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE sh -c "prettier" --check '**/*.cjs' '*GOINSECURE GOPROXY 64/bin/go GOSUMDB GOWORK 64/bin/go git (http block)
  • https://api.github.com/repos/docker/setup-buildx-action/git/ref/tags/v3
    • Triggering command: /usr/bin/gh gh api /repos/docker/setup-buildx-action/git/ref/tags/v3 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE sh -c npx prettier --check '**/*.cjs' GOINSECURE GOPROXY r: $owner, name: $name) { hasDiscussionsEnabled } } GOSUMDB GOWORK 64/bin/go node (http block)
    • Triggering command: /usr/bin/gh gh api /repos/docker/setup-buildx-action/git/ref/tags/v3 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE node /opt�� prettier --check 64/bin/go **/*.ts **/*.json --ignore-path go (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/a70c5eada06553e3510ac27f2c3bda9d3705bccb
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/a70c5eada06553e3510ac27f2c3bda9d3705bccb --jq .object.sha heck '**/*.cjs' GOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env ck 'scripts/**/*GOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.0.0
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha HEAD (http block)
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha --show-toplevel bash /usr/bin/git --noprofile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD GOMODCACHE go env nxGvlmA2K GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE ortcfg (http block)
  • https://api.github.com/repos/githubnext/agentics/git/ref/tags/
    • Triggering command: /usr/bin/gh gh api /repos/githubnext/agentics/git/ref/tags/# --jq .object.sha ck '**/*.cjs' '*GOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/nonexistent/action/git/ref/tags/v999.999.999
    • Triggering command: /usr/bin/gh gh api /repos/nonexistent/action/git/ref/tags/v999.999.999 --jq .object.sha HEAD (http block)
    • Triggering command: /usr/bin/gh gh api /repos/nonexistent/action/git/ref/tags/v999.999.999 --jq .object.sha --show-toplevel qpNXUOr2HjTe /usr/bin/git --noprofile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/nonexistent/action/git/ref/tags/v999.999.999 --jq .object.sha GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>[refactor] Semantic Function Clustering Analysis: Misplaced Functions and Duplicate Patterns in pkg/workflow</issue_title>
<issue_description>Automated semantic analysis of the pkg/ directory (563 non-test Go files across 18 packages) identified concrete refactoring opportunities in the pkg/workflow package. Below are actionable findings organized by priority.

Summary

  • Total Go files analyzed: 563 (280 in workflow, 192 in cli, 37 in parser, 27 in console, 27 in smaller utility packages)
  • Outlier functions found: 4 (functions clearly in the wrong file)
  • Near-duplicate function pairs: 2 (same logic, duplicated in parallel files)
  • Missing helpers causing repetition: 2 (patterns repeated in 4–15+ call sites)
  • Fragmented file candidates for consolidation: 1 (two single-function files covering the same struct)

Issue 1: validateTargetRepoSlug Misplaced in validation_helpers.go

File: pkg/workflow/validation_helpers.go:186
Function: func validateTargetRepoSlug(targetRepoSlug string, log *logger.Logger) bool

Problem:

  • Lives in validation_helpers.go alongside generic validators (ValidateRequired, ValidateMaxLength, ValidateInList, etc.) but is specific to the safe-outputs GitHub entity domain
  • Has an inconsistent return convention: returns bool instead of error like every other function in the file
  • Called exclusively by 6 safe-outputs files: add_comment.go, add_reviewer.go, close_entity_helpers.go, create_discussion.go, create_issue.go, create_pull_request.go

Recommendation: Move to pkg/workflow/safe_outputs_validation.go (or safe_outputs_target_validation.go) and align the return type to error.


Issue 2: collectPackagesFromWorkflow is Extraction Logic, Not Validation

File: pkg/workflow/runtime_validation.go:196
Function: func collectPackagesFromWorkflow(workflowData *WorkflowData, extractor func(string) []string, toolCommand string) []string

Problem:

  • Produces no errors and performs no validation — it is a pure extraction/collection helper that returns a deduplicated []string
  • Lives in runtime_validation.go, surrounded by validation methods like validateExpressionSizes, validateContainerImages, validateRuntimePackages
  • Conceptually belongs in the package-extraction infrastructure (package_extraction.go, npm.go, pip.go)

Recommendation: Move to pkg/workflow/package_extraction.go.


Issue 3: isValidFullSHA Defined in Wrong Domain File

File: pkg/workflow/features_validation.go:96
Function: func isValidFullSHA(s string) bool

Problem:

  • Checks if a string is a valid 40-character hexadecimal SHA — a general-purpose utility
  • Defined in features_validation.go (feature-flag value validation) but also called from pkg/workflow/action_pins.go:165, which is a completely different domain (action pin management)
  • A general SHA-validation helper should not live in a domain-specific validation file

Recommendation: Move to pkg/workflow/validation_helpers.go or pkg/workflow/strings.go.


Issue 4: Near-Duplicate Mount Validation Functions

Files:

  • pkg/workflow/sandbox_validation.go:24func validateMountsSyntax(mounts []string) error
  • pkg/workflow/mcp_config_validation.go:299func validateMCPMountsSyntax(toolName string, mountsRaw any) error

Problem:
Both functions:

  1. Iterate over mount strings
  2. Call the shared helper validateMountStringFormat(mount) from validation_helpers.go
  3. Use the same source == "" && dest == "" && mode == "" sentinel to distinguish error types
  4. Produce identical conceptual errors ("must follow 'source:destination:mode' format", "mode must be 'ro' or 'rw'")

The only differences are: validateMCPMountsSyntax normalizes a []any/[]string input and adds toolName context; validateMountsSyntax works directly on []string. Both carry separate doc-URL references (constants.DocsSandboxURL vs constants.DocsToolsURL).

View Duplicate Logic Pattern
// sandbox_validation.go
func validateMountsSyntax(mounts []string) error {
    var errors []string
    for _, mount := range mounts {
        source, dest, mode, err := validateMountStringFormat(mount)
        if err != nil {
            if source == "" && dest == "" && mode == "" {
                errors = append(errors, fmt.Sprintf("mount '%s' must follow 'source:destination:mode' format", mount))
            } else {
                errors = append(errors, fmt.Sprintf("mount '%s' mode must be 'ro' or 'rw', got '%s'", mount, mode))
            }
        }
    }
    // ...
}

// mcp_config_validation.go
func validateMCPMountsSyntax(toolName string, mountsRaw any) error {
    // normalize to []string ...
    for _, mount := range mounts {
        source, dest, mode, err := vali...

</details>



<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes github/gh-aw#18388

<!-- START COPILOT CODING AGENT TIPS -->
---Let Copilot coding agent [set things up for you](https://github.com/github/gh-aw/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 3 commits February 26, 2026 02:50
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…rmissions

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor semantic function clustering in pkg/workflow refactor(workflow): resolve 8 semantic function clustering issues in pkg/workflow Feb 26, 2026
@pelikhan pelikhan marked this pull request as ready for review February 26, 2026 03:46
Copilot AI review requested due to automatic review settings February 26, 2026 03:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors pkg/workflow by relocating semantically misplaced helpers, introducing shared utilities to reduce repeated patterns, and aligning validation/error-aggregation behavior across the workflow compiler.

Changes:

  • Relocates helpers (isValidFullSHA, collectPackagesFromWorkflow, validateTargetRepoSlug) into more appropriate domain files and updates call sites.
  • Adds shared helpers (Compiler.emitWarning, findWriteScopesForPolicy, validateMountStrings) to deduplicate repeated logic across multiple validators.
  • Standardizes validation error aggregation by migrating several validators to NewErrorCollector and consolidates firewall validation into a single file.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.

Show a summary per file
File Description
pkg/workflow/validation_helpers.go Adds shared SHA validation and shared mount-validation loop; removes validateTargetRepoSlug from generic helpers.
pkg/workflow/strict_mode_validation.go Switches strict permission write-scope detection to shared findWriteScopesForPolicy.
pkg/workflow/safe_outputs_target_validation.go Moves validateTargetRepoSlug into safe-outputs domain and changes return type to error.
pkg/workflow/runtime_validation.go Migrates runtime/container validation to NewErrorCollector; removes collectPackagesFromWorkflow from validation file.
pkg/workflow/permissions_operations.go Introduces findWriteScopesForPolicy helper for consistent write-scope checks.
pkg/workflow/package_extraction.go Relocates collectPackagesFromWorkflow into extraction-focused code.
pkg/workflow/npm_validation.go Migrates npx validation error aggregation to NewErrorCollector.
pkg/workflow/network_firewall_validation.go Removes standalone firewall validation file (function moved to firewall_validation.go).
pkg/workflow/mcp_config_validation.go Reuses shared validateMountStrings for MCP mount validation messaging.
pkg/workflow/frontmatter_extraction_yaml.go Uses Compiler.emitWarning to centralize warning emission + counting.
pkg/workflow/firewall_validation.go Absorbs validateNetworkFirewallConfig and adds docs import for error messaging.
pkg/workflow/features_validation.go Removes SHA helper/regex from feature validation after relocation.
pkg/workflow/engine_firewall_support.go Uses Compiler.emitWarning instead of repeated stderr + counter logic.
pkg/workflow/data/action_pins.json Adds an additional pinned action entry (anchore/sbom-action@v0).
pkg/workflow/dangerous_permissions_validation.go Reuses findWriteScopesForPolicy to unify write-permission detection logic.
pkg/workflow/create_pull_request.go Updates validateTargetRepoSlug call site for error return type.
pkg/workflow/create_issue.go Updates validateTargetRepoSlug call site for error return type.
pkg/workflow/create_discussion.go Updates validateTargetRepoSlug call site for error return type.
pkg/workflow/compiler_orchestrator_tools.go Uses Compiler.emitWarning to deduplicate warning emission.
pkg/workflow/compiler.go Adds Compiler.emitWarning helper and updates internal warning call sites.
pkg/workflow/close_entity_helpers.go Updates validateTargetRepoSlug call site for error return type.
pkg/workflow/agent_validation.go Uses Compiler.emitWarning for unsupported web-search warning emission.
pkg/workflow/add_reviewer.go Updates validateTargetRepoSlug call site for error return type.
pkg/workflow/add_comment.go Updates validateTargetRepoSlug call site for error return type.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@pelikhan pelikhan closed this Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants