Fix: Allow reading large Claude session files by disabling size limit #3166
Conversation
- Pass disableLimit: true when reading Claude session files - Update MockFileSystemService to accept disableLimit parameter - Add test for large session files (>5MB) Co-authored-by: TylerLeonhardt <2644648+TylerLeonhardt@users.noreply.github.com>
TylerLeonhardt
left a comment
TylerLeonhardt
left a comment
There was a problem hiding this comment.
Fixes the immediate problem but we should lazily load these.
There was a problem hiding this comment.
Pull request overview
Enables loading Claude session .jsonl files larger than the platform IFileSystemService 5MB read limit by opting out of the size guard during session load.
Changes:
- Pass
disableLimit=truewhen reading Claude session files inClaudeCodeSessionService. - Update
MockFileSystemService.readFilesignature to accept the optionaldisableLimitparameter. - Add a unit test intended to validate loading a >5MB session file.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/extension/agents/claude/node/claudeCodeSessionService.ts | Reads session files with disableLimit=true so large session JSONL files load successfully. |
| src/platform/filesystem/node/test/mockFileSystemService.ts | Aligns mock readFile signature with IFileSystemService’s optional disableLimit parameter. |
| src/extension/agents/claude/node/test/claudeCodeSessionService.spec.ts | Adds a test that constructs and loads a ~6MB session file. |
| mockFs.mockDirectory(dirUri, [[fileName, FileType.File]]); | ||
| mockFs.mockFile(URI.joinPath(dirUri, fileName), largeFileContents, 1000); | ||
|
|
||
| // Should not throw an error for large files | ||
| const sessions = await service.getAllSessions(CancellationToken.None); | ||
|
|
There was a problem hiding this comment.
The new “large session file” test doesn’t currently validate the regression it’s meant to cover: MockFileSystemService.readFile doesn’t enforce the 5MB limit, so this test would have passed even before the production change. To make this test meaningful, either (a) make the mock apply the same size check unless disableLimit is true (e.g. reuse assertReadFileSizeLimit), or (b) assert via a spy that readFile was called with disableLimit=true when loading sessions.
| } | ||
|
|
||
| async readFile(uri: URI): Promise<Uint8Array> { | ||
| async readFile(uri: URI, disableLimit?: boolean): Promise<Uint8Array> { |
There was a problem hiding this comment.
The mock readFile signature was updated to accept disableLimit, but the parameter isn’t used and the mock still doesn’t emulate the real 5MB read limit. Since the new behavior depends on bypassing that limit, consider applying the same limit logic in this mock (and honoring disableLimit) so unit tests can catch regressions around large-file reads.
| // Claude session files can be large (>5MB), so we disable the size limit | ||
| const content = await this._fileSystem.readFile(fileUri, true); |
There was a problem hiding this comment.
Passing disableLimit=true removes the 5MB safeguard for these session files. Since this code auto-loads all *.jsonl sessions, a very large/corrupted file could now cause high memory usage or extension host instability. Consider adding a separate (higher) hard cap for session files (and/or a streaming reader) so “large but reasonable” files work while still protecting against extreme sizes.
Claude session JSONL files can exceed 5MB in practice. The FileSystemService currently enforces a 5MB read limit, causing session loading to fail with
EXCEEDS max file sizeerrors.Changes
ClaudeCodeSessionService: Pass
disableLimit: truewhen reading session filesMockFileSystemService: Add
disableLimitparameter to match interface signatureTest: Validate loading 6MB session files (6000 messages)
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.