[5417] fix(api): declare mount binary responses and regenerate the Fern client - #5518
Open
mmabrouk wants to merge 1 commit into
Open
[5417] fix(api): declare mount binary responses and regenerate the Fern client#5518mmabrouk wants to merge 1 commit into
mmabrouk wants to merge 1 commit into
Conversation
The three mount routes that return raw bytes declared no response media type, so FastAPI advertised application/json for them. The generated clients then parsed a zip or a file body as JSON and corrupted it, which is why the frontend had to call these routes through raw axios. Declare the real media types (application/zip for the archive export, application/octet-stream for the two file downloads). Setting responses= alone is not enough: FastAPI keeps its default application/json entry unless the route also declares a response_class with no media type, so both are set. Runtime behavior is unchanged, since each handler already returns its own Response. Regenerate the TypeScript client once from the resulting spec, replacing the exportMountFiles method that was hand-written into the generated directory after the /files/export rename. The three methods now return core.BinaryResponse with responseType binary-response, which exposes both .blob() and .stream(). The regeneration also corrects two pieces of drift the hand-edits had left behind: GetMountFilesRequest.order becomes a generated enum, and MountFile moves under the mounts resource. Refs #5417 Claude-Session: https://claude.ai/code/session_011npNAXwcM2adqdSX6QGcsz
dosubot
Bot
added
the
size:L
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughFastAPI mount archive and file download routes now explicitly declare streaming or binary response classes and OpenAPI schemas for ZIP and octet-stream payloads. ChangesBinary response metadata
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Contributor
Railway Preview Environment
|
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.
Closes part of #5417.
The problem
Three mount routes return raw bytes, but none of them declared a response media type:
POST /mounts/files/exportapplication/jsonGET /mounts/{mount_id}/files/downloadapplication/jsonGET /sessions/mounts/{mount_id}/files/downloadapplication/jsonFastAPI advertises
application/jsonfor any route that declares nothing, so the Fern generator produced methods that parse the body as JSON. Parsing a zip as JSON corrupts it. That is the reason the drive UI could never call these routes through the generated client and had to reach for raw axios and rawfetchinstead, which is the deviation fromweb/AGENTS.mdthat #5417 was filed about.The export route also carried a hand-written
exportMountFilesmethod inside the generated directory, added indaf3f6a18when the route was renamed to/files/export, with a note that it "stays hand-written until the Fern regen ticket". This is that ticket.The change
Declare the real media types.
application/zipfor the archive export,application/octet-streamfor the two downloads. The declarations live inmounts/utils.pynext to the helpers that produce the bytes, since both routers share them.Setting
responses=alone is not enough, and this is the part that is easy to get wrong: FastAPI keeps its defaultapplication/jsonentry unless the route also declares aresponse_classwhose media type is empty. I verified this against the pinned FastAPI version before writing it. Withresponses=only, the 200 lists bothapplication/jsonandapplication/zip, and the generator still picks JSON.Runtime behavior does not change. Each handler already returns its own
Response, soresponse_classonly affects the generated schema.Regenerate the client once, from the spec the modified code actually serves. Before and after, for all three methods:
core.BinaryResponseexposes both.blob()and.stream(), which is what the follow-up needs: the buffered download path wants a blob, and the stream-to-disk path wants aReadableStream.The regeneration also picks up two pieces of drift the hand-edits had left behind:
GetMountFilesRequest.orderbecomes a generated enum instead of a loosestring | null, andMountFilemoves under the mounts resource.How the spec was produced
Worth recording, because generating from the wrong source silently destroys the client. A regeneration from the local OSS stack deletes 39 files: the entire auth layer (that spec has no security schemes) plus the
billingandeventsresources. The committed client comes from an EE spec.So the spec here came from the EE image running this branch's code, served over HTTP rather than dumped in-process, because FastAPI only adds the
root_pathserver entry when it serves the document. Dumping it in-process instead rewritesenvironments.tsfrom/apito an absolute host URL.The resulting spec differs from the deployed EE spec in exactly six ways: the three routes each losing
application/jsonand gaining their binary type. Nothing else.Verification
tsccompiles the regenerated client, and@agenta/shared,@agenta/ui,@agenta/sdk,@agenta/entitiesall typecheck against it.main.ruff formatandruff checkare clean.What this does not do
The frontend still calls these routes through raw axios. Moving it onto the generated client is the second half of #5417 and lands in a PR stacked on this one, since it needs the
BinaryResponsemethods this PR generates.https://claude.ai/code/session_011npNAXwcM2adqdSX6QGcsz