Distinguish undeliverable auth prompts from user declines

An elicitation prompt that the client cannot deliver (a transport or
protocol failure) was treated the same as a user actively declining: any
display error cancelled the flow. That conflated a system failure with a
deliberate "no", so a client that advertised URL elicitation but failed
to deliver it would hard-fail the login instead of degrading.

Add an ErrPromptUnavailable sentinel alongside ErrPromptDeclined and have
the MCP adapter return it when Elicit fails at the transport level. The
manager now falls back to the manual user-action channel on an
undeliverable prompt (keeping the background flow alive so the user can
still authorize out of band), while a genuine decline still aborts. A
context-cancelled prompt is checked first so an ending flow is never
misread as a transport failure.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: SamMorrowDrums

internal/ghmcp/oauth.go

@@ -44,7 +44,10 @@ func (p *sessionPrompter) PromptURL(ctx context.Context, prompt oauth.Prompt) er
 		ElicitationID: rand.Text(),
 	})
 	if err != nil {
-		return err
+		// The client advertised URL elicitation but the request itself failed:
+		// classify it as undeliverable (not a user decision) so the flow can fall
+		// back to a channel that needs no client capability.
+		return fmt.Errorf("%w: %w", oauth.ErrPromptUnavailable, err)
 	}
 	if res.Action != "accept" {
 		return oauth.ErrPromptDeclined
@@ -71,7 +74,9 @@ func (p *sessionPrompter) PromptForm(ctx context.Context, prompt oauth.Prompt) e
 		Message: prompt.Message,
 	})
 	if err != nil {
-		return err
+		// As with PromptURL, a delivery failure is undeliverable rather than a
+		// decline, so the flow can fall back instead of aborting.
+		return fmt.Errorf("%w: %w", oauth.ErrPromptUnavailable, err)
 	}
 	if res.Action != "accept" {
 		return oauth.ErrPromptDeclined

internal/ghmcp/oauth_test.go

@@ -2,6 +2,7 @@ package ghmcp
 
 import (
 	"context"
+	"errors"
 	"io"
 	"log/slog"
 	"net/http"
@@ -193,6 +194,51 @@ func TestSessionPrompterPromptActions(t *testing.T) {
 	}
 }
 
+// TestSessionPrompterTransportError verifies that a prompt which fails to be
+// delivered (the client errors instead of returning an action) is reported as
+// ErrPromptUnavailable, not ErrPromptDeclined. The manager relies on this
+// distinction to fall back to manual instructions instead of aborting.
+func TestSessionPrompterTransportError(t *testing.T) {
+	t.Parallel()
+
+	caps := &mcp.ClientCapabilities{Elicitation: &mcp.ElicitationCapabilities{
+		URL:  &mcp.URLElicitationCapabilities{},
+		Form: &mcp.FormElicitationCapabilities{},
+	}}
+
+	for _, mode := range []string{"url", "form"} {
+		t.Run(mode, func(t *testing.T) {
+			t.Parallel()
+
+			handler := func(_ context.Context, _ *mcp.ElicitRequest) (*mcp.ElicitResult, error) {
+				return nil, errors.New("client cannot deliver elicitation")
+			}
+
+			got := runProbe(t, caps, handler, func(ctx context.Context, p *sessionPrompter) string {
+				var err error
+				if mode == "url" {
+					err = p.PromptURL(ctx, oauth.Prompt{Message: "msg", URL: "https://example.com/auth"})
+				} else {
+					err = p.PromptForm(ctx, oauth.Prompt{Message: "msg"})
+				}
+				switch {
+				case err == nil:
+					return "ok"
+				case errors.Is(err, oauth.ErrPromptDeclined):
+					return "declined"
+				case errors.Is(err, oauth.ErrPromptUnavailable):
+					return "unavailable"
+				default:
+					return "error: " + err.Error()
+				}
+			})
+
+			assert.Equal(t, "unavailable", got,
+				"a delivery failure must be classified as undeliverable, not a decline")
+		})
+	}
+}
+
 // fakeAuthenticator is a deterministic stand-in for *oauth.Manager that lets the
 // middleware be tested at each branch without standing up live GitHub flows.
 type fakeAuthenticator struct {

internal/oauth/flow.go

@@ -24,9 +24,15 @@ type flowPlan struct {
 	// poll the device endpoint) and returns the token.
 	run func(context.Context) (*oauth2.Token, error)
 	// display, if set, presents the prompt to the user via the Prompter and
-	// blocks until they act. A non-nil error (including ErrPromptDeclined)
-	// aborts the flow.
+	// blocks until they act. ErrPromptDeclined (the user said no) or any other
+	// error aborts the flow, except ErrPromptUnavailable, which degrades to
+	// fallback when that is set.
 	display func(context.Context) error
+	// fallback, if set alongside display, is the manual user action to surface
+	// when the display prompt cannot be delivered (ErrPromptUnavailable). It lets
+	// a runtime elicitation failure degrade to the manual channel — keeping the
+	// background flow alive — instead of aborting.
+	fallback *UserAction
 	// userAction, if set, indicates the last-resort channel: the caller must
 	// surface it and the user retries after authorizing out of band.
 	userAction *UserAction
@@ -124,23 +130,27 @@ func (m *Manager) beginPKCE(prompter Prompter) (*flowPlan, error) {
 		m.logger.Debug("browser auto-open unavailable", "reason", browserErr)
 	}
 
+	// The manual instructions double as the fallback if a chosen display channel
+	// turns out to be undeliverable at runtime, so build them once here.
+	manual := &UserAction{
+		URL: authURL,
+		Message: fmt.Sprintf(
+			"To authorize the GitHub MCP Server, open this URL in your browser:\n\n%s\n\nAfter authorizing, retry your request.\n\n%s",
+			authURL, securityAdvisory,
+		),
+	}
+
 	if canPromptURL(prompter) {
 		display := func(ctx context.Context) error {
 			return prompter.PromptURL(ctx, Prompt{
 				Message: "Authorize the GitHub MCP Server in your browser to continue.",
 				URL:     authURL,
 			})
 		}
-		return &flowPlan{run: run, display: display}, nil
+		return &flowPlan{run: run, display: display, fallback: manual}, nil
 	}
 
-	return &flowPlan{run: run, userAction: &UserAction{
-		URL: authURL,
-		Message: fmt.Sprintf(
-			"To authorize the GitHub MCP Server, open this URL in your browser:\n\n%s\n\nAfter authorizing, retry your request.\n\n%s",
-			authURL, securityAdvisory,
-		),
-	}}, nil
+	return &flowPlan{run: run, userAction: manual}, nil
 }
 
 // beginDevice prepares the device authorization flow. It requests a device code
@@ -164,6 +174,17 @@ func (m *Manager) beginDevice(prompter Prompter) (*flowPlan, error) {
 		return tok, nil
 	}
 
+	// As with PKCE, the manual instructions double as the runtime fallback, so
+	// build them once and reuse for both display plans and the last resort.
+	manual := &UserAction{
+		URL:      da.VerificationURI,
+		UserCode: da.UserCode,
+		Message: fmt.Sprintf(
+			"%s\n\nAfter authorizing, retry your request.\n\n%s",
+			deviceInstruction(da), securityAdvisory,
+		),
+	}
+
 	if canPromptURL(prompter) {
 		display := func(ctx context.Context) error {
 			return prompter.PromptURL(ctx, Prompt{
@@ -172,7 +193,7 @@ func (m *Manager) beginDevice(prompter Prompter) (*flowPlan, error) {
 				UserCode: da.UserCode,
 			})
 		}
-		return &flowPlan{run: run, display: display}, nil
+		return &flowPlan{run: run, display: display, fallback: manual}, nil
 	}
 
 	if canPromptForm(prompter) {
@@ -183,17 +204,10 @@ func (m *Manager) beginDevice(prompter Prompter) (*flowPlan, error) {
 				UserCode: da.UserCode,
 			})
 		}
-		return &flowPlan{run: run, display: display}, nil
+		return &flowPlan{run: run, display: display, fallback: manual}, nil
 	}
 
-	return &flowPlan{run: run, userAction: &UserAction{
-		URL:      da.VerificationURI,
-		UserCode: da.UserCode,
-		Message: fmt.Sprintf(
-			"%s\n\nAfter authorizing, retry your request.\n\n%s",
-			deviceInstruction(da), securityAdvisory,
-		),
-	}}, nil
+	return &flowPlan{run: run, userAction: manual}, nil
 }
 
 // securityAdvisory nudges users on clients without URL elicitation to ask their

internal/oauth/manager.go

@@ -190,14 +190,32 @@ func (m *Manager) Authenticate(ctx context.Context, prompter Prompter) (*Outcome
 }
 
 // runFlow executes a prepared flow in the background and records the result. The
-// optional display prompt runs concurrently; if it ends in error or decline it
-// cancels the flow.
+// optional display prompt runs concurrently: a decline (or other failure) aborts
+// the flow, while an undeliverable prompt degrades to the manual fallback without
+// tearing the flow down, so the user can still authorize out of band.
 func (m *Manager) runFlow(ctx context.Context, cancel context.CancelFunc, plan *flowPlan) {
 	defer cancel()
 
 	if plan.display != nil {
 		go func() {
-			if err := plan.display(ctx); err != nil {
+			err := plan.display(ctx)
+			switch {
+			case err == nil:
+				// Prompt shown; the flow completes when the token arrives.
+			case ctx.Err() != nil:
+				// The flow is already ending (timed out or cancelled elsewhere),
+				// so there is nothing to fall back to. Checking this before the
+				// fallback also prevents misreading a context-cancelled prompt as
+				// a transport failure.
+			case errors.Is(err, ErrPromptUnavailable) && plan.fallback != nil:
+				// The client advertised the capability but could not deliver the
+				// prompt. Surface the manual instructions instead of failing, and
+				// keep the background flow alive so the user can still authorize.
+				m.logger.Debug("authorization prompt undeliverable; falling back to manual instructions", "reason", err)
+				m.fallBackToUserAction(plan.fallback)
+			default:
+				// A user decline (ErrPromptDeclined) or any other prompt failure
+				// ends the flow.
 				m.logger.Debug("authorization prompt closed", "reason", err)
 				cancel()
 			}
@@ -208,6 +226,26 @@ func (m *Manager) runFlow(ctx context.Context, cancel context.CancelFunc, plan *
 	m.complete(tok, err)
 }
 
+// fallBackToUserAction promotes a running secure flow to the manual user-action
+// channel after its prompt could not be delivered. The background flow keeps
+// running, so the user can complete authorization out of band and retry. It is a
+// no-op if the flow has already resolved.
+func (m *Manager) fallBackToUserAction(ua *UserAction) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	if m.status != statusInProgress {
+		return
+	}
+	m.status = statusAwaitingUser
+	m.pending = ua
+	// Wake any callers joined on this flow so they receive the action, and clear
+	// done so complete() does not double-close it when run() later finishes.
+	if m.done != nil {
+		close(m.done)
+		m.done = nil
+	}
+}
+
 // complete records the flow result, installing a refreshing token source on
 // success, and wakes any joined callers.
 func (m *Manager) complete(tok *oauth2.Token, err error) {
@@ -236,16 +274,22 @@ func (m *Manager) complete(tok *oauth2.Token, err error) {
 	}
 }
 
-// joinWait blocks until the running flow finishes or ctx is cancelled.
+// joinWait blocks until the running flow finishes or ctx is cancelled. If the
+// flow was promoted to the manual channel while waiting (its prompt could not be
+// delivered), it returns that user action rather than an error.
 func (m *Manager) joinWait(ctx context.Context, done chan struct{}) (*Outcome, error) {
 	select {
 	case <-done:
 		if m.AccessToken() != "" {
 			return nil, nil
 		}
 		m.mu.Lock()
+		pending := m.pending
 		err := m.lastErr
 		m.mu.Unlock()
+		if pending != nil {
+			return &Outcome{UserAction: pending}, nil
+		}
 		if err != nil {
 			return nil, err
 		}

internal/oauth/manager_test.go

@@ -117,6 +117,42 @@ func TestAuthenticateDeclinedPromptFails(t *testing.T) {
 	assert.Empty(t, m.AccessToken())
 }
 
+func TestAuthenticateUndeliverablePromptFallsBack(t *testing.T) {
+	f := newFakeGitHub(t)
+	m := newManager(t, f)
+	m.openURL = func(string) error { return errors.New("no browser") }
+
+	// The client advertised URL elicitation but delivering the prompt fails (a
+	// transport/protocol error, not a user decision). This must degrade to the
+	// manual instructions rather than aborting like a decline does.
+	prompter := &fakePrompter{
+		urlCapable: true,
+		onURL: func(_ context.Context, _ Prompt) error {
+			return ErrPromptUnavailable
+		},
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	out, err := m.Authenticate(ctx, prompter)
+	require.NoError(t, err, "an undeliverable prompt must not abort the flow")
+	require.NotNil(t, out)
+	require.NotNil(t, out.UserAction, "an undeliverable prompt must fall back to a user action")
+	assert.NotEmpty(t, out.UserAction.URL)
+	assert.Contains(t, out.UserAction.Message, securityAdvisory)
+
+	// A concurrent retry while awaiting the user returns the same fallback action.
+	out2, err := m.Authenticate(ctx, nil)
+	require.NoError(t, err)
+	require.NotNil(t, out2.UserAction)
+	assert.Equal(t, out.UserAction.URL, out2.UserAction.URL)
+
+	// The background flow stayed alive: opening the URL out of band completes it.
+	require.NoError(t, browserGet(out.UserAction.URL))
+	assert.Equal(t, "gho_access", waitForToken(t, m))
+}
+
 func TestAuthenticateLastDitchUserAction(t *testing.T) {
 	f := newFakeGitHub(t)
 	m := newManager(t, f)

internal/oauth/prompter.go

@@ -5,10 +5,18 @@ import (
 	"errors"
 )
 
-// ErrPromptDeclined is returned by a Prompter when the user cancels or declines
-// the authorization prompt.
+// ErrPromptDeclined is returned by a Prompter when the user actively cancels or
+// declines the authorization prompt. It is a deliberate "no", so the flow stops
+// rather than falling back to another channel.
 var ErrPromptDeclined = errors.New("authorization declined by user")
 
+// ErrPromptUnavailable is returned by a Prompter when the prompt could not be
+// delivered at all — for example the client advertised an elicitation capability
+// but the request failed at the transport or protocol level. Unlike
+// ErrPromptDeclined it reflects no user decision, so the flow falls back to a
+// channel that needs no client capability instead of giving up.
+var ErrPromptUnavailable = errors.New("authorization prompt could not be delivered")
+
 // Prompt is the content shown to the user when asking them to authorize.
 type Prompt struct {
 	// Message is a human-readable instruction.
@@ -36,15 +44,17 @@ type Prompter interface {
 	// until the user acknowledges, declines, or ctx is done. Returning nil means
 	// the prompt was shown (not that authorization completed); the caller waits
 	// for the OAuth flow itself to finish. It returns ErrPromptDeclined if the
-	// user declines or cancels.
+	// user declines or cancels, or ErrPromptUnavailable if the prompt could not
+	// be delivered.
 	PromptURL(ctx context.Context, p Prompt) error
 
 	// CanPromptForm reports whether the client supports form elicitation, used
 	// to display a device code when URL elicitation is unavailable.
 	CanPromptForm() bool
 
 	// PromptForm presents a textual acknowledgement prompt and blocks until the
-	// user responds. It returns ErrPromptDeclined if the user declines.
+	// user responds. It returns ErrPromptDeclined if the user declines, or
+	// ErrPromptUnavailable if the prompt could not be delivered.
 	PromptForm(ctx context.Context, p Prompt) error
 }