diff --git a/docs/README.md b/docs/README.md index db692c50..71262168 100644 --- a/docs/README.md +++ b/docs/README.md @@ -70,6 +70,11 @@ Architecture and design rationale: - [ADR-005: Documentation Linting](adr/adr-005-documentation-linting.md) - [ADR-006: Drop Local Namespace](adr/adr-006-drop-local-namespace.md) - [ADR-007: Merge Generate Into Build](adr/adr-007-merge-generate-into-build-command.md) +- [ADR-008: Staged Pipeline Architecture](adr/adr-008-staged-pipeline-architecture.md) +- [ADR-009: External Ingester Stage](adr/adr-009-external-ingester-stage.md) +- [ADR-010: Stable Document Identity via UID Aliases](adr/adr-010-stable-uid-aliases.md) +- [ADR-011: Set lastmod When Fingerprint Changes](adr/adr-011-lastmod-on-fingerprint-change.md) +- [ADR-012: Link-Safe File Normalization](adr/adr-012-link-safe-file-normalization.md) ## Quick Start diff --git a/docs/adr/adr-012-link-safe-file-normalization.md b/docs/adr/adr-012-link-safe-file-normalization.md new file mode 100644 index 00000000..c573ea25 --- /dev/null +++ b/docs/adr/adr-012-link-safe-file-normalization.md @@ -0,0 +1,118 @@ +--- +uid: 93bcd5b0-7d17-48c0-ac61-e41e2ae93baf +aliases: + - /_uid/93bcd5b0-7d17-48c0-ac61-e41e2ae93baf/ +title: "ADR-012: Link-Safe File Normalization" +date: 2026-01-20 +categories: + - architecture-decisions +tags: + - linting + - refactor + - file-system + - links +fingerprint: 77b435d1d6a32e5d38ef388679752e8e8308d6fd8640e950ba3cde8bad676713 +lastmod: 2026-01-20 +--- + +# ADR-012: Link-Safe File Normalization + +**Status**: Accepted +**Date**: 2026-01-20 +**Decision Makers**: DocBuilder Core Team + +## Context and Problem Statement + +DocBuilder's linting system ([ADR-005](adr-005-documentation-linting.md)) identifies violations of filename conventions (e.g., spaces, uppercase characters, non-kebab-case names). While these violations are reported as errors, fixing them manually is a complex task because renaming a Markdown file breaks all internal links pointing to it from other files within the same repository. + +To maintain a healthy documentation set, we need an automated way to rename files to match conventions while ensuring that no links are broken in the process. + +## Decision + +We will implement a link-aware self-healing system integrated into the existing `docbuilder lint --fix` command. This system will utilize Git for advanced features like history-based link healing and reliable rollbacks when available. + +To maintain consistency with the rest of the DocBuilder codebase, the implementation will: +- Use the `internal/foundation/errors` package for uniform error reporting ([ADR-000](adr-000-uniform-error-handling.md)). +- Leverage the `github.com/go-git/go-git/v5` library (already utilized in `internal/git`) for repository state inspection and history-based rename detection. +- Gracefully skip structural operations if not running within a Git repository. + +### 1. Unified Healing Workflow + +The `docbuilder lint --fix` command will be the single entry point for maintaining structural integrity: + +1. **Automated Normalization**: If the linter identifies a filename violation (e.g., uppercase, spaces), the fixer will attempt to perform a `git mv` to a valid name and simultaneously update all inbound links. +2. **External Rename Recovery**: If the linter finds a broken relative link and is running in a Git repo, it will consult the Git index/history to identify if the target was renamed, then heal the link. + +### 2. Git Integration and Graceful Degradation + +While Git is the preferred mechanism for structural changes, the system is designed to degrade gracefully: + +- **Git-Aware Renaming**: All renames are performed using `git mv` when available to preserve file history and metadata. +- **Git-Based Rollback**: + - If a Git repository is detected, operations (renames and content edits) are staged. If an error occurs, the fixer automatically rolls back all changes using `git checkout` or `git reset`, returning the workspace to its exact state before the transaction began. + - If no Git repository is found, the system skips structural normalization and link healing phases. Other linter fixes that do not depend on file moving (such as frontmatter updates or formatting) will still proceed. This ensures the tool remains useful for local development and non-Git documentation sets. +- **Commit Boundary**: The fixer does not perform Git commits; it leaves changes staged for the user to review and commit. + +### 3. Repository-Scoped Link Discovery + +The fixer needs a view of links within the local documentation repository. +- **Scan Phase**: Before performing renames, the fixer scans Markdown files in the local path to build a map of link references. +- **Inbound Tracking**: For every file slated for renaming, the fixer identifies all "inbound" links pointing to it from other files in the same repository. + +### 4. Safe Content Updates + +When updating links within a Markdown file: +- **Reverse Order**: Updates are applied from the bottom of the file to the top (descending line numbers). This ensures that modifying a line does not invalidate the line numbers for subsequent updates in the same file. +- **Atomic Write**: Updated content is written to a temporary file which is then used to replace the original file, ensuring that the file is always in a valid state on disk. + +### 5. Name Mapping and Validation + +The system relies on the core linting standards to drive automated renames. + +- **Standard Verification**: Every rename performed by the fixer must adhere to the naming conventions defined in [ADR-005](adr-005-documentation-linting.md) (lowercase, kebab-case, no special characters). +- **Collision Prevention**: The system must verify that the target destination does not already exist and is not part of another pending rename operation in the same transaction. + +### 6. Git-Aware Link Recovery + +The `docbuilder lint --fix` command will be extended to automatically repair broken relative links by consulting the Git history. + +- **Heuristic Recovery**: If a relative link points to a non-existent file, the fixer will inspect the latest Git commit(s) to check if the target file was recently renamed. +- **Git Rename Detection**: The system will use `git log --summary` or `git diff --name-status -M` to identify files that were moved or renamed. +- **Link Healing**: If a rename match is found (e.g., `OldName.md` moved to `old-name.md`), the fixer will automatically rewrite the broken link in the source file to point to the new location. +- **Scope**: This recovery is primarily focused on the most recent changes to ensure that link breaking is caught immediately after a structural change that might have happened outside of the `docbuilder mv` command (e.g., manual `git mv` or IDE-based renames). + +## Consequences + +### Pros + +- **Consistency**: Ensures all documentation follows the same naming conventions without manual toil. +- **Reliability**: Guarantees that automated fixes do not introduce broken links. +- **Git-Powered Recovery**: Utilizes native Git operations for reliable rollbacks when available. +- **Self-Healing**: Automatically repairs broken links by detecting external renames via Git history. +- **Graceful Degradation**: Safely skips complex structural fixes when running in non-Git environments without failing the tool. +- **Preservation**: Maintains Git history, which is crucial for long-lived documentation projects. +- **Developer Experience**: Allows developers to focus on content while the tooling handles structural normalization. + +### Cons + +- **Conditional Functionality**: Link healing and automated renames are only available in Git-managed repositories. +- **Complexity**: The `lint` package becomes significantly more complex to support transactional multi-file updates and Git integration. +- **Performance**: Scanning all files for links and querying Git history can be slow on very large documentation sets. +- **Edge Cases**: Complex relative paths (e.g., those involving symlinks or deep nesting) require careful handling. + +### Implementation and Reuse Strategy + +DocBuilder already possesses significant infrastructure for file operations and link detection. The implementation will heavily reuse and refactor existing components rather than building from scratch. + +- **`internal/lint/fixer.go`**: Reused as the central orchestration point. The existing `gitAware` logic will be enhanced to use `internal/git` instead of direct shell execution. +- **`internal/lint/fixer_broken_links.go`**: The `detectBrokenLinks` function will be the foundation for the "External Rename Recovery" phase. It will be extended to pass broken links to a new history inspector. +- **`internal/lint/fixer_file_ops.go`**: The existing `renameFile`, `shouldUseGitMv`, and `gitMv` functions will be refactored to support the atomic transaction requirement, likely replacing `os/exec` with `internal/git` calls for consistency. +- **`internal/lint/fixer_link_updates.go`**: Existing logic for rewriting links will be leveraged to handle the actual content updates during both healing and normalization. + +## Implementation References + +- `internal/lint/fixer.go`: Core orchestration logic using `internal/foundation/errors`. +- `internal/lint/fixer_file_ops.go`: `git mv` and Git-based rollback operations. +- `internal/lint/fixer_link_updates.go`: Atomic multi-file link rewriting. +- `internal/lint/fixer_link_detection.go`: Repository-scoped link discovery and `go-git` history tracking. +- `internal/git/errors.go`: Shared error classification for Git-related failures. diff --git a/internal/daemon/build_integration_test.go b/internal/daemon/build_integration_test.go index d6633bed..f450f80b 100644 --- a/internal/daemon/build_integration_test.go +++ b/internal/daemon/build_integration_test.go @@ -14,6 +14,7 @@ import ( "git.home.luguber.info/inful/docbuilder/internal/hugo" "git.home.luguber.info/inful/docbuilder/internal/hugo/stages" "git.home.luguber.info/inful/docbuilder/internal/state" + helpers "git.home.luguber.info/inful/docbuilder/internal/testutil/testutils" ) // TestDaemonStateBuildCounters ensures that after a full build the state manager records per-repo build counts > 0. @@ -28,21 +29,13 @@ func TestDaemonStateBuildCounters(t *testing.T) { } // Initialize a real local git repository so clone stage succeeds. - repoDir := filepath.Join(out, "remote-repoA") + _, wt, repoDir := helpers.SetupTestGitRepo(t) if err := os.MkdirAll(filepath.Join(repoDir, "docs"), 0o750); err != nil { t.Fatalf("mkdir docs: %v", err) } if err := os.WriteFile(filepath.Join(repoDir, "docs", "page.md"), []byte("# Page\n"), 0o600); err != nil { t.Fatalf("write page: %v", err) } - r, err := git.PlainInit(repoDir, false) - if err != nil { - t.Fatalf("init repo: %v", err) - } - wt, err := r.Worktree() - if err != nil { - t.Fatalf("worktree: %v", err) - } if _, addErr := wt.Add("docs/page.md"); addErr != nil { t.Fatalf("add: %v", addErr) } diff --git a/internal/daemon/discovery_state_integration_test.go b/internal/daemon/discovery_state_integration_test.go index cce0eca5..ee2dfd3b 100644 --- a/internal/daemon/discovery_state_integration_test.go +++ b/internal/daemon/discovery_state_integration_test.go @@ -14,6 +14,7 @@ import ( "git.home.luguber.info/inful/docbuilder/internal/hugo" "git.home.luguber.info/inful/docbuilder/internal/hugo/stages" "git.home.luguber.info/inful/docbuilder/internal/state" + helpers "git.home.luguber.info/inful/docbuilder/internal/testutil/testutils" ) // TestDiscoveryStagePersistsPerRepoDocFilesHash exercises the public GenerateFullSite API @@ -32,21 +33,13 @@ func TestDiscoveryStagePersistsPerRepoDocFilesHash(t *testing.T) { } // Initialize a local git repository with one markdown file in docs/. - remote := filepath.Join(tmp, "remote-repo-one") + _, wt, remote := helpers.SetupTestGitRepo(t) if err := os.MkdirAll(filepath.Join(remote, "docs"), 0o750); err != nil { t.Fatalf("mkdir docs: %v", err) } if err := os.WriteFile(filepath.Join(remote, "docs", "page.md"), []byte("# Page\n"), 0o600); err != nil { t.Fatalf("write page: %v", err) } - repo, err := git.PlainInit(remote, false) - if err != nil { - t.Fatalf("init repo: %v", err) - } - wt, err := repo.Worktree() - if err != nil { - t.Fatalf("worktree: %v", err) - } if _, addErr := wt.Add("docs/page.md"); addErr != nil { t.Fatalf("add: %v", addErr) } diff --git a/internal/git/git_ancestor_test.go b/internal/git/git_ancestor_test.go index 702daa65..35f28a05 100644 --- a/internal/git/git_ancestor_test.go +++ b/internal/git/git_ancestor_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + helpers "git.home.luguber.info/inful/docbuilder/internal/testutil/testutils" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" "github.com/go-git/go-git/v5/plumbing/object" @@ -34,11 +35,7 @@ func addCommit(t *testing.T, repo *git.Repository, repoPath, filename, content, } func TestIsAncestorEdgeCases(t *testing.T) { - tmp := t.TempDir() - repo, err := git.PlainInit(tmp, false) - if err != nil { - t.Fatalf("init: %v", err) - } + repo, _, tmp := helpers.SetupTestGitRepo(t) // Create a simple linear history: A -> B -> C a := addCommit(t, repo, tmp, "a.txt", "A", "A") diff --git a/internal/git/hash_test.go b/internal/git/hash_test.go index bc797fc3..7f2ba047 100644 --- a/internal/git/hash_test.go +++ b/internal/git/hash_test.go @@ -5,19 +5,14 @@ import ( "path/filepath" "testing" + helpers "git.home.luguber.info/inful/docbuilder/internal/testutil/testutils" "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing/object" ) func TestComputeRepoHashConsistency(t *testing.T) { // Create a temporary repository - tmpDir := t.TempDir() - repoPath := filepath.Join(tmpDir, "test-repo") - - repo, err := git.PlainInit(repoPath, false) - if err != nil { - t.Fatalf("Failed to init repo: %v", err) - } + repo, _, repoPath := helpers.SetupTestGitRepo(t) // Create test files testFile := filepath.Join(repoPath, "test.txt") @@ -78,13 +73,7 @@ func TestComputeRepoHashConsistency(t *testing.T) { func TestComputeRepoHashWithPaths(t *testing.T) { // Create a temporary repository - tmpDir := t.TempDir() - repoPath := filepath.Join(tmpDir, "test-repo") - - repo, err := git.PlainInit(repoPath, false) - if err != nil { - t.Fatalf("Failed to init repo: %v", err) - } + repo, _, repoPath := helpers.SetupTestGitRepo(t) // Create multiple directories dirs := []string{"docs", "src", "config"} @@ -152,13 +141,7 @@ func TestComputeRepoHashWithPaths(t *testing.T) { } func TestComputeRepoHashChangesWithContent(t *testing.T) { - tmpDir := t.TempDir() - repoPath := filepath.Join(tmpDir, "test-repo") - - repo, err := git.PlainInit(repoPath, false) - if err != nil { - t.Fatalf("Failed to init repo: %v", err) - } + repo, _, repoPath := helpers.SetupTestGitRepo(t) testFile := filepath.Join(repoPath, "test.txt") if writeFileErr := os.WriteFile(testFile, []byte("version 1"), 0o600); writeFileErr != nil { @@ -259,13 +242,7 @@ func TestComputeRepoHashFromWorkdir(t *testing.T) { } func TestGetRepoTree(t *testing.T) { - tmpDir := t.TempDir() - repoPath := filepath.Join(tmpDir, "test-repo") - - repo, err := git.PlainInit(repoPath, false) - if err != nil { - t.Fatalf("Failed to init repo: %v", err) - } + repo, _, repoPath := helpers.SetupTestGitRepo(t) testFile := filepath.Join(repoPath, "test.txt") if writeFileErr := os.WriteFile(testFile, []byte("test"), 0o600); writeFileErr != nil { @@ -315,13 +292,7 @@ func TestGetRepoTree(t *testing.T) { } func TestComputeRepoHashNonexistentPath(t *testing.T) { - tmpDir := t.TempDir() - repoPath := filepath.Join(tmpDir, "test-repo") - - repo, err := git.PlainInit(repoPath, false) - if err != nil { - t.Fatalf("Failed to init repo: %v", err) - } + repo, _, repoPath := helpers.SetupTestGitRepo(t) testFile := filepath.Join(repoPath, "test.txt") if writeErr := os.WriteFile(testFile, []byte("test"), 0o600); writeErr != nil { diff --git a/internal/git/update_branch_resolution_test.go b/internal/git/update_branch_resolution_test.go index fe8047f0..24f28edd 100644 --- a/internal/git/update_branch_resolution_test.go +++ b/internal/git/update_branch_resolution_test.go @@ -12,6 +12,7 @@ import ( "github.com/go-git/go-git/v5/plumbing/object" appcfg "git.home.luguber.info/inful/docbuilder/internal/config" + helpers "git.home.luguber.info/inful/docbuilder/internal/testutil/testutils" ) const ( @@ -41,11 +42,7 @@ func addSimpleCommit(t *testing.T, repo *git.Repository, repoPath, name string) } func TestResolveTargetBranchExplicit(t *testing.T) { - tmp := t.TempDir() - repo, err := git.PlainInit(tmp, false) - if err != nil { - t.Fatalf("init: %v", err) - } + repo, _, tmp := helpers.SetupTestGitRepo(t) addSimpleCommit(t, repo, tmp, "a.txt") cfg := appcfg.Repository{Name: "r", URL: "https://example/repo.git", Branch: "feature-x"} // explicit branch should always win @@ -56,11 +53,7 @@ func TestResolveTargetBranchExplicit(t *testing.T) { } func TestResolveTargetBranchFromHead(t *testing.T) { - tmp := t.TempDir() - repo, err := git.PlainInit(tmp, false) - if err != nil { - t.Fatalf("init: %v", err) - } + repo, _, tmp := helpers.SetupTestGitRepo(t) addSimpleCommit(t, repo, tmp, "a.txt") cfg := appcfg.Repository{Name: "r", URL: "https://example/repo.git"} b := resolveTargetBranch(repo, cfg) diff --git a/internal/lint/fixer.go b/internal/lint/fixer.go index a3c217c9..eacfdfa0 100644 --- a/internal/lint/fixer.go +++ b/internal/lint/fixer.go @@ -10,6 +10,8 @@ import ( "strings" "time" + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing" "github.com/inful/mdfp" ) @@ -25,20 +27,33 @@ type Fixer struct { dryRun bool force bool gitAware bool + gitRepo *git.Repository + initialSHA plumbing.Hash autoConfirm bool // Skip confirmation prompts (for CI/automated use) nowFn func() time.Time } // NewFixer creates a new fixer with the given linter and options. func NewFixer(linter *Linter, dryRun, force bool) *Fixer { - return &Fixer{ + f := &Fixer{ linter: linter, dryRun: dryRun, force: force, - gitAware: isGitRepository("."), autoConfirm: false, nowFn: time.Now, } + + // Try to open git repository + repo, err := git.PlainOpen(".") + if err == nil { + f.gitRepo = repo + f.gitAware = true + if head, hErr := repo.Head(); hErr == nil { + f.initialSHA = head.Hash() + } + } + + return f } func (f *Fixer) todayUTC() string { @@ -52,7 +67,14 @@ func (f *Fixer) todayUTC() string { // Fix attempts to automatically fix issues found in the given path. // For interactive use with confirmation prompts, use FixWithConfirmation instead. func (f *Fixer) Fix(path string) (*FixResult, error) { - return f.fix(path) + 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 } // FixWithConfirmation fixes issues with interactive confirmation prompts. @@ -63,13 +85,13 @@ func (f *Fixer) Fix(path string) (*FixResult, error) { 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) + return f.Fix(path) } // Phase 1: Preview changes (dry-run mode internally) originalDryRun := f.dryRun f.dryRun = true - previewResult, err := f.fix(path) + previewResult, err := f.Fix(path) f.dryRun = originalDryRun if err != nil { @@ -107,11 +129,29 @@ func (f *Fixer) FixWithConfirmation(path string) (*FixResult, error) { } // Phase 4: Apply fixes - return f.fix(path) + 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() + 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 { @@ -178,12 +218,15 @@ func (f *Fixer) fix(path string) (*FixResult, error) { // Phase 2: add missing uid-based aliases (for files that already have valid uids). f.applyUIDAliasesFixes(uidAliasTargets, uidAliasIssueCounts, fixResult, fingerprintTargets) - // Phase 3: perform renames + link updates. + // Phase 3: heal broken links by detecting external renames via Git history. + f.healBrokenLinks(fixResult, fingerprintTargets, rootPath) + + // Phase 4: perform renames + link updates. for filePath, issues := range fileIssues { f.processFileWithIssues(filePath, issues, rootPath, fixResult, fingerprintTargets, fingerprintIssueCounts) } - // Phase 4: regenerate fingerprints LAST, for all affected files. + // Phase 5: regenerate fingerprints LAST, for all affected files. // (This must remain the final fixer phase.) f.applyFingerprintFixes(fingerprintTargets, fingerprintIssueCounts, fixResult) diff --git a/internal/lint/fixer_broken_links.go b/internal/lint/fixer_broken_links.go index 5cec4576..3c10c0b7 100644 --- a/internal/lint/fixer_broken_links.go +++ b/internal/lint/fixer_broken_links.go @@ -122,6 +122,7 @@ func checkInlineLinksBroken(line string, lineNum int, sourceFile string) []Broke LineNumber: lineNum, Target: linkInfo.target, LinkType: LinkTypeInline, + FullMatch: line[linkInfo.start : linkInfo.end+1], }) } } @@ -191,6 +192,7 @@ func checkReferenceLinksBroken(line string, lineNum int, sourceFile string) []Br LineNumber: lineNum, Target: linkTarget, LinkType: LinkTypeReference, + FullMatch: line, }) } @@ -233,6 +235,7 @@ func checkImageLinksBroken(line string, lineNum int, sourceFile string) []Broken LineNumber: lineNum, Target: linkInfo.target, LinkType: LinkTypeImage, + FullMatch: line[linkInfo.start : linkInfo.end+1], }) } } diff --git a/internal/lint/fixer_file_ops.go b/internal/lint/fixer_file_ops.go index 80f41a8b..26a1b1bb 100644 --- a/internal/lint/fixer_file_ops.go +++ b/internal/lint/fixer_file_ops.go @@ -2,12 +2,14 @@ package lint import ( "context" - "errors" - "fmt" "io" "os" "os/exec" "path/filepath" + "strings" + + "git.home.luguber.info/inful/docbuilder/internal/foundation/errors" + "github.com/go-git/go-git/v5" ) // renameFile renames a file to fix filename issues. @@ -22,7 +24,7 @@ func (f *Fixer) renameFile(oldPath string) RenameOperation { suggestedName := SuggestFilename(filename) if suggestedName == "" || suggestedName == filename { - op.Error = errors.New("could not determine suggested filename or file is already correct") + op.Error = errors.NewError(errors.CategoryValidation, "could not determine suggested filename or file is already correct").Build() return op } @@ -33,7 +35,9 @@ func (f *Fixer) renameFile(oldPath string) RenameOperation { // Check if target already exists if _, err := os.Stat(newPath); err == nil && !f.force { - op.Error = fmt.Errorf("target file already exists: %s", newPath) + op.Error = errors.NewError(errors.CategoryAlreadyExists, "target file already exists"). + WithContext("path", newPath). + Build() return op } @@ -48,14 +52,14 @@ func (f *Fixer) renameFile(oldPath string) RenameOperation { // Use git mv to preserve history err := f.gitMv(oldPath, newPath) if err != nil { - op.Error = fmt.Errorf("git mv failed: %w", err) + op.Error = errors.WrapError(err, errors.CategoryGit, "git mv failed").Build() return op } } else { // Use regular file system rename err := os.Rename(oldPath, newPath) if err != nil { - op.Error = fmt.Errorf("rename failed: %w", err) + op.Error = errors.WrapError(err, errors.CategoryFileSystem, "rename failed").Build() return op } } @@ -64,25 +68,81 @@ func (f *Fixer) renameFile(oldPath string) RenameOperation { return op } -// shouldUseGitMv checks if a file is under Git version control. +// shouldUseGitMv checks if a file is under Git version control and tracked. func (f *Fixer) shouldUseGitMv(filePath string) bool { - if !f.gitAware { + if f.gitRepo == nil { return false } - // Check if file is tracked by Git - cmd := exec.CommandContext(context.Background(), "git", "ls-files", "--error-unmatch", filePath) - err := cmd.Run() + // Use the git index to check if the file is tracked. + // Only tracked files can be moved with 'git mv'. + idx, err := f.gitRepo.Storer.Index() + if err != nil { + return false + } + + // Git paths in the index are always relative to the repository root and use forward slashes. + // We assume f.gitRepo was opened at the root of the workspace being fixed (.). + absPath, err := filepath.Abs(filePath) + if err != nil { + return false + } + root, err := filepath.Abs(".") + if err != nil { + return false + } + + relPath, err := filepath.Rel(root, absPath) + if err != nil || strings.HasPrefix(relPath, "..") { + return false // Path is outside repository + } + + relPath = filepath.ToSlash(relPath) + _, err = idx.Entry(relPath) return err == nil } -// gitMv performs a git mv operation. +// gitMv performs a git mv operation using the Git library. func (f *Fixer) gitMv(oldPath, newPath string) error { - cmd := exec.CommandContext(context.Background(), "git", "mv", oldPath, newPath) - output, err := cmd.CombinedOutput() + if f.gitRepo == nil { + return errors.NewError(errors.CategoryGit, "git repository not initialized").Build() + } + + w, err := f.gitRepo.Worktree() + if err != nil { + return errors.WrapError(err, errors.CategoryGit, "failed to get git worktree").Build() + } + + // Calculate paths relative to the repository root (.) + root, err := filepath.Abs(".") + if err != nil { + return errors.WrapError(err, errors.CategoryFileSystem, "failed to get absolute path of current directory").Build() + } + + absOld, err := filepath.Abs(oldPath) + if err != nil { + return errors.WrapError(err, errors.CategoryFileSystem, "failed to get absolute path of old file").Build() + } + relOld, err := filepath.Rel(root, absOld) + if err != nil { + return errors.WrapError(err, errors.CategoryFileSystem, "failed to get relative path of old file").Build() + } + + absNew, err := filepath.Abs(newPath) if err != nil { - return fmt.Errorf("%w: %s", err, string(output)) + return errors.WrapError(err, errors.CategoryFileSystem, "failed to get absolute path of new file").Build() } + relNew, err := filepath.Rel(root, absNew) + if err != nil { + return errors.WrapError(err, errors.CategoryFileSystem, "failed to get relative path of new file").Build() + } + + // Perform the move using repository-relative paths + _, err = w.Move(filepath.ToSlash(relOld), filepath.ToSlash(relNew)) + if err != nil { + return errors.WrapError(err, errors.CategoryGit, "failed to move file in git").Build() + } + return nil } @@ -93,6 +153,48 @@ func isGitRepository(dir string) bool { return err == nil } +// isGitClean checks if the Git repository has uncommitted changes. +func (f *Fixer) isGitClean() (bool, error) { + if f.gitRepo == nil { + return true, nil + } + + w, err := f.gitRepo.Worktree() + if err != nil { + return false, errors.WrapError(err, errors.CategoryGit, "failed to get git worktree").Build() + } + + status, err := w.Status() + if err != nil { + return false, errors.WrapError(err, errors.CategoryGit, "failed to get git status").Build() + } + + return status.IsClean(), nil +} + +// rollback reverts all changes made during the fixer session using Git. +func (f *Fixer) rollback() error { + if f.gitRepo == nil || f.dryRun { + return nil + } + + w, err := f.gitRepo.Worktree() + if err != nil { + return errors.WrapError(err, errors.CategoryGit, "failed to get git worktree").Build() + } + + // Reset to initial SHA + err = w.Reset(&git.ResetOptions{ + Commit: f.initialSHA, + Mode: git.HardReset, + }) + if err != nil { + return errors.WrapError(err, errors.CategoryGit, "git reset failed").Build() + } + + return nil +} + // backupFile copies a file to the backup directory, preserving directory structure. func (f *Fixer) backupFile(filePath, backupDir, rootPath string) error { // Get relative path from root diff --git a/internal/lint/fixer_healing.go b/internal/lint/fixer_healing.go new file mode 100644 index 00000000..b0e2a6e8 --- /dev/null +++ b/internal/lint/fixer_healing.go @@ -0,0 +1,208 @@ +package lint + +import ( + stdErrors "errors" + "os" + "path/filepath" + "strings" + + "git.home.luguber.info/inful/docbuilder/internal/foundation/errors" + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/object" +) + +var errStop = errors.NewError(errors.CategoryInternal, "stop iteration").Build() + +// healBrokenLinks attempts to fix links that were broken by external renames/moves +// using Git history to find where the files went. +func (f *Fixer) healBrokenLinks(fixResult *FixResult, _ map[string]struct{}, root string) { + if f.gitRepo == nil { + return + } + + brokenLinks, err := detectBrokenLinks(root) + if err != nil || len(brokenLinks) == 0 { + return + } + + for _, bl := range brokenLinks { + // Resolve the absolute path of the broken link target + absTarget, err := resolveRelativePath(bl.SourceFile, bl.Target) + if err != nil { + continue + } + + // Attempt to find where absTarget went + newPath, err := f.findMovedFileInHistory(absTarget, root) + if err == nil && newPath != "" { + // Calculate relative link from SourceFile to newPath + relLink, err := filepath.Rel(filepath.Dir(bl.SourceFile), newPath) + if err == nil { + // Normalize for Markdown + relLink = filepath.ToSlash(relLink) + + err = f.updateLinkInFile(bl, relLink) + if err == nil { + fixResult.AddLinkUpdate(bl.SourceFile, bl.Target, relLink) + } + } + } + } +} + +// updateLinkInFile replaces a broken link target with a new one in the source file. +// It uses precise replacement based on the FullMatch metadata to avoid corrupting +// non-link content or link text. +func (f *Fixer) updateLinkInFile(bl BrokenLink, newRelTarget string) error { + // #nosec G304 -- bl.SourceFile is from discovery walkFiles, not user input + content, err := os.ReadFile(bl.SourceFile) + if err != nil { + return errors.WrapError(err, errors.CategoryFileSystem, "failed to read file"). + WithContext("file", bl.SourceFile). + Build() + } + + // Preserve fragment if present in original target + newLinkWithFragment := newRelTarget + if idx := strings.Index(bl.Target, "#"); idx != -1 { + newLinkWithFragment += bl.Target[idx:] + } + + // Calculate the precise new full match + // We replace the target part within the full markdown link syntax. + newFullMatch := strings.Replace(bl.FullMatch, bl.Target, newLinkWithFragment, 1) + + // Replace the old full match with the new one in the content. + // We use ReplaceAll because if the same exact link match appears multiple times in the file, + // they should all be healed. + newContent := strings.ReplaceAll(string(content), bl.FullMatch, newFullMatch) + + if newContent == string(content) { + return errors.NewError(errors.CategoryNotFound, "link not found in file"). + WithContext("file", bl.SourceFile). + WithContext("match", bl.FullMatch). + Build() + } + + err = os.WriteFile(bl.SourceFile, []byte(newContent), 0o600) + if err != nil { + return errors.WrapError(err, errors.CategoryFileSystem, "failed to write file"). + WithContext("file", bl.SourceFile). + Build() + } + return nil +} + +// findMovedFileInHistory searches Git logs to find a rename or move of the target path. +func (f *Fixer) findMovedFileInHistory(targetPath string, root string) (string, error) { + ref, err := f.gitRepo.Head() + if err != nil { + return "", errors.WrapError(err, errors.CategoryGit, "failed to get head ref").Build() + } + + cIter, err := f.gitRepo.Log(&git.LogOptions{From: ref.Hash()}) + if err != nil { + return "", errors.WrapError(err, errors.CategoryGit, "failed to get git log").Build() + } + + // We only look back a few commits to avoid heavy processing + const maxCommits = 10 + count := 0 + + relativeTarget, err := filepath.Rel(root, targetPath) + if err != nil { + return "", errors.WrapError(err, errors.CategoryFileSystem, "failed to calculate relative path"). + WithContext("root", root). + WithContext("target", targetPath). + Build() + } + relativeTarget = filepath.ToSlash(relativeTarget) + + var foundPath string + err = cIter.ForEach(func(c *object.Commit) error { + if count >= maxCommits { + return errStop + } + count++ + + stats, sErr := c.Stats() + if sErr != nil { + return errors.WrapError(sErr, errors.CategoryGit, "failed to get commit stats").Build() + } + + // Check if targetPath was deleted or renamed in this commit + deleted := false + for _, stat := range stats { + // Handle regular deletion + if stat.Name == relativeTarget && stat.Addition == 0 && stat.Deletion > 0 { + deleted = true + break + } + + // Handle git-detected rename "old => new" + if strings.HasPrefix(stat.Name, relativeTarget+" => ") { + parts := strings.Split(stat.Name, " => ") + if len(parts) == 2 { + foundPath = filepath.Join(root, parts[1]) + return errStop + } + } + } + + if deleted { + foundPath = f.findSameContentFile(c, relativeTarget, root) + if foundPath != "" { + return errStop + } + } + + return nil + }) + + if err != nil && !stdErrors.Is(err, errStop) { + return "", err + } + + return foundPath, nil +} + +// findSameContentFile searches for a file with the same content as the deleted one in the same commit. +func (f *Fixer) findSameContentFile(c *object.Commit, relativeTarget, root string) string { + // Get tree of this commit + tree, err := c.Tree() + if err != nil { + return "" + } + + // We need to find the blob hash of the targetPath in the PREVIOUS commit + if c.NumParents() == 0 { + return "" + } + parent, err := c.Parent(0) + if err != nil { + return "" + } + parentTree, err := parent.Tree() + if err != nil { + return "" + } + + entry, err := parentTree.FindEntry(relativeTarget) + if err != nil { + return "" + } + + targetHash := entry.Hash + + var foundPath string + // Search the current commit's tree for the same hash + _ = tree.Files().ForEach(func(file *object.File) error { + if file.Hash == targetHash && file.Name != relativeTarget { + foundPath = filepath.Join(root, file.Name) + return errStop + } + return nil + }) + + return foundPath +} diff --git a/internal/lint/fixer_healing_test.go b/internal/lint/fixer_healing_test.go new file mode 100644 index 00000000..f6df8a0e --- /dev/null +++ b/internal/lint/fixer_healing_test.go @@ -0,0 +1,368 @@ +package lint + +import ( + "os" + "path/filepath" + "testing" + + helpers "git.home.luguber.info/inful/docbuilder/internal/testutil/testutils" + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/object" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFixer_HealBrokenLinks(t *testing.T) { + // Setup temporary directory and git repo + repo, w, tempDir := helpers.SetupTestGitRepo(t) + + // 1. Create a file and commit it + oldFile := filepath.Join(tempDir, "target.md") + err := os.WriteFile(oldFile, []byte("# Target File\nContent"), 0o600) + require.NoError(t, err) + + _, err = w.Add("target.md") + require.NoError(t, err) + + _, err = w.Commit("Initial commit", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com"}, + }) + require.NoError(t, err) + + // 2. Rename the file manually and commit + newFile := filepath.Join(tempDir, "moved_target.md") + err = os.Rename(oldFile, newFile) + require.NoError(t, err) + + _, err = w.Add("target.md") // Mark as deleted + require.NoError(t, err) + _, err = w.Add("moved_target.md") // Mark as added + require.NoError(t, err) + + _, err = w.Commit("Move target file", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com"}, + }) + require.NoError(t, err) + + // 3. Create a file with a broken link to the old path + sourceFile := filepath.Join(tempDir, "index.md") + err = os.WriteFile(sourceFile, []byte("# Index\n[Link](./target.md)"), 0o600) + require.NoError(t, err) + + // 4. Initialize Fixer in the temp directory + // We need to override the repo discovery for testing + linter := NewLinter(nil) + fixer := &Fixer{ + linter: linter, + gitRepo: repo, + gitAware: true, + nowFn: nil, + } + + // 5. Run healing + result := &FixResult{} + fixer.healBrokenLinks(result, nil, tempDir) + + // 6. Verify link was updated + // #nosec G304 -- sourceFile is deterministic in tests + content, err := os.ReadFile(sourceFile) + require.NoError(t, err) + assert.Contains(t, string(content), "[Link](moved_target.md)") + assert.Len(t, result.LinksUpdated, 1) + assert.Equal(t, "./target.md", result.LinksUpdated[0].OldTarget) + assert.Equal(t, "moved_target.md", result.LinksUpdated[0].NewTarget) +} + +func TestFixer_UpdateLinkInFile(t *testing.T) { + tempDir := t.TempDir() + fixer := &Fixer{} + + t.Run("successful update", func(t *testing.T) { + sourceFile := filepath.Join(tempDir, "source.md") + originalContent := "# Title\n[Link](old.md)\nSome more text." + err := os.WriteFile(sourceFile, []byte(originalContent), 0o600) + require.NoError(t, err) + + bl := BrokenLink{ + SourceFile: sourceFile, + Target: "old.md", + FullMatch: "[Link](old.md)", + } + + err = fixer.updateLinkInFile(bl, "new.md") + require.NoError(t, err) + + // #nosec G304 -- test file + content, err := os.ReadFile(sourceFile) + require.NoError(t, err) + assert.Equal(t, "# Title\n[Link](new.md)\nSome more text.", string(content)) + }) + + t.Run("multiple occurrences of exactly same match", func(t *testing.T) { + sourceFile := filepath.Join(tempDir, "multiple_exact.md") + originalContent := "[Link](old.md) and another [Link](old.md)." + err := os.WriteFile(sourceFile, []byte(originalContent), 0o600) + require.NoError(t, err) + + bl := BrokenLink{ + SourceFile: sourceFile, + Target: "old.md", + FullMatch: "[Link](old.md)", + } + + err = fixer.updateLinkInFile(bl, "new.md") + require.NoError(t, err) + + // #nosec G304 -- test file + content, err := os.ReadFile(sourceFile) + require.NoError(t, err) + assert.Equal(t, "[Link](new.md) and another [Link](new.md).", string(content)) + }) + + t.Run("multiple occurrences with different text", func(t *testing.T) { + sourceFile := filepath.Join(tempDir, "multiple_diff.md") + originalContent := "[Link1](old.md) and [Link2](old.md)." + err := os.WriteFile(sourceFile, []byte(originalContent), 0o600) + require.NoError(t, err) + + // First match + bl1 := BrokenLink{ + SourceFile: sourceFile, + Target: "old.md", + FullMatch: "[Link1](old.md)", + } + err = fixer.updateLinkInFile(bl1, "new.md") + require.NoError(t, err) + + // Second match + bl2 := BrokenLink{ + SourceFile: sourceFile, + Target: "old.md", + FullMatch: "[Link2](old.md)", + } + err = fixer.updateLinkInFile(bl2, "new.md") + require.NoError(t, err) + + // #nosec G304 -- test file + content, err := os.ReadFile(sourceFile) + require.NoError(t, err) + assert.Equal(t, "[Link1](new.md) and [Link2](new.md).", string(content)) + }) + + t.Run("preserve fragment", func(t *testing.T) { + sourceFile := filepath.Join(tempDir, "fragment.md") + originalContent := "[Link](old.md#section)" + err := os.WriteFile(sourceFile, []byte(originalContent), 0o600) + require.NoError(t, err) + + bl := BrokenLink{ + SourceFile: sourceFile, + Target: "old.md#section", + FullMatch: "[Link](old.md#section)", + } + + err = fixer.updateLinkInFile(bl, "new.md") + require.NoError(t, err) + + // #nosec G304 -- test file + content, err := os.ReadFile(sourceFile) + require.NoError(t, err) + assert.Equal(t, "[Link](new.md#section)", string(content)) + }) + + t.Run("link not found error", func(t *testing.T) { + sourceFile := filepath.Join(tempDir, "notfound.md") + originalContent := "[Link](other.md)" + err := os.WriteFile(sourceFile, []byte(originalContent), 0o600) + require.NoError(t, err) + + bl := BrokenLink{ + SourceFile: sourceFile, + Target: "old.md", + FullMatch: "[Link](old.md)", + } + + err = fixer.updateLinkInFile(bl, "new.md") + assert.Error(t, err) + assert.Contains(t, err.Error(), "link not found in file") + }) + + t.Run("reference-style link", func(t *testing.T) { + sourceFile := filepath.Join(tempDir, "reference.md") + originalContent := "# Ref\n[id]: old.md\n" + err := os.WriteFile(sourceFile, []byte(originalContent), 0o600) + require.NoError(t, err) + + bl := BrokenLink{ + SourceFile: sourceFile, + Target: "old.md", + FullMatch: "[id]: old.md", + LinkType: LinkTypeReference, + } + + err = fixer.updateLinkInFile(bl, "new.md") + require.NoError(t, err) + + // #nosec G304 -- test file + content, err := os.ReadFile(sourceFile) + require.NoError(t, err) + assert.Equal(t, "# Ref\n[id]: new.md\n", string(content)) + }) + + t.Run("mixed styles in one file", func(t *testing.T) { + sourceFile := filepath.Join(tempDir, "mixed.md") + originalContent := "[Inline](old.md)\n\n[id]: old.md" + err := os.WriteFile(sourceFile, []byte(originalContent), 0o600) + require.NoError(t, err) + + // Fix inline + err = fixer.updateLinkInFile(BrokenLink{ + SourceFile: sourceFile, + Target: "old.md", + FullMatch: "[Inline](old.md)", + }, "new.md") + require.NoError(t, err) + + // Fix reference + err = fixer.updateLinkInFile(BrokenLink{ + SourceFile: sourceFile, + Target: "old.md", + FullMatch: "[id]: old.md", + }, "new.md") + require.NoError(t, err) + + // #nosec G304 -- test file + content, err := os.ReadFile(sourceFile) + require.NoError(t, err) + assert.Equal(t, "[Inline](new.md)\n\n[id]: new.md", string(content)) + }) +} + +func TestFixer_FindSameContentFile(t *testing.T) { + repo, w, tempDir := helpers.SetupTestGitRepo(t) + + signature := &object.Signature{Name: "Test", Email: "test@example.com"} + + // 1. Initial commit with two files + f1Path := "file1.md" + f2Path := "file2.md" + content1 := []byte("Initial content") + content2 := []byte("Different content") + + err := os.WriteFile(filepath.Join(tempDir, f1Path), content1, 0o600) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(tempDir, f2Path), content2, 0o600) + require.NoError(t, err) + + _, err = w.Add(f1Path) + require.NoError(t, err) + _, err = w.Add(f2Path) + require.NoError(t, err) + + initialCommitHash, err := w.Commit("Initial commit", &git.CommitOptions{Author: signature}) + require.NoError(t, err) + + fixer := &Fixer{} + + t.Run("finds same content file after rename", func(t *testing.T) { + // Second commit: Rename file1.md to renamed.md + newName := "renamed.md" + err = os.Rename(filepath.Join(tempDir, f1Path), filepath.Join(tempDir, newName)) + require.NoError(t, err) + + _, err = w.Add(f1Path) // Delete old + require.NoError(t, err) + _, err = w.Add(newName) // Add new + require.NoError(t, err) + + secondCommitHash, err := w.Commit("Rename file", &git.CommitOptions{Author: signature}) + require.NoError(t, err) + + secondCommit, err := repo.CommitObject(secondCommitHash) + require.NoError(t, err) + + found := fixer.findSameContentFile(secondCommit, f1Path, tempDir) + assert.Equal(t, filepath.Join(tempDir, newName), found) + }) + + t.Run("returns empty if content is different", func(t *testing.T) { + // Rename file2.md BUT change content too + err = os.Rename(filepath.Join(tempDir, f2Path), filepath.Join(tempDir, "renamed2.md")) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(tempDir, "renamed2.md"), []byte("Now it's different"), 0o600) + require.NoError(t, err) + + _, err = w.Add(f2Path) + require.NoError(t, err) + _, err = w.Add("renamed2.md") + require.NoError(t, err) + + thirdCommitHash, err := w.Commit("Change content during rename", &git.CommitOptions{Author: signature}) + require.NoError(t, err) + + thirdCommit, err := repo.CommitObject(thirdCommitHash) + require.NoError(t, err) + + // Search for content of file2.md as it was in initialCommit (via its parent) + found := fixer.findSameContentFile(thirdCommit, f2Path, tempDir) + assert.Empty(t, found) + }) + + t.Run("returns empty if no parent commit", func(t *testing.T) { + initialCommit, err := repo.CommitObject(initialCommitHash) + require.NoError(t, err) + + found := fixer.findSameContentFile(initialCommit, f1Path, tempDir) + assert.Empty(t, found) + }) + + t.Run("returns empty if file not in parent", func(t *testing.T) { + // Create a new commit where we add a file that wasn't there before + err = os.WriteFile(filepath.Join(tempDir, "newfile.md"), []byte("New file content"), 0o600) + require.NoError(t, err) + _, err = w.Add("newfile.md") + require.NoError(t, err) + + newCommitHash, err := w.Commit("Add new file", &git.CommitOptions{Author: signature}) + require.NoError(t, err) + + newCommit, err := repo.CommitObject(newCommitHash) + require.NoError(t, err) + + // Search for a path that didn't exist in parent + found := fixer.findSameContentFile(newCommit, "something_else.md", tempDir) + assert.Empty(t, found) + }) + + t.Run("finds one among multiple copies", func(t *testing.T) { + // Commit two files with exact same content + sameContent := []byte("Identical content") + err = os.WriteFile(filepath.Join(tempDir, "copy1.md"), sameContent, 0o600) + require.NoError(t, err) + err = os.WriteFile(filepath.Join(tempDir, "copy2.md"), sameContent, 0o600) + require.NoError(t, err) + + _, err = w.Add("copy1.md") + require.NoError(t, err) + _, err = w.Add("copy2.md") + require.NoError(t, err) + + _, err = w.Commit("Base for copies", &git.CommitOptions{Author: signature}) + require.NoError(t, err) + + // Remove copy1.md in next commit + err = os.Remove(filepath.Join(tempDir, "copy1.md")) + require.NoError(t, err) + _, err = w.Add("copy1.md") + require.NoError(t, err) + + nextHash, err := w.Commit("Remove copy1", &git.CommitOptions{Author: signature}) + require.NoError(t, err) + + nextCommit, err := repo.CommitObject(nextHash) + require.NoError(t, err) + + found := fixer.findSameContentFile(nextCommit, "copy1.md", tempDir) + assert.Equal(t, filepath.Join(tempDir, "copy2.md"), found) + }) +} diff --git a/internal/lint/fixer_result.go b/internal/lint/fixer_result.go index 73ecf332..b466968f 100644 --- a/internal/lint/fixer_result.go +++ b/internal/lint/fixer_result.go @@ -24,6 +24,15 @@ type FingerprintUpdate struct { Error error } +// AddLinkUpdate adds a link update to the result. +func (f *FixResult) AddLinkUpdate(sourceFile, oldTarget, newTarget string) { + f.LinksUpdated = append(f.LinksUpdated, LinkUpdate{ + SourceFile: sourceFile, + OldTarget: oldTarget, + NewTarget: newTarget, + }) +} + // RenameOperation represents a file rename operation. type RenameOperation struct { OldPath string @@ -42,10 +51,11 @@ type LinkUpdate struct { // BrokenLink represents a link to a non-existent file. type BrokenLink struct { - SourceFile string // File containing the broken link - LineNumber int // Line number of the link - Target string // Link target that doesn't exist - LinkType LinkType + SourceFile string // File containing the broken link + LineNumber int // Line number of the link + Target string // Link target that doesn't exist + LinkType LinkType // Type of link (inline, reference, image) + FullMatch string // Complete original link text for precise replacement } // LinkType represents the type of markdown link. diff --git a/internal/lint/fixer_result_test.go b/internal/lint/fixer_result_test.go index a501d60e..50f0e6a3 100644 --- a/internal/lint/fixer_result_test.go +++ b/internal/lint/fixer_result_test.go @@ -66,6 +66,26 @@ func TestFixResult_DetailedPreview(t *testing.T) { assert.Contains(t, preview, "./missing.md (file not found)", "should show broken link target") } +// TestFixResult_AddLinkUpdate tests adding link updates to the result. +func TestFixResult_AddLinkUpdate(t *testing.T) { + result := &FixResult{} + + result.AddLinkUpdate("src/index.md", "old/doc.md", "new/doc.md") + result.AddLinkUpdate("src/guide.md", "old/img.png", "new/img.png") + + assert.Len(t, result.LinksUpdated, 2) + + // Check first update + assert.Equal(t, "src/index.md", result.LinksUpdated[0].SourceFile) + assert.Equal(t, "old/doc.md", result.LinksUpdated[0].OldTarget) + assert.Equal(t, "new/doc.md", result.LinksUpdated[0].NewTarget) + + // Check second update + assert.Equal(t, "src/guide.md", result.LinksUpdated[1].SourceFile) + assert.Equal(t, "old/img.png", result.LinksUpdated[1].OldTarget) + assert.Equal(t, "new/img.png", result.LinksUpdated[1].NewTarget) +} + // TestFixResult_HasChanges tests the HasChanges method. func TestFixResult_HasChanges(t *testing.T) { tests := []struct { diff --git a/internal/lint/fixer_rollback_test.go b/internal/lint/fixer_rollback_test.go new file mode 100644 index 00000000..e0ddeac6 --- /dev/null +++ b/internal/lint/fixer_rollback_test.go @@ -0,0 +1,82 @@ +package lint + +import ( + "os" + "path/filepath" + "testing" + + "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/plumbing/object" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + helpers "git.home.luguber.info/inful/docbuilder/internal/testutil/testutils" +) + +func TestFixer_Rollback(t *testing.T) { + // Setup temporary directory and git repo + repo, w, tempDir := helpers.SetupTestGitRepo(t) + + // 1. Create a file and commit it (Initial State) + file1 := filepath.Join(tempDir, "file1.md") + err := os.WriteFile(file1, []byte("Content 1"), 0o600) + require.NoError(t, err) + + _, err = w.Add("file1.md") + require.NoError(t, err) + + headSHA, err := w.Commit("Initial commit", &git.CommitOptions{ + Author: &object.Signature{Name: "Test", Email: "test@example.com"}, + }) + require.NoError(t, err) + + // 2. Initialize Fixer + fixer := &Fixer{ + gitRepo: repo, + initialSHA: headSHA, + gitAware: true, + } + + // 3. Make some "fixes" + // Modify existing file + err = os.WriteFile(file1, []byte("Modified Content"), 0o600) + require.NoError(t, err) + _, err = w.Add("file1.md") + require.NoError(t, err) + + // Create new file + newFile := filepath.Join(tempDir, "file2.md") + err = os.WriteFile(newFile, []byte("New File"), 0o600) + require.NoError(t, err) + _, err = w.Add("file2.md") + require.NoError(t, err) + + // Simulate a rename (staged) + oldPathInRepo := "file1.md" + newPathInRepo := "file1_new.md" + err = os.Rename(file1, filepath.Join(tempDir, newPathInRepo)) + require.NoError(t, err) + _, err = w.Remove(oldPathInRepo) + require.NoError(t, err) + _, err = w.Add(newPathInRepo) + require.NoError(t, err) + + // 4. Perform rollback + err = fixer.rollback() + require.NoError(t, err) + + // 5. Verify state + // Original file should be restored + // #nosec G304 -- test file + content, err := os.ReadFile(file1) + require.NoError(t, err) + assert.Equal(t, "Content 1", string(content)) + + // New file should be gone + _, err = os.Stat(newFile) + assert.True(t, os.IsNotExist(err), "file2.md should have been removed by hard reset") + + // Renamed file should be gone + _, err = os.Stat(filepath.Join(tempDir, newPathInRepo)) + assert.True(t, os.IsNotExist(err), "file1_new.md should have been removed by hard reset") +} diff --git a/internal/lint/fixer_workflow_test.go b/internal/lint/fixer_workflow_test.go index f800af03..757e6d9c 100644 --- a/internal/lint/fixer_workflow_test.go +++ b/internal/lint/fixer_workflow_test.go @@ -231,7 +231,7 @@ func TestFix_ApplyLinkUpdatesError(t *testing.T) { defer func() { // Restore write permissions for cleanup // #nosec G302 -- intentional permission change for test cleanup - _ = os.Chmod(linkingFile, 0o644) + _ = os.Chmod(linkingFile, 0o600) }() linter := NewLinter(&Config{Format: "text"}) diff --git a/internal/lint/link_update_test.go b/internal/lint/link_update_test.go index b7db4731..3c3faa0e 100644 --- a/internal/lint/link_update_test.go +++ b/internal/lint/link_update_test.go @@ -356,7 +356,7 @@ func TestApplyLinkUpdates_AtomicRollback(t *testing.T) { // Clean up read-only file // #nosec G302 -- intentional permission change for test cleanup - _ = os.Chmod(source2, 0o644) + _ = os.Chmod(source2, 0o600) } // TestApplyLinkUpdates_EmptyLinks tests behavior with no links to update. @@ -595,7 +595,7 @@ func TestApplyLinkUpdates_RollbackOnFailure(t *testing.T) { require.NoError(t, os.Chmod(source2, 0o444)) // Make it read-only defer func() { // #nosec G302 -- intentional permission change for test cleanup - _ = os.Chmod(source2, 0o644) // Clean up (ignore error) + _ = os.Chmod(source2, 0o600) // Clean up (ignore error) }() // Create link references - source1 will succeed, source2 will fail diff --git a/internal/testutil/testutils/git_helpers.go b/internal/testutil/testutils/git_helpers.go new file mode 100644 index 00000000..7a579923 --- /dev/null +++ b/internal/testutil/testutils/git_helpers.go @@ -0,0 +1,27 @@ +package helpers + +import ( + "testing" + + "github.com/go-git/go-git/v5" +) + +// SetupTestGitRepo initializes a temporary git repository for testing. +// Returns the repository, its worktree, and the absolute path to the temporary directory. +func SetupTestGitRepo(t *testing.T) (*git.Repository, *git.Worktree, string) { + t.Helper() + + tempDir := t.TempDir() + + repo, err := git.PlainInit(tempDir, false) + if err != nil { + t.Fatalf("failed to initialize git repo: %v", err) + } + + w, err := repo.Worktree() + if err != nil { + t.Fatalf("failed to get worktree: %v", err) + } + + return repo, w, tempDir +}