[jbeckwith-oai]

Summary

• Stream explicit file outputs, forced stdout, and redirected stdout directly with bounded memory instead of loading complete binary responses into memory.
• For automatic terminal output, classify at most 515 response bytes, handle UTF-8 sequences crossing the sample boundary, and stream directly to stdout or an exclusively created owner-only file while preserving inferred filenames and collision behavior.
• Verify an automatically selected filename still identifies the completed file before reporting success, and preserve existing explicit-file inode, hard-link, symlink, and writable-directory behavior.
• Document the accepted direct-streaming tradeoffs: bytes after a text-looking prefix are not reclassified; existing explicit outputs are truncated when opened; and canceled, truncated, or failed downloads can leave a partial output file.

Validation

• Exact-head GitHub Actions checks passed: CI, CodeQL, and Castiron custom code.
• Focused regression coverage includes large and unbounded responses, chunked and truncated HTTP bodies, context cancellation, short writes, close errors, terminal output, bounded-prefix UTF-8 classification, filename collisions, replaced or missing automatic destinations, retained partial files, hard links, symlinks, and all six generated binary-download endpoints.
• Repeated race-detector runs, repository integration tests, ./scripts/lint, go vet ./..., go mod verify, Castiron checker tests, and Linux/Windows/macOS cross-platform build checks passed.
• All 33 review threads are resolved, including explicit discussion of the bounded-prefix and direct-output compatibility tradeoffs.

[Copilot]

Pull request overview

This PR updates the CLI’s binary download handling to stream response bodies directly to their destinations (stdout or explicit output files) instead of buffering entire responses in memory, while preserving the existing auto-detection behavior for terminal output via a temporary spool.

Changes:

• Stream response bodies with io.Copy for forced-stdout and explicit --output file paths.
• Add writeAutomaticBinaryResponse to preserve full-response UTF-8 validation without in-memory buffering, using a private temporary spool.
• Add extensive regression tests for large/chunked/unbounded responses, cancellation, short writes, temporary file privacy, and generated download endpoints.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
pkg/cmd/cmdutil.go	Switches binary response handling to streaming; adds temp-spool-based automatic (terminal) output behavior.
pkg/cmd/binaryresponse_test.go	Adds regression tests covering streaming, chunked responses, cancellation, writer failures, and generated download commands.

Suppressed comments (1)

pkg/cmd/cmdutil.go:293

• writeAutomaticBinaryResponse writes to the created download file but only defers file.Close(), so any close-time error (flush/sync failure) is silently ignored. Since this code is explicitly dealing with large/binary downloads, it’s important to surface close failures just like the explicit-output branch does.

Close the file explicitly after io.Copy, and avoid the deferred close so the error can be returned.

	file, err := createDownloadFile(response, sample)
	if err != nil {
		return "", err
	}
	defer file.Close()
	if _, err := io.Copy(file, spool); err != nil {
		return "", err
	}
	return fmt.Sprintf("Wrote output to: %s", file.Name()), nil

---

💡 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 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (2)

pkg/cmd/cmdutil.go:251

• writeAutomaticBinaryResponse validates UTF-8 by iterating rune-by-rune via bufio.Reader.ReadRune(). For large text downloads this adds significant per-rune overhead compared to validating in byte chunks (with a small carry buffer for split runes), while still requiring a full pass over the file. Consider switching to a chunked UTF-8 validator to reduce CPU time on large terminal downloads (the main case where this path is exercised).

		r, size, err := reader.ReadRune()

pkg/cmd/binaryresponse_test.go:73

• This test is gated on isTerminal(os.Stdout) and will be skipped in most CI environments where go test runs with non-TTY stdout, so it won’t provide regression coverage there. Consider making the terminal/non-terminal decision injectable (or using a PTY in the test) so the terminal auto-download path is exercised deterministically in CI.

	if !isTerminal(os.Stdout) {
		t.Skip("automatic terminal output requires an actual terminal")
	}

[Copilot]

Pull request overview

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

Suppressed comments (2)

pkg/cmd/cmdutil.go:252

