Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/09563086-dd5d-4587-a6c6-a3516d95592c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…n wrapper Agent-Logs-Url: https://github.com/github/gh-aw/sessions/09563086-dd5d-4587-a6c6-a3516d95592c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
|
Hey The PR is well-described, the diff is focused, and existing test files are updated. One gap: Please add direct unit tests for the two new public methods to bring the
|
There was a problem hiding this comment.
Pull request overview
Refactors version and git-root helpers to remove duplicate types/wrappers in pkg/cli and centralize semantics in shared utility packages.
Changes:
- Added
IsPreciseVersion()andIsNewer()methods tosemverutil.SemanticVersionand updated callers to use them. - Removed the local
semanticVersiontype frompkg/cli/semver.go;parseVersion()now returns*semverutil.SemanticVersiondirectly. - Removed the
findGitRoot()passthrough wrapper frompkg/cli/git.goand updatedpkg/clicall sites to callgitutil.FindGitRoot()directly.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| pkg/semverutil/semverutil.go | Adds SemanticVersion methods for precision detection and version ordering. |
| pkg/cli/semver.go | Removes duplicate semver struct; returns *semverutil.SemanticVersion directly. |
| pkg/cli/update_workflows.go | Switches version comparison/type usage to semverutil.SemanticVersion. |
| pkg/cli/update_actions.go | Switches release filtering/sorting/comparison to semverutil.SemanticVersion methods/fields. |
| pkg/cli/update_actions_test.go | Updates tests to use semverutil.SemanticVersion and new methods. |
| pkg/cli/semver_test.go | Updates tests to assert on semverutil.SemanticVersion fields and IsNewer(). |
| pkg/cli/semver_precise_test.go | Updates tests to use IsPreciseVersion()/IsNewer() on the shared type. |
| pkg/cli/git.go | Removes findGitRoot() wrapper; uses gitutil.FindGitRoot() directly. |
| pkg/cli/gitroot_test.go | Updates test to call gitutil.FindGitRoot() directly. |
| pkg/cli/actionlint.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/poutine.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/zizmor.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/init.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/run_push.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/file_tracker.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/devcontainer.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/deps_go_mod.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/copilot_agents.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/compile_watch.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/compile_pipeline.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/compile_orchestrator_stability_test.go | Updates tests to call gitutil.FindGitRoot() directly. |
| pkg/cli/compile_command_test.go | Updates tests to call gitutil.FindGitRoot() directly. |
| pkg/cli/audit.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/add_workflow_compilation.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| pkg/cli/add_command.go | Replaces findGitRoot() with gitutil.FindGitRoot(). |
| go.mod | Removes an unused indirect dependency. |
| go.sum | Drops sums for removed dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // (i.e., at least two dots in the version string, e.g. "v6.0.0" is precise, "v6" is not). | ||
| func (v *SemanticVersion) IsPreciseVersion() bool { | ||
| versionPart := strings.TrimPrefix(v.Raw, "v") |
There was a problem hiding this comment.
IsPreciseVersion() currently counts dots in v.Raw, but Raw can include prerelease/build metadata (e.g. v6.0-rc.1, v6.0+build.1). Dots in those suffixes can cause versions without an explicit patch component to be misclassified as “precise”. Consider stripping any -... / +... suffix before counting dots (and adding a test case for these forms).
| // (i.e., at least two dots in the version string, e.g. "v6.0.0" is precise, "v6" is not). | |
| func (v *SemanticVersion) IsPreciseVersion() bool { | |
| versionPart := strings.TrimPrefix(v.Raw, "v") | |
| // (i.e., at least two dots in the version string, e.g. "v6.0.0" is precise, "v6" is not). | |
| // Prerelease and build-metadata suffixes (e.g. "-rc.1", "+build.1") are ignored. | |
| func (v *SemanticVersion) IsPreciseVersion() bool { | |
| versionPart := strings.TrimPrefix(v.Raw, "v") | |
| // Strip any prerelease ("-...") or build-metadata ("+...") suffix before counting dots. | |
| if idx := strings.IndexAny(versionPart, "-+"); idx != -1 { | |
| versionPart = versionPart[:idx] | |
| } |
Two dead-weight indirections identified in semantic analysis: a local
semanticVersionstruct inpkg/cli/semver.gothat duplicatedsemverutil.SemanticVersionfield-for-field, and a one-linerfindGitRoot()wrapper inpkg/cli/git.gothat just calledgitutil.FindGitRoot().Changes
pkg/semverutil/semverutil.go— AddedIsPreciseVersion()andIsNewer()as methods onSemanticVersionso callers don't need to re-implement version comparison logic locallypkg/cli/semver.go— RemovedsemanticVersionstruct and its methods;parseVersion()now returns*semverutil.SemanticVersiondirectly instead of copying all five fields into a parallel typepkg/cli/git.go— RemovedfindGitRoot()passthrough; 17 call sites acrosspkg/cli/now callgitutil.FindGitRoot()directly and importgitutilthemselvesBefore:
After:
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/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 6947320/b250/vetrev-parse ndor/bin/bash git rev-�� --show-toplevel ache/go/1.25.0/x64/pkg/tool/linu/tmp/go-build1257888443/b111/vet.cfg /usr/bin/git se 6947320/b371/vetrev-parse cal/bin/node git(http block)https://api.github.com/orgs/test-owner/actions/secrets/usr/bin/gh gh api /orgs/test-owner/actions/secrets --jq .secrets[].name get --local x_amd64/vet user.email(http block)https://api.github.com/repos/actions/ai-inference/git/ref/tags/v1/usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha --show-toplevel 64/pkg/tool/linuremote.origin.url /usr/bin/git b/workflows .cfg _modules/.bin/sh--show-toplevel git rev-�� --show-toplevel 64/pkg/tool/linuremote.origin.url /usr/bin/gh ./../pkg/workflogit .cfg 64/pkg/tool/linu--show-toplevel gh(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v3/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v3 --jq .object.sha --write ../../../**/*.json ache/node/24.14.0/x64/bin/node --ignore-path ../../../.prettirev-parse om /tmp/go-build1257888443/b414/gitutil.test t-14�� sistency_WithImports3428064209/001/main.md -test.v=true /usr/bin/git -test.timeout=10git -test.run=^Test -test.short=true--show-toplevel git(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v5/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha .js' --ignore-path .prettierigno-test.timeout=10m0s 1 0/x64/lib/node_modules/npm/node_-test.short=true(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha list --json /usr/bin/git --workflow nonexistent-workrev-parse --limit git rev-�� --show-toplevel ache/go/1.25.0/xorigin /usr/bin/git '**/*.ts' '**/*.git 6947320/b036/vetrev-parse ache/go/1.25.0/x--show-toplevel git(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha --show-toplevel erignore /usr/bin/git -unreachable=falgit /tmp/go-build330rev-parse rgo/bin/bash git rev-�� --show-toplevel /opt/hostedtoolcache/go/1.25.0/x^remote\..*\.gh-resolved$ n-dir/node -bool -buildtags /opt/hostedtoolc--show-toplevel git(http block)https://api.github.com/repos/actions/checkout/git/ref/tags/v6/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha -test.paniconexit0 -test.v=true /usr/bin/git -test.timeout=10git -test.run=^Test -test.short=true--show-toplevel git -C /tmp/gh-aw-test-runs/20260331-131105-13981/test-1106410898 status /usr/bin/git .github/workflowgit .cfg 64/pkg/tool/linu--show-toplevel git(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha /tmp/TestHashConsistency_GoAndJavaScript1881313396/001/test-inli-test.timeout=10m0s x_amd64/vet /usr/bin/git --abbrev-ref HEAD 64/pkg/tool/linu--show-toplevel git rese�� HEAD .github/workflows/test.md /usr/bin/git json' --ignore-pgit .cfg 64/pkg/tool/linu--show-toplevel git(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha --show-toplevel /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linuremote.origin.url /usr/bin/git 1105-13981/test-git /tmp/go-build330rev-parse ache/uv/0.11.2/x--show-toplevel git rev-�� --show-toplevel /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/vet /usr/bin/git ithub/workflows /tmp/go-build330rev-parse 64/pkg/tool/linu--show-toplevel git(http block)https://api.github.com/repos/actions/github-script/git/ref/tags/v8/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha get --local x_amd64/vet user.name 2w/fnHxk27dow1V3/opt/hostedtoolcache/node/24.14.0/x64/bin/npx(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha get --local x_amd64/vet core.hooksPath(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha oot()/g --local x_amd64/vet core.hooksPath(http block)https://api.github.com/repos/actions/setup-go/git/ref/tags/v4/usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v4 --jq .object.sha ue.number x_amd64/vet /usr/bin/git set-url origin ode_modules/.bin--show-toplevel git add .github/workflows/test.md siWHJxF12LLI /opt/hostedtoolcache/node/24.14.0/x64/bin/node json' --ignore-pgit(http block)https://api.github.com/repos/actions/setup-node/git/ref/tags/v4/usr/bin/gh gh api /repos/actions/setup-node/git/ref/tags/v4 --jq .object.sha /tmp/go-build1257888443/b445/_pkg_.a -trimpath /usr/bin/git -p github.com/githurev-parse -lang=go1.25 git rese�� HEAD .github/workflows/test.md /usr/bin/git -c=4 -nolocalimports -importcfg git(http block)https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v0.1.2/usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v0.1.2 --jq .object.sha k/gh-aw/gh-aw/.github/workflows/blog-auditor.md -buildtags /usr/bin/git -errorsas -ifaceassert -nilfunc git -C nt/action/git/ref/tags/v999.999.999 rev-parse /usr/bin/git @{u}(http block)https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v1.0.0/usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v1.0.0 --jq .object.sha 3Hdb/bUNV7REv_YPf1BO_3Hdb(http block)https://api.github.com/repos/github/gh-aw-actions/git/ref/tags/v1.2.3/usr/bin/gh gh api /repos/github/gh-aw-actions/git/ref/tags/v1.2.3 --jq .object.sha 1105-13981/test-1106410898(http block)https://api.github.com/repos/github/gh-aw/actions/runs/1/artifacts/usr/bin/gh gh run download 1 --dir test-logs/run-1 .cfg .cfg(http block)https://api.github.com/repos/github/gh-aw/actions/runs/12345/artifacts/usr/bin/gh gh run download 12345 --dir test-logs/run-12345(http block)https://api.github.com/repos/github/gh-aw/actions/runs/12346/artifacts/usr/bin/gh gh run download 12346 --dir test-logs/run-12346(http block)https://api.github.com/repos/github/gh-aw/actions/runs/2/artifacts/usr/bin/gh gh run download 2 --dir test-logs/run-2(http block)https://api.github.com/repos/github/gh-aw/actions/runs/3/artifacts/usr/bin/gh gh run download 3 --dir test-logs/run-3(http block)https://api.github.com/repos/github/gh-aw/actions/runs/4/artifacts/usr/bin/gh gh run download 4 --dir test-logs/run-4(http block)https://api.github.com/repos/github/gh-aw/actions/runs/5/artifacts/usr/bin/gh gh run download 5 --dir test-logs/run-5(http block)https://api.github.com/repos/github/gh-aw/actions/workflows/usr/bin/gh gh workflow list --json name,state,path get om/github/gh-aw(http block)/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(http block)/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 dd5d-4587-a6c6-acheckout(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v0.47.4/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v0.47.4 --jq .object.sha --show-toplevel ache/go/1.25.0/xorigin /usr/bin/git ty-test.md 6947320/b047/vetrev-parse de_modules/.bin/--show-toplevel git rev-�� --show-toplevel ache/go/1.25.0/x64/pkg/tool/linurev-parse /usr/bin/git 33639880/001(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.0.0/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.0.0 --jq .object.sha ty-test.md security .cfg OUTPUT -d 168.63.129.16 ache/go/1.25.0/x64/pkg/tool/linux_amd64/vet -o b/workflows -trimpath ache/go/1.25.0/x64/pkg/tool/linux_amd64/vet -p github.com/githurev-parse -lang=go1.25 ache/go/1.25.0/x64/pkg/tool/linux_amd64/vet(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v1.2.3/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.2.3 --jq .object.sha re --log-level=e!../../../pkg/workflow/js/**/*.json user.name /node(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v2.0.0/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha re --log-level=error user.name x_amd64/vet(http block)/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha re --log-level=error pull.rebase x_amd64/vet(http block)https://api.github.com/repos/github/gh-aw/git/ref/tags/v3.0.0/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v3.0.0 --jq .object.sha re --log-level=error pull.rebase x_amd64/vet(http block)https://api.github.com/repos/nonexistent/action/git/ref/tags/v999.999.999/usr/bin/gh gh api /repos/nonexistent/action/git/ref/tags/v999.999.999 --jq .object.sha(http block)https://api.github.com/repos/nonexistent/repo/actions/runs/12345/usr/bin/gh gh run view 12345 --repo nonexistent/repo --json status,conclusion(http block)https://api.github.com/repos/owner/repo/actions/workflows/usr/bin/gh gh workflow list --json name,state,path --repo owner/repo x_amd64/vet committer.name(http block)https://api.github.com/repos/owner/repo/contents/file.md/tmp/go-build1257888443/b396/cli.test /tmp/go-build1257888443/b396/cli.test -test.testlogfile=/tmp/go-build1257888443/b396/testlog.txt -test.paniconexit0 -test.v=true -test.parallel=4 -test.timeout=10m0s -test.run=^Test -test.short=true credential.helpenode(http block)https://api.github.com/repos/test-owner/test-repo/actions/secrets/usr/bin/gh gh api /repos/test-owner/test-repo/actions/secrets --jq .secrets[].name get --local x_amd64/vet user.name(http block)If you need me to access, download, or install something from one of these locations, you can either: