Skip to content

refactor: consolidate duplicate logic in role_checks.go#24870

Merged
pelikhan merged 2 commits intomainfrom
copilot/refactor-role-checks-duplicate-logic
Apr 6, 2026
Merged

refactor: consolidate duplicate logic in role_checks.go#24870
pelikhan merged 2 commits intomainfrom
copilot/refactor-role-checks-duplicate-logic

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 6, 2026

pkg/workflow/role_checks.go contained three instances of near-identical logic that were never consolidated when shared helpers were introduced.

Changes

  • Finding 1 — extractSkipRoles / extractSkipBots: Both functions duplicated the same on: map lookup pattern. Extracted extractSkipField(frontmatter, fieldName string) helper; each function now delegates with its field name literal.

  • Finding 2 — mergeSkipRoles / mergeSkipBots: Both functions duplicated a manual set-based ordered deduplication loop. Replaced with mergeStringSlicesDedup(top, imported []string, logLabel string) that delegates to the already-available sliceutil.Deduplicate.

  • Finding 3 — parseBotsValue: Predated extractStringSliceField and duplicated its string / []string / []any type-switch. Replaced body with a direct delegation call.

// Before: ~45 lines of duplicated set loop in mergeSkipRoles + mergeSkipBots
func mergeStringSlicesDedup(top, imported []string, logLabel string) []string {
    result := sliceutil.Deduplicate(append(top, imported...))
    if len(result) > 0 {
        roleLog.Printf("Merged %s: %v (top=%d, imported=%d, total=%d)", logLabel, result, len(top), len(imported), len(result))
    }
    return result
}

func (c *Compiler) mergeSkipRoles(top, imported []string) []string { return mergeStringSlicesDedup(top, imported, "skip-roles") }
func (c *Compiler) mergeSkipBots(top, imported []string) []string  { return mergeStringSlicesDedup(top, imported, "skip-bots") }

