refactor: split trial_command.go (1,007 lines) into focused files#23917
refactor: split trial_command.go (1,007 lines) into focused files#23917
Conversation
…files - trial_command.go: trimmed to NewTrialCommand() only (141 lines, down from 1007) - trial_types.go (new, 45 lines): WorkflowTrialResult, CombinedTrialResult, RepoConfig, TrialOptions - trial_runner.go (new, 261 lines): RunWorkflowTrials + getCurrentGitHubUsername - trial_helpers.go (new, 374 lines): executeTrialRun + triggerWorkflowRun + parseIssueSpec + saveTrialResult + copyTrialResultsToHostRepo - trial_confirmation.go (new, 220 lines): showTrialConfirmation No exported names or function signatures changed. All tests pass. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9a0ed25c-8ccf-4d01-8b17-719913f928b9 Co-authored-by: pelikhan <[email protected]>
There was a problem hiding this comment.
Pull request overview
Refactors the trial CLI command implementation by splitting the former monolithic trial_command.go into smaller, focused files while keeping the command wrapper thin and preserving existing public APIs.
Changes:
- Reduced
trial_command.goto Cobra command construction + flag parsing only. - Extracted trial execution flow into a dedicated runner (
RunWorkflowTrials) and helper functions. - Moved trial data types and interactive confirmation UI into their own files.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/trial_command.go | Thin Cobra command wrapper: args validation, flag parsing, and call into RunWorkflowTrials. |
| pkg/cli/trial_runner.go | Main orchestration for trial execution (mode selection, repo setup, repeat loop, username lookup). |
| pkg/cli/trial_helpers.go | Extracted helper logic for executing a trial run, triggering runs, parsing trigger context, and persisting results. |
| pkg/cli/trial_confirmation.go | Interactive confirmation/plan rendering extracted from the command file. |
| pkg/cli/trial_types.go | Centralized trial-related structs (TrialOptions, results types, repo config). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // It is called (possibly multiple times) by RunWorkflowTrials via ExecuteWithRepeat. | ||
| func executeTrialRun(ctx context.Context, parsedSpecs []*WorkflowSpec, hostRepoSlug, logicalRepoSlug, cloneRepoSlug string, directTrialMode bool, opts TrialOptions) error { | ||
| // Generate a unique datetime-ID for this trial session | ||
| dateTimeID := fmt.Sprintf("%s-%d", time.Now().Format("20060102-150405"), time.Now().UnixNano()%1000000) |
There was a problem hiding this comment.
dateTimeID is built using two separate time.Now() calls. If the clock ticks between them, the formatted timestamp portion and the nano suffix can disagree (e.g., crossing a second boundary), which is confusing for users/debugging. Capture a single time value once and derive both components from it.
| dateTimeID := fmt.Sprintf("%s-%d", time.Now().Format("20060102-150405"), time.Now().UnixNano()%1000000) | |
| now := time.Now() | |
| dateTimeID := fmt.Sprintf("%s-%d", now.Format("20060102-150405"), now.UnixNano()%1000000) |
| WorkflowName string `json:"workflow_name"` | ||
| RunID string `json:"run_id"` | ||
| SafeOutputs map[string]any `json:"safe_outputs"` | ||
| //AgentStdioLogs []string `json:"agent_stdio_logs,omitempty"` |
There was a problem hiding this comment.
There is commented-out struct field code (AgentStdioLogs) left in WorkflowTrialResult. Keeping commented code in a result type makes the JSON contract harder to reason about; either remove it or reintroduce the field properly (with tests/usage) if it’s still needed.
| WorkflowName string `json:"workflow_name"` | |
| RunID string `json:"run_id"` | |
| SafeOutputs map[string]any `json:"safe_outputs"` | |
| //AgentStdioLogs []string `json:"agent_stdio_logs,omitempty"` | |
| WorkflowName string `json:"workflow_name"` | |
| RunID string `json:"run_id"` | |
| SafeOutputs map[string]any `json:"safe_outputs"` |
| // First try to match GitHub issue URLs | ||
| urlRegex := regexp.MustCompile(`https://github\.com/[^/]+/[^/]+/issues/(\d+)`) | ||
| if matches := urlRegex.FindStringSubmatch(input); len(matches) >= 2 { | ||
| return matches[1] | ||
| } |
There was a problem hiding this comment.
parseIssueSpec recompiles regexp.MustCompile patterns on every call. The codebase often precompiles regexes at package scope for efficiency/readability (e.g., pkg/cli/firewall_log.go:20-32). Consider moving these regexes to package-level vars.
trial_command.gohad grown to 1,007 lines mixing Cobra setup, business logic, types, and UI helpers — violating the codebase's thin-command pattern.Changes
trial_command.go— trimmed toNewTrialCommand()+ flag definitions only (141 lines)trial_types.go(new, 45 lines) —WorkflowTrialResult,CombinedTrialResult,RepoConfig,TrialOptionstrial_runner.go(new, 261 lines) —RunWorkflowTrials()+getCurrentGitHubUsername(); the innerrunAllTrialsclosure is extracted toexecuteTrialRun()to keep the file under 350 linestrial_helpers.go(new, 374 lines) —executeTrialRun,triggerWorkflowRun,parseIssueSpec,saveTrialResult,copyTrialResultsToHostRepotrial_confirmation.go(new, 220 lines) —showTrialConfirmationNo exported names or function signatures changed.
RunWorkflowTrialsis now called from the thinRunEwrapper exactly as before: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 gh repo view owner/repo(http block)/usr/bin/gh gh repo view owner/host-repo(http block)/usr/bin/gh gh repo view owner/repo -j ACCEPT(http block)https://api.github.com/orgs/test-owner/actions/secrets/usr/bin/gh gh api /orgs/test-owner/actions/secrets --jq .secrets[].name -goversion go1.25.0 -c=4 -nolocalimports -importcfg /tmp/go-build2615293733/b207/importcfg -pack -o /tmp/go-build362-p o 64/bin/go -p main -lang=go1.25 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 x_amd64/compile /usr/bin/git -json GO111MODULE x_amd64/compile git rev-�� --show-toplevel x_amd64/compile /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 ithub/workflows/audit-workflows.md l /usr/bin/git -p os/exec -lang=go1.25 git rev-�� --show-toplevel xHcmtjwuXM7iv3Cv3_5D/xHcmtjwuXM7iv3Cv3_5D /usr/bin/git -goversion go1.25.0 -c=4 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 g_.a 0/internal/language/common.go 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env 14/001/test-frontmatter-with-env-template-expressions.md WfTa2tL3Z ache/go/1.25.0/x64/pkg/tool/linux_amd64/asm GOINSECURE GOMOD GOMODCACHE ache/go/1.25.0/x64/pkg/tool/linux_amd64/asm(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha xterm-color x_amd64/compile /usr/bin/git -json GO111MODULE 64/pkg/tool/linu--show-toplevel git rev-�� --show-toplevel 64/pkg/tool/linu/tmp/go-build1225676781/b434/_testmain.go /usr/bin/git g_.a poll/fd.go 64/pkg/tool/linu--show-toplevel git(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v5 --jq .object.sha --show-toplevel ache/go/1.25.0/xowner=github /usr/bin/git 5293733/b089/_pkgit d/gh-aw-wasm/mairev-parse ache/go/1.25.0/x--show-toplevel git rev-�� --show-toplevel ache/go/1.25.0/x64/pkg/tool/linu1 86_64/node -json W94_/e_Qf3Rdg4GCrev-parse 0/x64/bin/node 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(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha /repos/actions/github-script/git/ref/tags/v8 --jq r,url,status,conclusion,workflowName,createdAt,startedAt,updatedAt,event,headBranch,headSha,disp--show-toplevel -json flow-12345 x_amd64/compile git remo�� add origin /usr/bin/git -json GO111MODULE x_amd64/compile git(http block)/usr/bin/gh gh api /repos/actions/checkout/git/ref/tags/v6 --jq .object.sha --show-toplevel ache/go/1.25.0/x64/pkg/tool/linux_amd64/compile /usr/bin/git 0618-31417/test-git om/goccy/go-yamlrev-parse ache/go/1.25.0/x--show-toplevel git rev-�� --show-toplevel ortcfg /usr/bin/git tions/setup/js/ngit zaQn/pTiF_vBrcOXrev-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 go1.25.0 -c=4 -nolocalimports -importcfg /tmp/go-build2615293733/b228/importcfg -pack /home/REDACTED/go/pkg/mod/github.com/modelcontextprotocol/[email protected]/internal/mcpgodebug/mcpgodebug.go env GOPATH sh 64/bin/go -d 64/pkg/tool/linu-o 64/bin/go go(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha --check scripts/**/*.js 64/bin/go -d 64/pkg/tool/linu-o 64/bin/go go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD e_wasm.s go(http block)/usr/bin/gh gh api /repos/actions/github-script/git/ref/tags/v8 --jq .object.sha -e -f 64/bin/go -- unsafe 64/bin/go go env -json go 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 --show-toplevel go /usr/bin/git -json GO111MODULE 64/bin/go git rese�� HEAD .github/workflows/test.md /usr/bin/git -json GO111MODULE x_amd64/compile git(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 uts.branch go /usr/bin/git -json GO111MODULE 64/bin/go git -C /tmp/gh-aw-test-runs/20260401-160618-31417/test-3854410201 status /usr/bin/git .github/workflowgit GO111MODULE x_amd64/compile 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/go-build1225676781/b446/timeutil.test -importcfg /usr/bin/git -s -w -buildmode=exe git -C /tmp/gh-aw-test-runs/20260401-160618-31417/test-3854410201 rev-parse /usr/bin/git @{u} GO111MODULE x_amd64/compile 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 licyMinIntegrityOnlymin-integrity_with_explicit_repo2681965911/0remote.origin.url -trimpath 5676781/b385/vet.cfg l container/list -lang=go1.25 /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/compile -o /tmp/go-build2615293733/b162/_pkg_.a -trimpath /tmp/go-build1225676781/b070/gh-aw.test -p encoding/asn1 -lang=go1.25 /tmp/go-build1225676781/b070/gh-aw.test(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 0618-31417/test-3854410201 -trimpath 5676781/b125/vet.cfg -I /tmp/go-build261rev-parse -I /opt/hostedtoolcache/go/1.25.0/x64/pkg/tool/linux_amd64/compile -o /tmp/go-build2615293733/b157/_pkg_.a -trimpath ache/go/1.25.0/x64/pkg/tool/linux_amd64/link -p crypto/internal/rev-parse -lang=go1.25 ache/go/1.25.0/x64/pkg/tool/linux_amd64/link(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/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/[email protected] env d 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/12345/artifacts/usr/bin/gh gh run download 12345 --dir test-logs/run-12345 GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linuTest User env g_.a 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/12346/artifacts/usr/bin/gh gh run download 12346 --dir test-logs/run-12346 GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/[email protected] env g_.a GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linuremote.origin.url(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/pkg/tool/linux_amd64/compile GOINSECURE 0V/0Jbfg3fkGPmTBrev-parse GOMODCACHE 64/pkg/tool/linux_amd64/compile env 2792053037/.github/workflows 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/3/artifacts/usr/bin/gh gh run download 3 --dir test-logs/run-3 GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env md tants.go 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/4/artifacts/usr/bin/gh gh run download 4 --dir test-logs/run-4 GO111MODULE 64/pkg/tool/linux_amd64/asm GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/asm env 2792053037/.github/workflows 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/usr/bin/gh gh run download 5 --dir test-logs/run-5 GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linuTest User env md 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/workflows/usr/bin/gh gh workflow list --json name,state,path go1.25.0 -c=4 -nolocalimports -importcfg /tmp/go-build2615293733/b245/importcfg -pack /home/REDACTED/go/pkg/mod/golang.org/x/[email protected]/message/catalog/catalog.go -o /tmp/go-build362-p -trimpath 64/bin/go -p main -lang=go1.25 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 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 64/pkg/tool/linuorigin env g_.a 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/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 u1NbgiD/uvljh3C4-extld=gcc /usr/bin/git on GO111MODULE 64/pkg/tool/linu--show-toplevel git rev-�� --show-toplevel 64/pkg/tool/linux_amd64/compile /usr/bin/git edOutput37626366git GO111MODULE 64/pkg/tool/linu--verify 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 64/pkg/tool/linux_amd64/asm GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/asm env -json 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/git/ref/tags/v1.2.3/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v1.2.3 --jq .object.sha -json(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 -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go(http block)/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha -json GO111MODULE 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE x_amd64/asm GOINSECURE GOMOD GOMODCACHE x_amd64/asm(http block)/usr/bin/gh gh api /repos/github/gh-aw/git/ref/tags/v2.0.0 --jq .object.sha -json sonrpc2/conn.go 64/bin/go GOINSECURE GOMOD GOMODCACHE go env -json GO111MODULE x_amd64/asm GOINSECURE GOMOD GOMODCACHE x_amd64/asm(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 -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/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 rty 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile env g_.a GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile(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 64/pkg/tool/linux_amd64/compile env g_.a GO111MODULE 64/pkg/tool/linux_amd64/compile GOINSECURE GOMOD GOMODCACHE 64/pkg/tool/linux_amd64/compile(http block)https://api.github.com/repos/owner/repo/actions/workflows/usr/bin/gh gh workflow list --json name,state,path --repo owner/repo -nolocalimports -importcfg /tmp/go-build2615293733/b253/importcfg -pack D7YWElo_KE64 -o /tmp/go-build362-p -trimpath 64/bin/go -p github.com/githu-o -lang=go1.25 go(http block)/usr/bin/gh gh workflow list --json name,state,path --repo owner/repo -nolocalimports -importcfg /tmp/go-build2615293733/b252/importcfg -pack /home/REDACTED/work/gh-aw/gh-aw/pkg/semverutil/semverutil.go -o /tmp/go-build362-p -trimpath 64/bin/go -p github.com/githu-o -lang=go1.25 go(http block)https://api.github.com/repos/owner/repo/contents/file.md/tmp/go-build1225676781/b396/cli.test /tmp/go-build1225676781/b396/cli.test -test.testlogfile=/tmp/go-build1225676781/b396/testlog.txt -test.paniconexit0 -test.v=true -test.parallel=4 -test.timeout=10m0s -test.run=^Test -test.short=true -nolocalimports -importcfg /tmp/go-build2615293733/b185/importcfg -pack -c "prettier" --che-p sh 64/bin/go tierignore 64/pkg/tool/linu-o 64/bin/go 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 -goversion go1.25.0 -c=4 -nolocalimports -importcfg /tmp/go-build2615293733/b146/importcfg -pack -o /tmp/go-build362-p -trimpath 64/bin/go -p github.com/githu-o -lang=go1.25 go(http block)If you need me to access, download, or install something from one of these locations, you can either: