Repository navigation
fix: non-image files upload again from Hub Member sessions and Bot chat drops - #68
Merged
Merged
Conversation
… file it uploads A client sends the begin frame right behind file.upload.request. The socket hands binary frames to the session at once, while a Project-scoped (Hub Member) request awaits resource authorization before beginUpload registers the upload. Frames that arrived in that window found no upload and were dropped: the file then failed with "Upload chunks arrived before file begin." or never finished uploading. Images travel inside the message and were unaffected. The session now holds that request's frames until the request is admitted or refused and applies them in arrival order; file-transfer authority is checked when a frame is applied.
… file The app uploads a chosen file when it is attached and again from its local copy at send; only the second copy is linked to the message. Each attached file therefore left one completed upload behind, and after 256 in one connection session every further file failed with "Upload storage overloaded: too many unfinished drafts" while images still sent. At the cap the store now forgets its oldest unlinked upload instead of refusing. The bytes stay on disk because a terminal may still hold the pasted path.
The Bot chat screen had no FileDropZone, so its composer's drop sink never registered and a dropped file, image or not, was ignored on web and desktop. The screen now provides the zone the agent panel already has. A browser journey sends MP4, PDF, zip and JSON through the Upload file menu and through one drop in a Bot chat, and through the menu in an agent composer, and checks the bytes the Host stored.
Review of the frame hold found that it counted every frame for as long as the hold existed and dropped frames past 64 without telling anyone. A file over 8 MiB whose frames arrived during a slow authorization, or while held frames were still being written, lost its end frame and the composer spun forever. Held frames now drain to the store one at a time, and the hold counts only what it still holds: up to the declared size and at most 64 MiB, so a whole 50 MiB file can wait for authorization, with 128 MiB across all sessions. If the hold overflows, the upload fails with the store's overload error. A refused request's frames are discarded unread. If the sender loses authority, the upload ends with an access_denied rpc_error, because a file.upload.response no longer reaches a session without workspace.write. Holds are keyed per connection.
…ry later file" Review found the eviction could invalidate a live attachment: New workspace, the workspace setup dialog and the draft create flow send the upload made when the file was chosen, so forgetting the oldest unlinked upload could fail that draft's send with "Uploaded file does not belong to this connection". The store refuses at its cap again; a test keeps an unsent draft linkable while other uploads fill the store. The cap itself remains: each file a composer sends is uploaded twice, so one connection session can still reach 256 unlinked uploads. Ending that needs the client to reuse or release the first upload.
…t finish Review asked what happens to a hold whose request never settles or whose connection closes first. The hold stayed in place, and so did its bytes in the process-wide budget, so later uploads in every session could be refused as overloaded. A hold now gives up after 60 s without an answer. It is released when its connection closes and when the session is disposed. Tests cover byte accounting after a drain, a refused request, a closed connection, a request that never settles, dispose, the shared 128 MiB budget across sessions, a duplicate request id, and an overflow during the drain.
Review found that the 60 s hold timeout only discarded frames. A client whose request never settled still waited forever. The timeout now sends one rpc_error (upload_authorization_timeout) to that connection and keeps a marker until the request settles. Frames still on the wire are dropped. A late admission is cancelled without a second answer and leaves no upload behind. A late refusal emits no second rpc_error. The timer is cleared at release, so it cannot fire while held frames drain. Session tests cover a permanently stalled authorization and late admission or refusal after the timeout answer, checking a single answer, no upload left on disk, and that the next file still goes through. Unit tests cover a connection closing mid-drain and a timeout during a long drain.
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.
Problem
Users reported that only images could be sent: "upload file PDF không được" (web), "MP4 hay JSON thì không được, chỉ được mỗi ảnh" (desktop), "em ko upload file được" (04/11). Two reports have screenshots:
Causes found and reproduced
A Project-scoped (Hub Member) session lost the binary frames of an upload. The client sends the begin frame right after
file.upload.request. The socket gives binary frames to the session immediately. The JSON request first awaits resource authorization and only then callsbeginUpload. A frame that arrived during that wait found no upload and was dropped:Images are sent inside the message, so they were unaffected. Owner/unrestricted sessions call
beginUploadwithout an await, which is why direct and relay probes passed.Fix: the session holds that request's frames until the request is admitted or refused, then drains them to the store one at a time, in arrival order. File-transfer authority is checked when each frame is applied.
workspace.write: anaccess_deniedrpc_error. Afile.upload.responsewould be filtered out once the session no longer has that permission.rpc_error(upload_authorization_timeout). A marker then drops late frames. A late admission is cancelled without a second answer and leaves no upload behind, and a late refusal is not emitted.(Reverted after review.) An earlier commit evicted the oldest unlinked completed upload once the per-session cap of 256 was reached. Review showed this could invalidate a live draft: New workspace, the setup dialog and the create flow send the upload made at pick time. The cap now refuses again, as on main, and a test keeps an unsent draft linkable while the store fills. The cap itself is still open (see below).
Bot chat ignored dropped files. The Bot chat screen had no
FileDropZone, so a drop did nothing on web or desktop, for images too.Fix: Bot chat now gets the same zone the agent panel has.
Evidence (synthetic data only)
managed-access/file-upload-order.e2e.test.ts: Project-scoped session, frames sent before authorizationfile-upload-order.e2e: 49 MiB with every frame (>64) before authorization; 12 MiB split across authorization; declared size exceeded; revoked while held; another connection reusing the request idheld-upload-frames.test.ts: bytes returned after drain, refusal, closed connection, request that never settles (answered once), dispose, shared 128 MiB cap, duplicate request id, overflow during drain, connection closing mid-drain, timeout during a long drainfile-upload-order.e2e: authorization permanently stalled; late admission or refusal after the timeout answer (exactly one answer, no upload left, next file works)file-upload/index.test.ts: unsent draft stays linkable while other uploads fill the storechat-file-attachment-types.spec.ts: Bot chat, MP4 6 MB, PDF, zip 2 MB and JSON dropped in one dropOther runs on the final head:
session.testand file-transfer e2e all pass.composer-attachments→ 13 passed, 1 skipped (that test needs GitHub auth).Failures that also happen on base:
websocket-server.liveness"resumed stale socket".daemon-client.e2etests that need a real Codex provider.Not covered or not changed
FILE_UPLOAD_MAX_BYTES), with a clear toast. Unchanged.~/.clisbot/uploads/<id>with no cleanup (pre-existing).uploadFileAttachmentsdrops theagentIdthat the composer passes. Not changed here: forwarding it would enable a durable-destination path whose agent ownership check needs its own review.file.upload.requestneedsworkspace.write, whilesend_agent_message_requestalso acceptshub.execute. A Hub relationship that only hashub.executewould show the "You do not have permission…" toast. None of the reports show that toast, and no permission was widened.