Conversation
…yaml, timeutil Agent-Logs-Url: https://github.com/github/gh-aw/sessions/804aaa7c-6a4c-4dd5-93d7-17215048fe8d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Refactors mixed-concern helper modules into smaller, domain-focused files and consolidates duplicated duration-formatting helpers into pkg/timeutil to improve navigability and auditability.
Changes:
- Split
pkg/workflow/compiler_yaml_helpers.gointo step conversion, lookups, and step generation files (leaving only minimal helpers incompiler_yaml_helpers.go). - Consolidated duration formatting into
pkg/timeutiland updated CLI renderers/parsers to delegate to the shared helpers. - Moved batch validation entrypoints (
RunActionlintOnFiles,RunZizmorOnFiles,RunPoutineOnDirectory) to the batch operations module.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/compiler_yaml_step_generation.go | Extracted workflow YAML step generation helpers and namespaced logger. |
| pkg/workflow/compiler_yaml_step_conversion.go | Extracted step→YAML conversion utilities and map-based renderer. |
| pkg/workflow/compiler_yaml_lookups.go | Extracted workflow compilation lookups (versions/models) and git-ref normalization. |
| pkg/workflow/compiler_yaml_helpers.go | Reduced to core helpers (ContainsCheckout, GetWorkflowIDFromPath) and logger. |
| pkg/timeutil/format.go | Added shared FormatDurationMs and FormatDurationNs helpers. |
| pkg/cli/util.go | Added shared boolPtr helper for CLI package. |
| pkg/cli/token_usage.go | Deprecated local FormatDurationMs and delegated to timeutil. |
| pkg/cli/mcp_repository.go | Extracted MCP repository lookup with caching. |
| pkg/cli/mcp_permissions.go | Removed mixed-concern helpers (now in dedicated MCP files). |
| pkg/cli/mcp_error.go | Extracted MCP JSON-RPC error helpers. |
| pkg/cli/compile_validation.go | Removed batch tool entrypoints (moved to batch operations). |
| pkg/cli/compile_batch_operations.go | Added exported batch validation entrypoints alongside orchestration logic. |
| pkg/cli/audit_cross_run_render.go | Delegated duration formatting to timeutil.FormatDurationNs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // that were added after the latest tag. | ||
| ref := versionToGitRef(c.version) | ||
|
|
||
| // Script mode: checkout .github folder from github/gh-aw to /tmp/gh-aw/actions-source/ |
There was a problem hiding this comment.
The comment says script mode checks out the ".github" folder, but the sparse-checkout path is "actions" and the destination path is "/tmp/gh-aw/actions-source". Please update the comment to reflect that this step checks out the actions folder (or adjust the sparse-checkout paths if the intent really is to fetch .github/.agents).
| // Script mode: checkout .github folder from github/gh-aw to /tmp/gh-aw/actions-source/ | |
| // Script mode: checkout actions folder from github/gh-aw to /tmp/gh-aw/actions-source/ |
| // If the version looks like `git describe` output with -N-gSHA, extract the SHA. | ||
| // Pattern: anything ending with -<digits>-g<hexchars> | ||
| shaRe := regexp.MustCompile(`-\d+-g([0-9a-f]+)$`) | ||
| if m := shaRe.FindStringSubmatch(clean); m != nil { |
There was a problem hiding this comment.
versionToGitRef recompiles the same regexp on every call (regexp.MustCompile(...) inside the function). Consider hoisting the compiled regexp to a package-level var so repeated compilations don’t pay the compile cost during workflow generation (this function is called in multiple code paths and has unit tests).
| // FormatDurationMs formats a duration given in milliseconds as a human-readable string. | ||
| // Examples: 500 -> "500ms", 1500 -> "1.5s", 90000 -> "1m30s" | ||
| func FormatDurationMs(ms int) string { | ||
| if ms < 1000 { | ||
| return fmt.Sprintf("%dms", ms) | ||
| } | ||
| seconds := float64(ms) / 1000.0 | ||
| if seconds < 60 { | ||
| return fmt.Sprintf("%.1fs", seconds) | ||
| } | ||
| minutes := int(seconds) / 60 | ||
| secs := math.Mod(seconds, 60) | ||
| return fmt.Sprintf("%dm%.0fs", minutes, secs) | ||
| } | ||
|
|
||
| // FormatDurationNs formats a duration given in nanoseconds as a human-readable string. | ||
| // Returns "—" for zero or negative values. Uses Go's standard duration rounding to seconds. | ||
| func FormatDurationNs(ns int64) string { | ||
| if ns <= 0 { | ||
| return "—" | ||
| } | ||
| d := time.Duration(ns) | ||
| return d.Round(time.Second).String() | ||
| } |
There was a problem hiding this comment.
New helpers FormatDurationMs and FormatDurationNs are added but format_test.go currently only covers FormatDuration. Since these helpers are now shared by multiple packages, please add unit tests covering representative and edge cases (e.g., <1s ms formatting, minute+ formatting, ns<=0 returning "—", and sub-second ns rounding behavior).
Four files mixed unrelated domains, making security audits harder and reducing navigability. Splits them by concern and consolidates duplicate duration formatters into a shared package.
pkg/cli/mcp_server_helpers.go→ 4 filesmcp_error.gonewMCPError,mcpErrorDatamcp_repository.gogetRepositorymcp_permissions.goqueryActorRole,hasWriteAccess,validateMCPWorkflowName,checkActorPermissionutil.goboolPtrPermission/authorization code is now isolated — easier to scope security reviews.
pkg/workflow/compiler_yaml_helpers.go→ 3 new filescompiler_yaml_step_conversion.goConvertStepToYAML,unquoteUsesWithComments,renderStepFromMapcompiler_yaml_lookups.gogetInstallationVersion,getDefaultAgentModel,versionToGitRefcompiler_yaml_step_generation.gogenerateCheckoutActionsFolder,generateRestoreActionsSetupStep,generateSetupStep,generateSetRuntimePathsStepcompiler_yaml_helpers.goretains onlyContainsCheckout,GetWorkflowIDFromPath, and the package logger. Each new file gets its own namespaced logger (workflow:compiler_yaml_lookups,workflow:compiler_yaml_step_generation).pkg/timeutil— two new formatterstoken_usage.FormatDurationMsandaudit_cross_run_render.formatDurationNsnow delegate totimeutil. Behavior is unchanged.compile_validation.go→compile_batch_operations.goRunActionlintOnFiles,RunZizmorOnFiles,RunPoutineOnDirectorymoved tocompile_batch_operations.gowhere the batch orchestration logic already lives.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 GO111MODULE x_amd64/link git rev-�� --show-toplevel x_amd64/link /usr/bin/git -json GO111MODULE ache/go/1.25.0/x--show-toplevel 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 GOSUMDB GOWORK 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/ai-inference/git/ref/tags/v1/usr/bin/gh gh api /repos/actions/ai-inference/git/ref/tags/v1 --jq .object.sha --show-toplevel go /opt/hostedtoolcache/node/24.14.0/x64/bin/node -json GO111MODULE ache/go/1.25.0/x--show-toplevel node /tmp�� /home/REDACTED/work/gh-aw/gh-aw/.github/workflows/artifacts-summary.md go /usr/bin/infocmp -json GO111MODULE 64/bin/go infocmp(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 4123630/b434/_pkg_.a GO111MODULE 4123630/b434=> GOINSECURE GOMOD GOMODCACHE /tmp/go-build2644123630/b411/fileutil.test -tes�� eOEC/u1ogOOgsHoxcKHcseOEC -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 ck 'scripts/**/*.js' --ignore-path .prettierignoGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE x_amd64/vet GOINSECURE GOMOD GOMODCACHE x_amd64/vet(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha --show-toplevel go /usr/bin/git -json GO111MODULE x_amd64/compile git rev-�� --show-toplevel x_amd64/compile /usr/bin/git heck '**/*.cjs' git GO111MODULE 64/bin/go git(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha --show-toplevel git /usr/bin/git log.showsignaturgit log 64/pkg/tool/linu--show-toplevel git rev-�� --show-toplevel 64/pkg/tool/linux_amd64/link /usr/bin/git ingutil.test GO111MODULE ortcfg.link 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 uts.branch --jq /usr/bin/git -json GO111MODULE n-dir/node git rese�� HEAD .github/workflows/test.md /usr/bin/git -json GO111MODULE ache/go/1.25.0/x--show-toplevel git(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha /tmp/TestHashStability_SameInputSameOutput1000688649/001/stability-test.md go r,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,disp--show-toplevel(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha --show-toplevel 2513942/b366/importcfg /usr/bin/git 5450-40691/test-git GOPROXY .cfg git rev-�� --show-toplevel /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linuremote.origin.url /usr/bin/git /tmp/go-build132git -trimpath 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 GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD GOMODCACHE go env json' --ignore-pGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha json' --ignore-pGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go ules�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --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/setup-go/git/ref/tags/v4/usr/bin/gh gh api /repos/actions/setup-go/git/ref/tags/v4 --jq .object.sha -test.paniconexit0 -test.v=true r,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,disp--show-toplevel -test.timeout=10git flow-12345 -test.short=true--show-toplevel git conf�� --get remote.origin.url /opt/hostedtoolcache/node/24.14.0/x64/bin/node -json GO111MODULE ache/go/1.25.0/x--show-toplevel node(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 xterm-color go /usr/bin/git -json GO111MODULE ache/go/1.25.0/x--show-toplevel git rev-�� --git-dir go /usr/bin/git -json GO111MODULE n-dir/sh 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 /tmp/file-tracker-test2906249278/existing.md go /usr/bin/git -json GO111MODULE 64/bin/go git rev-�� --show-toplevel go /usr/bin/git -json GO111MODULE ache/go/1.25.0/x--show-toplevel git(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 5450-40691/test-3421959636 GO111MODULE 4123630/b401/vet.cfg l GOMOD GOMODCACHE go env -json GO111MODULE /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/compile(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 -json GO111MODULE 4123630/b385/vet.cfg GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE /tmp/go-build2644123630/b070/gh-aw.test GOINSECURE GOMOD GOMODCACHE /tmp/go-build2644123630/b070/gh-aw.test(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 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go ache�� 686081023/.github/workflows GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE erignore(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 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env heck '**/*.cjs' '**/*.ts' '**/*.json' --ignore-pGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(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 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env heck '**/*.cjs' '**/*.ts' '**/*.json' --ignore-pGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(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 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go ache�� 1254520507/.github/workflows GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE erignore(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 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go ache�� 686081023/.github/workflows GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE erignore(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 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/github/gh-aw/actions/runs/5/artifacts/usr/bin/gh gh run download 5 --dir test-logs/run-5 GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go ache�� 1254520507/.github/workflows GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE erignore(http block)https://api.github.com/repos/github/gh-aw/actions/workflows/usr/bin/gh gh workflow list --json name,state,path GOSUMDB GOWORK 64/bin/go GOINSECURE GOMOD GOMODCACHE go env on' --ignore-patGOINSECURE GO111MODULE x_amd64/compile GOINSECURE GOMOD GOMODCACHE x_amd64/compile(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 GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(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 GOMOD GOMODCACHE go estl�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(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 x_amd64/compile /usr/bin/git ty-test.md GO111MODULE 64/bin/go git rev-�� --show-toplevel go /usr/bin/git edOutput26647391git GO111MODULE x_amd64/compile git(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 -json GO111MODULE At,event,headBranch,headSha,displayTitle GOINSECURE GOMOD GOMODCACHE go k/gh�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD erignore go(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 th .prettierignoGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE n-dir/node GOINSECURE GOMOD GOMODCACHE go(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 th .prettierignoGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE ache/go/1.25.0/x-lang=go1.25 GOINSECURE GOMOD igFiles,SwigCXXF/tmp/TestGuardPolicyMinIntegrityOnlyCompiledOutput3190901286/001 go(http block)/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha th .prettierignoGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE tions/setup/node-nilfunc GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha th .prettierignoGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE de_modules/.bin/-test.short=true GOINSECURE GOMOD GOMODCACHE go(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 th .prettierignoGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 0/x64/bin/node GOINSECURE GOMOD GOMODCACHE go(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 -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE R9/-4LzASpRV2zK6St_2P28/4-jKiyOZwE2ToBGe9z02 k/gh�� -json GO111MODULE 64/bin/go GOINSECURE GOMOD erignore go(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 GOINSECURE GOMOD GOMODCACHE al/zW7PK4gxNw14mfhXSJPK/Pe1lUVPh-trimpath env 1065855875/.github/workflows GO111MODULE 64/bin/go GOINSECURE GOMOD ode-gyp-bin/node--git-dir go(http block)https://api.github.com/repos/owner/repo/actions/workflows/usr/bin/gh gh workflow list --json name,state,path --repo owner/repo 64/bin/go GOINSECURE GOMOD GOMODCACHE go env on' --ignore-patGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh workflow list --json name,state,path --repo owner/repo odules/npm/node_GOMODCACHE GOINSECURE GOMOD GOMODCACHE go env on' --ignore-patGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)https://api.github.com/repos/owner/repo/contents/file.md/tmp/go-build2644123630/b396/cli.test /tmp/go-build2644123630/b396/cli.test -test.testlogfile=/tmp/go-build2644123630/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' --ignore-pGOINSECURE GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(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 GOSUMDB GOWORK odules/npm/node_GOMODCACHE 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: