From 8ca0c461596593b21647065adf7fb85c62c32fb4 Mon Sep 17 00:00:00 2001 From: hannesrudolph Date: Tue, 19 Aug 2025 16:40:17 -0600 Subject: [PATCH] checkpoints: make large-file threshold configurable via ROO_CHECKPOINTS_LARGE_FILE_THRESHOLD_MB; improve error reporting for auto-exclude (ripgrep/fs.stat); log diagnostics in ShadowCheckpointService; add tests --- .../checkpoints/ShadowCheckpointService.ts | 9 +- .../checkpoints/__tests__/excludes.spec.ts | 74 +++++++++ src/services/checkpoints/excludes.ts | 144 +++++++++++------- 3 files changed, 168 insertions(+), 59 deletions(-) diff --git a/src/services/checkpoints/ShadowCheckpointService.ts b/src/services/checkpoints/ShadowCheckpointService.ts index 1f4b22e33d..6fa8ee469b 100644 --- a/src/services/checkpoints/ShadowCheckpointService.ts +++ b/src/services/checkpoints/ShadowCheckpointService.ts @@ -142,12 +142,19 @@ export abstract class ShadowCheckpointService extends EventEmitter { const { patterns, stats } = await getExcludePatternsWithStats(this.workspaceDir) await fs.writeFile(path.join(this.dotGitDir, "info", "exclude"), patterns.join("\n")) + const mb = Math.round(stats.thresholdBytes / (1024 * 1024)) + if (stats?.largeFilesExcluded && stats.largeFilesExcluded > 0) { - const mb = Math.round(stats.thresholdBytes / (1024 * 1024)) this.log( `[${this.constructor.name}#writeExcludeFile] auto-excluding ${stats.largeFilesExcluded} large files (>= ${mb}MB) from checkpoints. Sample: ${stats.sample.join(", ")}`, ) } + + if (stats?.errorCounts && (stats.errorCounts.ripgrepErrors > 0 || stats.errorCounts.fsStatErrors > 0)) { + this.log( + `[${this.constructor.name}#writeExcludeFile] auto-exclude encountered errors (ripgrepErrors=${stats.errorCounts.ripgrepErrors}, fsStatErrors=${stats.errorCounts.fsStatErrors}). Check environment and filesystem permissions.`, + ) + } } private async stageAll(git: SimpleGit) { diff --git a/src/services/checkpoints/__tests__/excludes.spec.ts b/src/services/checkpoints/__tests__/excludes.spec.ts index f194fa68b5..acb1c13d40 100644 --- a/src/services/checkpoints/__tests__/excludes.spec.ts +++ b/src/services/checkpoints/__tests__/excludes.spec.ts @@ -264,4 +264,78 @@ readme.md text expect(result.stats.sample).not.toContain("big.ts") }) }) + + describe("configurable threshold and error reporting", () => { + it("respects ROO_CHECKPOINTS_LARGE_FILE_THRESHOLD_MB override", async () => { + // Ensure no LFS patterns + vi.mocked(fileExistsAtPath).mockResolvedValue(false) + + // Set threshold to 1 MB + const prev = process.env.ROO_CHECKPOINTS_LARGE_FILE_THRESHOLD_MB + process.env.ROO_CHECKPOINTS_LARGE_FILE_THRESHOLD_MB = "1" + + try { + // Mock file listing + vi.mocked(executeRipgrep).mockResolvedValue([ + { path: "large.bin", type: "file", label: "large.bin" }, + { path: "code.js", type: "file", label: "code.js" }, + ]) + + // Mock sizes: 2MB for large.bin, 2MB for code.js (but code is allowlisted) + vi.mocked(fs.stat).mockImplementation(async (p) => { + const s = p.toString() + if (s.includes("large.bin") || s.includes("code.js")) { + return { size: 2 * 1024 * 1024 } as any + } + return { size: 1024 } as any + }) + + const result = await getExcludePatternsWithStats(testWorkspacePath) + + expect(result.stats.thresholdBytes).toBe(1 * 1024 * 1024) + expect(result.stats.largeFilesExcluded).toBe(1) + expect(result.stats.sample).toContain("large.bin") + // code.js should never be excluded even if large + expect(result.stats.sample).not.toContain("code.js") + } finally { + // cleanup + if (prev === undefined) { + delete process.env.ROO_CHECKPOINTS_LARGE_FILE_THRESHOLD_MB + } else { + process.env.ROO_CHECKPOINTS_LARGE_FILE_THRESHOLD_MB = prev + } + } + }) + + it("records ripgrep failures without breaking pattern generation", async () => { + vi.mocked(fileExistsAtPath).mockResolvedValue(false) + // Force executeRipgrep to throw + vi.mocked(executeRipgrep).mockRejectedValue(new Error("ripgrep failed")) + + const result = await getExcludePatternsWithStats(testWorkspacePath) + + // No dynamic large files because ripgrep failed + expect(result.stats.largeFilesExcluded).toBe(0) + expect(result.stats.sample.length).toBe(0) + // Error counts should reflect one ripgrep error + expect(result.stats.errorCounts?.ripgrepErrors).toBe(1) + expect(result.stats.errorCounts?.fsStatErrors).toBe(0) + // Base patterns should still include .git/ + expect(result.patterns).toContain(".git/") + }) + + it("counts fs.stat errors for diagnostics", async () => { + vi.mocked(fileExistsAtPath).mockResolvedValue(false) + vi.mocked(executeRipgrep).mockResolvedValue([{ path: "mystery.bin", type: "file", label: "mystery.bin" }]) + // Make stat fail + vi.mocked(fs.stat).mockRejectedValue(new Error("stat failure")) + + const result = await getExcludePatternsWithStats(testWorkspacePath) + + expect(result.stats.largeFilesExcluded).toBe(0) + expect(result.stats.sample.length).toBe(0) + expect(result.stats.errorCounts?.ripgrepErrors).toBe(0) + expect(result.stats.errorCounts?.fsStatErrors).toBe(1) + }) + }) }) diff --git a/src/services/checkpoints/excludes.ts b/src/services/checkpoints/excludes.ts index 6225a5be39..800866f94f 100644 --- a/src/services/checkpoints/excludes.ts +++ b/src/services/checkpoints/excludes.ts @@ -2,10 +2,20 @@ import fs from "fs/promises" import * as path from "path" import { fileExistsAtPath } from "../../utils/fs" -import { executeRipgrepForFiles, executeRipgrep } from "../search/file-search" +import { executeRipgrep } from "../search/file-search" const DEFAULT_LARGE_FILE_THRESHOLD_BYTES = 10 * 1024 * 1024 // 10 MB +function getConfiguredLargeFileThresholdBytes(): number { + // Allow override via environment variable (in MB), e.g. ROO_CHECKPOINTS_LARGE_FILE_THRESHOLD_MB=25 + const env = process.env.ROO_CHECKPOINTS_LARGE_FILE_THRESHOLD_MB + const parsed = env ? Number(env) : NaN + if (Number.isFinite(parsed) && parsed > 0) { + return Math.round(parsed * 1024 * 1024) + } + return DEFAULT_LARGE_FILE_THRESHOLD_BYTES +} + // Common code/text extensions that should not be auto-excluded by size const CODE_EXT_ALLOWLIST: Set = new Set([ ".ts", @@ -273,62 +283,71 @@ const getGameEnginePatterns = () => [ */ async function getLargeFileAutoExcludePatterns( workspacePath: string, - thresholdBytes: number = DEFAULT_LARGE_FILE_THRESHOLD_BYTES, + thresholdBytes: number, lfsPatterns: string[] = [], -): Promise { +): Promise<{ patterns: string[]; errorCounts: { ripgrepErrors: number; fsStatErrors: number } }> { + // Build ripgrep args with common ignores + const args = [ + "--files", + "--follow", + "--hidden", + "-g", + "!**/node_modules/**", + "-g", + "!**/.git/**", + "-g", + "!**/out/**", + "-g", + "!**/dist/**", + ] + + // Pre-filter git-lfs patterns at ripgrep level + for (const pattern of lfsPatterns) { + const rgPattern = pattern.startsWith("!") ? pattern.substring(1) : `!${pattern}` + args.push("-g", rgPattern) + } + + args.push(workspacePath) + + let items: Array<{ path: string; type: string }> = [] + let ripgrepErrors = 0 + let fsStatErrors = 0 + try { - // Create a custom ripgrep execution that excludes git-lfs patterns - const args = [ - "--files", - "--follow", - "--hidden", - "-g", - "!**/node_modules/**", - "-g", - "!**/.git/**", - "-g", - "!**/out/**", - "-g", - "!**/dist/**", - ] - - // Add git-lfs patterns as exclusions to ripgrep - // This pre-filters files before we check their sizes - for (const pattern of lfsPatterns) { - // Convert git-lfs patterns to ripgrep glob patterns - // Git patterns like "*.psd" need to be "!*.psd" for ripgrep - const rgPattern = pattern.startsWith("!") ? pattern.substring(1) : `!${pattern}` - args.push("-g", rgPattern) - } - - args.push(workspacePath) - - const items = await executeRipgrep({ args, workspacePath, limit: 50000 }) - const large: string[] = [] - - for (const item of items) { - if (item.type !== "file") continue - - const rel = item.path - const ext = path.extname(rel).toLowerCase() - - // Keep code/text files even if large - if (CODE_EXT_ALLOWLIST.has(ext)) continue - - try { - const stat = await fs.stat(path.join(workspacePath, rel)) - if (stat.size >= thresholdBytes) { - // Normalize to forward slashes for git exclude - large.push(rel.replace(/\\/g, "/")) - } - } catch { - // Ignore stat errors for individual files - } - } - - return Array.from(new Set(large)) + const rgResult = await executeRipgrep({ args, workspacePath, limit: 50000 }) + items = Array.isArray(rgResult) ? rgResult : [] } catch { - return [] + // If ripgrep fails, record error and continue with empty items to avoid breaking checkpoints + ripgrepErrors = 1 + items = [] + } + + const large: string[] = [] + + for (const item of items) { + if ((item as any).type !== "file") continue + + const rel = (item as any).path + const ext = path.extname(rel).toLowerCase() + + // Keep code/text files even if large + if (CODE_EXT_ALLOWLIST.has(ext)) continue + + try { + const stat = await fs.stat(path.join(workspacePath, rel)) + if (stat.size >= thresholdBytes) { + // Normalize to forward slashes for git exclude + large.push(rel.replace(/\\/g, "/")) + } + } catch { + // Count stat errors for diagnostics + fsStatErrors++ + } + } + + return { + patterns: Array.from(new Set(large)), + errorCounts: { ripgrepErrors, fsStatErrors }, } } @@ -337,7 +356,12 @@ async function getLargeFileAutoExcludePatterns( */ export async function getExcludePatternsWithStats(workspacePath: string): Promise<{ patterns: string[] - stats: { largeFilesExcluded: number; thresholdBytes: number; sample: string[] } + stats: { + largeFilesExcluded: number + thresholdBytes: number + sample: string[] + errorCounts?: { ripgrepErrors: number; fsStatErrors: number } + } }> { // Get git-lfs patterns first const lfsPatterns = await getLfsPatterns(workspacePath) @@ -356,10 +380,13 @@ export async function getExcludePatternsWithStats(workspacePath: string): Promis ...lfsPatterns, ] + // Determine threshold (env override supported) + const thresholdBytes = getConfiguredLargeFileThresholdBytes() + // Pass lfs patterns to the large file scanner to pre-filter them - const dynamicLarge = await getLargeFileAutoExcludePatterns( + const { patterns: dynamicLarge, errorCounts } = await getLargeFileAutoExcludePatterns( workspacePath, - DEFAULT_LARGE_FILE_THRESHOLD_BYTES, + thresholdBytes, lfsPatterns, ) @@ -369,8 +396,9 @@ export async function getExcludePatternsWithStats(workspacePath: string): Promis patterns, stats: { largeFilesExcluded: dynamicLarge.length, - thresholdBytes: DEFAULT_LARGE_FILE_THRESHOLD_BYTES, + thresholdBytes, sample: dynamicLarge.slice(0, 10), + errorCounts, }, } }