cmd/prompt: remove uses of github.com/AlecAivazis/survey/v2 by thaJeztah · Pull Request #14161 · docker/compose

thaJeztah · 2026-09-01T18:12:22Z

taken from cmd/prompt: use ReadString instead of Fscanln, fix default value, and remove uses github.com/AlecAivazis/survey/v2 #13900

The github.com/AlecAivazis/survey/v2 module was archived and is no longer
maintained. Looking at the code, we didn't really use most of its features;
current callers already include the confirmation hint in the prompt
message, so survey mostly provided terminal input handling around a simple
yes/no question.

Replace the interactive confirmation implementation with
golang.org/x/term while keeping the exported prompt types and existing
non-terminal prompt behavior unchanged. Interactive prompts continue to
use raw terminal input so Ctrl+C is handled by the prompt instead of
requiring a second interrupt to terminate the process.

What I did

Related issue

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

docker-agent

Assessment: 🟡 NEEDS ATTENTION

This PR cleanly removes the archived survey/v2 dependency and replaces it with a minimal local line-based implementation. The overall approach is sound. Two behavioral regressions in the new confirm() function are worth addressing before merge.

docker-agent · 2026-09-01T18:17:01Z

 }
+
+func confirm(stdin *bufio.Reader, stdout io.Writer, message string, defaultValue bool) (bool, error) {
+	_, _ = fmt.Fprint(stdout, message)


[medium] confirm() does not print [y/N] hint; behavioral regression from survey.Confirm

The new confirm() function writes message verbatim to stdout with no indication of valid responses or the current default:

_, _ = fmt.Fprint(stdout, message)

survey.Confirm automatically appended (y/N) or (Y/n) (depending on defaultValue) to every prompt. The new implementation omits this entirely. Unless every caller already embeds the hint in its own message string, users running in a terminal will see a bare prompt with no cue that y/n are the expected inputs or which value is the default.

Consider appending the hint in confirm() itself so callers don't have to know about this detail:

Suggested change

_, _ = fmt.Fprint(stdout, message)

if defaultValue {

_, _ = fmt.Fprintf(stdout, "%s [Y/n] ", message)

} else {

_, _ = fmt.Fprintf(stdout, "%s [y/N] ", message)

}

Confidence Score

🟡 moderate 70/100

That's already handled by callers, which include it, so we just print verbatim; e.g.;

msg := "\nDo you want to proceed with these variables? [Y/n]: "

You're right, thanks for the clarification. I see that both call sites in options.go already include the hint in their message strings (line 166 uses [Y/n] and line 290 uses [y/N]), so the verbatim printing is intentional and there's no regression here.

I should have checked all the callers before flagging this. Apologies for the noise!

glours

I confirm the 2 catches made by @docker-agent and found a 3rd one

glours · 2026-09-02T13:17:07Z

+func confirm(stdin *bufio.Reader, stdout io.Writer, message string, defaultValue bool) (bool, error) {
+	_, _ = fmt.Fprint(stdout, message)
+
+	answer, err := stdin.ReadString('\n')


This blocking read has no way to observe cancellation. Since AdaptCmd's SIGINT handler (cmd/compose/compose.go:112) now intercepts Ctrl+C to cancel the context instead of letting the OS kill the process, a single Ctrl+C at a prompt no longer aborts it, the read stays blocked here and a second Ctrl+C is needed to actually terminate.
survey's raw-mode reader handled this internally (ISIG disabled, Ctrl+C caught as a keypress).

thaJeztah · 2026-09-02T13:23:47Z

Ah, yes, I need to look at this one; I thought I'd move this separate, but didn't look too closely yet 😂

docker-agent

Assessment: 🟢 APPROVE

The PR correctly removes the archived github.com/AlecAivazis/survey/v2 dependency and replaces it with golang.org/x/term. The core Confirm implementation is structurally sound: SetRawTerminal / defer RestoreTerminal is safe, term.NewTerminal usage is idiomatic, the error propagation path is correct, and go.mod properly promotes golang.org/x/term from indirect to direct. The new tests cover the happy path, invalid-input retry, and Ctrl+C interruption.

Lower-confidence findings (not posted inline)

[medium → verified low] cmd/prompt/prompt.go:94 — Ctrl+C handling relies on term.ReadLine returning io.EOF on byte 0x03; if golang.org/x/term changes this behaviour in a future release the loop would spin indefinitely until the TTY is closed. Currently working as intended in v0.45.0 — the verifier assessed this as speculative/low, score 40/100 (🟠 weak). No action required now; worth a comment in the code or a version note in go.mod if the team wants to guard against it long-term.

thaJeztah · 2026-09-05T12:19:11Z

if golang.org/x/term changes this behaviour in a future release the loop would spin indefinitely until the TTY is closed. Currently working as intended in v0.45.0

Yeah, not fixing that; we have tests, and they shouldn't break behavior.

codecov · 2026-09-05T12:35:22Z

Codecov Report

❌ Patch coverage is 71.42857% with 6 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cmd/prompt/prompt.go	71.42%	5 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

The github.com/AlecAivazis/survey/v2 module was archived and is no longer maintained. Looking at the code, we didn't really use most of its features; current callers already include the confirmation hint in the prompt message, so survey mostly provided terminal input handling around a simple yes/no question. Replace the interactive confirmation implementation with golang.org/x/term while keeping the exported prompt types and existing non-terminal prompt behavior unchanged. Interactive prompts continue to use raw terminal input so Ctrl+C is handled by the prompt instead of requiring a second interrupt to terminate the process. Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2026-09-08T14:07:16Z

Rebased; probably should be OK for review now 😅

ndeloof

Legitimate change, worth landing: survey/v2 is archived, compose only used a yes/no out of it, and this drops five transitive dependencies while promoting an already-present x/term. Exported API intact, non-terminal Pipe path untouched, the pty-backed tests cover the right level, deferred RestoreTerminal covers error paths, and the retry/Enter-default semantics match survey.

One real compatibility regression and two smaller notes, inline. On top of those, one thing CI cannot tell us: the interactive path now goes through streams SetRawTerminal + x/term.Terminal on a Windows console, a different plumbing than survey's (go-colorable); the pty tests only exercise unix. Worth one manual check on Windows before merge, or a note in the description.

ndeloof · 2026-09-09T06:17:59Z

@@ -74,17 +75,34 @@ func (s streamsFileReader) Fd() uintptr {

 // Confirm asks for yes or no input
 func (u User) Confirm(message string, defaultValue bool) (bool, error) {


The "(y/N)" hint survey auto-appended is lost, and three call sites relied on it — the description's "current callers already include the confirmation hint" holds for the two prompts in cmd/compose/options.go only. These don't carry any hint in their message:

pkg/compose/publish.go — "Are you ok to publish these bind mount declarations?" and "…these sensitive data?" (via confirmOrCancel);

pkg/bridge/convert.go — "Output directory … will be permanently deleted. Continue?".

After this change those render as a bare question: the user no longer sees the expected input format nor the default — on destructive confirmations. Minimal fix that preserves every caller at once: have Confirm append " [y/N]: " / " [Y/n]: " (per defaultValue) when the message doesn't already end with a hint; alternatively, fix the three messages in this same PR.

If you go with option 2 (fixing individual messages rather than the centralized approach), don't miss two more call sites through the same Confirm with the same bare-question issue:

pkg/compose/publish.go:619 — buildEnvPromptMessage: "...Are you ok to publish these env declarations?"

pkg/compose/publish.go:629 — buildConfigContentPromptMessage: "...Are you ok to publish these config contents?"

ndeloof · 2026-09-09T06:17:59Z

+			return false, err
+		}
+
+		switch strings.ToLower(answer) {


nit: strings.ToLower without strings.TrimSpace — a y (stray whitespace) silently loops back to the prompt, where survey tolerated it. One-liner: switch strings.ToLower(strings.TrimSpace(answer)).

ndeloof · 2026-09-09T06:17:59Z

+
+	select {
+	case err := <-done:
+		assert.ErrorIs(t, err, io.EOF)


Behavior note this test locks in: Ctrl+C now surfaces as io.EOF (that's x/term's mapping), where survey returned InterruptErr ("interrupt"). No compose caller special-cased InterruptErr (they all return err), so nothing breaks logically — but the user-facing failure message degrades to "EOF", and ^C becomes indistinguishable from ^D by construction. Non-blocking suggestion: intercept 0x03 in the wrapping Reader and return a dedicated error ("prompt interrupted by user"), which also keeps the door open for a proper exit 130 later.

thaJeztah requested review from a team as code owners September 1, 2026 18:12

thaJeztah requested review from glours and ndeloof September 1, 2026 18:12

thaJeztah force-pushed the rm_survey branch from 8fe8459 to 8e20cce Compare September 1, 2026 18:14

docker-agent reviewed Sep 1, 2026

View reviewed changes

glours requested changes Sep 2, 2026

View reviewed changes

thaJeztah force-pushed the rm_survey branch 2 times, most recently from bed0960 to e61e69a Compare September 5, 2026 12:09

thaJeztah requested a review from docker-agent September 5, 2026 12:09

docker-agent reviewed Sep 5, 2026

View reviewed changes

thaJeztah force-pushed the rm_survey branch from e61e69a to 3b8ee52 Compare September 5, 2026 12:16

thaJeztah force-pushed the rm_survey branch from 3b8ee52 to 363d60d Compare September 5, 2026 12:23

thaJeztah force-pushed the rm_survey branch from 363d60d to 6463e95 Compare September 8, 2026 14:06

ndeloof reviewed Sep 9, 2026

View reviewed changes

-	_, _ = fmt.Fprint(stdout, message)
+	if defaultValue {
+		_, _ = fmt.Fprintf(stdout, "%s [Y/n] ", message)
+	} else {
+		_, _ = fmt.Fprintf(stdout, "%s [y/N] ", message)
+	}

		@@ -74,17 +75,34 @@ func (s streamsFileReader) Fd() uintptr {

		// Confirm asks for yes or no input
		func (u User) Confirm(message string, defaultValue bool) (bool, error) {

Uh oh!

Conversation

thaJeztah commented Sep 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

docker-agent left a comment

Choose a reason for hiding this comment

Assessment: 🟡 NEEDS ATTENTION

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

glours left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Sep 2, 2026

Uh oh!

docker-agent left a comment

Choose a reason for hiding this comment

Assessment: 🟢 APPROVE

Lower-confidence findings (not posted inline)

Uh oh!

thaJeztah commented Sep 5, 2026

Uh oh!

codecov Bot commented Sep 5, 2026

Codecov Report

Uh oh!

thaJeztah commented Sep 8, 2026

Uh oh!

ndeloof left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thaJeztah commented Sep 1, 2026 •

edited

Loading