Add init command regression coverage for custom agent creation#44656
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories (41 additions, threshold is 100). |
There was a problem hiding this comment.
Pull request overview
Adds explicit command-level regression coverage for gh aw init to ensure the default init path creates the expected custom agent file under .github/agents/.
Changes:
- Adds a unit test that executes
NewInitCommand()with default args and runscmd.Execute(). - Verifies
.github/agents/agentic-workflows.mdis created and contains the expectedAgentic Workflowsname marker.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/init_command_test.go | Adds a command-level regression test asserting default init creates the Agentic Workflows custom agent artifact. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Low
| if err := exec.Command("git", "init").Run(); err != nil { | ||
| t.Skip("Git not available") | ||
| } |
| if !strings.Contains(string(agentContent), "name: Agentic Workflows") { | ||
| t.Error("Expected Agentic Workflows custom agent file to use the Agentic Workflows name") | ||
| } |
There was a problem hiding this comment.
The new test is well-structured and consistent with existing patterns in the file. It correctly covers the NewInitCommand() entrypoint, sets up a real git repo, and verifies the expected agent artifact and identity string. No issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 12.2 AIC · ⌖ 4.92 AIC · ⊞ 4.8K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd — two issues to address before merge.
📋 Key Themes & Highlights
Issues Found
- Missing inverse test (line 208): The PR claims to guard against flag wiring regressions, but
--no-agentsuppression is never tested. A regression on that path would go undetected. - Potential
os.Chdirrace (line 183): The test omitst.Parallel()while sibling tests in the file run in parallel; if the scheduler interleaves them the working directory mutation will corrupt other tests.
Positive Highlights
- ✅ Tests the real CLI entrypoint (
NewInitCommand) rather than lower-level helpers — good regression anchor. - ✅ Asserts the user-visible artifact content (
name: Agentic Workflows), not just file existence. - ✅ Proper temp-dir setup via
testutil.TempDirand deferredos.Chdirrestore.
@copilot please address the review comments above.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 31.8 AIC · ⌖ 4.62 AIC · ⊞ 6.6K
Comment /matt to run again
| t.Fatalf("Failed to set git user.email: %v", err) | ||
| } | ||
|
|
||
| cmd := NewInitCommand() |
There was a problem hiding this comment.
[/tdd] The PR description says this guards against flag wiring regressions, but only the positive case (default path creates the agent) is tested — the inverse is missing.
💡 Suggested inverse test
Add a companion test that passes --no-agent and asserts the agent file is not created:
cmd := NewInitCommand()
cmd.SetArgs([]string{"--no-agent"})
if err := cmd.Execute(); err != nil {
t.Fatalf("init command failed: %v", err)
}
agentPath := filepath.Join(".github", "agents", "agentic-workflows.md")
if _, err := os.Stat(agentPath); !os.IsNotExist(err) {
t.Error("Expected --no-agent to suppress custom agent file creation")
}Without this, the flag wiring can silently break in the other direction (e.g., --no-agent stops suppressing) and no test will catch it.
@copilot please address this.
| } | ||
| } | ||
|
|
||
| func TestInitCommandCreatesCustomAgentByDefault(t *testing.T) { |
There was a problem hiding this comment.
[/tdd] The new test mutates global process state (os.Chdir) without t.Parallel(), while several sibling tests do run in parallel — this creates a race condition if the Go test runner schedules them concurrently.
💡 Why this matters and how to fix it
Tests that call os.Chdir change the working directory for the entire process, not just the current goroutine. If two such tests run concurrently one will silently operate in the wrong directory.
The safest fix is to not use os.Chdir at all. Pass the working directory explicitly to the command, or use t.Chdir(tmpDir) (Go 1.24+) which automatically serialises directory changes and restores on cleanup.
If the command API requires the cwd to be implicit, mark the test with a serial-safe pattern or accept that the suite must run with -p 1.
Existing tests in this file (TestInitRepositoryBasic, etc.) follow the same os.Chdir pattern, so this is a pre-existing issue — but the new test should not perpetuate it without a comment.
@copilot please address this.
There was a problem hiding this comment.
Review
The existing review comments already cover the two meaningful issues in this change (the overly broad t.Skip and the fragile YAML string assertion). No additional blocking concerns — the rest of the test structure is consistent with the file's existing patterns.
🔎 Code quality review by PR Code Quality Reviewer · 27.7 AIC · ⌖ 4.28 AIC · ⊞ 5.4K
Comment /review to run again
Test Quality Sentinel ReportScore: 60/100 - Acceptable Analyzed 1 test(s): 1 design, 0 implementation, 0 violations. See details below. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "awmgmcpg"See Network Configuration for more information.
|
|
🎉 This pull request is included in a new release. Release: |
gh aw initis expected to materialize the default custom agent under.github/agents/, but that behavior was only covered indirectly. This change adds command-level regression coverage so the default init path is explicitly exercised and the agent artifact is verified.What changed
gh aw initthroughNewInitCommand()rather than calling lower-level helpers directly..github/agents/agentic-workflows.md.Agentic Workflowsagent identity, keeping the test anchored to the user-visible artifact.Why this matters
InitRepository.Example