From 35c0886e85a6a8d65c581ebe4c8be66000ad859f Mon Sep 17 00:00:00 2001
From: Adhik Joshi <adhik3035@gmail.com>
Date: Wed, 9 Sep 2026 23:00:58 +0530
Subject: [PATCH] Stop the CLI misdiagnosing why a login failed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A customer was blocked for ~12 days on "cannot authenticate". Their account is
Google-only, so `auth login --email/--password` can never work for them — OAuth
signups are stored with a random password — and every failure the CLI showed
them pointed somewhere useless.

The control plane wraps failures as
{"data":null,"error":{"code","message"},"meta":{}}. parseAPIError read only the
top level, where "error" is an object rather than a string, so it fell through
to printing the entire JSON body as the message. Underneath it, the login hint
was a fixed "Check your email and password / run forgot-password" — advice that
is wrong for an account that has no password to check.

Error reporting
- Parse the control-plane envelope: message, machine code, and the per-field
  `details` bag. Unrecognised bodies are truncated on a rune boundary, and a
  proxy's HTML error page is reduced to its <title> ("502 Bad Gateway").
- Hint from the code, not blanket text. invalid_credentials now names
  `--browser` for OAuth accounts; email_not_verified offers resend-verification;
  validation_error prints the fields the server actually rejected. An EMPTY code
  is no longer folded in with invalid_credentials — a 502 during a deploy used
  to be reported as a wrong password, which is exactly the rabbit hole above.
- Map APIError.ExitCode for every command, not just `auth login`. Everything
  else exited 1, so a script could not tell an expired token from a bad flag. A
  401 with nothing stored now says so instead of just "Unauthenticated."
- Honour Retry-After on 429, keep the server's own message, and return
  immediately rather than sleeping through a 60s window.

Credential handling
- `.modelslab/config.toml` is read from the CURRENT WORKING DIRECTORY and was
  merged wholesale, so a committed `base_url` in any repo you cd into redirected
  the next command — with the stored bearer token attached — to that host. Proven
  end to end. base_url, api_key and token are now ignored from project config;
  harmless preferences still merge. Use --base-url or MODELSLAB_BASE_URL.
- `auth login` dropped the StoreToken/StoreEmail/StoreAPIKey return values and
  printed "Logged in" regardless. A locked keychain plus an unwritable
  ~/.config meant success on screen, nothing stored, and 401s afterwards.
- The MCP server's auth-login returned a token without ever assigning it to the
  long-lived client, so an agent that logged in through MCP got 401 from every
  later tool with a valid token in its own transcript. Same for api-keys-create.

Prompts and the browser flow
- Replace fmt.Scanln: it stopped at the first space, so signup recorded "Ada"
  for "Ada Lovelace", and on an empty line it let the command run on with an
  empty email. term.ReadPassword now falls back to reading stdin when it is not
  a TTY, so piped and CI logins work instead of dying with "inappropriate ioctl
  for device" — previously the only non-interactive option was --password on the
  command line, which lands in shell history and CI logs.
- Print the authorize URL unconditionally. openBrowser uses exec.Start(), which
  returns nil as soon as the child is spawned, so a headless box or a browser
  that dies on launch looked like success: the user waited five minutes and was
  never shown the URL. The timeout message now says what to do next.

Companion PR in ModelsLab/modelslab-frontend-v2 fixes the server-side cause:
the Google and GitHub callbacks ignored Laravel's `url.intended`, so
`auth login --browser` never returned to the CLI for a signed-out user.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QgkQePXPha8ShvoBerrVXL
---
 README.md                       |   4 +
 cmd/modelslab/main.go           |   4 +-
 internal/api/client.go          | 209 +++++++++++++++++++++++++++-----
 internal/api/client_test.go     | 122 +++++++++++++++++++
 internal/cmd/auth.go            | 145 ++++++++++++++++------
 internal/cmd/auth_hints_test.go |  62 ++++++++++
 internal/cmd/exit.go            |  56 +++++++++
 internal/cmd/exit_test.go       |  32 +++++
 internal/cmd/mcp.go             |   4 +-
 internal/cmd/prompt.go          |  76 ++++++++++++
 internal/cmd/prompt_test.go     |  66 ++++++++++
 internal/config/config.go       |  40 +++++-
 internal/config/config_test.go  |  40 ++++++
 internal/mcp/server.go          |  69 ++++++++++-
 internal/mcp/server_test.go     |  61 ++++++++++
 15 files changed, 915 insertions(+), 75 deletions(-)
 create mode 100644 internal/cmd/auth_hints_test.go
 create mode 100644 internal/cmd/exit.go
 create mode 100644 internal/cmd/exit_test.go
 create mode 100644 internal/cmd/prompt.go
 create mode 100644 internal/cmd/prompt_test.go
 create mode 100644 internal/mcp/server_test.go

diff --git a/README.md b/README.md
index 41edad5..2037478 100644
--- a/README.md
+++ b/README.md
@@ -183,6 +183,10 @@ modelslab auth login --browser
 # Email/password login also gets both token and API key
 modelslab auth login --email you@example.com --password "..."
 
+# NOTE: accounts created with "Continue with Google" or "Continue with GitHub"
+# have no password, so --email/--password can never work for them. Use
+# --browser, or set a password first with `modelslab auth forgot-password`.
+
 # Or set API key manually
 modelslab config set api_key "your-api-key"
 
