Conversation
… gitutil)" - Add NormalizeGitHubHostURL to pkg/stringutil/urls.go - Add ExtractBaseRepo and FindGitRoot to pkg/gitutil/gitutil.go - Update pkg/parser/github.go and pkg/cli/github.go to use stringutil.NormalizeGitHubHostURL - Update pkg/workflow/action_resolver.go and pkg/cli/update_actions.go to use gitutil.ExtractBaseRepo - Update pkg/cli/git.go to delegate findGitRoot to gitutil.FindGitRoot - Update pkg/workflow/git_helpers.go to delegate findGitRoot to gitutil.FindGitRoot - Update test files to use the shared functions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors duplicated helper functions across packages by extracting shared GitHub URL normalization and git-related utilities into canonical utility packages, and updating all callers/tests to use them.
Changes:
- Added
stringutil.NormalizeGitHubHostURLand replaced duplicated private normalizers inpkg/parserandpkg/cli. - Added
gitutil.ExtractBaseRepoandgitutil.FindGitRoot, updating CLI/workflow callers and removing local duplicates. - Updated affected tests to call the shared utilities.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/stringutil/urls.go | Adds exported NormalizeGitHubHostURL utility for consistent host normalization. |
| pkg/gitutil/gitutil.go | Adds shared ExtractBaseRepo and FindGitRoot utilities. |
| pkg/parser/github.go | Switches host normalization to stringutil.NormalizeGitHubHostURL and removes duplicate helper. |
| pkg/cli/github.go | Switches host normalization to stringutil.NormalizeGitHubHostURL and removes duplicate helper. |
| pkg/cli/git.go | Delegates findGitRoot() to gitutil.FindGitRoot(). |
| pkg/workflow/git_helpers.go | Delegates internal findGitRoot() to gitutil.FindGitRoot() while preserving wrapper signature. |
| pkg/workflow/action_resolver.go | Uses gitutil.ExtractBaseRepo and removes local duplicate implementation. |
| pkg/cli/update_actions.go | Uses gitutil.ExtractBaseRepo and removes local duplicate implementation. |
| pkg/workflow/action_resolver_test.go | Updates tests to call gitutil.ExtractBaseRepo. |
| pkg/cli/update_actions_test.go | Updates tests to call gitutil.ExtractBaseRepo. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg/stringutil/urls.go
Outdated
| // NormalizeGitHubHostURL ensures the host URL has https:// scheme and no trailing slashes. | ||
| // It is safe to call with URLs that already have the https:// scheme. |
There was a problem hiding this comment.
The doc comment says NormalizeGitHubHostURL is safe to call with URLs that already have the https:// scheme, but the implementation also explicitly preserves http://. Update the comment to reflect both schemes (or refer to “a scheme”).
| // NormalizeGitHubHostURL ensures the host URL has https:// scheme and no trailing slashes. | |
| // It is safe to call with URLs that already have the https:// scheme. | |
| // NormalizeGitHubHostURL ensures the host URL has a scheme (defaulting to https://) and no trailing slashes. | |
| // It is safe to call with URLs that already have an http:// or https:// scheme. |
pkg/stringutil/urls.go
Outdated
| // NormalizeGitHubHostURL ensures the host URL has https:// scheme and no trailing slashes. | ||
| // It is safe to call with URLs that already have the https:// scheme. | ||
| func NormalizeGitHubHostURL(rawHostURL string) string { | ||
| // Remove all trailing slashes | ||
| normalized := strings.TrimRight(rawHostURL, "/") | ||
|
|
||
| // Add https:// scheme if no scheme is present | ||
| if !strings.HasPrefix(normalized, "https://") && !strings.HasPrefix(normalized, "http://") { | ||
| normalized = "https://" + normalized | ||
| } | ||
|
|
||
| return normalized |
There was a problem hiding this comment.
NormalizeGitHubHostURL is a newly exported helper and is now used by both pkg/parser and pkg/cli, but there are no unit tests covering its behavior (scheme preservation/addition, trailing-slash trimming). Adding focused tests in pkg/stringutil/urls_test.go would help prevent regressions in host resolution across the app.
|
@copilot apply comments, review test errors |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in d475bcd:
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in 2d8c14b. The CI failure was 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:
If you need me to access, download, or install something from one of these locations, you can either:
|
normalizeGitHubHostURL→pkg/stringutil.NormalizeGitHubHostURLpkg/stringutil/urls.goNormalizeGitHubHostURLinpkg/stringutil/urls_test.gopkg/parser/github.goto use shared functionpkg/cli/github.goto use shared functionextractBaseRepo→pkg/gitutil.ExtractBaseRepopkg/gitutil/gitutil.gopkg/workflow/action_resolver.goto use shared functionpkg/cli/update_actions.goto use shared functionaction_resolver_test.go,update_actions_test.go,update_actions_integration_test.go) to use shared functionFindGitRoot() (string, error)topkg/gitutil/gitutil.gopkg/cli/git.goto delegate togitutil.FindGitRootpkg/workflow/git_helpers.goto delegate togitutil.FindGitRootmake fmt,make build,make lint— all passingupdate_actions_integration_test.gowas still calling the old privateextractBaseRepo; updated togitutil.ExtractBaseRepoOriginal prompt
This section details on the original issue you should resolve
<issue_title>[refactor] Semantic Function Clustering Analysis: Duplicates and Outliers in pkg/</issue_title>
<issue_description>Automated semantic analysis of 553 non-test Go files across
pkg/identified confirmed cross-package duplicates, outlier functions, and structural patterns that warrant attention.Analysis date: 2026-03-02 | Workflow run: §22587119823
Executive Summary
pkg/workflow(272+ files),pkg/cli(188+ files),pkg/parser(31 files)pkg/gitutil)Identified Issues
1. Exact Duplicate:
normalizeGitHubHostURLSeverity: High — identical private function implemented in two separate packages.
pkg/parser/github.go:42pkg/cli/github.go:37Both implementations are byte-for-byte identical:
View duplicate code
Recommendation: Extract to
pkg/stringutilasNormalizeGitHubHostURL(rawHostURL string) string(exported). This fits alongside the existing URL utilities inpkg/stringutil/urls.go.2. Near-Duplicate:
extractBaseRepoSeverity: High — nearly identical private function across two packages, same algorithm, different variable names only.
pkg/workflow/action_resolver.go:111repo stringpkg/cli/update_actions.go:21actionPath stringView near-duplicate code
Recommendation: Extract to
pkg/gitutilasExtractBaseRepo(repoPath string) string. Thepkg/gitutilpackage already exists for shared git-related utilities.3. Similar Logic:
findGitRootin Three LocationsSeverity: Medium — the same
git rev-parse --show-toplevelshell invocation is independently implemented in two packages, with a third stub for WASM.pkg/workflow/git_helpers.go:53string""on errorpkg/cli/git.go:23(string, error)pkg/workflow/git_helpers_wasm.go:9string"."(WASM stub)The
pkg/gitutilpackage exists (withIsAuthErrorandIsHexString) but does not contain git root detection despite being the natural home for it.Recommendation: Add
FindGitRoot() (string, error)topkg/gitutil/gitutil.go. Thepkg/cli/git.goversion (with proper error return) is the more idiomatic implementation to use as the basis.4. Engine Log Parser Structural Duplication
Severity: Medium — four engine log parsers each independently implement the same
ParseLogMetricsinterface plus a privateparseXxxToolCallsWithSequencemethod, with no shared base utilities.🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.