fix(corpus): also skip (not abort) on stat failure and TOCTOU read races

Review found the stat-based size pre-check protected only the common
case: a file that grows past the cap between the Stat and the
subsequent bytelimit.ReadFileLimited call still returned a hard error,
reintroducing the exact whole-build-abort bug the pre-check was meant
to fix, just for a narrower race window. Both the Stat error branch and
the fallback ReadFileLimited error branch now skip-and-report instead
of failing collectFile's caller. Added direct collectFile unit tests
for both branches (a vanished path, and a directory passed as a file,
which reliably fails at Read without depending on file-permission bits
that root ignores) and rewrote the oversized-file test to use two
Sources, proving an earlier source's records survive a later source's
bad file — Collect's per-source loop returns on first error, discarding
every prior record, which a single-source test can't exercise.

Also: correct a stale comment in internal/linkgraph/linkgraph.go
describing ColumnOfOffset's old O(column) backward scan (it's now
O(log lines) via binary search), and add an integration-level test
driving pkg/goldmark/parser's full HTML block Open() path with
mixed-case tags — the existing equivalence/upstream-parity harness and
TestHTMLBlock_AllSevenTypes only ever exercise lowercase tags, so a
wiring mistake in the new tag-lookup helpers could otherwise only be
caught by their own unit tests.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UCh2j2oeSw6UgNsmpjRonL

Author: claude

internal/corpus/collect.go

@@ -176,16 +176,19 @@ func collectFile(
 		return Record{}, false, nil
 	}
 
-	// Skip files over the shared byte-limit cap rather than failing the
-	// whole source: collectFromRoot's caller aborts the entire walk (and
-	// every record already collected from every source before it, since
-	// Collect returns on the first error) on any error from this
-	// function. A cloned third-party repository can legitimately contain
-	// one oversized file (a large CHANGELOG, a vendored spec) without
-	// that file being reason to discard the rest of the corpus build.
+	// Skip a file this function cannot read — oversized, vanished, or
+	// permission-denied — rather than failing the whole source:
+	// collectFromRoot's caller aborts the entire walk (and every record
+	// already collected from every source before it, since Collect
+	// returns on the first error) on any error from this function. A
+	// cloned third-party repository can legitimately contain one
+	// oversized or racy file (a large CHANGELOG, a vendored spec, a file
+	// removed mid-walk) without that file being reason to discard the
+	// rest of the corpus build.
 	info, err := os.Stat(fullPath)
 	if err != nil {
-		return Record{}, false, fmt.Errorf("stat file %s: %w", fullPath, err)
+		reportProgress(cfg, fmt.Sprintf("skipping %s: %v", relPath, err))
+		return Record{}, false, nil
 	}
 	if info.Size() > bytelimit.DefaultMaxInputBytes {
 		reportProgress(cfg, fmt.Sprintf(
@@ -194,9 +197,14 @@ func collectFile(
 		return Record{}, false, nil
 	}
 
+	// bytelimit.ReadFileLimited re-checks the size on the actual read,
+	// so a file that grows past the cap in the window between the Stat
+	// above and this call (or otherwise becomes unreadable) is caught
+	// here too — skipped the same way, not treated as fatal.
 	content, err := bytelimit.ReadFileLimited(fullPath, bytelimit.DefaultMaxInputBytes)
 	if err != nil {
-		return Record{}, false, fmt.Errorf("read file %s: %w", fullPath, err)
+		reportProgress(cfg, fmt.Sprintf("skipping %s: %v", relPath, err))
+		return Record{}, false, nil
 	}
 	raw := normalizeContent(string(content))
 	words := countWords(raw)

internal/corpus/collect_test.go

@@ -219,46 +219,132 @@ func TestCollect_ErrorPath(t *testing.T) {
 // into memory in full — and, since a real source repository can contain
 // one large file among many good ones (a big CHANGELOG, a vendored
 // spec), skipping it must not abort collection of the rest of that
-// source or of sources collected earlier in the same run.
+// source, or of sources collected *earlier* in the same run (Collect's
+// loop over cfg.Sources returns on the first error, discarding every
+// record gathered so far — so this uses two sources, not one, to prove
+// the first source's record survives a later source's oversized file).
 func TestCollect_OversizedFile_SkippedNotFatal(t *testing.T) {
 	t.Parallel()
 
-	root := filepath.Join(t.TempDir(), "docs")
-	if err := os.MkdirAll(root, 0o755); err != nil {
-		t.Fatalf("mkdir: %v", err)
-	}
+	const prose = "# Title\n\nword word word word word word\n"
+
+	goodRoot := filepath.Join(t.TempDir(), "good")
+	mustMkdirAll(t, goodRoot)
+	mustWriteFile(t, filepath.Join(goodRoot, "early.md"), []byte(prose))
+
+	mixedRoot := filepath.Join(t.TempDir(), "mixed")
+	mustMkdirAll(t, mixedRoot)
 	oversized := bytes.Repeat([]byte("a "), int(bytelimit.DefaultMaxInputBytes)/2+1)
-	if err := os.WriteFile(filepath.Join(root, "huge.md"), oversized, 0o644); err != nil {
-		t.Fatalf("write oversized markdown: %v", err)
-	}
-	normalContent := []byte("# Title\n\nword word word word word word\n")
-	if err := os.WriteFile(filepath.Join(root, "normal.md"), normalContent, 0o644); err != nil {
-		t.Fatalf("write normal markdown: %v", err)
-	}
+	mustWriteFile(t, filepath.Join(mixedRoot, "huge.md"), oversized)
+	mustWriteFile(t, filepath.Join(mixedRoot, "normal.md"), []byte(prose))
 
 	cfg := &Config{
 		CollectedAt:      "2026-02-16",
 		MinWords:         1,
 		MinChars:         1,
 		LicenseAllowlist: []string{"MIT"},
-		Sources: []SourceConfig{{
-			Name:       "seed",
-			Repository: "github.com/acme/seed",
-			Root:       root,
-			CommitSHA:  "abc123",
-			License:    "MIT",
-		}},
+		Sources: []SourceConfig{
+			{
+				Name:       "early",
+				Repository: "github.com/acme/early",
+				Root:       goodRoot,
+				CommitSHA:  "abc123",
+				License:    "MIT",
+			},
+			{
+				Name:       "mixed",
+				Repository: "github.com/acme/mixed",
+				Root:       mixedRoot,
+				CommitSHA:  "def456",
+				License:    "MIT",
+			},
+		},
 	}
 
 	records, err := Collect(cfg, t.TempDir())
 	if err != nil {
 		t.Fatalf("Collect: unexpected error, oversized file should be skipped: %v", err)
 	}
-	if len(records) != 1 {
-		t.Fatalf("record count = %d, want 1 (only normal.md; huge.md must be skipped)", len(records))
+	if len(records) != 2 {
+		t.Fatalf("record count = %d, want 2 (early.md from the first source, "+
+			"normal.md from the second; huge.md must be skipped)", len(records))
+	}
+	paths := make([]string, len(records))
+	for i, r := range records {
+		paths[i] = r.Source + "/" + r.Path
+	}
+	if paths[0] != "early/early.md" || paths[1] != "mixed/normal.md" {
+		t.Fatalf("records = %v, want [early/early.md mixed/normal.md]", paths)
+	}
+}
+
+// mustMkdirAll creates dir and all parents, failing the test on error.
+func mustMkdirAll(t *testing.T, dir string) {
+	t.Helper()
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatalf("mkdir %s: %v", dir, err)
+	}
+}
+
+// mustWriteFile writes content to path, failing the test on error.
+func mustWriteFile(t *testing.T, path string, content []byte) {
+	t.Helper()
+	if err := os.WriteFile(path, content, 0o644); err != nil {
+		t.Fatalf("write %s: %v", path, err)
+	}
+}
+
+// TestCollectFile_StatError_SkippedNotFatal covers collectFile's os.Stat
+// error branch directly: a file that vanishes between WalkDir listing it
+// and the Stat call inside collectFile (or any other stat failure) must
+// be skipped, not treated as fatal — the same reasoning as the oversized-
+// file case above.
+func TestCollectFile_StatError_SkippedNotFatal(t *testing.T) {
+	t.Parallel()
+
+	root := t.TempDir()
+	missing := filepath.Join(root, "gone.md")
+
+	cfg := &Config{MinWords: 1, MinChars: 1}
+	record, keep, err := collectFile(cfg, SourceConfig{Name: "seed"}, missing, "gone.md", root)
+	if err != nil {
+		t.Fatalf("collectFile: unexpected error for a stat failure: %v", err)
+	}
+	if keep {
+		t.Fatal("collectFile: keep = true, want false for a stat failure")
+	}
+	if record != (Record{}) {
+		t.Fatalf("collectFile: record = %+v, want zero value", record)
+	}
+}
+
+// TestCollectFile_ReadError_SkippedNotFatal covers collectFile's fallback
+// bytelimit.ReadFileLimited error branch directly: a path that passes the
+// Stat-based size pre-check but then fails to read must be skipped, not
+// treated as fatal — the same reasoning as the stat-failure and
+// oversized-file cases above. A directory Stats successfully (size 0,
+// under the cap) but fails to Read as a file ("is a directory"),
+// deterministically reaching this branch regardless of the running
+// user's privileges (unlike a permission-bit test, which root ignores).
+func TestCollectFile_ReadError_SkippedNotFatal(t *testing.T) {
+	t.Parallel()
+
+	root := t.TempDir()
+	notAFile := filepath.Join(root, "not-a-file.md")
+	if err := os.Mkdir(notAFile, 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+
+	cfg := &Config{MinWords: 1, MinChars: 1}
+	record, keep, err := collectFile(cfg, SourceConfig{Name: "seed"}, notAFile, "not-a-file.md", root)
+	if err != nil {
+		t.Fatalf("collectFile: unexpected error reading a directory as a file: %v", err)
+	}
+	if keep {
+		t.Fatal("collectFile: keep = true, want false when the read fails")
 	}
-	if records[0].Path != "normal.md" {
-		t.Fatalf("Path = %q, want normal.md", records[0].Path)
+	if record != (Record{}) {
+		t.Fatalf("collectFile: record = %+v, want zero value", record)
 	}
 }
 

internal/linkgraph/linkgraph.go

@@ -251,10 +251,10 @@ func linkPosition(f *lint.File, n ast.Node) (int, int) {
 	if offset < 0 {
 		return 1, 1
 	}
-	// f.ColumnOfOffset scans backward from offset to the previous
-	// newline, so it's O(column) per call instead of the O(offset)
-	// forward scan a hand-rolled version would do — meaningful for
-	// `mdsmith list backlinks` which can call this many times per file.
+	// f.ColumnOfOffset binary-searches the cached newline index, so
+	// it's O(log lines) per call instead of the O(column) backward scan
+	// a hand-rolled version would do — meaningful for `mdsmith list
+	// backlinks` which can call this many times per file.
 	return f.LineOfOffset(offset), f.ColumnOfOffset(offset)
 }
 

pkg/goldmark/parser/html_blocks_test.go

@@ -56,6 +56,38 @@ func TestHTMLBlock_AllSevenTypes(t *testing.T) {
 	}
 }
 
+// TestHTMLBlock_TagCaseInsensitive drives the full Open() path (not just
+// the tagInAllowedSet/isRawTextTag unit tests) with mixed-case tag names,
+// so a wiring mistake in html_block.go's tag lookups (e.g. swapping which
+// helper a call site uses) would be caught here even though
+// TestHTMLBlock_AllSevenTypes only exercises lowercase tags.
+func TestHTMLBlock_TagCaseInsensitive(t *testing.T) {
+	cases := []struct {
+		name string
+		src  string
+		kind ast.NodeKind
+	}{
+		{"type1-script-mixed-case", "<Script>alert('x')</Script>\n", ast.KindHTMLBlock},
+		{"type6-block-tag-uppercase", "<DIV>\nblock\n</DIV>\n", ast.KindHTMLBlock},
+		{"type7-tag-mixed-case", "<A href=\"x\">\n\n", ast.KindHTMLBlock},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			root := parseWithDefaults(tc.src)
+			found := false
+			_ = ast.Walk(root, func(n ast.Node, entering bool) (ast.WalkStatus, error) {
+				if entering && n.Kind() == tc.kind {
+					found = true
+				}
+				return ast.WalkContinue, nil
+			})
+			if !found {
+				t.Errorf("expected %v for %q", tc.kind, tc.src)
+			}
+		})
+	}
+}
+
 func TestRawHTML_InlineTags(t *testing.T) {
 	cases := []struct {
 		name string