diff --git a/cmd/modelslab/main.go b/cmd/modelslab/main.go
index 6cb3c44..ba61e06 100644
--- a/cmd/modelslab/main.go
+++ b/cmd/modelslab/main.go
@@ -1,7 +1,6 @@
 package main
 
 import (
-	"fmt"
 	"os"
 
 	"github.com/ModelsLab/modelslab-cli/internal/cmd"
@@ -17,7 +16,6 @@ var (
 func main() {
 	cmd.SetVersion(version, commit, date)
 	if err := cmd.Execute(); err != nil {
-		fmt.Fprintln(os.Stderr, err)
-		os.Exit(1)
+		os.Exit(cmd.ReportError(err))
 	}
 }
diff --git a/internal/api/client.go b/internal/api/client.go
index 7ad71ae..d1e4bd0 100644
--- a/internal/api/client.go
+++ b/internal/api/client.go
@@ -8,26 +8,61 @@ import (
 	"math"
 	"net/http"
 	"os"
+	"regexp"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
+	"unicode/utf8"
 
 	"github.com/google/uuid"
 )
 
 // Exit codes matching the design document
 const (
-	ExitSuccess       = 0
-	ExitGeneralError  = 1
-	ExitUsageError    = 2
-	ExitAuthError     = 3
-	ExitRateLimited   = 4
-	ExitNotFound      = 5
-	ExitPaymentError  = 6
-	ExitGenTimeout    = 7
-	ExitNetworkError  = 10
+	ExitSuccess      = 0
+	ExitGeneralError = 1
+	ExitUsageError   = 2
+	ExitAuthError    = 3
+	ExitRateLimited  = 4
+	ExitNotFound     = 5
+	ExitPaymentError = 6
+	ExitGenTimeout   = 7
+	ExitNetworkError = 10
 )
 
+// maxRawErrorBody caps how much of an unrecognised error body is echoed back.
+const maxRawErrorBody = 300
+
+// htmlTitle pulls the <title> out of a proxy's error page.
+var htmlTitle = regexp.MustCompile(`(?is)<title[^>]*>(.*?)</title>`)
+
+// maxRateLimitWait is the longest window the client will sleep through before it
+// gives the terminal back. Laravel's throttle routinely names 60s; blocking that
+// long without a word looks like a hang.
+const maxRateLimitWait = 30 * time.Second
+
+// rateLimitWait reads how long the server asked us to wait. Retry-After is the
+// standard header and the one Laravel's throttle middleware sends;
+// X-RateLimit-Reset is an absolute unix timestamp.
+func rateLimitWait(header http.Header) time.Duration {
+	if value := header.Get("Retry-After"); value != "" {
+		if seconds, err := strconv.ParseInt(value, 10, 64); err == nil && seconds > 0 {
+			return time.Duration(seconds) * time.Second
+		}
+	}
+
+	if value := header.Get("X-RateLimit-Reset"); value != "" {
+		if resetAt, err := strconv.ParseInt(value, 10, 64); err == nil {
+			if seconds := resetAt - time.Now().Unix(); seconds > 0 {
+				return time.Duration(seconds) * time.Second
+			}
+		}
+	}
+
+	return 0
+}
+
 type Client struct {
 	BaseURL    string
 	Token      string // Bearer token for control plane
@@ -38,8 +73,34 @@ type Client struct {
 
 type APIError struct {
 	StatusCode int
+	Code       string
 	Message    string
-	ExitCode   int
+	// Details carries the control plane's per-field validation errors, keyed by
+	// field name. Dropping it left the CLI guessing which field the server
+	// actually rejected.
+	Details  map[string][]string
+	ExitCode int
+}
+
+// FieldErrors renders Details as "field: message" lines, sorted for stable output.
+func (e *APIError) FieldErrors() []string {
+	if len(e.Details) == 0 {
+		return nil
+	}
+
+	fields := make([]string, 0, len(e.Details))
+	for field := range e.Details {
+		fields = append(fields, field)
+	}
+	sort.Strings(fields)
+
+	lines := make([]string, 0, len(fields))
+	for _, field := range fields {
+		for _, message := range e.Details[field] {
+			lines = append(lines, field+": "+message)
+		}
+	}
+	return lines
 }
 
 func (e *APIError) Error() string {
@@ -171,20 +232,22 @@ func (c *Client) doRequest(method, path string, body interface{}, result interfa
 
 		// Handle rate limiting
 		if resp.StatusCode == 429 {
-			if attempt < maxRetries {
-				waitTime := math.Pow(2, float64(attempt))
-				if reset := resp.Header.Get("X-RateLimit-Reset"); reset != "" {
-					if resetTime, err := strconv.ParseInt(reset, 10, 64); err == nil {
-						waitSecs := resetTime - time.Now().Unix()
-						if waitSecs > 0 && waitSecs < 30 {
-							waitTime = float64(waitSecs)
-						}
-					}
+			retryAfter := rateLimitWait(resp.Header)
+			if attempt < maxRetries && retryAfter <= maxRateLimitWait {
+				waitTime := time.Duration(math.Pow(2, float64(attempt))) * time.Second
+				if retryAfter > 0 {
+					waitTime = retryAfter
 				}
-				time.Sleep(time.Duration(waitTime) * time.Second)
+				time.Sleep(waitTime)
 				continue
 			}
-			return &APIError{StatusCode: 429, Message: "Rate limited. Please try again later.", ExitCode: ExitRateLimited}
+			// Keep the server's own message and the window it named. The old
+			// blanket "try again later" sent people straight back into the wall.
+			apiErr := parseAPIError(429, respBody)
+			if retryAfter > 0 {
+				apiErr.Message = fmt.Sprintf("%s Retry in %s.", apiErr.Message, retryAfter)
+			}
+			return apiErr
 		}
 
 		// Handle server errors with retry
@@ -230,18 +293,106 @@ func parseAPIError(statusCode int, body []byte) *APIError {
 		exitCode = ExitRateLimited
 	}
 
-	// Try to extract message from JSON response
+	code, message, details := extractAPIErrorFields(body)
+	if message == "" {
+		message = summarizeRawBody(body, statusCode)
+	}
+
+	return &APIError{StatusCode: statusCode, Code: code, Message: message, Details: details, ExitCode: exitCode}
+}
+
+// extractAPIErrorFields pulls the machine code and the human message out of an
+// error body. The control plane wraps every failure as
+//
+//	{"data":null,"error":{"code":"invalid_credentials","message":"..."},"meta":{...}}
+//
+// so the message is NOT at the top level and "error" is an object, not a string.
+// Reading only the top level printed the whole JSON blob back at the user on
+// every failed login. The flatter shapes are still handled: some legacy
+// endpoints answer {"message":...} or {"error":"..."}.
+func extractAPIErrorFields(body []byte) (string, string, map[string][]string) {
 	var errResp map[string]interface{}
-	message := string(body)
-	if err := json.Unmarshal(body, &errResp); err == nil {
-		if msg, ok := errResp["message"].(string); ok {
-			message = msg
-		} else if msg, ok := errResp["error"].(string); ok {
-			message = msg
+	if err := json.Unmarshal(body, &errResp); err != nil {
+		return "", "", nil
+	}
+
+	if errObj, ok := errResp["error"].(map[string]interface{}); ok {
+		code, _ := errObj["code"].(string)
+		message, _ := errObj["message"].(string)
+		if message == "" {
+			message, _ = errResp["message"].(string)
 		}
+		return code, message, parseErrorDetails(errObj["details"])
+	}
+
+	if msg, ok := errResp["message"].(string); ok && msg != "" {
+		code, _ := errResp["code"].(string)
+		return code, msg, parseErrorDetails(errResp["errors"])
+	}
+
+	if msg, ok := errResp["error"].(string); ok && msg != "" {
+		code, _ := errResp["code"].(string)
+		return code, msg, parseErrorDetails(errResp["errors"])
+	}
+
+	return "", "", nil
+}
+
+// parseErrorDetails normalises Laravel's per-field error bag. It arrives as
+// {"field": ["message", ...]}, but an empty bag serialises as [] rather than {},
+// and some codes put a flat object there instead.
+func parseErrorDetails(raw interface{}) map[string][]string {
+	bag, ok := raw.(map[string]interface{})
+	if !ok || len(bag) == 0 {
+		return nil
 	}
 
-	return &APIError{StatusCode: statusCode, Message: message, ExitCode: exitCode}
+	details := make(map[string][]string, len(bag))
+	for field, value := range bag {
+		switch typed := value.(type) {
+		case string:
+			details[field] = []string{typed}
+		case []interface{}:
+			for _, item := range typed {
+				if message, ok := item.(string); ok {
+					details[field] = append(details[field], message)
+				}
+			}
+		}
+	}
+
+	if len(details) == 0 {
+		return nil
+	}
+	return details
+}
+
+// summarizeRawBody is the last resort when the body carries no message we
+// recognise. An unparseable body is often an HTML error page from a proxy, and
+// dumping the whole thing into the terminal helps nobody.
+func summarizeRawBody(body []byte, statusCode int) string {
+	raw := strings.TrimSpace(string(body))
+	if raw == "" {
+		return fmt.Sprintf("Request failed with HTTP %d.", statusCode)
+	}
+
+	// A proxy error page is HTML. Its <title> ("502 Bad Gateway") is the only
+	// part worth showing; the markup around it is noise in a terminal.
+	if match := htmlTitle.FindSubmatch(body); match != nil {
+		if title := strings.TrimSpace(string(match[1])); title != "" {
+			return fmt.Sprintf("%s (HTTP %d)", title, statusCode)
+		}
+	}
+
+	if len(raw) > maxRawErrorBody {
+		// Slice on a rune boundary; a body cut mid-rune renders as U+FFFD.
+		cut := maxRawErrorBody
+		for cut > 0 && !utf8.RuneStart(raw[cut]) {
+			cut--
+		}
+		return raw[:cut] + "…"
+	}
+	return raw
 }
 
 // GenerateIdempotencyKey creates a UUID for idempotent billing operations.
diff --git a/internal/api/client_test.go b/internal/api/client_test.go
index bbc2745..daac42f 100644
--- a/internal/api/client_test.go
+++ b/internal/api/client_test.go
@@ -4,7 +4,11 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"strconv"
+	"strings"
 	"testing"
+	"time"
+	"unicode/utf8"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -286,3 +290,121 @@ func TestDoControlPlane_ServerError_Retry(t *testing.T) {
 	assert.Equal(t, "ok", result["status"])
 	assert.GreaterOrEqual(t, attempts, 3)
 }
+
+// The control plane wraps every failure as {"data":null,"error":{...},"meta":{...}}.
+// Reading only the top level used to print the whole JSON blob at the user.
+func TestParseAPIError_ControlPlaneEnvelope(t *testing.T) {
+	body := `{"data":null,"error":{"code":"invalid_credentials","message":"Email or password is incorrect.","details":[]},"meta":{"request_id":"abc"}}`
+
+	err := parseAPIError(401, []byte(body))
+
+	assert.Equal(t, "Email or password is incorrect.", err.Message)
+	assert.Equal(t, "invalid_credentials", err.Code)
+	assert.Equal(t, ExitAuthError, err.ExitCode)
+}
+
+func TestParseAPIError_ControlPlaneEnvelope_Unverified(t *testing.T) {
+	body := `{"data":null,"error":{"code":"email_not_verified","message":"Email is not verified.","details":{"verification_required":true}},"meta":{}}`
+
+	err := parseAPIError(403, []byte(body))
+
+	assert.Equal(t, "Email is not verified.", err.Message)
+	assert.Equal(t, "email_not_verified", err.Code)
+	assert.Equal(t, ExitAuthError, err.ExitCode)
+}
+
+func TestParseAPIError_TruncatesUnrecognisedBody(t *testing.T) {
+	body := strings.Repeat("<html>proxy error</html>", 100)
+
+	err := parseAPIError(502, []byte(body))
+
+	assert.LessOrEqual(t, len(err.Message), maxRawErrorBody+len("…"))
+	assert.Empty(t, err.Code)
+}
+
+func TestParseAPIError_EmptyBody(t *testing.T) {
+	err := parseAPIError(500, nil)
+
+	assert.Equal(t, "Request failed with HTTP 500.", err.Message)
+}
+
+func TestParseAPIError_KeepsValidationDetails(t *testing.T) {
+	body := `{"data":null,"error":{"code":"validation_error","message":"Invalid login payload.","details":{"token_expiry":["The selected token expiry is invalid."],"email":["The email field is required."]}},"meta":{}}`
+
+	err := parseAPIError(422, []byte(body))
+
+	assert.Equal(t, "validation_error", err.Code)
+	assert.Equal(t, []string{
+		"email: The email field is required.",
+		"token_expiry: The selected token expiry is invalid.",
+	}, err.FieldErrors())
+}
+
+// An empty details bag serialises as [] rather than {}.
+func TestParseAPIError_EmptyDetailsBag(t *testing.T) {
+	err := parseAPIError(401, []byte(`{"data":null,"error":{"code":"invalid_credentials","message":"Nope.","details":[]}}`))
+
+	assert.Nil(t, err.Details)
+	assert.Nil(t, err.FieldErrors())
+}
+
+func TestParseAPIError_TruncationLandsOnARuneBoundary(t *testing.T) {
+	err := parseAPIError(502, []byte(strings.Repeat("é", 400)))
+
+	assert.True(t, utf8.ValidString(err.Message), "truncated body must stay valid UTF-8")
+}
+
+// Laravel's throttle sends Retry-After; the old code read only
+// X-RateLimit-Reset and replaced the server's message with a blanket one.
+func TestRateLimitWait_PrefersRetryAfter(t *testing.T) {
+	header := http.Header{}
+	header.Set("Retry-After", "45")
+
+	assert.Equal(t, 45*time.Second, rateLimitWait(header))
+}
+
+func TestRateLimitWait_FallsBackToResetTimestamp(t *testing.T) {
+	header := http.Header{}
+	header.Set("X-RateLimit-Reset", strconv.FormatInt(time.Now().Unix()+12, 10))
+
+	wait := rateLimitWait(header)
+
+	assert.Greater(t, wait, 10*time.Second)
+	assert.LessOrEqual(t, wait, 12*time.Second)
+}
+
+func TestRateLimitWait_NoHeaders(t *testing.T) {
+	assert.Zero(t, rateLimitWait(http.Header{}))
+}
+
+// A long Retry-After must surface immediately instead of blocking the terminal,
+// and must keep the server's own wording and window.
+func TestDoControlPlane_LongRateLimitReturnsImmediately(t *testing.T) {
+	attempts := 0
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		attempts++
+		w.Header().Set("Retry-After", "60")
+		w.WriteHeader(429)
+		w.Write([]byte(`{"data":null,"error":{"code":"rate_limited","message":"Too Many Attempts."}}`))
+	}))
+	defer server.Close()
+
+	client := NewClient(server.URL, "token", "")
+	err := client.DoControlPlane("GET", "/api-keys", nil, nil)
+
+	require.Error(t, err)
+	apiErr := err.(*APIError)
+	assert.Equal(t, ExitRateLimited, apiErr.ExitCode)
+	assert.Contains(t, apiErr.Message, "Too Many Attempts.")
+	assert.Contains(t, apiErr.Message, "1m0s")
+	assert.Equal(t, 1, attempts, "a 60s window must not be slept through")
+}
+
+// A proxy error page is HTML; its <title> is the only part worth a terminal line.
+func TestParseAPIError_SummarisesAProxyErrorPage(t *testing.T) {
+	body := `<html><head><title>502 Bad Gateway</title></head><body><center><h1>502 Bad Gateway</h1></center><hr><center>nginx</center></body></html>`
+
+	err := parseAPIError(502, []byte(body))
+
+	assert.Equal(t, "502 Bad Gateway (HTTP 502)", err.Message)
+}
diff --git a/internal/cmd/auth.go b/internal/cmd/auth.go
index 9c2bca7..0bfb473 100644
--- a/internal/cmd/auth.go
+++ b/internal/cmd/auth.go
@@ -15,14 +15,12 @@ import (
 	"runtime"
 	"strconv"
 	"strings"
-	"syscall"
 	"time"
 
 	"github.com/ModelsLab/modelslab-cli/internal/api"
 	"github.com/ModelsLab/modelslab-cli/internal/auth"
 	"github.com/ModelsLab/modelslab-cli/internal/output"
 	"github.com/spf13/cobra"
-	"golang.org/x/term"
 )
 
 var authCmd = &cobra.Command{
@@ -61,17 +59,18 @@ var authLoginCmd = &cobra.Command{
 		deviceName, _ := cmd.Flags().GetString("device-name")
 
 		if email == "" {
-			fmt.Print("Email: ")
-			fmt.Scanln(&email)
+			value, err := promptLine("Email: ")
+			if err != nil {
+				return err
+			}
+			email = value
 		}
 		if password == "" {
-			fmt.Print("Password: ")
-			bytePw, err := term.ReadPassword(int(syscall.Stdin))
+			value, err := promptSecret("Password: ")
 			if err != nil {
-				return fmt.Errorf("could not read password: %w", err)
+				return err
 			}
-			password = string(bytePw)
-			fmt.Println()
+			password = value
 		}
 
 		if expiry == "" {
@@ -94,7 +93,7 @@ var authLoginCmd = &cobra.Command{
 		if err != nil {
 			apiErr, ok := err.(*api.APIError)
 			if ok {
-				output.PrintError(apiErr.Message, "Check your email and password.", "Run: modelslab auth forgot-password")
+				output.PrintError(apiErr.Message, loginFailureHints(apiErr, email)...)
 				os.Exit(apiErr.ExitCode)
 			}
 			return err
@@ -110,7 +109,9 @@ var authLoginCmd = &cobra.Command{
 			}
 			// Also store API key if returned
 			if k, ok := data["api_key"].(string); ok && k != "" {
-				auth.StoreAPIKey(flagProfile, k)
+				if err := auth.StoreAPIKey(flagProfile, k); err != nil {
+					return fmt.Errorf("logged in, but could not store the API key: %w", err)
+				}
 			}
 		} else if t, ok := result["token"].(string); ok {
 			token = t
@@ -122,9 +123,17 @@ var authLoginCmd = &cobra.Command{
 			return fmt.Errorf("no token returned from login")
 		}
 
-		// Store credentials
-		auth.StoreToken(flagProfile, token)
-		auth.StoreEmail(flagProfile, email)
+		// Checked, not fire-and-forget. When both the keychain and the file
+		// fallback fail — a read-only ~/.config, a denied keychain prompt — the
+		// CLI used to print "Logged in" and exit 0 while storing nothing, and
+		// every later command 401'd for no visible reason.
+		if err := auth.StoreToken(flagProfile, token); err != nil {
+			return fmt.Errorf("logged in, but could not store the access token: %w", err)
+		}
+		if err := auth.StoreEmail(flagProfile, email); err != nil {
+			return fmt.Errorf("logged in, but could not store the account email: %w", err)
+		}
+		apiClient = nil
 
 		outputResult(result, func() {
 			output.PrintSuccess(fmt.Sprintf("Logged in as %s (profile: %s)", email, flagProfile))
@@ -134,6 +143,50 @@ var authLoginCmd = &cobra.Command{
 	},
 }
 
+// loginFailureHints turns a control-plane error code into next steps a user can
+// actually act on.
+//
+// The old blanket "check your email and password" was wrong for the two failures
+// people actually hit. An account created through Google or GitHub is stored with
+// a random password nobody ever sees, so email+password login can never succeed
+// for it and the only way in is `--browser`. And an unverified account fails with
+// credentials that are perfectly correct.
+func loginFailureHints(apiErr *api.APIError, email string) []string {
+	switch apiErr.Code {
+	case "email_not_verified":
+		hint := "Run: modelslab auth resend-verification"
+		if email != "" {
+			hint += " --email " + email
+		}
+		return []string{"This account exists but its email is not verified yet.", hint}
+	case "access_denied":
+		return []string{"Contact support@modelslab.com if you think this is a mistake."}
+	case "validation_error":
+		// Say which field the server rejected instead of guessing at --email.
+		if fields := apiErr.FieldErrors(); len(fields) > 0 {
+			return append([]string{"The server rejected these values:"}, fields...)
+		}
+		return []string{"The server rejected the login payload — check --email and --expiry."}
+	case "invalid_credentials":
+		return []string{
+			"Check your email and password.",
+			"Signed up with Google or GitHub? That account has no password — run: modelslab auth login --browser",
+			"Otherwise run: modelslab auth forgot-password",
+		}
+	case "":
+		// No code means we could not read the response at all — a proxy error
+		// page, a captive portal, an outage. Saying "check your password" here
+		// sends people to reset a password that was never the problem.
+		return []string{
+			fmt.Sprintf("The server did not return a recognisable error (HTTP %d).", apiErr.StatusCode),
+			"This is usually a network or service problem, not your credentials. Try again shortly.",
+			"Check https://modelslab.com/status, or pass --base-url if you are pointing at a non-default host.",
+		}
+	default:
+		return []string{"Run: modelslab auth login --browser", "Or run: modelslab auth forgot-password"}
+	}
+}
+
 func runBrowserLogin(cmd *cobra.Command) error {
 	expiry, _ := cmd.Flags().GetString("expiry")
 	deviceName, _ := cmd.Flags().GetString("device-name")
@@ -199,13 +252,21 @@ func runBrowserLogin(cmd *cobra.Command) error {
 		return err
 	}
 
-	if noOpen {
-		fmt.Fprintf(os.Stderr, "Open this URL in Chrome to authorize ModelsLab CLI:\n%s\n\n", loginURL)
-	} else {
-		fmt.Fprintln(os.Stderr, "Opening Google Chrome for ModelsLab login...")
+	/*
+	 * The URL is printed unconditionally, before any attempt to open it.
+	 *
+	 * openBrowser uses exec.Start(), which returns nil the moment the child is
+	 * spawned — so `xdg-open` with no display, or a Chrome that dies on launch,
+	 * both looked like success. The user saw "Waiting for browser
+	 * authorization..." and then a timeout five minutes later, and was never
+	 * once shown the URL they could have pasted somewhere that works.
+	 */
+	fmt.Fprintf(os.Stderr, "Authorize ModelsLab CLI at:\n%s\n\n", loginURL)
+
+	if !noOpen {
+		fmt.Fprintln(os.Stderr, "Opening your browser...")
 		if err := openBrowser(loginURL); err != nil {
-			fmt.Fprintf(os.Stderr, "Could not open Chrome automatically: %v\n", err)
-			fmt.Fprintf(os.Stderr, "Open this URL manually:\n%s\n\n", loginURL)
+			fmt.Fprintf(os.Stderr, "Could not open a browser automatically (%v) — open the URL above yourself.\n", err)
 		}
 	}
 	fmt.Fprintln(os.Stderr, "Waiting for browser authorization...")
@@ -219,7 +280,13 @@ func runBrowserLogin(cmd *cobra.Command) error {
 	case err := <-serveErrCh:
 		return fmt.Errorf("OAuth callback server failed: %w", err)
 	case <-timer.C:
-		return fmt.Errorf("browser login timed out after %s", timeout)
+		return fmt.Errorf(
+			"browser login timed out after %s.\n"+
+				"  Open the URL above and finish the grant there. If the browser sent you to the model\n"+
+				"  catalogue instead of a grant page, sign in at https://modelslab.com/login first and retry.\n"+
+				"  Raise the wait with --timeout, or print the URL without opening a browser with --no-open",
+			timeout,
+		)
 	}
 
 	if callback.Error != "" {
@@ -402,21 +469,25 @@ var authSignupCmd = &cobra.Command{
 		name, _ := cmd.Flags().GetString("name")
 
 		if email == "" {
-			fmt.Print("Email: ")
-			fmt.Scanln(&email)
+			value, err := promptLine("Email: ")
+			if err != nil {
+				return err
+			}
+			email = value
 		}
 		if password == "" {
-			fmt.Print("Password: ")
-			bytePw, err := term.ReadPassword(int(syscall.Stdin))
+			value, err := promptSecret("Password: ")
 			if err != nil {
-				return fmt.Errorf("could not read password: %w", err)
+				return err
 			}
-			password = string(bytePw)
-			fmt.Println()
+			password = value
 		}
 		if name == "" {
-			fmt.Print("Name: ")
-			fmt.Scanln(&name)
+			value, err := promptLine("Name: ")
+			if err != nil {
+				return err
+			}
+			name = value
 		}
 
 		client := getClient()
@@ -523,8 +594,11 @@ var authForgotPasswordCmd = &cobra.Command{
 	RunE: func(cmd *cobra.Command, args []string) error {
 		email, _ := cmd.Flags().GetString("email")
 		if email == "" {
-			fmt.Print("Email: ")
-			fmt.Scanln(&email)
+			value, err := promptLine("Email: ")
+			if err != nil {
+				return err
+			}
+			email = value
 		}
 
 		client := getClient()
@@ -578,8 +652,11 @@ var authResendVerificationCmd = &cobra.Command{
 	RunE: func(cmd *cobra.Command, args []string) error {
 		email, _ := cmd.Flags().GetString("email")
 		if email == "" {
-			fmt.Print("Email: ")
-			fmt.Scanln(&email)
+			value, err := promptLine("Email: ")
+			if err != nil {
+				return err
+			}
+			email = value
 		}
 
 		client := getClient()
diff --git a/internal/cmd/auth_hints_test.go b/internal/cmd/auth_hints_test.go
new file mode 100644
index 0000000..af60e49
--- /dev/null
+++ b/internal/cmd/auth_hints_test.go
@@ -0,0 +1,62 @@
+package cmd
+
+import (
+	"testing"
+
+	"github.com/ModelsLab/modelslab-cli/internal/api"
+	"github.com/stretchr/testify/assert"
+)
+
+func hintsFor(code string, status int, details map[string][]string) []string {
+	return loginFailureHints(&api.APIError{Code: code, StatusCode: status, Details: details}, "maxx@example.com")
+}
+
+// An OAuth-only account has no password, so "check your email and password" is a
+// dead end — the hint has to name the path that can actually work.
+func TestLoginFailureHints_InvalidCredentialsPointsAtBrowserLogin(t *testing.T) {
+	hints := hintsFor("invalid_credentials", 401, nil)
+
+	assert.Contains(t, joined(hints), "auth login --browser")
+	assert.Contains(t, joined(hints), "forgot-password")
+}
+
+// A proxy 502 carries no error code. Telling that user their password is wrong
+// is the multi-day rabbit hole this whole ticket is about.
+func TestLoginFailureHints_UnrecognisedErrorIsNotBlamedOnCredentials(t *testing.T) {
+	hints := hintsFor("", 502, nil)
+
+	assert.NotContains(t, joined(hints), "Check your email and password")
+	assert.NotContains(t, joined(hints), "forgot-password")
+	assert.Contains(t, joined(hints), "502")
+	assert.Contains(t, joined(hints), "network or service problem")
+}
+
+func TestLoginFailureHints_UnverifiedEmailOffersResend(t *testing.T) {
+	hints := hintsFor("email_not_verified", 403, nil)
+
+	assert.Contains(t, joined(hints), "resend-verification --email maxx@example.com")
+}
+
+// The old hint blamed --email whatever the server actually rejected.
+func TestLoginFailureHints_ValidationErrorNamesTheRejectedField(t *testing.T) {
+	hints := hintsFor("validation_error", 422, map[string][]string{
+		"token_expiry": {"The selected token expiry is invalid."},
+	})
+
+	assert.Contains(t, joined(hints), "token_expiry: The selected token expiry is invalid.")
+	assert.NotContains(t, joined(hints), "Check the email address")
+}
+
+func TestLoginFailureHints_ValidationErrorWithoutDetailsStillGuides(t *testing.T) {
+	hints := hintsFor("validation_error", 422, nil)
+
+	assert.Contains(t, joined(hints), "--email")
+}
+
+func joined(hints []string) string {
+	out := ""
+	for _, hint := range hints {
+		out += hint + "\n"
+	}
+	return out
+}
diff --git a/internal/cmd/exit.go b/internal/cmd/exit.go
new file mode 100644
index 0000000..f2540be
--- /dev/null
+++ b/internal/cmd/exit.go
@@ -0,0 +1,56 @@
+package cmd
+
+import (
+	"errors"
+	"os"
+
+	"github.com/ModelsLab/modelslab-cli/internal/api"
+	"github.com/ModelsLab/modelslab-cli/internal/auth"
+	"github.com/ModelsLab/modelslab-cli/internal/output"
+)
+
+// ReportError prints a failed command's error and returns the process exit code.
+//
+// Only `auth login` used to map an APIError to its documented exit code; every
+// other command exited 1, so a script could not tell an expired token (3) from a
+// bad flag (2) or a rate limit (4). And a 401 was reported as a bare
+// "Unauthenticated." with no hint that the CLI had simply never been logged in —
+// which is what it looks like when the keychain is locked and getClient()
+// silently sends no Authorization header at all.
+func ReportError(err error) int {
+	var apiErr *api.APIError
+	if !errors.As(err, &apiErr) {
+		output.PrintError(err.Error())
+		return api.ExitGeneralError
+	}
+
+	output.PrintError(apiErr.Message, commandFailureHints(apiErr)...)
+
+	return apiErr.ExitCode
+}
+
+func commandFailureHints(apiErr *api.APIError) []string {
+	hints := apiErr.FieldErrors()
+
+	if apiErr.ExitCode == api.ExitAuthError && !hasStoredCredentials() {
+		hints = append(hints,
+			"No credentials are stored for profile "+flagProfile+".",
+			"Run: modelslab auth login --browser",
+			"If you are logged in, your OS keychain may be locked — check: modelslab auth status",
+		)
+	}
+
+	return hints
+}
+
+func hasStoredCredentials() bool {
+	if os.Getenv("MODELSLAB_TOKEN") != "" || flagAPIKey != "" {
+		return true
+	}
+	if token, err := auth.GetToken(flagProfile); err == nil && token != "" {
+		return true
+	}
+	key, err := auth.GetAPIKey(flagProfile)
+
+	return err == nil && key != ""
+}
diff --git a/internal/cmd/exit_test.go b/internal/cmd/exit_test.go
new file mode 100644
index 0000000..5cca2e2
--- /dev/null
+++ b/internal/cmd/exit_test.go
@@ -0,0 +1,32 @@
+package cmd
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/ModelsLab/modelslab-cli/internal/api"
+	"github.com/stretchr/testify/assert"
+)
+
+// Only `auth login` used to map an APIError to its documented exit code; every
+// other command exited 1, so a script could not tell an expired token from a
+// bad flag.
+func TestReportError_UsesTheApiExitCode(t *testing.T) {
+	cases := map[int]error{
+		api.ExitAuthError:    &api.APIError{StatusCode: 401, Message: "Unauthenticated.", ExitCode: api.ExitAuthError},
+		api.ExitRateLimited:  &api.APIError{StatusCode: 429, Message: "Too Many Attempts.", ExitCode: api.ExitRateLimited},
+		api.ExitNotFound:     &api.APIError{StatusCode: 404, Message: "Not found.", ExitCode: api.ExitNotFound},
+		api.ExitPaymentError: &api.APIError{StatusCode: 402, Message: "Insufficient balance.", ExitCode: api.ExitPaymentError},
+		api.ExitGeneralError: errors.New("something else"),
+	}
+
+	for want, err := range cases {
+		assert.Equal(t, want, ReportError(err))
+	}
+}
+
+func TestReportError_WrappedApiErrorStillMaps(t *testing.T) {
+	wrapped := errors.Join(errors.New("context"), &api.APIError{StatusCode: 401, ExitCode: api.ExitAuthError})
+
+	assert.Equal(t, api.ExitAuthError, ReportError(wrapped))
+}
diff --git a/internal/cmd/mcp.go b/internal/cmd/mcp.go
index c8e30ab..971b609 100644
--- a/internal/cmd/mcp.go
+++ b/internal/cmd/mcp.go
@@ -20,7 +20,7 @@ var mcpServeCmd = &cobra.Command{
 		transport, _ := cmd.Flags().GetString("transport")
 
 		client := getClient()
-		server := mcpserver.NewServer(client)
+		server := mcpserver.NewServer(client, flagProfile)
 
 		switch transport {
 		case "stdio":
@@ -39,7 +39,7 @@ var mcpToolsCmd = &cobra.Command{
 	Short: "List available MCP tools",
 	RunE: func(cmd *cobra.Command, args []string) error {
 		client := getClient()
-		server := mcpserver.NewServer(client)
+		server := mcpserver.NewServer(client, flagProfile)
 		tools := server.ListTools()
 
 		outputResult(tools, func() {
diff --git a/internal/cmd/prompt.go b/internal/cmd/prompt.go
new file mode 100644
index 0000000..320f573
--- /dev/null
+++ b/internal/cmd/prompt.go
@@ -0,0 +1,76 @@
+package cmd
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"strings"
+	"syscall"
+
+	"golang.org/x/term"
+)
+
+// errNoInput is returned when a prompt gets nothing to work with.
+var errNoInput = errors.New("no input provided")
+
+// stdinReader is shared so a secret read from a pipe does not lose whatever the
+// next prompt was going to read. bufio would swallow it in its own buffer.
+var stdinReader = bufio.NewReader(os.Stdin)
+
+// promptLine reads one whole line.
+//
+// It replaces fmt.Scanln, which had two failure modes here: it stops at the
+// first space, so `auth signup` recorded "Ada" for "Ada Lovelace" and left the
+// rest to corrupt the next prompt; and on an empty line it returns an error the
+// caller ignored, so the command carried on with an empty email and the server
+// answered with a validation error about a flag the user never passed.
+func promptLine(label string) (string, error) {
+	fmt.Fprint(os.Stderr, label)
+
+	line, err := stdinReader.ReadString('\n')
+	if err != nil && !errors.Is(err, io.EOF) {
+		return "", fmt.Errorf("could not read %s: %w", fieldName(label), err)
+	}
+
+	line = strings.TrimSpace(line)
+	if line == "" {
+		return "", fmt.Errorf("%s: %w", fieldName(label), errNoInput)
+	}
+
+	return line, nil
+}
+
+// promptSecret reads a secret without echoing it.
+//
+// When stdin is not a terminal — a pipe, a heredoc, CI — term.ReadPassword fails
+// with "inappropriate ioctl for device", which used to abort the login entirely.
+// The only remaining non-interactive option was --password on the command line,
+// which lands in shell history, ps output and CI logs. Fall back to reading the
+// line instead; there is no echo to suppress when nobody is typing.
+func promptSecret(label string) (string, error) {
+	if !term.IsTerminal(int(syscall.Stdin)) {
+		return promptLine(label)
+	}
+
+	fmt.Fprint(os.Stderr, label)
+	raw, err := term.ReadPassword(int(syscall.Stdin))
+	fmt.Fprintln(os.Stderr)
+	if err != nil {
+		return "", fmt.Errorf("could not read %s: %w", fieldName(label), err)
+	}
+
+	secret := strings.TrimRight(string(raw), "\r\n")
+	if secret == "" {
+		return "", fmt.Errorf("%s: %w", fieldName(label), errNoInput)
+	}
+
+	return secret, nil
+}
+
+// fieldName turns a prompt label ("Email: ") into something an error can read
+// naturally ("email").
+func fieldName(label string) string {
+	return strings.ToLower(strings.TrimSpace(strings.TrimSuffix(strings.TrimSpace(label), ":")))
+}
diff --git a/internal/cmd/prompt_test.go b/internal/cmd/prompt_test.go
new file mode 100644
index 0000000..372fd67
--- /dev/null
+++ b/internal/cmd/prompt_test.go
@@ -0,0 +1,66 @@
+package cmd
+
+import (
+	"bufio"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func withStdin(t *testing.T, input string) {
+	t.Helper()
+	original := stdinReader
+	stdinReader = bufio.NewReader(strings.NewReader(input))
+	t.Cleanup(func() { stdinReader = original })
+}
+
+// fmt.Scanln stopped at the first space, so `auth signup` recorded "Ada" for
+// "Ada Lovelace" and left "Lovelace" to corrupt the next prompt.
+func TestPromptLine_KeepsTheWholeLine(t *testing.T) {
+	withStdin(t, "Ada Lovelace\nnext-value\n")
+
+	name, err := promptLine("Name: ")
+	require.NoError(t, err)
+	assert.Equal(t, "Ada Lovelace", name)
+
+	next, err := promptLine("Next: ")
+	require.NoError(t, err)
+	assert.Equal(t, "next-value", next)
+}
+
+// An empty line used to leave the field empty and let the command run on, so the
+// server answered with a validation error about a flag the user never passed.
+func TestPromptLine_RejectsEmptyInput(t *testing.T) {
+	withStdin(t, "\n")
+
+	_, err := promptLine("Email: ")
+
+	require.ErrorIs(t, err, errNoInput)
+	assert.Contains(t, err.Error(), "email")
+}
+
+func TestPromptLine_RejectsClosedStdin(t *testing.T) {
+	withStdin(t, "")
+
+	_, err := promptLine("Email: ")
+
+	require.ErrorIs(t, err, errNoInput)
+}
+
+// term.ReadPassword fails with "inappropriate ioctl for device" when stdin is a
+// pipe, which aborted every non-interactive login.
+func TestPromptSecret_FallsBackWhenStdinIsNotATerminal(t *testing.T) {
+	withStdin(t, "hunter2\n")
+
+	secret, err := promptSecret("Password: ")
+
+	require.NoError(t, err)
+	assert.Equal(t, "hunter2", secret)
+}
+
+func TestFieldName(t *testing.T) {
+	assert.Equal(t, "email", fieldName("Email: "))
+	assert.Equal(t, "password", fieldName("Password: "))
+}
diff --git a/internal/config/config.go b/internal/config/config.go
index 03c2e9f..b804051 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/spf13/viper"
 )
@@ -74,13 +75,50 @@ func Init() error {
 		projectViper.SetConfigFile(projectConfig)
 		projectViper.SetConfigType("toml")
 		if err := projectViper.ReadInConfig(); err == nil {
-			viper.MergeConfigMap(projectViper.AllSettings())
+			viper.MergeConfigMap(stripUntrustedProjectKeys(projectViper.AllSettings()))
 		}
 	}
 
 	return nil
 }
 
+/*
+ * untrustedProjectKeys are the settings a project-level config may NOT set.
+ *
+ * .modelslab/config.toml is read from the CURRENT WORKING DIRECTORY, so any
+ * repository you clone and cd into gets a say in it. Merged wholesale, a
+ * committed `base_url = "http://attacker/"` was enough to make the next
+ * `modelslab` command send the user's stored bearer token to that host —
+ * no prompt, no warning. `api_key` and `token` are just as bad in the other
+ * direction: a checked-out repo could silently swap in its own credentials and
+ * bill someone else's account.
+ *
+ * Everything harmless — default model, output format, output dir — still merges.
+ * Point the CLI at another host with --base-url or MODELSLAB_BASE_URL, both of
+ * which the user types themselves.
+ */
+var untrustedProjectKeys = []string{"api_key", "base_url", "token", "defaults.base_url"}
+
+func stripUntrustedProjectKeys(settings map[string]interface{}) map[string]interface{} {
+	for _, key := range untrustedProjectKeys {
+		parts := strings.Split(key, ".")
+		scope := settings
+		for _, part := range parts[:len(parts)-1] {
+			nested, ok := scope[part].(map[string]interface{})
+			if !ok {
+				scope = nil
+				break
+			}
+			scope = nested
+		}
+		if scope != nil {
+			delete(scope, parts[len(parts)-1])
+		}
+	}
+
+	return settings
+}
+
 func ConfigDir() string {
 	return configDir
 }
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 8b43433..0b12d65 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -61,3 +61,43 @@ func TestAllSettings(t *testing.T) {
 	settings := AllSettings()
 	assert.NotNil(t, settings)
 }
+
+// A .modelslab/config.toml is read from the CURRENT WORKING DIRECTORY, so any
+// repository you clone and cd into gets a say in it. Merged wholesale, a
+// committed base_url was enough to redirect the user's stored bearer token to an
+// attacker-controlled host on the next command.
+func TestStripUntrustedProjectKeys(t *testing.T) {
+	settings := map[string]interface{}{
+		"base_url": "http://attacker.example",
+		"api_key":  "ml-attacker-key",
+		"token":    "attacker-token",
+		"defaults": map[string]interface{}{
+			"base_url": "http://attacker.example",
+			"output":   "json",
+		},
+		"generation": map[string]interface{}{
+			"default_model": "sdxl",
+			"output_dir":    "./out",
+		},
+	}
+
+	cleaned := stripUntrustedProjectKeys(settings)
+
+	assert.NotContains(t, cleaned, "base_url")
+	assert.NotContains(t, cleaned, "api_key")
+	assert.NotContains(t, cleaned, "token")
+
+	defaults := cleaned["defaults"].(map[string]interface{})
+	assert.NotContains(t, defaults, "base_url")
+	assert.Equal(t, "json", defaults["output"], "harmless preferences must still merge")
+
+	generation := cleaned["generation"].(map[string]interface{})
+	assert.Equal(t, "sdxl", generation["default_model"])
+	assert.Equal(t, "./out", generation["output_dir"])
+}
+
+func TestStripUntrustedProjectKeys_ToleratesMissingSections(t *testing.T) {
+	cleaned := stripUntrustedProjectKeys(map[string]interface{}{"generation": "not-a-map"})
+
+	assert.Equal(t, "not-a-map", cleaned["generation"])
+}
diff --git a/internal/mcp/server.go b/internal/mcp/server.go
index dbf8e2d..0eccdaf 100644
--- a/internal/mcp/server.go
+++ b/internal/mcp/server.go
@@ -7,6 +7,7 @@ import (
 	"net/http"
 
 	"github.com/ModelsLab/modelslab-cli/internal/api"
+	"github.com/ModelsLab/modelslab-cli/internal/auth"
 	mcpgo "github.com/mark3labs/mcp-go/mcp"
 	"github.com/mark3labs/mcp-go/server"
 )
@@ -18,15 +19,56 @@ type ToolInfo struct {
 
 type Server struct {
 	client    *api.Client
+	profile   string
 	mcpServer *server.MCPServer
 }
 
-func NewServer(client *api.Client) *Server {
-	s := &Server{client: client}
+func NewServer(client *api.Client, profile string) *Server {
+	s := &Server{client: client, profile: profile}
 	s.init()
 	return s
 }
 
+/*
+ * applyCredentials pushes an access token or API key from a tool response onto
+ * the live client, and persists it for the next process.
+ *
+ * Storage failures are deliberately not fatal: the in-memory client is already
+ * updated, so the session works either way, and an MCP tool call is the wrong
+ * place to abort over a locked keychain.
+ */
+func (s *Server) applyCredentials(result map[string]interface{}) {
+	data, ok := result["data"].(map[string]interface{})
+	if !ok {
+		return
+	}
+
+	token, _ := data["access_token"].(string)
+	if token == "" {
+		token, _ = data["token"].(string)
+	}
+	if token != "" {
+		s.client.Token = token
+		if s.profile != "" {
+			_ = auth.StoreToken(s.profile, token)
+		}
+	}
+
+	if key, _ := data["api_key"].(string); key != "" {
+		s.client.APIKey = key
+		if s.profile != "" {
+			_ = auth.StoreAPIKey(s.profile, key)
+		}
+	}
+
+	if key, _ := data["key"].(string); key != "" {
+		s.client.APIKey = key
+		if s.profile != "" {
+			_ = auth.StoreAPIKey(s.profile, key)
+		}
+	}
+}
+
 func (s *Server) init() {
 	s.mcpServer = server.NewMCPServer(
 		"ModelsLab",
@@ -52,8 +94,20 @@ func (s *Server) registerControlPlaneTools() {
 		"required": []string{"email", "password"},
 	}, func(args map[string]interface{}) (interface{}, error) {
 		var result map[string]interface{}
-		err := s.client.DoControlPlane("POST", "/auth/login", args, &result)
-		return result, err
+		if err := s.client.DoControlPlane("POST", "/auth/login", args, &result); err != nil {
+			return nil, err
+		}
+		/*
+		 * Apply the credentials, do not just hand them back.
+		 *
+		 * The client is built once in `mcp serve` and lives for the whole
+		 * process. Returning the token without assigning it meant an agent that
+		 * started the server unauthenticated, called auth-login, and got a token
+		 * in the response then got 401 from every other tool for the life of the
+		 * process — with a valid token sitting in its own transcript.
+		 */
+		s.applyCredentials(result)
+		return result, nil
 	})
 
 	s.addTool("auth-signup", "Create a new ModelsLab account", map[string]interface{}{
@@ -106,8 +160,11 @@ func (s *Server) registerControlPlaneTools() {
 		},
 	}, func(args map[string]interface{}) (interface{}, error) {
 		var result map[string]interface{}
-		err := s.client.DoControlPlane("POST", "/api-keys", args, &result)
-		return result, err
+		if err := s.client.DoControlPlane("POST", "/api-keys", args, &result); err != nil {
+			return nil, err
+		}
+		s.applyCredentials(result)
+		return result, nil
 	})
 
 	// Models tools
diff --git a/internal/mcp/server_test.go b/internal/mcp/server_test.go
new file mode 100644
index 0000000..cba7bda
--- /dev/null
+++ b/internal/mcp/server_test.go
@@ -0,0 +1,61 @@
+package mcp
+
+import (
+	"testing"
+
+	"github.com/ModelsLab/modelslab-cli/internal/api"
+	"github.com/stretchr/testify/assert"
+)
+
+// The client is built once in `mcp serve` and lives for the whole process.
+// auth-login used to return the token without ever assigning it, so an agent
+// that logged in through MCP got 401 from every other tool for the life of the
+// process — with a valid token sitting in its own transcript.
+//
+// profile is empty here so the test only touches the in-memory client and never
+// writes to the developer's real keychain.
+func TestApplyCredentials_UpdatesTheLiveClient(t *testing.T) {
+	s := &Server{client: api.NewClient("https://modelslab.com", "", "")}
+
+	s.applyCredentials(map[string]interface{}{
+		"data": map[string]interface{}{
+			"access_token": "tok-123",
+			"api_key":      "ml-abc",
+		},
+	})
+
+	assert.Equal(t, "tok-123", s.client.Token)
+	assert.Equal(t, "ml-abc", s.client.APIKey)
+}
+
+func TestApplyCredentials_AcceptsTheLegacyTokenField(t *testing.T) {
+	s := &Server{client: api.NewClient("https://modelslab.com", "", "")}
+
+	s.applyCredentials(map[string]interface{}{
+		"data": map[string]interface{}{"token": "tok-legacy"},
+	})
+
+	assert.Equal(t, "tok-legacy", s.client.Token)
+}
+
+// api-keys-create returns the new key under "key".
+func TestApplyCredentials_PicksUpANewlyCreatedApiKey(t *testing.T) {
+	s := &Server{client: api.NewClient("https://modelslab.com", "tok", "ml-old")}
+
+	s.applyCredentials(map[string]interface{}{
+		"data": map[string]interface{}{"key": "ml-new"},
+	})
+
+	assert.Equal(t, "ml-new", s.client.APIKey)
+	assert.Equal(t, "tok", s.client.Token)
+}
+
+func TestApplyCredentials_LeavesTheClientAloneOnAnUnexpectedShape(t *testing.T) {
+	s := &Server{client: api.NewClient("https://modelslab.com", "tok", "ml-old")}
+
+	s.applyCredentials(map[string]interface{}{"data": "not-a-map"})
+	s.applyCredentials(map[string]interface{}{})
+
+	assert.Equal(t, "tok", s.client.Token)
+	assert.Equal(t, "ml-old", s.client.APIKey)
+}