Net: −72 lines, no behavior change.

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 6176174/b039/vetrev-parse .cfg git rev-�� --show-toplevel ache/go/1.25.8/xv1.0.0 /usr/bin/git 0510-31954/test-git 6176174/b121/vetrev-parse tartedAt,updated--show-toplevel git (http block)
  • https://api.github.com/orgs/test-owner/actions/secrets
    • Triggering command: /usr/bin/gh gh api /orgs/test-owner/actions/secrets --jq .secrets[].name -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env -json 1.4.1/internal/x-ifaceassert x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh api /orgs/test-owner/actions/secrets --jq .secrets[].name echo "Syncing acGOINSECURE git 64/bin/go --ignore-path ..sh git /usr/bin/git go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (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 --show-toplevel x_amd64/vet /usr/bin/git ions-build/main.git rrG8ct2Bi 64/pkg/tool/linu--show-toplevel git rev-�� --show-toplevel 64/pkg/tool/linu-goversion /usr/bin/gh g_.a GO111MODULE 64/pkg/tool/linu--show-toplevel gh (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 -v go /usr/bin/gh 4113032/001' 4113032/001' x_amd64/compile gh api /repos/actions/github-script/git/ref/tags/v8 --jq ache/node/24.14.1/x64/bin/node -json GO111MODULE layTitle /opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet (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 .test 6176174/b037/vet.cfg ortcfg.link GOINSECURE GOMOD GOMODCACHE C8-1p4BWMcZFj5zQGi/dCI4RP8ziW6hVzy26jkN/Qr-c2nIRrev-parse (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha xterm-color 64/pkg/tool/linurev-parse /usr/bin/git ortcfg .cfg 64/pkg/tool/linu--show-toplevel git rev-�� --show-toplevel 64/pkg/tool/linux_amd64/vet /usr/bin/git 9452140/b187/_pkgit GO111MODULE 64/pkg/tool/linu--show-toplevel git (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha --show-toplevel /opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linu/tmp/go-build1936176174/b114/vet.cfg /usr/bin/git faultBranchFromLgit faultBranchFromLrev-parse 6176174/b347/vet--show-toplevel git rev-�� --show-toplevel /opt/hostedtoolcache/go/1.25.8/x64/pkg/tool/linux_amd64/vet /usr/bin/git licyMinIntegritygit /tmp/go-build193rev-parse /opt/hostedtoolc--show-toplevel git (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 --show-toplevel epo}/actions/runs/1/artifacts /usr/bin/git e GO111MODULE x_amd64/vet git conf�� user.name Test User /usr/bin/git g_.a GO111MODULE 64/pkg/tool/linu--show-toplevel git (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha /tmp/gh-aw-test-runs/20260406-120510-31954/test-23574315 rev-parse /usr/bin/git @{u} GO111MODULE x_amd64/vet git init�� GOMODCACHE x_amd64/vet /usr/bin/git g_.a GO111MODULE x_amd64/compile git (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha --show-toplevel ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet /usr/bin/git ithub/workflows -trimpath ache/go/1.25.8/x--show-toplevel git rev-�� --show-toplevel ache/go/1.25.8/x64/pkg/tool/linux_amd64/compile /usr/bin/git athSetup_GorootOgit /tmp/go-build193rev-parse 6176174/b427=> git (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 -json GO111MODULE x_amd64/asm GOINSECURE GOMOD GOMODCACHE x_amd64/asm env -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha -json 8601/parse.go x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env -json GO111MODULE x_amd64/link GOINSECURE GOMOD GOMODCACHE x_amd64/link (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 -m epo}/actions/runs/2/artifacts /opt/hostedtoolcache/node/24.14.1/x64/bin/node e GO111MODULE x_amd64/vet node /tmp�� /home/REDACTED/work/gh-aw/gh-aw/.github/workflows/ai-moderator.md x_amd64/vet /usr/bin/git g_.a nLaxVxxol 64/pkg/tool/linu--show-toplevel git (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 /tmp/gh-aw-test-runs/20260406-120510-31954/test-23574315 epo}/actions/runs/3/artifacts /usr/bin/git e elect.go x_amd64/vet git rev-�� --git-dir x_amd64/vet /usr/bin/git g_.a GO111MODULE x_amd64/compile git (http block)
  • https://api.github.com/repos/actions/upload-artifact/git/ref/tags/v4
    • Triggering command: /usr/bin/gh gh api /repos/actions/upload-artifact/git/ref/tags/v4 --jq .object.sha -bool -buildtags ache/node/24.14.1/x64/bin/node -errorsas -ifaceassert -nilfunc /usr/lib/git-core/git t-37�� sistency_GoAndJavaScript1046422632/001/test-inlined-imports-enabled-with-env-template-expressiongit --format=%(objectname) 1/x64/bin/node -json GO111MODULE x_amd64/compile 1/x64/bin/node (http block)
  • https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v0.1.2
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v0.1.2 --jq .object.sha test.txt vL9a24R/kExe3dXfremote /usr/bin/git g_.a GO111MODULE x_amd64/vet git conf�� user.email test@example.com /usr/bin/git g_.a GO111MODULE x_amd64/compile git (http block)
  • https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v1.0.0
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v1.0.0 --jq .object.sha t0 -buildtags (http block)
  • https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v1.2.3
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v1.2.3 --jq .object.sha -bool -buildtags 1/x64/bin/node -errorsas -ifaceassert -nilfunc git t-ha�� ithub/workflows/blog-auditor.md (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/1/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/1/artifacts --jq .artifacts[].name .cfg 64/pkg/tool/linux_amd64/vet GOINSECURE 9452140/b011/ GOMODCACHE 64/pkg/tool/linuTest User env 9452140/b166/_pkg_.a GO111MODULE x_amd64/vet GOINSECURE 9452140/b011/sysrev-parse ache/go/1.25.8/x--show-toplevel x_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh run download 1 --dir test-logs/run-1 GO111MODULE 64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/vet env 9452140/b208/_pkg_.a GO111MODULE .cfg GOINSECURE able GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/1/artifacts --jq .artifacts[].name GO111MODULE ache/go/1.25.8/x64/bin/go GOINSECURE GOMOD GOMODCACHE go 1/x6�� -json GO111MODULE ache/go/1.25.8/x64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/12345/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/12345/artifacts --jq .artifacts[].name mLsRemoteWithRealGitbranch_with_hyphen3535467423/001' 64/pkg/tool/linux_amd64/vet GOINSECURE b/gh-aw/pkg/typerev-parse GOMODCACHE 64/pkg/tool/linux_amd64/vet env 9452140/b161/_pkg_.a .cfg 64/pkg/tool/linux_amd64/vet GOINSECURE fips140cache GOMODCACHE 64/pkg/tool/linuTest User (http block)
    • Triggering command: /usr/bin/gh gh run download 12345 --dir test-logs/run-12345 .cfg 64/pkg/tool/linux_amd64/vet GOINSECURE g/x/crypto/interconfig GOMODCACHE 64/pkg/tool/linuTest User env 9452140/b170/_pkg_.a vMoO/r1c5PlYHcFDLvhFNvMoO 64/pkg/tool/linux_amd64/link GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/link (http block)
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/12345/artifacts --jq .artifacts[].name GO111MODULE ache/go/1.25.8/x64/bin/go GOINSECURE GOMOD GOMODCACHE go env 30/001/test-inlined-imports-enabled-with-env-template-expressions-in-body.md GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/12346/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/12346/artifacts --jq .artifacts[].name GO111MODULE 64/pkg/tool/linux_amd64/vet GOINSECURE contextprotocol/rev-parse GOMODCACHE 64/pkg/tool/linux_amd64/vet env b/workflows .cfg 64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linutest@example.com (http block)
    • Triggering command: /usr/bin/gh gh run download 12346 --dir test-logs/run-12346 .cfg 64/pkg/tool/linux_amd64/vet GOINSECURE 9452140/b011/ GOMODCACHE 64/pkg/tool/linux_amd64/vet env 9452140/b177/_pkg_.a .cfg 64/pkg/tool/linux_amd64/link GOINSECURE 9452140/b011/intinit ache/go/1.25.8/x64/src/runtime/iuser.email 64/pkg/tool/linux_amd64/link (http block)
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/12346/artifacts --jq .artifacts[].name GO111MODULE ndor/bin/sh GOINSECURE GOMOD GOMODCACHE go env '**/*.ts' '**/*.json' --ignore-path ../../../.pr--ignore-path GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/2/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/2/artifacts --jq .artifacts[].name .cfg 64/pkg/tool/linux_amd64/vet GOINSECURE fips140/subtle GOMODCACHE 64/pkg/tool/linux_amd64/vet env 9452140/b181/_pkg_.a xxol/GL-tkTNtkvunLaxVxxol ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linuremote.origin.url (http block)
    • Triggering command: /usr/bin/gh gh run download 2 --dir test-logs/run-2 GO111MODULE 64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linutest@example.com env 286903479 bYse/Agvt9vB4Z3tFs27lbYse .cfg GOINSECURE g/x/net/http2/hprev-parse GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linu-importcfg (http block)
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/2/artifacts --jq .artifacts[].name GO111MODULE 1/x64/lib/node_modules/npm/node_modules/@npmcli/run-script/lib/node-gyp-bin/node GOINSECURE GOMOD GOMODCACHE go 1/x6�� y_with_repos_array_c3117545948/001 GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/3/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/3/artifacts --jq .artifacts[].name .cfg 64/pkg/tool/linux_amd64/vet GOINSECURE 9452140/b006/ GOMODCACHE 64/pkg/tool/linux_amd64/vet env til.go o 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD 9452140/b006/sym--show-toplevel 64/pkg/tool/linux_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh run download 3 --dir test-logs/run-3 .cfg 64/pkg/tool/linu-nolocalimports GOINSECURE 9452140/b078/ GOMODCACHE 64/pkg/tool/linuTest User env 286903479 GO111MODULE ache/go/1.25.8/x64/pkg/tool/linux_amd64/link GOINSECURE g/x/text/unicoderev-parse ache/go/1.25.8/x--show-toplevel ache/go/1.25.8/x64/pkg/tool/linux_amd64/link (http block)
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/3/artifacts --jq .artifacts[].name mLsRemoteWithRealGitcustom_branch155733751/001' ache/go/1.25.8/x64/bin/go GOINSECURE GOMOD GOMODCACHE go 1/x6�� y_with_repos_array_c3117545948/001 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/4/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/4/artifacts --jq .artifacts[].name .cfg 64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linutest@example.com env 9452140/b173/_pkg_.a zm1t/ybsydLQ-bM8eUCGDzm1t x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh run download 4 --dir test-logs/run-4 GO111MODULE 64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/vet env 9452140/b200/_pkg_.a eFae/0ahu769BnKYz-hV-eFae .cfg GOINSECURE g/x/text/secure/rev-parse GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/4/artifacts --jq .artifacts[].name GO111MODULE ache/go/1.25.8/x64/bin/go GOINSECURE GOMOD GOMODCACHE go 1/x6�� y_with_repos_array_c3117545948/0remote.origin.url GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile (http block)
  • https://api.github.com/repos/github/gh-aw/actions/runs/5/artifacts
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/5/artifacts --jq .artifacts[].name .cfg 64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/vet env l.go l_test.go x_amd64/link GOINSECURE GOMOD GOMODCACHE x_amd64/link (http block)
    • Triggering command: /usr/bin/gh gh run download 5 --dir test-logs/run-5 GO111MODULE 64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/vet env 286903479 go .cfg GOINSECURE g/x/text/unicoderev-parse GOMODCACHE ache/go/1.25.8/x64/pkg/tool/linu-trimpath (http block)
    • Triggering command: /usr/bin/gh gh api --paginate repos/{owner}/{repo}/actions/runs/5/artifacts --jq .artifacts[].name GO111MODULE 64/bin/node GOINSECURE GOMOD GOMODCACHE go 1/x6�� -json GO111MODULE 64/pkg/tool/linux_amd64/asm GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linuorigin (http block)
  • https://api.github.com/repos/github/gh-aw/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env -json 2/compile.go x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 100 GOMOD emclr_wasm.s x_amd64/vet env -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh run list --json databaseId,number,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,displayTitle --workflow nonexistent-workflow-12345 --limit 6 GOMOD GOMODCACHE 64/pkg/tool/linuremote.origin.url env edOutput3353440096/001 .cfg 64/pkg/tool/linux_amd64/vet GOINSECURE 9452140/b132/ GOMODCACHE 64/pkg/tool/linux_amd64/vet (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/v0.47.4
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v0.47.4 --jq .object.sha -t security /usr/bin/git -nxv .cfg 64/pkg/tool/linu--show-toplevel git rev-�� --show-toplevel 64/pkg/tool/linux_amd64/vet /usr/bin/git se 6176174/b009/vet-c x_amd64/vet git (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 edOutput3353440096/001 .cfg 64/pkg/tool/linux_amd64/vet GOINSECURE /go-yaml/token GOMODCACHE 64/pkg/tool/linux_amd64/vet env 9452140/b001/exe/a.out GO111MODULE k GOINSECURE bjd8Uilp95fYc/qOrun GOMODCACHE ache/go/1.25.8/x--json (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.2.3
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.2.3 --jq .object.sha -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.2.3 --jq .object.sha re 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/github/gh-aw/git/ref/tags/v2.0.0
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env 113032/001 113032/002/work x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha -json GO111MODULE x_amd64/vet GOINSECURE json GOMODCACHE x_amd64/vet env -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet (http block)
  • https://api.github.com/repos/github/gh-aw/git/ref/tags/v3.0.0
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v3.0.0 --jq .object.sha -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet env -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet (http block)
    • Triggering command: /usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v3.0.0 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE es/.bin/sh 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 edOutput3353440096/001 .cfg x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env gh-aw.wasm ($(du -h gh-aw.wasm | cut -f1))" .cfg ache/go/1.25.8/x64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD 9452140/b132/symxterm-color ache/go/1.25.8/x64/pkg/tool/linuconfig (http block)
  • https://api.github.com/repos/nonexistent/repo/actions/runs/12345
    • Triggering command: /usr/bin/gh gh run view 12345 --repo nonexistent/repo --json status,conclusion GOINSECURE fips140/aes GOMODCACHE 64/pkg/tool/linux_amd64/vet env 9452140/b198/_pkg_.a qrnP/bIu9B-2Kyy25-yTJqrnP ger.test GOINSECURE g/x/net/http/httinit GOMODCACHE ger.test (http block)
    • Triggering command: /usr/bin/gh gh run view 12345 --repo nonexistent/repo --json status,conclusion GOINSECURE GOMOD GOMODCACHE go tion�� /workflows GO111MODULE 64/pkg/tool/linux_amd64/vet GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/vet (http block)
  • https://api.github.com/repos/owner/repo/actions/workflows
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path --repo owner/repo -importcfg /tmp/go-build1936176174/b410/importcfg -pack /home/REDACTED/work/gh-aw/gh-aw/pkg/fileutil/fileutil.go /home/REDACTED/work/gh-aw/gh-aw/pkg/fileutil/tar.go env -json age/common.go x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path --repo owner/repo x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile env -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh workflow list --json name,state,path --repo owner/repo 64/bin/go -n1 --format=format:/home/REDACTED/work/gh-aw/gh-aw/actions/setup/js/node_modules/.bin/pre�� --end-of-options--check go env q "All matched f**/*.json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go (http block)
  • https://api.github.com/repos/owner/repo/contents/file.md
    • Triggering command: /tmp/go-build1936176174/b396/cli.test /tmp/go-build1936176174/b396/cli.test -test.testlogfile=/tmp/go-build1936176174/b396/testlog.txt -test.paniconexit0 -test.v=true -test.parallel=4 -test.timeout=10m0s -test.run=^Test -test.short=true GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
    • Triggering command: /tmp/go-build2001551124/b396/cli.test /tmp/go-build2001551124/b396/cli.test -test.testlogfile=/tmp/go-build2001551124/b396/testlog.txt -test.paniconexit0 -test.v=true -test.parallel=4 -test.timeout=10m0s -test.run=^Test -test.short=true tierignore /usr/lib/git-corenv /usr/bin/git make lint�� /usr/bin/git git (http block)
  • https://api.github.com/repos/test-owner/test-repo/actions/secrets
    • Triggering command: /usr/bin/gh gh api /repos/test-owner/test-repo/actions/secrets --jq .secrets[].name -json GO111MODULE x_amd64/compile GOINSECURE R5/Ba16K1_zLw9CV-atomic GOMODCACHE x_amd64/compile env -json eyset.go x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile (http block)
    • Triggering command: /usr/bin/gh gh api /repos/test-owner/test-repo/actions/secrets --jq .secrets[].name echo "��� Code fGOINSECURE pkg/workflow/conGOMOD 64/bin/go --ignore-path ..sh git /usr/bin/git 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:

- Extract extractSkipField helper; extractSkipRoles/extractSkipBots delegate to it (Finding 1)
- Extract mergeStringSlicesDedup using sliceutil.Deduplicate; mergeSkipRoles/mergeSkipBots delegate to it (Finding 2)
- Simplify parseBotsValue to delegate to extractStringSliceField (Finding 3)

Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9e504884-9233-4ae6-89d8-4a3499e01455

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor role_checks.go to reduce duplicate logic refactor: consolidate duplicate logic in role_checks.go Apr 6, 2026
Copilot AI requested a review from pelikhan April 6, 2026 12:17
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

Hey @Copilot 👋 — great cleanup work on pkg/workflow/role_checks.go! Consolidating three instances of near-identical logic into shared helpers (extractSkipField, mergeStringSlicesDedup, and delegating parseBotsValue to extractStringSliceField) is a clean, focused refactor that improves maintainability. The PR description is excellent — precise, with before/after code samples.

One thing to address before this is merge-ready:

  • Add tests for the new helper functionsextractSkipField and mergeStringSlicesDedup are now standalone package-level helpers, but the diff includes no new test cases for them. While the existing tests in role_checks_test.go and skip_roles_test.go cover the public-facing methods (extractSkipRoles, extractSkipBots, mergeSkipRoles, mergeSkipBots), the new shared helpers themselves aren't directly tested. Adding a few focused unit tests would guard against future regressions and align with the project's testing standards in scratchpad/testing.md.

If you'd like a hand, you can use this prompt with your coding agent:

Add unit tests for the two new package-level helpers introduced in pkg/workflow/role_checks.go.

File to add tests to: pkg/workflow/role_checks_test.go (or a new file pkg/workflow/role_checks_helpers_test.go if preferred)

Use the existing table-driven test patterns already in pkg/workflow/role_checks_test.go and pkg/workflow/skip_roles_test.go as style reference. Use the testify assert package consistent with the rest of the test suite.

## extractSkipField(frontmatter map[string]any, fieldName string) []string

Test cases:
1. frontmatter has no "on" key → returns nil
2. frontmatter "on" key is nil → returns nil
3. frontmatter "on" is not a map (e.g., a string) → returns nil
4. frontmatter "on" map exists but fieldName key is absent → returns nil
5. frontmatter "on" map has fieldName with nil value → returns nil
6. frontmatter "on" map has fieldName as a single string → returns []string{value}
7. frontmatter "on" map has fieldName as []string → returns the slice
8. frontmatter "on" map has fieldName as []any of strings → returns string slice

## mergeStringSlicesDedup(top, imported []string, logLabel string) []string

Test cases:
1. Both slices empty → returns empty (or nil)
2. top only, no imported → returns top values
3. imported only, no top → returns imported values
4. No duplicates → returns concatenation in order
5. Duplicates within top → deduped, top values preserved once
6. Duplicates between top and imported → top values win (first occurrence preserved)
7. logLabel is used in log output (verify no panic; exact log content not required)

Run `make test-unit` to verify all tests pass before finishing.

Generated by Contribution Check · ● 1.8M ·

@pelikhan pelikhan marked this pull request as ready for review April 6, 2026 14:13
Copilot AI review requested due to automatic review settings April 6, 2026 14:13
Copy link
Copy Markdown
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/role_checks.go to consolidate duplicated parsing/merging logic for skip fields (roles/bots) and bot value parsing.

Changes:

  • Introduced extractSkipField(frontmatter, fieldName) to unify on: map lookup for skip fields.
  • Added mergeStringSlicesDedup(top, imported, logLabel) using sliceutil.Deduplicate to replace duplicated merge/dedup loops.
  • Simplified parseBotsValue by delegating to extractStringSliceField.
Show a summary per file
File Description
pkg/workflow/role_checks.go Consolidates skip-field extraction/merge logic and reuses shared string-slice parsing helper.

Copilot's findings

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 3

// mergeStringSlicesDedup merges two string slices with deduplication (preserving order of first occurrence).
// Logs the merged result under the given logLabel when the result is non-empty.
func mergeStringSlicesDedup(top, imported []string, logLabel string) []string {
result := sliceutil.Deduplicate(append(top, imported...))
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

mergeStringSlicesDedup builds the input via append(top, imported...), which can mutate the backing array of top (and thus potentially the frontmatter-provided []string) if top has spare capacity. This introduces a side effect that the previous implementation didn’t have. To keep this merge pure, allocate a new slice before appending (e.g., copy top into a new buffer, then append imported).

Suggested change
result := sliceutil.Deduplicate(append(top, imported...))
merged := make([]string, len(top), len(top)+len(imported))
copy(merged, top)
merged = append(merged, imported...)
result := sliceutil.Deduplicate(merged)

Copilot uses AI. Check for mistakes.
Comment on lines 163 to 166
// parseBotsValue parses a bots value from frontmatter (supports string, []any, []string)
func parseBotsValue(botsValue any, fieldName string) []string {
switch v := botsValue.(type) {
case []any:
// Array of bot identifiers
var bots []string
for _, item := range v {
if str, ok := item.(string); ok {
bots = append(bots, str)
}
}
roleLog.Printf("Extracted %d bot identifiers from '%s' array: %v", len(bots), fieldName, bots)
return bots
case []string:
// Already a string slice
roleLog.Printf("Extracted %d bot identifiers from '%s': %v", len(v), fieldName, v)
return v
case string:
// Single bot identifier as string
roleLog.Printf("Extracted single bot identifier from '%s': %s", fieldName, v)
return []string{v}
}
return nil
return extractStringSliceField(botsValue, fieldName)
}
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

parseBotsValue now delegates to extractStringSliceField, which returns nil for empty strings / empty arrays and filters out empty string elements in []any. The previous parseBotsValue implementation would return a non-nil slice in these cases (e.g., []string{""}). If this change is intentional, it should be called out explicitly; otherwise, consider preserving the old behavior (or updating extractBots/parseBotsValue to match prior semantics).

Copilot uses AI. Check for mistakes.
Comment on lines 144 to 166
// extractBots extracts the 'bots' field from frontmatter to determine allowed bot identifiers
func (c *Compiler) extractBots(frontmatter map[string]any) []string {
// Check on.bots
if onValue, exists := frontmatter["on"]; exists {
if onMap, ok := onValue.(map[string]any); ok {
if botsValue, hasBots := onMap["bots"]; hasBots {
bots := parseBotsValue(botsValue, "on.bots")
if bots != nil {
return bots
}
}
}
}

// No bots specified, return empty array
roleLog.Print("No bots specified")
return []string{}
}

// parseBotsValue parses a bots value from frontmatter (supports string, []any, []string)
func parseBotsValue(botsValue any, fieldName string) []string {
switch v := botsValue.(type) {
case []any:
// Array of bot identifiers
var bots []string
for _, item := range v {
if str, ok := item.(string); ok {
bots = append(bots, str)
}
}
roleLog.Printf("Extracted %d bot identifiers from '%s' array: %v", len(bots), fieldName, bots)
return bots
case []string:
// Already a string slice
roleLog.Printf("Extracted %d bot identifiers from '%s': %v", len(v), fieldName, v)
return v
case string:
// Single bot identifier as string
roleLog.Printf("Extracted single bot identifier from '%s': %s", fieldName, v)
return []string{v}
}
return nil
return extractStringSliceField(botsValue, fieldName)
}
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

There are tests covering skip-roles extraction (e.g., pkg/workflow/skip_roles_test.go), but no corresponding tests for extractBots/parseBotsValue. Given this refactor can affect edge cases (empty string / empty array / []any with non-string entries), it would be good to add a small table-driven test for bot parsing to lock in the intended semantics.

Copilot uses AI. Check for mistakes.
@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented Apr 6, 2026

@copilot review comments

@pelikhan pelikhan merged commit 43454ba into main Apr 6, 2026
67 of 68 checks passed
@pelikhan pelikhan deleted the copilot/refactor-role-checks-duplicate-logic branch April 6, 2026 14:24
Copilot stopped work on behalf of pelikhan due to an error April 6, 2026 14:25
Sign up for free to 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.

[refactor] Reduce duplicate logic in role_checks.go via consolidation refactoring

3 participants