Allow add-wizard to initialize empty repositories - #51895
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #51895 does not have the 'implementation' label and has only 59 new lines of code in business logic directories (threshold: 100).
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ PR Code Quality Reviewer completed the code quality review. Warning Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding. What happenedThe threat detection engine failed to produce results. Review the workflow run logs for details. Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
To allow these domains, add them to the network:
allowed:
- defaults
- "api.individual.githubcopilot.com"See Network Configuration for more information.
|
|
✅ Test Quality Sentinel completed test quality analysis.
|
There was a problem hiding this comment.
Pull request overview
Enables add-wizard to tolerate and stage repository initialization files while still rejecting unrelated changes.
Changes:
- Adds clean-tree checks with explicit path exclusions.
- Passes initialization files through PR creation and stages them.
- Adds regression coverage for ignored initialization files.
Show a summary per file
| File | Description |
|---|---|
pkg/cli/git.go |
Adds path-aware clean-tree validation. |
pkg/cli/git_test.go |
Tests ignored generated and modified files. |
pkg/cli/add_workflow_pr.go |
Tracks initialization files for staging. |
pkg/cli/add_interactive_git.go |
Passes initialization paths into add options. |
pkg/cli/add_command.go |
Excludes initialization paths from PR cleanliness checks. |
Review details
Tip
Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Balanced
| // Create file tracker for rollback capability | ||
| tracker := NewFileTracker() | ||
| for _, initializedFile := range opts.initializedFiles { | ||
| tracker.TrackModified(initializedFile) |
| for _, initializedFile := range opts.initializedFiles { | ||
| tracker.TrackModified(initializedFile) |
ReviewOne correctness issue found in the rollback logic. Bug: In
Otherwise the path normalisation in
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
One blocking correctness issue: in add_workflow_pr.go, initializedFiles are newly-created files but are passed to tracker.TrackModified() instead of tracker.TrackCreated(). On rollback failure, RollbackModifiedFiles will find no stored original content and silently skip cleanup, leaving stale init-files on disk. Change TrackModified to TrackCreated for these files.
@copilot please address this.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 37.4 AIC · ⌖ 6.41 AIC · ⊞ 5.4K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — requesting changes on two correctness issues and a test gap.
📋 Key Themes & Highlights
Key Themes
- Rollback semantics: Init files are newly created by
InitRepositorybut tracked withTrackModified, which restores content instead of deleting on rollback. Should useTrackCreated. - Flag inconsistency:
--untracked-files=allis only added when paths are excluded, creating a silent behavioural difference in the baseline code path. - Test coverage gap: The staged-but-not-committed case for an excluded init file is not tested.
Positive Highlights
- ✅ Clean layering: the ignore list is plumbed through
AddOptionswithout touching unrelated paths - ✅ Path normalization (forward-slash, top-pathspec) is thoughtful and cross-platform
- ✅ Regression tests cover untracked and modified-tracked cases, and verify unrelated files still block
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 43.4 AIC · ⌖ 6.75 AIC · ⊞ 7.1K
Comment /matt to run again
| // Create file tracker for rollback capability | ||
| tracker := NewFileTracker() | ||
| for _, initializedFile := range opts.initializedFiles { | ||
| tracker.TrackModified(initializedFile) |
There was a problem hiding this comment.
[/diagnosing-bugs] TrackModified is used here for init files that were just created by InitRepository, not pre-existing files. If rollback is triggered, TrackModified restores the stored "original" content (captured after creation) rather than deleting the file — leaving orphaned init files in the repo.
💡 Suggested fix
Init files are brand-new, so register them with TrackCreated, which deletes them on rollback — matching how workflow and lock files are handled:
for _, initializedFile := range opts.initializedFiles {
tracker.TrackCreated(initializedFile)
}@copilot please address this.
|
|
||
| // checkCleanWorkingDirectoryIgnoring checks for uncommitted changes except for | ||
| // the provided repository-relative paths. | ||
| func checkCleanWorkingDirectoryIgnoring(verbose bool, ignoredPaths []string) error { |
There was a problem hiding this comment.
[/diagnosing-bugs] --untracked-files=all is added to git status only when ignoredPaths is non-empty. The baseline checkCleanWorkingDirectory path now uses different flags than before (it omits --untracked-files=all), which may silently miss untracked files in subdirectories in some Git configurations.
💡 Suggested fix
Always pass --untracked-files=all regardless of whether paths are excluded:
args := []string{"status", "--porcelain", "--untracked-files=all"}
if len(ignoredPaths) > 0 {
args = append(args, "--", ":(top)**")
for _, p := range ignoredPaths {
...
}
}@copilot please address this.
| require.NoError(t, exec.Command("git", "add", generatedFile).Run()) | ||
| require.NoError(t, exec.Command("git", "commit", "-m", "initial commit").Run()) | ||
| require.NoError(t, os.WriteFile(generatedFile, []byte("updated"), 0644)) | ||
|
|
There was a problem hiding this comment.
[/tdd] The test creates a SKILL.md file as the representative init file, but the test covers only the untracked and modified-tracked cases — not a staged-but-not-committed init file. A staged init file that is excluded should also pass the check.
💡 Suggested additional case
// Staged init file should still be ignored
require.NoError(t, exec.Command("git", "add", generatedFile).Run())
require.NoError(t, checkCleanWorkingDirectoryIgnoring(false, []string{generatedFile}))@copilot please address this.
🧪 Test Quality Sentinel Report✅ Test Quality Score: 100/100 — Excellent
📊 Metrics (1 test)
📋 Test Details
This test verifies the behavioral contract of the new
Classification: ✅ Design test verifying public API contract, not implementation details. Error coverage: Strong — 3 explicit error assertions ( Test inflation: 1.38:1 ratio (36 test lines / 26 production lines) — acceptable and well below the 2:1 threshold. Quality signals:
Verdict
|
|
@copilot quick triage: this PR has two fresh blocking github-actions reviews requesting changes. Please address the rollback tracking issue in
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed both review issues:
Commit: dbbb8ac → latest |
|
@copilot add git integration test |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Added two tests:
|
|
@copilot quick triage: there are still unresolved review threads, but you already replied with follow-up fixes on head Open review context (newest first):
Branch refresh was requested. Run: https://github.com/github/gh-aw/actions/runs/31452822169
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed all open threads:
Latest commit: Add staged init-file test case for checkCleanWorkingDirectoryIgnoring |
|
🎉 This pull request is included in a new release. Release: |
gh aw add-wizardrejected empty repositories after its own initialization files made the working tree appear dirty.Clean-tree validation
PR contents
Regression coverage
Run URL: https://github.com/github/gh-aw/actions/runs/31450173394> Generated by 👨🍳 PR Sous Chef · gpt54 · 24.1 AIC · ⌖ 5.21 AIC · ⊞ 6.1K · ◷