feat(cli): actionable errors for missing models and credentials (phase 1 of #3442)

cmd/root/flags.go

@@ -103,6 +103,13 @@ func addGatewayFlags(cmd *cobra.Command, runConfig *config.RuntimeConfig, loadUs
 
 		env := runConfig.EnvProvider()
 
+		// A missing or malformed --env-from-file must abort the run: the flag
+		// is the documented way to supply credentials, so silently continuing
+		// without it leads to confusing "env var must be set" errors later.
+		if err := runConfig.EnvFilesError(); err != nil {
+			return fmt.Errorf("--env-from-file: %w", err)
+		}
+
 		// Precedence: CLI flag > environment variable > user config
 		if runConfig.ModelsGateway == "" {
 			if gateway, _ := env.Get(ctx, envModelsGateway); gateway != "" {

cmd/root/flags_test.go

@@ -2,6 +2,8 @@ package root
 
 import (
 	"context"
+	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/spf13/cobra"
@@ -327,3 +329,59 @@ func TestDefaultModelLogic(t *testing.T) {
 		})
 	}
 }
+
+// A missing or malformed --env-from-file must abort the run instead of being
+// logged and skipped: the flag is the documented way to supply credentials
+// (issue #3442).
+func TestEnvFromFileErrorsAbortPreRun(t *testing.T) {
+	t.Parallel()
+
+	loadUserConfig := func() (*userconfig.Config, error) {
+		return &userconfig.Config{}, nil
+	}
+
+	t.Run("missing file", func(t *testing.T) {
+		t.Parallel()
+
+		cmd := &cobra.Command{
+			SilenceErrors: true,
+			SilenceUsage:  true,
+			RunE:          func(*cobra.Command, []string) error { return nil },
+		}
+		runConfig := config.RuntimeConfig{
+			Config: config.Config{EnvFiles: []string{filepath.Join(t.TempDir(), "missing.env")}},
+		}
+		addGatewayFlags(cmd, &runConfig, loadUserConfig)
+
+		cmd.SetArgs(nil)
+		err := cmd.Execute()
+
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "--env-from-file")
+		assert.Contains(t, err.Error(), "missing.env")
+	})
+
+	t.Run("malformed file", func(t *testing.T) {
+		t.Parallel()
+
+		bad := filepath.Join(t.TempDir(), "bad.env")
+		require.NoError(t, os.WriteFile(bad, []byte("NOT_A_PAIR\n"), 0o600))
+
+		cmd := &cobra.Command{
+			SilenceErrors: true,
+			SilenceUsage:  true,
+			RunE:          func(*cobra.Command, []string) error { return nil },
+		}
+		runConfig := config.RuntimeConfig{
+			Config: config.Config{EnvFiles: []string{bad}},
+		}
+		addGatewayFlags(cmd, &runConfig, loadUserConfig)
+
+		cmd.SetArgs(nil)
+		err := cmd.Execute()
+
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "--env-from-file")
+		assert.Contains(t, err.Error(), "bad.env")
+	})
+}

pkg/cli/runner.go

