diff --git a/pkg/cli/add_command.go b/pkg/cli/add_command.go index 44758f6049a..5dc8dcd16b6 100644 --- a/pkg/cli/add_command.go +++ b/pkg/cli/add_command.go @@ -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 @@ -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) } } diff --git a/pkg/cli/add_command_test.go b/pkg/cli/add_command_test.go index f22ae73c02f..5b7d6f110cb 100644 --- a/pkg/cli/add_command_test.go +++ b/pkg/cli/add_command_test.go @@ -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() { diff --git a/pkg/cli/add_init.go b/pkg/cli/add_init.go index ff9ad8d0bbc..95f2d54d649 100644 --- a/pkg/cli/add_init.go +++ b/pkg/cli/add_init.go @@ -3,6 +3,7 @@ package cli import ( "errors" "fmt" + "path/filepath" "github.com/github/gh-aw/pkg/gitutil" ) @@ -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{ @@ -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 { diff --git a/pkg/cli/add_interactive_git.go b/pkg/cli/add_interactive_git.go index 9a0d99aa00a..7f8f22db316 100644 --- a/pkg/cli/add_interactive_git.go +++ b/pkg/cli/add_interactive_git.go @@ -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 { diff --git a/pkg/cli/add_workflow_pr.go b/pkg/cli/add_workflow_pr.go index 1bd62c28be0..d5a32bbc616 100644 --- a/pkg/cli/add_workflow_pr.go +++ b/pkg/cli/add_workflow_pr.go @@ -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() { diff --git a/pkg/cli/git.go b/pkg/cli/git.go index fc08c490135..b2b7d115983 100644 --- a/pkg/cli/git.go +++ b/pkg/cli/git.go @@ -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 { 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) diff --git a/pkg/cli/git_test.go b/pkg/cli/git_test.go index 2dd2b62e53d..b5a74562541 100644 --- a/pkg/cli/git_test.go +++ b/pkg/cli/git_test.go @@ -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)) + + 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-*")