fix(vcs): address review findings on PR #88

Findings addressed:
- F1/G1: Add doc comment to GetAllFilesInPath documenting fail-fast contract
- F2/G2: Add explicit backslash-prefix guard to skip '\ No newline' markers
- F3: Add comment explaining position > 0 guard (skip lines before first hunk)
- F4: Refactor parseHunkNewStart to use strconv.Atoi instead of per-char concat
- F5: Add error propagation tests (ListContents, GetFileContent, nested, ctx cancel)
- F6: Wrap errors.ErrUnsupported in DismissReview for programmatic checking
- S1: Add ctx.Err() checks + max file count/byte constants with clear errors
- S2: Addressed by S1 — input bounds are now enforced via the same constants

vcs/util_test.go

@@ -2,6 +2,7 @@ package vcs_test
 
 import (
 	"context"
+	"strings"
 	"fmt"
 	"testing"
 
@@ -248,3 +249,83 @@ func TestBuildLineToPositionMap(t *testing.T) {
 		}
 	})
 }
+
+func TestGetAllFilesInPath_ErrorPropagation(t *testing.T) {
+	ctx := context.Background()
+
+	t.Run("ListContents error propagates", func(t *testing.T) {
+		client := &mockFileReader{
+			contents: map[string][]vcs.ContentEntry{
+				// "src" not in map, so ListContents will fail
+			},
+		}
+		_, err := vcs.GetAllFilesInPath(ctx, client, "owner", "repo", "src")
+		if err == nil {
+			t.Fatal("expected error, got nil")
+		}
+		if !strings.Contains(err.Error(), "list contents") {
+			t.Errorf("expected error about list contents, got: %v", err)
+		}
+	})
+
+	t.Run("GetFileContent error propagates", func(t *testing.T) {
+		client := &mockFileReader{
+			contents: map[string][]vcs.ContentEntry{
+				"src": {
+					{Name: "main.go", Path: "src/main.go", Type: "file"},
+				},
+			},
+			files: map[string]string{
+				// "src/main.go" not in files map, so GetFileContent will fail
+			},
+		}
+		_, err := vcs.GetAllFilesInPath(ctx, client, "owner", "repo", "src")
+		if err == nil {
+			t.Fatal("expected error, got nil")
+		}
+		if !strings.Contains(err.Error(), "get file") {
+			t.Errorf("expected error about get file, got: %v", err)
+		}
+	})
+
+	t.Run("nested ListContents error propagates", func(t *testing.T) {
+		client := &mockFileReader{
+			contents: map[string][]vcs.ContentEntry{
+				"src": {
+					{Name: "pkg", Path: "src/pkg", Type: "dir"},
+				},
+				// "src/pkg" not in map, so recursive ListContents will fail
+			},
+		}
+		_, err := vcs.GetAllFilesInPath(ctx, client, "owner", "repo", "src")
+		if err == nil {
+			t.Fatal("expected error, got nil")
+		}
+		if !strings.Contains(err.Error(), "list contents") {
+			t.Errorf("expected error about list contents, got: %v", err)
+		}
+	})
+
+	t.Run("cancelled context propagates", func(t *testing.T) {
+		ctx, cancel := context.WithCancel(context.Background())
+		cancel() // Cancel immediately
+
+		client := &mockFileReader{
+			contents: map[string][]vcs.ContentEntry{
+				"src": {
+					{Name: "main.go", Path: "src/main.go", Type: "file"},
+				},
+			},
+			files: map[string]string{
+				"src/main.go": "package main",
+			},
+		}
+		_, err := vcs.GetAllFilesInPath(ctx, client, "owner", "repo", "src")
+		if err == nil {
+			t.Fatal("expected error from cancelled context, got nil")
+		}
+		if !strings.Contains(err.Error(), "context cancelled") {
+			t.Errorf("expected context cancellation error, got: %v", err)
+		}
+	})
+}