fix: only update .gitattributes if needed, skip if already up to date

[Copilot]

Summary

ensureGitAttributes() previously returned only error, making it impossible for callers to distinguish between "file was written" and "file was already up to date". All callers unconditionally showed success messages and tracked the file, even when nothing changed.

Changes

• pkg/cli/git.go: Changed ensureGitAttributes() signature from error to (bool, error) — the bool is true only when the file was actually written to disk.
• pkg/cli/add_workflow_compilation.go: Moved .gitattributes existence check to before compilation (so we know whether to use TrackCreated vs TrackModified), and deferred file tracker registration to after ensureGitAttributes() returns — only registering when updated=true. Added a comment explaining that gitattributes errors are non-fatal.
• pkg/cli/init.go: Only prints "Configured .gitattributes" when the file was actually modified.
• pkg/cli/add_command.go: Only prints "Configured .gitattributes" when the file was actually modified.
• pkg/cli/compile_post_processing.go: Only prints "Updated .gitattributes" when the file was actually modified; logs "already up to date" otherwise.
• pkg/cli/compile_file_operations.go: Updated to use _, err pattern for the new return signature.
• pkg/cli/gitattributes_test.go: Added expectUpdated bool field to each test case and asserts the return value. Also verifies that a second call on an already-correct file returns updated=false.

[Copilot]

Pull request overview

Updates .gitattributes handling so callers can detect whether it was actually changed, avoiding noisy success messages and unnecessary file tracking when the file is already up to date.

Changes:

• Changed ensureGitAttributes() to return (updated bool, err error) and updated all call sites accordingly.
• Updated compilation/add/init flows to only message/track .gitattributes when updated=true.
• Expanded unit tests to assert the new updated return value and basic idempotency behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/cli/git.go	Changes ensureGitAttributes() signature to (bool, error) and returns updated status based on whether the file was modified.
pkg/cli/add_workflow_compilation.go	Moves .gitattributes existence check earlier and only registers file tracking when ensureGitAttributes() reports an update.
pkg/cli/init.go	Only prints the success message when .gitattributes was actually modified.
pkg/cli/add_command.go	Only prints the success message when .gitattributes was actually modified.
pkg/cli/compile_post_processing.go	Only prints “updated” messaging when .gitattributes was actually modified; logs up-to-date otherwise.
pkg/cli/compile_file_operations.go	Updates call sites to ignore the new updated return value where it’s not needed.
pkg/cli/gitattributes_test.go	Adds expectUpdated to cases and asserts updated; adds a follow-up call check for idempotency in some cases.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The inline comment // Not in a git repository, skip is misleading because this branch returns a non-nil error (which some callers treat as fatal). Consider rewording to reflect that the error is propagated so callers can decide whether to skip or fail (or adjust behavior to truly “skip” by returning (false, nil) if that’s the intent).

Suggested change

	return false, err // Not in a git repository, skip
	return false, err // Propagate error (e.g., when not in a git repository) so caller can decide how to handle it

[Copilot]

This “second call” assertion only runs when the first call returned updated=true. To actually verify the stated behavior “calling again when already up to date returns updated=false”, it should also run for cases where the initial file was already correct (expectUpdated=false). Consider always doing a second call at the end of each test case and asserting updatedAgain == false (and that the content is unchanged).

Suggested change

	if tt.expectUpdated {
	updatedAgain, errAgain := ensureGitAttributes()
	if errAgain != nil {
	t.Fatalf("ensureGitAttributes() second call returned error: %v", errAgain)
	}
	if updatedAgain {
	t.Errorf("ensureGitAttributes() second call updated=true, want false (already up to date)")
	}
	}
	updatedAgain, errAgain := ensureGitAttributes()
	if errAgain != nil {
	t.Fatalf("ensureGitAttributes() second call returned error: %v", errAgain)
	}
	if updatedAgain {
	t.Errorf("ensureGitAttributes() second call updated=true, want false (already up to date)")
	}
	// Verify that the content is unchanged after the second call
	contentAfter, errAfter := os.ReadFile(gitAttributesPath)
	if errAfter != nil {
	t.Fatalf("Failed to read .gitattributes after second call: %v", errAfter)
	}
	if string(contentAfter) != string(content) {
	t.Errorf("Expected .gitattributes content to remain unchanged after second call.\nBefore:\n%s\n\nAfter:\n%s", string(content), string(contentAfter))
	}