Autoheal

[inful]

• Refactor Fixer to use go-git for atomic transactions and rollback
• Add healing phase to fix broken links using Git history
• Implement content-aware move detection in fixer_healing.go
• Refactor file operations to use Git library instead of commands
• Add unit tests for healing and rollback functionality

[Copilot]

Pull request overview

This pull request implements an "Autoheal" feature that adds Git-based atomic transactions and self-healing capabilities to the linting system. The PR introduces automatic broken link repair by analyzing Git history to detect file renames/moves, and adds rollback functionality to safely revert changes when errors occur.

Changes:

• Refactored Git operations from shell commands to use go-git library directly
• Added healing phase that detects and fixes broken links using Git history analysis
• Implemented rollback mechanism using Git hard reset to initial commit SHA
• Added helper method AddLinkUpdate to the FixResult type

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
internal/lint/fixer_healing.go	New module implementing link healing via Git history analysis and content-aware move detection
internal/lint/fixer_healing_test.go	Basic integration test for healing broken links after external file renames
internal/lint/fixer_rollback_test.go	Basic test for rollback functionality using Git hard reset
internal/lint/fixer_file_ops.go	Refactored Git operations to use go-git library; added isGitClean and rollback functions
internal/lint/fixer.go	Added rollback calls on error paths and Git cleanliness check before fix operations
internal/lint/fixer_result.go	Added AddLinkUpdate helper method to append link updates
docs/adr/adr-012-link-safe-file-normalization.md	New ADR documenting the link-safe file normalization design
docs/README.md	Added ADR-012 and other missing ADRs to the index

Comments suppressed due to low confidence (1)

internal/lint/fixer.go:180

• Error handling uses standard library functions instead of the required internal/foundation/errors package. All error construction and wrapping should use the fluent API for consistent error classification. The fmt.Errorf calls on lines 73, 135, 148, 158, 172, and 180 should use errors.WrapError() or errors.NewError() as specified in ADR-000.

			return nil, fmt.Errorf("fix failed: %w (rollback also failed: %w)", err, rollbackErr)
		}
		return nil, err
	}
	return result, nil
}

// FixWithConfirmation fixes issues with interactive confirmation prompts.
// Shows a preview of changes and prompts the user before applying.
// Creates a backup of modified files before making changes.
//
//nolint:forbidigo // fmt is used for user-facing messages
func (f *Fixer) FixWithConfirmation(path string) (*FixResult, error) {
	// If in dry-run mode, no need for confirmation or backup
	if f.dryRun {
		return f.Fix(path)
	}

	// Phase 1: Preview changes (dry-run mode internally)
	originalDryRun := f.dryRun
	f.dryRun = true
	previewResult, err := f.Fix(path)
	f.dryRun = originalDryRun

	if err != nil {
		return nil, err
	}

	// If no changes to make, return early
	if !previewResult.HasChanges() {
		return previewResult, nil
	}

	// Phase 2: Show preview and get confirmation (unless auto-confirm)
	confirmed, err := f.ConfirmChanges(previewResult)
	if err != nil {
		return nil, fmt.Errorf("confirmation failed: %w", err)
	}

	if !confirmed {
		return previewResult, errors.New("user canceled operation")
	}

	// Phase 3: Create backup (always, even with auto-confirm)
	rootPath, err := filepath.Abs(path)
	if err != nil {
		return nil, fmt.Errorf("failed to get absolute path: %w", err)
	}

	backupDir, err := f.CreateBackup(previewResult, rootPath)
	if err != nil {
		return nil, fmt.Errorf("failed to create backup: %w", err)
	}

	if backupDir != "" {
		fmt.Printf("Created backup: %s\n", backupDir)
	}

	// Phase 4: Apply fixes
	result, err := f.fix(path)
	if err != nil {
		if rollbackErr := f.rollback(); rollbackErr != nil {
			return nil, fmt.Errorf("fix failed: %w (rollback also failed: %w)", err, rollbackErr)
		}
		return nil, err
	}
	return result, nil
}

