fix(progress): probe stderr (not stdout) for TTY auto-detection by glours · Pull Request #13837 · docker/compose

glours · 2026-06-11T21:14:15Z

What I did
The progress UI writes to dockerCli.Err() but the auto-mode selector was probing dockerCli.Out().IsTerminal(), introduced when the EventProcessor was moved to the CLI layer. Any context that pipes stdout while keeping stderr attached — docker compose up | tee log, some CI wrappers, PowerShell native-command capture — silently dropped to plain mode.

Align the detection with the stream the renderer actually targets, restoring v4 behavior. Extract the switch into selectEventProcessor so the auto-mode logic can be unit-tested with a real pty pair.

Fixes #13570

Related issue

(not mandatory) A picture of a cute animal, if possible in relation to what you did

Copilot

Pull request overview

This PR fixes Compose’s progress renderer auto-detection by probing the TTY status of stderr (the stream actually used by the progress UI) instead of stdout, restoring expected behavior when stdout is redirected but stderr remains attached to a terminal.

Changes:

Switch auto-mode TTY probing from dockerCli.Out().IsTerminal() to dockerCli.Err().IsTerminal().
Extract progress EventProcessor selection into selectEventProcessor(...) to make the logic unit-testable.
Add unit tests covering auto-mode and explicit --progress modes.

Reviewed changes

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

File	Description
cmd/compose/compose.go	Routes progress auto-detection to stderr and factors selection logic into `selectEventProcessor`.
cmd/compose/compose_progress_test.go	Adds tests to validate stream probing behavior and explicit progress modes.

+func newMockCli(t *testing.T, out, errStream *streams.Out) *mocks.MockCli {
+	t.Helper()
+	cli := mocks.NewMockCli(gomock.NewController(t))
+	cli.EXPECT().Out().Return(out).AnyTimes()
+	cli.EXPECT().Err().Return(errStream).AnyTimes()
+	return cli
+}


+import (
+	"fmt"
+	"os"
+	"testing"
+
+	"github.com/creack/pty"
+	"github.com/docker/cli/cli/streams"
+	"go.uber.org/mock/gomock"
+	"gotest.tools/v3/assert"
+
+	"github.com/docker/compose/v5/cmd/display"
+	"github.com/docker/compose/v5/pkg/mocks"
+)
+
+// saveDisplayMode snapshots display.Mode and restores it on cleanup, since
+// selectEventProcessor mutates this global as a side effect.
+func saveDisplayMode(t *testing.T) {
+	t.Helper()
+	original := display.Mode
+	t.Cleanup(func() { display.Mode = original })
+}


docker-agent

Assessment: 🟢 APPROVE

The fix correctly aligns TTY auto-detection with the stream the renderer actually writes to — probing Err() (stderr) instead of Out() (stdout). This restores the expected behavior for docker compose up | tee log and similar piping scenarios.

What was reviewed:

selectEventProcessor correctly calls dockerCli.Err().IsTerminal() (was dockerCli.Out().IsTerminal())
All explicit --progress modes (tty, plain, quiet, json, none) are correctly handled in the extracted function
Error handling is properly propagated via (api.EventProcessor, error) return signature
The new test file covers the core regression scenario (stderr TTY, stdout piped → Full) and all explicit-mode branches
saveDisplayMode correctly saves/restores the display.Mode global to prevent test pollution

Minor observations (not blocking):

In the ModeAuto + TTY branch, display.Mode is not updated to ModeTTY (stays ModeAuto). This is consistent with the old code and has no observable effect since no downstream code checks display.Mode == display.ModeTTY — only display.ModeJSON is gated on.
With ansi=always and a non-TTY stderr, display.Mode is left as ModeTTY (set by the outer ansi switch) while a plainWriter is returned. Again, no current code path is broken by this inconsistency.
Tests do not use t.Parallel(), which is fine given the display.Mode global mutation; just worth noting if parallelism is added later.

The core bugfix is sound and the test coverage for the regression is well-structured.

docker-agent

Assessment: 🟢 APPROVE

The fix correctly aligns TTY auto-detection with the stream that the progress renderer actually writes to. The extracted selectEventProcessor function is well-structured and the new tests validate the regression path.

No high or medium severity issues were found in the changed code.

codecov · 2026-06-11T21:26:20Z

Codecov Report

❌ Patch coverage is 93.93939% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cmd/compose/compose.go	93.93%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

glours · 2026-06-11T21:46:32Z

/review

docker-agent

Assessment: 🟢 APPROVE

