Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions src/services/checkpoints/ShadowCheckpointService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { executeRipgrep } from "../../services/search/file-search"

import { CheckpointDiff, CheckpointResult, CheckpointEventMap } from "./types"
import { getExcludePatterns } from "./excludes"
import { MAX_CHECKPOINT_FILE_SIZE_BYTES } from "../constants/file-limits"

export abstract class ShadowCheckpointService extends EventEmitter {
public readonly taskId: string
Expand Down Expand Up @@ -292,9 +293,25 @@ export abstract class ShadowCheckpointService extends EventEmitter {
const absPath = path.join(cwdPath, relPath)
const before = await this.git.show([`${from}:${relPath}`]).catch(() => "")

const after = to
? await this.git.show([`${to}:${relPath}`]).catch(() => "")
: await fs.readFile(absPath, "utf8").catch(() => "")
let after = ""
if (to) {
after = await this.git.show([`${to}:${relPath}`]).catch(() => "")
} else {
// Check file size before reading from filesystem
try {
const stats = await fs.stat(absPath)
if (stats.size > MAX_CHECKPOINT_FILE_SIZE_BYTES) {
console.warn(
`Checkpoint file ${absPath} exceeds size limit (${stats.size} bytes > ${MAX_CHECKPOINT_FILE_SIZE_BYTES} bytes)`,
)
after = `[File too large to display: ${stats.size} bytes]`
} else {
after = await fs.readFile(absPath, "utf8")
}
} catch (err) {
after = ""
}
}

result.push({ paths: { relative: relPath, absolute: absPath }, content: { before, after } })
}
Expand Down
28 changes: 28 additions & 0 deletions src/services/constants/file-limits.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* File size limits for various file types across Roo-Code services
* These constants help prevent memory exhaustion and performance issues
*/

// General file size limit (1MB) - used for code files and general file operations
export const MAX_FILE_SIZE_BYTES = 1 * 1024 * 1024 // 1MB

// Configuration files should be smaller (100KB)
export const MAX_CONFIG_FILE_SIZE_BYTES = 100 * 1024 // 100KB

// .gitignore files should be even smaller (50KB)
export const MAX_GITIGNORE_FILE_SIZE_BYTES = 50 * 1024 // 50KB

// Checkpoint files can be larger but still need a reasonable limit (5MB)
export const MAX_CHECKPOINT_FILE_SIZE_BYTES = 5 * 1024 * 1024 // 5MB

/**
* Error class for file size limit violations
*/
export class FileSizeLimitError extends Error {
constructor(filePath: string, size: number, limit: number) {
const sizeInKB = Math.round(size / 1024)
const limitInKB = Math.round(limit / 1024)
super(`File ${filePath} exceeds size limit (${sizeInKB}KB > ${limitInKB}KB)`)
this.name = "FileSizeLimitError"
}
}
175 changes: 175 additions & 0 deletions src/services/glob/__tests__/list-files-gitignore-size.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
import { vi, describe, it, expect, beforeEach, afterEach } from "vitest"
import * as path from "path"
import { MAX_GITIGNORE_FILE_SIZE_BYTES } from "../../constants/file-limits"

// Use vi.hoisted to ensure mocks are available during hoisting
const { mockConsoleWarn, mockStat, mockReadFile, mockAccess } = vi.hoisted(() => ({
mockConsoleWarn: vi.fn(),
mockStat: vi.fn(),
mockReadFile: vi.fn(),
mockAccess: vi.fn(),
}))

vi.mock("fs", () => ({
promises: {
stat: mockStat,
readFile: mockReadFile,
access: mockAccess,
readdir: vi.fn().mockResolvedValue([]),
},
}))

// Mock other dependencies
vi.mock("../../ripgrep", () => ({
getBinPath: vi.fn(() => Promise.resolve("/mock/path/to/rg")),
}))

vi.mock("vscode", () => ({
env: {
appRoot: "/mock/app/root",
},
}))

vi.mock("child_process", () => ({
spawn: vi.fn(() => {
const mockProcess = {
stdout: {
on: vi.fn((event, callback) => {
if (event === "data") {
setTimeout(() => callback("test-file.txt\n"), 10)
}
}),
},
stderr: { on: vi.fn() },
on: vi.fn((event, callback) => {
if (event === "close") {
setTimeout(() => callback(0), 20)
}
}),
kill: vi.fn(),
}
return mockProcess
}),
}))

vi.mock("../../path", () => ({
arePathsEqual: vi.fn().mockReturnValue(false),
}))

// Import the function to test after mocks are set up
import { listFiles } from "../list-files"

describe("list-files .gitignore size validation", () => {
beforeEach(() => {
vi.clearAllMocks()
console.warn = mockConsoleWarn
})

afterEach(() => {
vi.restoreAllMocks()
})

it("should skip .gitignore files that exceed size limit", async () => {
// Mock finding a .gitignore file
mockAccess.mockResolvedValue(undefined) // File exists

// Mock .gitignore file that exceeds size limit
const largeSize = MAX_GITIGNORE_FILE_SIZE_BYTES + 1000
mockStat.mockResolvedValue({ size: largeSize })

// Call listFiles
await listFiles("/test/dir", false, 100)

// Verify that console.warn was called with the appropriate message
expect(mockConsoleWarn).toHaveBeenCalledWith(expect.stringContaining("Skipping large .gitignore file"))
expect(mockConsoleWarn).toHaveBeenCalledWith(
expect.stringContaining(`${largeSize} bytes > ${MAX_GITIGNORE_FILE_SIZE_BYTES} bytes`),
)

// Verify that readFile was NOT called for the large .gitignore
expect(mockReadFile).not.toHaveBeenCalledWith(expect.stringContaining(".gitignore"), "utf8")
})

it("should read .gitignore files within size limit", async () => {
// Mock finding a .gitignore file
mockAccess.mockResolvedValue(undefined) // File exists

// Mock .gitignore file within size limit
const normalSize = 1024 // 1KB, well under limit
mockStat.mockResolvedValue({ size: normalSize })
mockReadFile.mockResolvedValue("*.log\nnode_modules/\n")

// Call listFiles
await listFiles("/test/dir", false, 100)

// Verify that readFile was called for the .gitignore
expect(mockReadFile).toHaveBeenCalledWith(expect.stringContaining(".gitignore"), "utf8")

// Verify no warning was logged
expect(mockConsoleWarn).not.toHaveBeenCalledWith(expect.stringContaining("Skipping large .gitignore file"))
})

it("should handle multiple .gitignore files with mixed sizes", async () => {
// Mock finding .gitignore files at different levels
mockAccess.mockResolvedValue(undefined) // All files exist

// Mock different .gitignore files with different sizes
// First call is for the .gitignore in the current directory
mockStat
.mockResolvedValueOnce({ size: 1024 }) // First .gitignore - OK
.mockResolvedValueOnce({ size: MAX_GITIGNORE_FILE_SIZE_BYTES + 1000 }) // Second - too large

mockReadFile.mockResolvedValueOnce("*.log\n")

// Call listFiles with recursive to trigger .gitignore checks
await listFiles("/test/dir", true, 100)

// Verify that readFile was called only for files within limit
expect(mockReadFile).toHaveBeenCalled()

// Verify warning was logged for the large file if multiple .gitignore files were found
// The test might only find one .gitignore depending on the mock setup
const warnCalls = mockConsoleWarn.mock.calls
const hasLargeFileWarning = warnCalls.some((call) => call[0].includes("Skipping large .gitignore file"))

// If we had multiple stat calls, we should have seen the warning
if (mockStat.mock.calls.length > 1) {
expect(hasLargeFileWarning).toBe(true)
}
})

it("should continue processing when .gitignore stat fails", async () => {
// Mock finding a .gitignore file
mockAccess.mockResolvedValue(undefined) // File exists

// Mock stat failure
mockStat.mockRejectedValue(new Error("Permission denied"))

// Call listFiles
const [results] = await listFiles("/test/dir", false, 100)

// Should still return results despite .gitignore stat failure
expect(results.length).toBeGreaterThan(0)

// Verify warning was logged
expect(mockConsoleWarn).toHaveBeenCalledWith(expect.stringContaining("Error reading .gitignore"))
})

it("should handle .gitignore at exactly the size limit", async () => {
// Mock finding a .gitignore file
mockAccess.mockResolvedValue(undefined) // File exists

// Mock .gitignore file at exactly the size limit
mockStat.mockResolvedValue({ size: MAX_GITIGNORE_FILE_SIZE_BYTES })
mockReadFile.mockResolvedValue("*.tmp\n")

// Call listFiles
await listFiles("/test/dir", false, 100)

// Verify that readFile was called (file at limit should be read)
expect(mockReadFile).toHaveBeenCalledWith(expect.stringContaining(".gitignore"), "utf8")

// Verify no warning was logged
expect(mockConsoleWarn).not.toHaveBeenCalledWith(expect.stringContaining("Skipping large .gitignore file"))
})
})
10 changes: 10 additions & 0 deletions src/services/glob/list-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import ignore from "ignore"
import { arePathsEqual } from "../../utils/path"
import { getBinPath } from "../../services/ripgrep"
import { DIRS_TO_IGNORE } from "./constants"
import { MAX_GITIGNORE_FILE_SIZE_BYTES } from "../constants/file-limits"

/**
* List files in a directory, with optional recursive traversal
Expand Down Expand Up @@ -257,6 +258,15 @@ async function createIgnoreInstance(dirPath: string): Promise<ReturnType<typeof
// Add patterns from all .gitignore files
for (const gitignoreFile of gitignoreFiles) {
try {
// Check file size before reading
const stats = await fs.promises.stat(gitignoreFile)
if (stats.size > MAX_GITIGNORE_FILE_SIZE_BYTES) {
console.warn(
`Skipping large .gitignore file: ${gitignoreFile} (${stats.size} bytes > ${MAX_GITIGNORE_FILE_SIZE_BYTES} bytes)`,
)
continue
}

const content = await fs.promises.readFile(gitignoreFile, "utf8")
ignoreInstance.add(content)
} catch (err) {
Expand Down
Loading
Loading