Skip to content
4 changes: 3 additions & 1 deletion pkg/cli/add_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ type AddOptions struct {
// the workflow frontmatter, enabling GitHub Actions token auth for Copilot.
// Set by the add-wizard when the user selects org-billing auth instead of a PAT.
AddCopilotRequestsPermission bool
// initializedFiles contains files created by add-wizard after its clean-tree check.
initializedFiles []string
}

// AddWorkflowsResult contains the result of adding workflows
Expand Down Expand Up @@ -262,7 +264,7 @@ func AddResolvedWorkflows(ctx context.Context, workflowStrings []string, resolve
}

// Check no other changes are present
if err := checkCleanWorkingDirectory(opts.Verbose); err != nil {
if err := checkCleanWorkingDirectoryIgnoring(opts.Verbose, opts.initializedFiles); err != nil {
return nil, fmt.Errorf("working directory is not clean: %w", err)
}
}
Expand Down
45 changes: 45 additions & 0 deletions pkg/cli/add_command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,51 @@ func TestEnsureAddRepositoryInitialized(t *testing.T) {
})
}

// TestEnsureAddRepositoryInitializedWithDetails_AbsolutePaths verifies that
// ensureAddRepositoryInitializedWithDetails returns absolute paths for files
// that were actually written by init, and skips files that init deliberately
// does not create (e.g. .gitattributes when --no-gitattributes is used).
func TestEnsureAddRepositoryInitializedWithDetails_AbsolutePaths(t *testing.T) {
repoDir := t.TempDir()

originalFindGitRoot := addFindGitRoot
originalInitRepository := addInitRepository
originalMissingInitMarkers := addMissingInitMarkers
t.Cleanup(func() {
addFindGitRoot = originalFindGitRoot
addInitRepository = originalInitRepository
addMissingInitMarkers = originalMissingInitMarkers
})

// Use a marker whose isBootstrapInitMarkerSatisfied check uses the default
// branch (file exists and size > 0), so the test does not need to reproduce
// marker-specific content such as a SKILL.md or MCP config.
writtenMarker := ".vscode/settings.json"
skippedMarker := ".gitattributes"

addFindGitRoot = func() (string, error) { return repoDir, nil }
addMissingInitMarkers = func(string, string) ([]string, error) {
return []string{writtenMarker, skippedMarker}, nil
}
addInitRepository = func(InitOptions) error {
// Simulate init: create the settings.json marker but skip .gitattributes.
p := filepath.Join(repoDir, filepath.FromSlash(writtenMarker))
if err := os.MkdirAll(filepath.Dir(p), 0755); err != nil {
return err
}
return os.WriteFile(p, []byte(`{}`), 0644)
}

files, err := ensureAddRepositoryInitializedWithDetails("", false, true)
require.NoError(t, err)

// Only the actually-written file should be returned.
require.Len(t, files, 1)
// The returned path must be absolute.
require.True(t, filepath.IsAbs(files[0]), "expected absolute path, got %q", files[0])
require.Equal(t, filepath.Join(repoDir, filepath.FromSlash(writtenMarker)), files[0])
}

