Repository navigation
fix(deno): transfer sandbox files as bytes instead of UTF-8 text - #986
Open
Mihir Inamdar (inamdarmihir) wants to merge 1 commit into
Open
Mihir Inamdar (inamdarmihir) wants to merge 1 commit into
Mihir Inamdar (inamdarmihir) wants to merge 1 commit into
Conversation
uploadFiles decoded the Uint8Array to a string and wrote it with writeTextFile, and downloadFiles re-encoded the text view of stdout. Any byte sequence that is not valid UTF-8 was replaced with U+FFFD while the call reported success, so images, PDFs and archives came back corrupted. Write with fs.writeFile and return the raw stdout bytes. The mock sandbox now keeps files as bytes and exposes fs.writeFile, so the tests can tell the two apart; the new tests fail on the old implementation. Fixes langchain-ai#888 Authored by Mihir
🦋 Changeset detectedLatest commit: c7b28d9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Mihir Inamdar (@inamdarmihir) is attempting to deploy a commit to the LangChain Team on Vercel. A member of the Team first needs to authorize it. |
deepagents-acp
deepagents
@langchain/sandbox-standard-tests
@langchain/daytona
@langchain/deno
@langchain/modal
@langchain/node-vfs
@langchain/quickjs
commit: |
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
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.
Fixes #888.
Problem
DenoSandbox.uploadFilesdecoded eachUint8Arrayto a string and wrote it withfs.writeTextFile.downloadFilesre-encoded thestdoutTextview ofcat. Any byte sequence that isn't valid UTF-8 was replaced with U+FFFD, and the call still reported success, so images, PDFs and archives moved through the sandbox came back corrupted.Change
uploadFileswrites the bytes withfs.writeFile.downloadFilesreturns the rawstdoutbytes instead of re-encodingstdoutText.@langchain/deno.The
catapproach and its error mapping (file_not_foundand so on) are unchanged.Tests
sandbox.test.tsnow stores files as bytes and exposes the SDK'sfs.writeFile, with a lossystdoutTextview like the real SDK. The old mock kept strings, so it couldn't tell the two paths apart.oxfmt --checkandoxlintare clean, andtscreports no errors insandbox.ts.libs/providers/denounit tests: 57 pass, 3 fail. The 3 failures (should create sandbox via SDK,should work with default options,should pass options to sandbox creation, allmemoryMbundefined) also fail on an unmodifiedmain; this change doesn't touch them. Six tests failed before my change, because the mock exposedwriteTextFileon the sandbox while the code callssandbox.fs.writeTextFile. Updating the mock for the upload path fixed the three upload tests.I did not run the integration tests, which need a Deno Deploy account, so this hasn't been verified against a real sandbox.
Authored by Mihir