Check media size before loading and keep unreadable files from aborting chat - #74
Draft
MiguelsPizza wants to merge 2 commits into
Draft
MiguelsPizza wants to merge 2 commits into
MiguelsPizza wants to merge 2 commits into
Conversation
Think rereads image and PDF tool results when assembling later model requests. An expired folder grant therefore aborted both ordinary turns and recovery, even when the next message supplied a fresh attachment. Render the filesystem failure as an error tool result while preserving the saved reference so reconnection restores the original media. Exercise AI SDK history conversion with denied PDF and image reads, unchanged saved history, and restored access. Include the existing workspace-tool suite in the SDK gate so this regression remains covered.
The inline media limit was enforced only after readFileBytes, so a hundreds-of-megabytes PDF could exhaust a browser worker before the tool rejected it. Check fresh metadata before loading media and leave oversized untyped files as metadata instead of reading them solely to sniff their format. Keep the post-read check for changing files and prove that a file which grows is rejected without a content read, while a smaller replacement renders from the same saved reference.
MiguelsPizza
changed the title
This branch has not been deployed
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.
A hundreds-of-megabytes PDF could exhaust a browser host before Think rejected it: the pinned renderer loaded the entire file before checking its 3.5 MiB inline limit. Separately, an expired folder grant made saved image/PDF reads abort later turns and recovery, even when the next message supplied a fresh attachment.
Check current metadata before loading inline media, preserve the post-read size check, and leave oversized untyped files as metadata instead of reading them just to detect their format. Convert failed rereads into explicit tool errors containing the path and original failure. Saved history remains unchanged, so restored access or a smaller replacement renders again. This belongs in Think under Rook’s SDK ownership policy.
Validation:
test:think.pnpm sdk:checkpasses. The preceding access-error commit passed the fullpnpm sdk:testgate (1,863 tests); the size change was checked with the targeted native suite.Remaining boundaries: Rook’s current just-bash
head/tailrequest whole-file data before slicing, and its composer serializes direct attachments as whole-file base64. This change does not make those paths stream or establish that the reported crash was an out-of-memory termination. Rook adoption still requires a new immutable SDK release and a verified consumer pin update; this PR does not deploy to the extension.