// fix is the internal implementation that actually performs the fixes.
func (f *Fixer) fix(path string) (*FixResult, error) {
	// If git-aware, check for uncommitted changes to ensure safe rollback
	if f.gitAware && !f.dryRun && !f.force {
		clean, err := f.isGitClean(path)
		if err != nil {
			return nil, fmt.Errorf("failed to check git status: %w", err)
		}
		if !clean {
			return nil, errors.New("cannot fix: git repository has uncommitted changes. commit or stash them first, or use --force")
		}
	}

	// First, run linter to find issues
	result, err := f.linter.LintPath(path)
	if err != nil {
		return nil, fmt.Errorf("failed to lint path: %w", err)
	}

	fixResult := &FixResult{
		FilesRenamed: make([]RenameOperation, 0),
		LinksUpdated: make([]LinkUpdate, 0),
		Fingerprints: make([]FingerprintUpdate, 0),
		BrokenLinks:  make([]BrokenLink, 0),
		Errors:       make([]error, 0),
	}

	// Get absolute path for the root directory (for searching links)
	rootPath, err := filepath.Abs(path)
	if err != nil {
		return nil, fmt.Errorf("failed to get absolute path: %w", err)
	}

	// Detect broken links before applying fixes
	brokenLinks, err := detectBrokenLinks(path)
	if err != nil {
		// Non-fatal: log but continue with fixes
		fixResult.Errors = append(fixResult.Errors,
			fmt.Errorf("failed to detect broken links: %w", err))

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 15 comments.

[Copilot]

The healBrokenLinks and updateLinkInFile methods don't check the dryRun flag before modifying files. This means that even in dry-run mode, the healing phase will actually modify file contents. Add dryRun checks to skip actual file writes during preview mode, similar to how other fixer operations handle dry runs.

[Copilot]

The rename detection pattern matching "old => new" using strings.Split may be brittle if filenames contain " => " as part of their name. While unlikely, this could cause incorrect parsing. Consider using a more robust parsing approach or validating that the split results in exactly 2 non-empty parts before using them.

[Copilot]

The rollback function doesn't validate that initialSHA is non-empty before attempting to reset. If the Fixer was created in a repository with no commits or if the HEAD couldn't be read during initialization (see NewFixer), initialSHA will be empty and the reset operation will fail or reset to an unexpected state. Add a check to ensure initialSHA is valid before attempting rollback.

[Copilot]

The rollback logic is called even when the Fixer is in dryRun mode. Although rollback() itself checks for dryRun, it's inefficient and semantically incorrect to attempt rollback during a dry run. Consider adding a check: "if !f.dryRun { if rollbackErr := f.rollback()... }" to skip the rollback call entirely during dry runs.

[Copilot]

Errors encountered during the healing process are silently ignored (lines 32, 40, 45). This makes debugging difficult and hides potential issues. Consider logging errors at an appropriate level or accumulating them in fixResult.Errors so users are aware when healing operations fail.

[Copilot]

Using a sentinel error built from the internal errors package as a control flow mechanism (errStop) is inappropriate. Sentinel errors should be simple, meaningful errors that can be compared directly. For internal iteration control, use a plain standard library error like "var errStop = stdErrors.New("stop iteration")" instead. The foundation errors package is meant for user-facing or logged errors that need rich context and categorization.

Suggested change

	var errStop = errors.NewError(errors.CategoryInternal, "stop iteration").Build()
	var errStop = stdErrors.New("stop iteration")

[Copilot]

Both shouldUseGitMv and gitMv resolve the repository root using filepath.Abs(".") which assumes the current working directory is the repository root. This is fragile and could fail if the fixer is called from a subdirectory. Consider storing the repository root path in the Fixer struct during initialization (obtained from gitRepo.Worktree().Filesystem.Root() or similar), or passing it as a parameter.

[Copilot]

The hardcoded limit of 10 commits (maxCommits) may be insufficient for repositories with frequent commits or long-running branches where a file was moved many commits ago. Consider making this configurable or documenting this limitation clearly so users understand why older renames might not be detected. Alternatively, consider using a time-based limit (e.g., last 30 days) instead of a commit count.

[Copilot]

The ADR references internal/lint/fixer_link_detection.go as an implementation file, but this file doesn't exist in the PR changes. The actual implementation appears to be in fixer_healing.go. Update the reference to point to the correct file.

Suggested change

	- `internal/lint/fixer_link_detection.go`: Repository-scoped link discovery and `go-git` history tracking.
	- `internal/lint/fixer_healing.go`: Repository-scoped link discovery and `go-git` history tracking.

[Copilot]

The updateLinkInFile method doesn't follow the same safety guarantees as applyLinkUpdates in fixer_link_updates.go. The healing code lacks backup creation before modifications and doesn't implement transactional rollback if an error occurs after partial writes. Consider refactoring to use the same backup/restore pattern or documenting why the simpler approach is acceptable for healing operations.