fix: stream multipart uploads without buffering

[jbeckwith-oai]

Closes #18.

This replaces #19 with a current-main implementation.

Root cause

The CLI opened file-backed values as readers, then synchronously copied those readers into a bytes.Buffer while constructing the multipart request option. Large uploads were therefore fully buffered before the request was sent, and CLI-owned file handles were not reliably closed.

Design decision

Multipart forms containing uploads are intentionally single-attempt streams:

• encode lazily through io.Pipe with bounded memory
• set WithMaxRetries(0) because an arbitrary reader cannot be guaranteed to produce the same bytes twice
• return an explicit error for POST 307/308 responses that require replay
• do not create upload-sized temporary snapshots or duplicate descriptors
• retain Content-Length for trustworthy regular-file sizes; use chunked transfer for unknown-length readers

Scalar-only multipart forms remain buffered and retain the SDK's normal retry behavior.

This is a deliberate fail-closed boundary. Transparent upload replay would require either buffering/spooling the body or a first-class SDK replay contract. Reimplementing that retry state machine in CLI middleware introduced substantially more lifecycle and platform risk than the original memory/descriptor problem. A network failure during an upload now returns the original transport error after one attempt instead of retrying a body whose identity cannot be guaranteed.

Correctness and cleanup

• source read failures abort the pipe without emitting a successful terminal multipart boundary
• exact-length regular-file readers report premature EOF if a file shrinks during the request
• owned upload files close exactly once across success, encoding failure, cancellation, close-before-read, and option-construction failure
• stdin and other unowned readers are not closed
• extensionless @value fallback occurs only for genuine not-found errors

Impact

In the original local 128 MiB end-to-end reproduction, peak RSS dropped from approximately 348 MiB on main to approximately 26 MiB while the complete multipart payload reached the server.

Behavioral tradeoff: multipart requests containing uploads no longer receive automatic SDK retries. Scalar-only multipart requests are unchanged.

Validation

• focused multipart regressions, including 307/308, 429/5xx single-attempt behavior, incomplete transport failures, Content-Length/chunked framing, premature EOF, cleanup, unavailable temp directories, low descriptor limits, and actual CLI receivers
• focused race suite
• stress runs for cancellation, incomplete transport failure, redirect rejection, and source shrinkage
• full ./scripts/test native suite and Windows compile gate
• ./scripts/lint
• ./scripts/build
• go vet ./...

[Copilot]

Pull request overview

This PR fixes high memory usage and file descriptor leaks during multipart uploads by switching the CLI from fully buffering multipart bodies in memory to streaming encoding through an io.Pipe, while ensuring owned upload files are reliably closed across success and error paths.

Changes:

• Introduces a streamed multipart request body (multipartRequestBody) that starts encoding lazily on first read and performs deterministic cleanup of owned uploads.
• Updates request option construction to use the streamed body (and disables SDK retries because the body is not replayable).
• Adds targeted tests covering backpressure/streaming behavior, error propagation, cancellation via Close, and correct upload ownership/cleanup.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
pkg/cmd/multipartbody.go	Adds streamed multipart request body implementation with lazy start, cleanup, and retry disabling options.
pkg/cmd/multipartbody_test.go	Adds tests validating streaming semantics, cancellation, error propagation, and upload closure behavior.
pkg/cmd/flagoptions.go	Switches multipart request construction from buffering to streaming and ensures partial uploads are closed on errors.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[HAYDEN-OAI]

I found four reproducible correctness and backward-compatibility regressions. Each inline finding was verified against both this exact PR commit and its parent with the actual CLI.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

pkg/cmd/multipartbody.go:341

• The comment says this only closes files opened by openFileUpload, but the implementation closes any fileUpload value it finds (even if constructed elsewhere). This is just documentation, but it’s important because it defines ownership/cleanup semantics for fileUpload.

// closeFileUploads closes every file opened by openFileUpload, including files
// nested inside maps and arrays. Other io.ReadClosers (notably stdin wrappers)
// are intentionally left alone because this code does not own them.

[HAYDEN-OAI]

Verified that the earlier redirect handling, source-read failures, scalar-only retries, and Content-Length findings were addressed. I found three additional, independently reproduced regressions in regular-file retry/replay and virtual-file length handling; details are inline.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 521f01ca48

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pkg/cmd/multipartbody.go:124

• encode() closes the pipe with errors.Join(writerCloseErr, cleanupErr), which means a local cleanup failure (e.g. a source file failing to close) is surfaced as a body read error. net/http treats any non-EOF body read error as a request failure, so this can fail an otherwise fully-encoded upload due to a post-read cleanup issue. Consider only propagating encoding/stream errors to the reader and returning cleanup errors via Close() instead.

	writerCloseErr := b.multipartWriter.Close()
	cleanupErr := b.cleanup()
	_ = b.writer.CloseWithError(errors.Join(writerCloseErr, cleanupErr))

pkg/cmd/multipartbody.go:366

• rejectUnreplayableRedirect closes res.Body and then returns a non-nil *http.Response alongside a non-nil error. Downstream error handling may still attempt to read/parse the response body and hit "read on closed response body" noise. Returning a nil response after closing the body avoids that ambiguity.

	location := res.Header.Get("Location")
	if res.Body != nil {
		_ = res.Body.Close()
	}
	return res, fmt.Errorf(
		"cannot follow HTTP %d redirect to %q: multipart upload contains a non-replayable source",
		res.StatusCode,
		location,
	)

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[HAYDEN-OAI]

Re-reviewed exact head 872425944c457cd1bbc71d4869e84e6b5aaff431 after the prior redirect, retry, virtual-file framing, and cleanup feedback was addressed. Direct comparisons against the pre-PR base confirm two remaining upload integrity issues; details and reproductions are inline.

[Copilot]

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

[HAYDEN-OAI]

Re-reviewed exact head af8013d49bdc4dd06563160b9eb33edc60513e77 against the pre-PR base after excluding all previously resolved findings. I found four newly introduced, independently validated runtime/security/resource regressions; the Linux cases were reproduced with the actual CLI and authenticated local HTTP receivers, and the Windows case was verified against exact-head release-toolchain binaries and the unmodified Go syscall implementation.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7e4a444fad

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pkg/cmd/multipartbody.go:166

• When multipart bodies contain uploads, multipartRequestOptions can return early (e.g., from multipartContentLength) without closing any fileUpload readers in bodyMap. Since openFileUpload opens *os.File handles, this can leak descriptors on option-construction failures (e.g., size/framing overflow). Consider joining closeFileUploads(bodyMap) into these error returns so uploads are always closed when options are not produced.

	if info.knownLength {
		var err error
		contentLength, err = multipartContentLength(bodyMap, encodingFormat, boundary)
		if err != nil {
			return nil, err

pkg/cmd/multipartfile_windows.go:46

• After os.Remove(path) succeeds, the later error paths still do errors.Join(err, os.Remove(path)). That adds noisy os.ErrNotExist to the returned error and can also delete an unrelated file if something recreates that path between steps. These branches should just return the original error (the placeholder has already been unlinked).

	pathPtr, err := windows.UTF16PtrFromString(path)
	if err != nil {
		return nil, errors.Join(err, os.Remove(path))

[Copilot]

Pull request overview

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.