func TestAddResolvedWorkflows_IgnoresBootstrapRequireOwnerTypeDuringInstall(t *testing.T) {
originalCheckOwnerType := bootstrapCheckOwnerType
t.Cleanup(func() {
Expand Down
17 changes: 16 additions & 1 deletion pkg/cli/add_init.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cli
import (
"errors"
"fmt"
"path/filepath"

"github.com/github/gh-aw/pkg/gitutil"
)
Expand Down Expand Up @@ -35,7 +36,6 @@ func ensureAddRepositoryInitializedWithDetails(engineOverride string, verbose bo
if len(missingMarkers) == 0 {
return nil
}
initializedFiles = append(initializedFiles, missingMarkers...)

addLog.Printf("Repository missing init markers; running init: %v", missingMarkers)
if err := addInitRepository(InitOptions{
Expand All @@ -53,6 +53,21 @@ func ensureAddRepositoryInitializedWithDetails(engineOverride string, verbose bo
return fmt.Errorf("failed to initialize repository for agentic workflows: %w", err)
}

// Record only the files that were actually written by init (some markers,
// e.g. .gitattributes with --no-gitattributes, may intentionally be skipped).
// Use absolute paths so callers don't need to resolve against gitRoot.
for _, marker := range missingMarkers {
ok, statErr := isBootstrapInitMarkerSatisfied(".", marker)
if statErr != nil || !ok {
continue
}
absPath, pathErr := filepath.Abs(marker)
if pathErr != nil {
return fmt.Errorf("failed to resolve path for initialized file %s: %w", marker, pathErr)
}
initializedFiles = append(initializedFiles, absPath)
}

return nil
})
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions pkg/cli/add_interactive_git.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func (c *AddInteractiveConfig) createWorkflowPRAndConfigureSecret(ctx context.Co
StopAfter: c.StopAfter,
DisableSecurityScanner: c.DisableSecurityScanner,
AddCopilotRequestsPermission: c.UseCopilotRequests,
initializedFiles: initFiles,
}
result, err := AddResolvedWorkflows(ctx, c.WorkflowSpecs, c.resolvedWorkflows, opts)
if err != nil {
Expand Down
3 changes: 3 additions & 0 deletions pkg/cli/add_workflow_pr.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,9 @@ func addWorkflowsWithPR(ctx context.Context, workflows []*ResolvedWorkflow, opts

// Create file tracker for rollback capability
tracker := NewFileTracker()
for _, initializedFile := range opts.initializedFiles {
tracker.TrackCreated(initializedFile)
}

// Ensure we switch back to original branch on exit
defer func() {
Expand Down
31 changes: 30 additions & 1 deletion pkg/cli/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -558,9 +558,38 @@ func hasPendingChanges() (bool, error) {

// checkCleanWorkingDirectory checks if there are uncommitted changes
func checkCleanWorkingDirectory(verbose bool) error {
return checkCleanWorkingDirectoryIgnoring(verbose, nil)
}

// checkCleanWorkingDirectoryIgnoring checks for uncommitted changes except for
// the provided paths (which may be absolute or repository-relative).
func checkCleanWorkingDirectoryIgnoring(verbose bool, ignoredPaths []string) error {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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.

console.LogVerbose(verbose, "Checking for uncommitted changes...")

cmd := exec.Command("git", "status", "--porcelain")
args := []string{"status", "--porcelain", "--untracked-files=all"}
if len(ignoredPaths) > 0 {
gitRoot, err := gitutil.FindGitRoot()
if err != nil {
return fmt.Errorf("failed to find git root for path resolution: %w", err)
}
args = append(args, "--", ":(top)**")
for _, ignoredPath := range ignoredPaths {
cleaned := filepath.Clean(ignoredPath)
// Convert absolute paths to paths relative to the git root so they
// work correctly as :(top,...) pathspecs.
if filepath.IsAbs(cleaned) {
rel, relErr := filepath.Rel(gitRoot, cleaned)
if relErr != nil {
return fmt.Errorf("failed to resolve %s relative to git root: %w", ignoredPath, relErr)
}
cleaned = rel
}
path := filepath.ToSlash(strings.TrimPrefix(cleaned, "."+string(filepath.Separator)))
args = append(args, ":(top,literal,exclude)"+path)
}
}

cmd := exec.Command("git", args...)
output, err := cmd.Output()
if err != nil {
return fmt.Errorf("failed to check git status: %w", err)
Expand Down
82 changes: 82 additions & 0 deletions pkg/cli/git_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,88 @@ func TestGetCurrentBranchNotInRepo(t *testing.T) {
assert.Error(t, err, "getCurrentBranch should return an error when not in a git repository")
}

func TestCheckCleanWorkingDirectoryIgnoring(t *testing.T) {
tmpDir := testutil.TempDir(t, "test-*")

originalDir, err := os.Getwd()
require.NoError(t, err)
defer func() {
require.NoError(t, os.Chdir(originalDir))
}()

require.NoError(t, os.Chdir(tmpDir))
require.NoError(t, exec.Command("git", "init").Run())
require.NoError(t, exec.Command("git", "config", "user.name", "Test User").Run())
require.NoError(t, exec.Command("git", "config", "user.email", "test@example.com").Run())

generatedFile := filepath.Join(".github", "skills", "agentic-workflows", "SKILL.md")
require.NoError(t, os.MkdirAll(filepath.Dir(generatedFile), 0755))
require.NoError(t, os.WriteFile(generatedFile, []byte("generated"), 0644))

require.NoError(t, checkCleanWorkingDirectoryIgnoring(false, []string{generatedFile}))
require.ErrorContains(t, checkCleanWorkingDirectory(false), "working directory has uncommitted changes")

// Staged (but not committed) init file should also be excluded.
require.NoError(t, exec.Command("git", "add", generatedFile).Run())
require.NoError(t, checkCleanWorkingDirectoryIgnoring(false, []string{generatedFile}))
require.ErrorContains(t, checkCleanWorkingDirectory(false), "working directory has uncommitted changes")

require.NoError(t, exec.Command("git", "commit", "-m", "initial commit").Run())
require.NoError(t, os.WriteFile(generatedFile, []byte("updated"), 0644))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[/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.

require.NoError(t, checkCleanWorkingDirectoryIgnoring(false, []string{generatedFile}))
require.ErrorContains(t, checkCleanWorkingDirectory(false), "working directory has uncommitted changes")

require.NoError(t, os.WriteFile("README.md", []byte("user file"), 0644))
require.ErrorContains(
t,
checkCleanWorkingDirectoryIgnoring(false, []string{generatedFile}),
"working directory has uncommitted changes",
)
}

// TestCheckCleanWorkingDirectoryIgnoringAbsolutePaths verifies that absolute
// paths are accepted when the current directory is a subdirectory of the repo.
// This is the case when the wizard is invoked from a nested directory and
// ensureAddRepositoryInitializedWithDetails returns absolute paths.
func TestCheckCleanWorkingDirectoryIgnoringAbsolutePaths(t *testing.T) {
repoDir := testutil.TempDir(t, "test-*")

originalDir, err := os.Getwd()
require.NoError(t, err)
defer func() {
require.NoError(t, os.Chdir(originalDir))
}()

require.NoError(t, os.Chdir(repoDir))
require.NoError(t, exec.Command("git", "init").Run())
require.NoError(t, exec.Command("git", "config", "user.name", "Test User").Run())
require.NoError(t, exec.Command("git", "config", "user.email", "test@example.com").Run())

// Create the init file at the repo root.
generatedFile := filepath.Join(".github", "skills", "agentic-workflows", "SKILL.md")
require.NoError(t, os.MkdirAll(filepath.Dir(generatedFile), 0755))
require.NoError(t, os.WriteFile(generatedFile, []byte("generated"), 0644))
absGenerated := filepath.Join(repoDir, generatedFile)

// Create a subdirectory and cd into it to simulate a nested invocation.
subDir := filepath.Join(repoDir, "subdir")
require.NoError(t, os.MkdirAll(subDir, 0755))
require.NoError(t, os.Chdir(subDir))

// Passing the absolute path from a nested CWD should still exclude the file.
require.NoError(t, checkCleanWorkingDirectoryIgnoring(false, []string{absGenerated}))
require.ErrorContains(t, checkCleanWorkingDirectory(false), "working directory has uncommitted changes")

// An unrelated untracked file must still be detected even when the init file is excluded.
require.NoError(t, os.WriteFile(filepath.Join(repoDir, "README.md"), []byte("user file"), 0644))
require.ErrorContains(
t,
checkCleanWorkingDirectoryIgnoring(false, []string{absGenerated}),
"working directory has uncommitted changes",
)
}

func TestCreateAndSwitchBranch(t *testing.T) {
tmpDir := testutil.TempDir(t, "test-*")

Expand Down
Loading