@@ -213,8 +213,10 @@ func Run(ctx context.Context, out *Printer, cfg Config, rt runtime.Runtime, sess
 				if strings.Contains(lowerErr, "context cancel") && ctx.Err() != nil { // treat Ctrl+C cancellations as non-errors
 					lastErr = nil
 				} else {
+					// Not printed here: the error is returned to the command layer,
+					// which prints it exactly once. Printing here too showed every
+					// failure twice in --exec runs.
 					lastErr = fmt.Errorf("%s", e.Error)
-					out.PrintError(lastErr)
 				}
 			case *runtime.MaxIterationsReachedEvent:
 				switch handleMaxIterationsAutoApprove(cfg.AutoApprove, &autoExtensions, e.MaxIterations) {
@@ -274,6 +276,9 @@ func Run(ctx context.Context, out *Printer, cfg Config, rt runtime.Runtime, sess
 		if err != nil {
 			return fmt.Errorf("failed to read from stdin: %w", err)
 		}
+		if strings.TrimSpace(string(buf)) == "" {
+			return errors.New(`no message received on stdin: with "-" the prompt is read from stdin, e.g. echo "Hello" | docker agent run <agent> -`)
+		}
 
 		if err := oneLoop(string(buf), os.Stdin); err != nil {
 			return err
@@ -291,6 +296,11 @@ func Run(ctx context.Context, out *Printer, cfg Config, rt runtime.Runtime, sess
 		if err != nil {
 			return fmt.Errorf("failed to read from stdin: %w", err)
 		}
+		// Exiting 0 with no output on empty input (e.g. bare `docker agent`
+		// in CI) would be a silent no-op; fail with the next steps instead.
+		if strings.TrimSpace(string(buf)) == "" {
+			return errors.New("no message provided and stdin is not a terminal; pass a message (docker agent run <agent> \"<message>\"), pipe one on stdin, or run from an interactive terminal to use the chat UI")
+		}
 
 		if err := oneLoop(string(buf), os.Stdin); err != nil {
 			return err

pkg/cli/runner_test.go

@@ -4,6 +4,8 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"os"
+	"strings"
 	"sync"
 	"testing"
 
@@ -481,3 +483,88 @@ func TestPrepareUserMessage_CommandResolution(t *testing.T) {
 
 	assert.Equal(t, "Fix the file main.go", msg.Message.Content, "Command should be resolved with args")
 }
+
+// swapStdin replaces os.Stdin with a pipe carrying the given content for the
+// duration of the test. A pipe is never a terminal, so it also exercises the
+// non-TTY stdin paths. Tests using it must not run in parallel: os.Stdin is
+// process-global state.
+func swapStdin(t *testing.T, content string) {
+	t.Helper()
+
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("creating stdin pipe: %v", err)
+	}
+	if _, err := w.WriteString(content); err != nil {
+		t.Fatalf("writing stdin pipe: %v", err)
+	}
+	if err := w.Close(); err != nil {
+		t.Fatalf("closing stdin pipe: %v", err)
+	}
+
+	old := os.Stdin
+	os.Stdin = r
+	t.Cleanup(func() {
+		os.Stdin = old
+		_ = r.Close()
+	})
+}
+
+// A run that would silently do nothing (no message, empty non-TTY stdin, e.g.
+// bare `docker agent` in CI) must fail with an actionable error instead of
+// exiting 0 with no output (issue #3442).
+func TestRunEmptyStdinNonTTYFails(t *testing.T) {
+	swapStdin(t, "")
+
+	rt := &mockRuntime{}
+	var buf bytes.Buffer
+	sess := session.New()
+
+	err := Run(t.Context(), NewPrinter(&buf), Config{}, rt, sess, nil)
+	assert.ErrorContains(t, err, "no message provided and stdin is not a terminal")
+	assert.ErrorContains(t, err, "interactive terminal")
+}
+
+func TestRunEmptyStdinWithDashFails(t *testing.T) {
+	swapStdin(t, "")
+
+	rt := &mockRuntime{}
+	var buf bytes.Buffer
+	sess := session.New()
+
+	err := Run(t.Context(), NewPrinter(&buf), Config{}, rt, sess, []string{"-"})
+	assert.ErrorContains(t, err, "no message received on stdin")
+}
+
+func TestRunPipedStdinStillWorks(t *testing.T) {
+	swapStdin(t, "hello agent\n")
+
+	rt := &mockRuntime{
+		events: []runtime.Event{&runtime.AgentChoiceEvent{Content: "4"}},
+	}
+	var buf bytes.Buffer
+	sess := session.New()
+
+	err := Run(t.Context(), NewPrinter(&buf), Config{}, rt, sess, nil)
+	assert.NilError(t, err)
+	assert.Equal(t, len(sess.GetAllMessages()) > 0, true)
+}
+
+// An ErrorEvent must surface exactly once: returned to the command layer
+// (which prints it), never also printed by the runner (issue #3442).
+func TestErrorEventReturnedNotPrinted(t *testing.T) {
+	t.Parallel()
+
+	rt := &mockRuntime{
+		events: []runtime.Event{runtime.Error("model failed: HTTP 404")},
+	}
+	var buf bytes.Buffer
+	sess := session.New()
+
+	err := Run(t.Context(), NewPrinter(&buf), Config{}, rt, sess, []string{"hello"})
+	assert.ErrorContains(t, err, "model failed: HTTP 404")
+
+	var runtimeErr RuntimeError
+	assert.Equal(t, errors.As(err, &runtimeErr), true)
+	assert.Equal(t, strings.Contains(buf.String(), "model failed"), false)
+}

pkg/config/config.go

@@ -146,7 +146,7 @@ func CheckRequiredEnvVars(ctx context.Context, cfg *latest.Config, modelsGateway
 		}
 	}
 
-	missing, err := gatherMissingEnvVars(ctx, cfg, modelsGateway, env)
+	missing, missingModelCreds, err := gatherMissingEnvVars(ctx, cfg, modelsGateway, env)
 	if err != nil {
 		// If there's a tool preflight error, log it but continue
 		slog.WarnContext(ctx, "Failed to preflight toolset environment variables; continuing", "error", err)
@@ -155,7 +155,8 @@ func CheckRequiredEnvVars(ctx context.Context, cfg *latest.Config, modelsGateway
 	// Return error if there are missing environment variables
 	if len(missing) > 0 {
 		return &environment.RequiredEnvError{
-			Missing: missing,
+			Missing:                 missing,
+			MissingModelCredentials: missingModelCreds,
 		}
 	}
 

pkg/config/config_test.go

@@ -283,6 +283,7 @@ func TestCheckRequiredEnvVars(t *testing.T) {
 				var reqErr *environment.RequiredEnvError
 				require.ErrorAs(t, err, &reqErr)
 				assert.Equal(t, test.expectedMissing, reqErr.Missing)
+				assert.True(t, reqErr.MissingModelCredentials, "missing vars are model credentials, so the local-model hint must be offered")
 			}
 		})
 	}

pkg/config/first_available.go

@@ -174,10 +174,15 @@ func (e *firstAvailableMissingEnvError) Error() string {
 
 	fmt.Fprintln(&msg, "No 'first_available' candidate has credentials configured.")
 	fmt.Fprintln(&msg, "Set the environment variables for at least one candidate:")
-	for _, candidate := range e.Candidates {
+	example := "OPENAI_API_KEY"
+	for i, candidate := range e.Candidates {
 		fmt.Fprintf(&msg, " - %s: %s\n", candidate.Ref, strings.Join(candidate.Missing, ", "))
+		if i == 0 && len(candidate.Missing) > 0 {
+			example = candidate.Missing[0]
+		}
 	}
-	fmt.Fprintln(&msg, "\nEither:\n - Set one of those groups of environment variables before running docker agent\n - Run docker agent with --env-from-file\n - Store those secrets using one of the built-in environment variable providers.")
+	msg.WriteString("\n")
+	msg.WriteString(environment.SecretSourcesHelp(example))
 
 	return msg.String()
 }

pkg/config/first_available_test.go

@@ -180,8 +180,9 @@ func TestResolveFirstAvailableModels_NoCandidateAvailable(t *testing.T) {
 	assert.Contains(t, err.Error(), "Set the environment variables for at least one candidate:")
 	assert.Contains(t, err.Error(), " - anthropic/claude-sonnet-4-6: ANTHROPIC_API_KEY")
 	assert.Contains(t, err.Error(), " - openai/gpt-5: OPENAI_API_KEY")
-	assert.Contains(t, err.Error(), "Set one of those groups of environment variables")
-	assert.Contains(t, err.Error(), "Run docker agent with --env-from-file")
+	assert.Contains(t, err.Error(), "export ANTHROPIC_API_KEY=<value>")
+	assert.Contains(t, err.Error(), "--env-from-file")
+	assert.Contains(t, err.Error(), environment.SecretsDocsURL)
 }
 
 func TestResolveFirstAvailableModels_InvalidCandidate(t *testing.T) {

pkg/config/gather.go

@@ -16,24 +16,26 @@ import (
 )
 
 // gatherMissingEnvVars finds out which environment variables are required by the models and tools.
-// It returns the missing variables and any non-fatal error encountered during tool discovery.
-func gatherMissingEnvVars(ctx context.Context, cfg *latest.Config, modelsGateway string, env environment.Provider) (missing []string, toolErr error) {
+// It returns the missing variables, whether any of them is a model-provider
+// credential (as opposed to a tool secret), and any non-fatal error
+// encountered during tool discovery.
+func gatherMissingEnvVars(ctx context.Context, cfg *latest.Config, modelsGateway string, env environment.Provider) (missing []string, missingModelCreds bool, toolErr error) {
 	requiredEnv := map[string]bool{}
+	modelEnv := map[string]bool{}
 
 	// Models
+	var modelNames []string
 	if modelsGateway == "" {
-		names := GatherEnvVarsForModels(ctx, cfg, env)
-		for _, e := range names {
-			requiredEnv[e] = true
-		}
+		modelNames = GatherEnvVarsForModels(ctx, cfg, env)
 	} else {
 		// A gateway supplies credentials for routed models, but models that
 		// bypass it dial their provider directly and still need their own
 		// credentials present.
-		names := gatherEnvVarsForModels(ctx, cfg, env, true)
-		for _, e := range names {
-			requiredEnv[e] = true
-		}
+		modelNames = gatherEnvVarsForModels(ctx, cfg, env, true)
+	}
+	for _, e := range modelNames {
+		requiredEnv[e] = true
+		modelEnv[e] = true
 	}
 
 	// Tools
@@ -52,10 +54,11 @@ func gatherMissingEnvVars(ctx context.Context, cfg *latest.Config, modelsGateway
 	for _, e := range sortedKeys(requiredEnv) {
 		if v, _ := env.Get(ctx, e); v == "" {
 			missing = append(missing, e)
+			missingModelCreds = missingModelCreds || modelEnv[e]
 		}
 	}
 
-	return missing, toolErr
+	return missing, missingModelCreds, toolErr
 }
 
 func GatherEnvVarsForModels(ctx context.Context, cfg *latest.Config, env environment.Provider) []string {

pkg/config/runtime.go

@@ -19,6 +19,7 @@ type RuntimeConfig struct {
 	EnvProviderForTests environment.Provider
 	envProviderCached   environment.Provider
 	envProviderOnce     sync.Once
+	envFilesErr         error
 
 	ModelsDevStoreOverride *modelsdev.Store
 	modelsDevStore         *modelsdev.Store
@@ -82,6 +83,7 @@ func (runConfig *RuntimeConfig) Clone() *RuntimeConfig {
 		Config:                 runConfig.Config,
 		EnvProviderForTests:    runConfig.EnvProviderForTests,
 		envProviderCached:      env,
+		envFilesErr:            runConfig.envFilesErr,
 		ModelsDevStoreOverride: runConfig.ModelsDevStoreOverride,
 		modelsDevStore:         store,
 		modelsDevStoreErr:      storeErr,
@@ -140,6 +142,15 @@ func (runConfig *RuntimeConfig) EnvProvider() environment.Provider {
 	return runConfig.envProviderCached
 }
 
+// EnvFilesError reports a failure to resolve, read, or parse the configured
+// env files (--env-from-file). The provider chain silently falls back to the
+// default sources in that case, so callers that must not run without the
+// file's variables (e.g. the CLI) should surface this error and abort.
+func (runConfig *RuntimeConfig) EnvFilesError() error {
+	runConfig.EnvProvider() // materialize the provider chain
+	return runConfig.envFilesErr
+}
+
 func (runConfig *RuntimeConfig) computedEnvProvider() environment.Provider {
 	defaultEnv := environment.NewDefaultProvider()
 
@@ -148,12 +159,14 @@ func (runConfig *RuntimeConfig) computedEnvProvider() environment.Provider {
 	runConfig.EnvFiles, err = environment.AbsolutePaths(runConfig.WorkingDir, runConfig.EnvFiles)
 	if err != nil {
 		slog.Error("Failed to make env file paths absolute", "error", err)
+		runConfig.envFilesErr = err
 		return defaultEnv
 	}
 
 	envFilesProviders, err := environment.NewEnvFilesProvider(runConfig.EnvFiles)
 	if err != nil {
 		slog.Error("Failed to read env files", "error", err)
+		runConfig.envFilesErr = err
 		return defaultEnv
 	}