fix: don't silently truncate parses or resume in the wrong directory#6
Merged
Conversation
Two robustness bugs from the codebase review: - parseConversationFile never checked scanner.Err(). A JSONL line over the buffer cap stops the scan, and the function returned the partial parse as if complete - silent data loss in a search tool (a phrase past the truncation point reports "no match"). Now the buffer cap is 64MB and a scan error skips the file instead of trusting a partial read. - On resume, os.Chdir ran after printing "Resuming in <cwd>" and, on failure, only warned then exec'd claude anyway - in the wrong directory, silently handing claude the wrong project config/MCP servers. Now chdir runs first and fails loudly if the directory is gone. No unit tests: the scanner path needs a >64MB line and the resume path calls os.Exit/syscall.Exec in main(); both are impractical to unit test. Existing parse tests still pass as regression cover.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two robustness bugs surfaced by the reliability lens of the codebase review.
1. Silent parse truncation
parseConversationFilenever checkedscanner.Err(). A JSONL line larger than the buffer cap (10MB) stopsscanner.Scan(), and the function returned whatever it had parsed so far as if the file were complete. A single big turn - a large tool result or a base64 image - silently drops the rest of the conversation, so searching for a phrase past that line reports "no match". Worst kind of bug in a search tool: wrong answer, no error.scanner.Err()is now checked; a scan error skips the file rather than trusting a half-parse.2. Resume in the wrong directory
os.Chdir(cwd)ran after printingResuming conversation … in <cwd>and, if it failed, only printed a warning thensyscall.Exec'dclaudeanyway - in whatever directoryccshappened to be in, silently giving claude the wrong project config / MCP servers. For a tool whose whole point is "resume in the right place", that quietly defeats the feature.os.Chdirnow runs before the announcement and exits with a clear error if the directory is gone, instead of resuming in the wrong place.Scope
previewScroll's discarded clamp (a separate review finding) is not here - the clean fix needs a small render refactor and it's lower stakes (a UI scroll annoyance, no data loss). Left as a follow-up.Tests
None added: the scanner path needs a >64MB line and the resume path calls
os.Exit/syscall.Execfrommain()- both impractical to unit test without restructuring. Existing parse tests pass as regression cover;go test -cover= 68.7%.