• writeAutomaticBinaryResponse validates UTF-8 by scanning the entire spool rune-by-rune even when the MIME sniffed prefix is clearly not text (e.g., PDFs/PNGs). In the terminal-auto path this can add an extra full pass over large binary downloads (copy-to-spool + full UTF-8 scan + copy-to-destination), which is avoidable without changing behavior: only do the full-response UTF-8 scan (and the invalid-after-sample adjustment) when the sampled prefix is classified as text by isUTF8TextFile(sample).

	validUTF8 := true
	reader := bufio.NewReader(spool)
	for {
		r, size, err := reader.ReadRune()
		if errors.Is(err, io.EOF) {

pkg/cmd/binaryresponse_test.go:407

• The test server handler writes the incoming request into a requests channel with capacity 1. If the CLI ever performs more than one HTTP request for a given command (e.g., a metadata call plus the download), the second handler invocation will block on requests <- request and can deadlock/hang the test. Making the send non-blocking (and/or increasing the buffer) avoids a brittle test hang without changing what the test asserts (it still reads the first request).

			requests := make(chan *http.Request, 1)
			server := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
				requests <- request
				response.Header().Set("Content-Type", "application/octet-stream")

[HAYDEN-OAI]

Reviewed the exact head and found three actionable P2 regressions in download correctness, disk usage, and interruption cleanup.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7f65e28f8

ℹ️ 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 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

pkg/cmd/cmdutil.go:251

• writeExplicitBinaryResponse always creates a staging temp file in filepath.Dir(target) and then renames it into place. This changes behavior for overwriting an existing regular file: the old os.WriteFile-based approach only needed write permission on the file itself, but the new transactional path also requires write permission on the containing directory (to create the temp file and perform the rename). That can cause unexpected EPERM even when the destination file is writable.

Consider falling back to an in-place stream overwrite when the destination exists and staging fails with a permission error (transactional replacement isn’t possible in that environment anyway).

	staged, err := os.CreateTemp(filepath.Dir(target), ".openai-cli-download-*")
	if err != nil {
		return err
	}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dbd5e1a0da

ℹ️ 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 6 out of 6 changed files in this pull request and generated 1 comment.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-23T00:54:22.919870Z	8362baf	New commits
🔒 Security Review	✅ Completed 2026-08-23T00:53:47.649702Z	8362baf	New commits

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

Castiron custom code

✅ No new custom-code files detected.

5 mixed files remain; 0 existing customizations changed.

Compared 681d595d302f → 8362baf1bc5e. Generated baselines verified.

5 existing customizations unchanged

• pkg/cmd/adminorganizationcertificate.go
• pkg/cmd/cmd.go
• scripts/castiron/README.md
• scripts/castiron/custom_code_report.py
• scripts/castiron/test_custom_code_report.py

A changed generated baseline means this report cannot reliably identify which handwritten lines changed.

Inspect the custom-code diff

Download the exact patch produced by this run (requires repository access):

gh run download 32622121073 --repo openai/openai-cli \
  --name castiron-custom-code-32622121073-1 --dir /tmp/castiron-custom-code-32622121073-1
git apply --stat /tmp/castiron-custom-code-32622121073-1/custom-code.patch
cat /tmp/castiron-custom-code-32622121073-1/custom-code.patch

Or reproduce it from an SDK checkout containing the vendored reporter:

git fetch --no-tags origin 681d595d302ffe9db8ec6a7efbd32c2762785ce1 8362baf1bc5e132078ac1f21f027ae9883ae5a75
python3 scripts/castiron/custom_code_report.py report \
  --base 681d595d302ffe9db8ec6a7efbd32c2762785ce1 \
  --head 8362baf1bc5e132078ac1f21f027ae9883ae5a75 --fetch --require-head-hash --public \
  --out /tmp/castiron-custom-code-8362baf1bc5e
cat /tmp/castiron-custom-code-8362baf1bc5e/custom-code.patch

This is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR.

Full report and patch

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a09f0f1266

ℹ️ 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 6 out of 6 changed files in this pull request and generated 1 comment.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f1228ddc69

ℹ️ 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 2 out of 2 changed files in this pull request and generated 1 comment.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9800c8c316

ℹ️ 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.

[Copilot]

Pull request overview

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8362baf1bc

ℹ️ 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".

[apcha-oai]

[sdk codex review] Completed two independent full reviews of all three changed files, six download callers and all 33 existing review threads at 8362baf1bc5e132078ac1f21f027ae9883ae5a75. No substantiated new blocking issue within the author's declared direct-streaming/bounded-prefix design; no duplicate inline comments.

The tradeoffs remain explicit: automatic routing samples up to 515 bytes, later nontext bytes can follow a text-looking prefix, explicit outputs are truncated before completion, and failed downloads can retain partial files. The removed spool/promotion/unlink architectures are not present. Separate human acceptance of these tradeoffs was not independently verified; the PR summary's spool description should also be aligned with current source.

Verified CI run 32608925033/test job 97118546458 checked out merge ba9f471 of this exact head into current main 681d595. Go 1.25.14 Linux package tests passed via ./scripts/test without skip arguments, followed by Windows test compilation. CI, CodeQL and trusted Castiron checks report success. Nonverbose logs do not prove execution of the conditional real-TTY test; Windows compilation is not Windows runtime testing. No new tests, builds or security scans were run for this review.

This completed automated review COMMENT does not provide the requested human SDK CODEOWNER approval. Human signoff remains outstanding. Head, base/merge-base, author, open/non-draft status, mergeability, checks and feedback were verified before submission.

[jbeckwith-oai]

Updated the PR Summary and Validation to match the approved final direct-streaming/bounded-prefix implementation, its explicit compatibility tradeoffs, current regression coverage, and passing hosted checks, as requested in #108 (review). No source or commit changes.

[HAYDEN-OAI]

Reviewed the exact head, including direct streaming across all binary-download callers, bounded-prefix terminal routing, response/body ownership, cancellation and short-write propagation, destination permissions and identity, and the documented direct-streaming tradeoffs. No new substantive correctness, security, or compatibility issues found.