The fix correctly aligns TTY auto-detection with the stream the renderer actually writes to (dockerCli.Err() / stderr instead of dockerCli.Out() / stdout). This restores the expected behavior when stdout is piped while stderr remains a terminal (e.g., docker compose up | tee log).

What was verified:

The core fix (Err().IsTerminal() replacing Out().IsTerminal()) is correct and directly addresses the reported issue.
The auto-mode TTY path in selectEventProcessor does not set display.Mode = display.ModeTTY — but this is pre-existing behavior (the old inline switch also omitted this), and all downstream consumers of display.Mode are consistent with it remaining "auto" in this path (none require it to be "tty" for correct behavior).
The regression test TestSelectEventProcessor_AutoMode_StdoutPipedStderrTTY uses genuinely distinct streams (a real PTY for stderr, a pipe for stdout), providing real discriminating power for the exact scenario being fixed.
Extraction of selectEventProcessor is a clean refactor that improves testability without behavioral regressions.

No confirmed or likely bugs found in the changed code.

Copilot

Pull request overview

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

The progress UI writes to dockerCli.Err() but the auto-mode selector was probing dockerCli.Out().IsTerminal(), introduced when the EventProcessor was moved to the CLI layer. Any context that pipes stdout while keeping stderr attached — `docker compose up | tee log`, some CI wrappers, PowerShell native-command capture — silently dropped to plain mode. Align the detection with the stream the renderer actually targets, restoring v4 behavior. Extract the switch into selectEventProcessor so the auto-mode logic can be unit-tested with a real pty pair. Fixes docker#13570 Signed-off-by: Guillaume Lours <glours@users.noreply.github.com>

glours · 2026-06-12T06:03:56Z

/review

docker-agent

Assessment: 🟢 APPROVE

This PR correctly fixes TTY auto-detection for the progress renderer by probing dockerCli.Err().IsTerminal() instead of dockerCli.Out().IsTerminal(), aligning the detection with the stream the renderer actually writes to.

What was reviewed:

selectEventProcessor extraction and TTY probe fix in cmd/compose/compose.go
New unit tests in cmd/compose/compose_progress_test.go using a real pty pair

No confirmed or likely bugs found. The refactor faithfully preserves existing behavior while fixing the stdout/stderr mismatch. All explicit progress modes (tty, plain, quiet, json) correctly set display.Mode. The new test coverage for auto-mode TTY detection is well-structured.

Copilot AI review requested due to automatic review settings June 11, 2026 21:14

glours requested a review from a team as a code owner June 11, 2026 21:14

glours requested a review from ndeloof June 11, 2026 21:14

Copilot started reviewing on behalf of glours June 11, 2026 21:14 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

docker-agent Bot reviewed Jun 11, 2026

View reviewed changes

glours force-pushed the fix/progress-tty-detection-stderr branch from ccbe129 to f0a76df Compare June 11, 2026 21:39

glours requested a review from Copilot June 11, 2026 21:46

Copilot started reviewing on behalf of glours June 11, 2026 21:46 View session

docker-agent Bot reviewed Jun 11, 2026

View reviewed changes

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Comment thread cmd/compose/compose_progress_test.go

glours force-pushed the fix/progress-tty-detection-stderr branch from f0a76df to 041ff77 Compare June 12, 2026 06:03

docker-agent Bot reviewed Jun 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(progress): probe stderr (not stdout) for TTY auto-detection#13837

fix(progress): probe stderr (not stdout) for TTY auto-detection#13837
glours wants to merge 1 commit into
docker:mainfrom
glours:fix/progress-tty-detection-stderr

glours commented Jun 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

docker-agent Bot left a comment

Uh oh!

docker-agent Bot left a comment

Uh oh!

codecov Bot commented Jun 11, 2026

Uh oh!

glours commented Jun 11, 2026

Uh oh!

docker-agent Bot left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

glours commented Jun 12, 2026

Uh oh!

docker-agent Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

glours commented Jun 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

docker-agent Bot left a comment

Choose a reason for hiding this comment

Assessment: 🟢 APPROVE

Uh oh!

docker-agent Bot left a comment

Choose a reason for hiding this comment

Assessment: 🟢 APPROVE

Uh oh!

codecov Bot commented Jun 11, 2026

Codecov Report

Uh oh!

glours commented Jun 11, 2026

Uh oh!

docker-agent Bot left a comment

Choose a reason for hiding this comment

Assessment: 🟢 APPROVE

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

glours commented Jun 12, 2026

Uh oh!

docker-agent Bot left a comment

Choose a reason for hiding this comment

Assessment: 🟢 APPROVE

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants