Don't drop discharges fetched before the interactive retry

go.mod

@@ -79,7 +79,7 @@ require (
 	github.com/superfly/fly-go v0.9.7
 	github.com/superfly/graphql v0.2.6
 	github.com/superfly/lfsc-go v0.1.1
-	github.com/superfly/macaroon v0.3.1
+	github.com/superfly/macaroon v0.3.2
 	github.com/superfly/tokenizer v0.0.3-0.20240826174224-a17a2e0a9dc0
 	github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd
 	github.com/vektah/gqlparser/v2 v2.5.36

go.sum

@@ -672,8 +672,8 @@ github.com/superfly/lfsc-go v0.1.1 h1:dGjLgt81D09cG+aR9lJZIdmonjZSR5zYCi7s54+ZU2
 github.com/superfly/lfsc-go v0.1.1/go.mod h1:zVb0VENz/Il8Nmvvd4XAsX2bWhQ+sr0nK8vv9PeezcE=
 github.com/superfly/ltx v0.3.12 h1:Z7z1sc4g34/jUi3XO84+zBlIsbaoh2RJ3b4zTQpBK/M=
 github.com/superfly/ltx v0.3.12/go.mod h1:ly+Dq7UVacQVEI5/b0r6j+PSNy9ibwx1yikcWAaSkhE=
-github.com/superfly/macaroon v0.3.1 h1:02JF233876dGI53tm/cb72yUkAYKzwQjL1npMYLqE10=
-github.com/superfly/macaroon v0.3.1/go.mod h1:ZAmlRD/Hmp/ddTxE8IonZ7NdTny2DcOffRvZhapQwJw=
+github.com/superfly/macaroon v0.3.2 h1:vrPnUZUsd0DkSO6ievTESgGE1B3LJE3trXnDYZU1KTc=
+github.com/superfly/macaroon v0.3.2/go.mod h1:ZAmlRD/Hmp/ddTxE8IonZ7NdTny2DcOffRvZhapQwJw=
 github.com/superfly/tokenizer v0.0.3-0.20240826174224-a17a2e0a9dc0 h1:0GZOxvuQ2u3XUY7Hr8N02zn4ZN9Iz2xgi3aNNaUpRO4=
 github.com/superfly/tokenizer v0.0.3-0.20240826174224-a17a2e0a9dc0/go.mod h1:w38ieJ28pCyIpQJzuDOKfN5z6Q6R92vOkAYtUv6FL9k=
 github.com/tailscale/hujson v0.0.0-20260302212456-ecc657c15afd h1:Rf9uhF1+VJ7ZHqxrG8pJ6YacmHvVCmByDmGbAWCc/gA=

internal/config/tokens.go

@@ -7,7 +7,6 @@ import (
 	"maps"
 	"slices"
 	"strconv"
-	"strings"
 	"sync"
 	"time"
 
@@ -19,6 +18,7 @@ import (
 	"github.com/superfly/flyctl/internal/task"
 	"github.com/superfly/macaroon"
 	"github.com/superfly/macaroon/flyio"
+	"github.com/superfly/macaroon/tp"
 )
 
 // UserURLCallback is a function that opens a URL in the user's browser. This is
@@ -189,18 +189,28 @@ func refreshDischargeTokens(ctx context.Context, t *tokens.Tokens, uucb UserURLC
 		tokens.WithAdvancePrune(advancePrune),
 	}
 
+	// tokens discharged by the first pass have to be reported even if the
+	// second one fails: our callers decide whether to persist the tokens from
+	// this return value, and dropping it means re-fetching those discharges on
+	// every command.
+	var updatedParallel bool
+
 	if uucb != nil {
 		// Update without UserURLCallback to fetch tokens in parallel.
 		updated, err := t.Update(ctx, updateOpts...)
-		if err == nil || !strings.Contains(err.Error(), "missing user-url callback") {
+		if err == nil || !errors.Is(err, tp.ErrMissingUserURLCallback) {
 			return updated, err
 		}
+		updatedParallel = updated
 
-		// Retry with UserURLCallback if we received a 'missing user-url callback' error.
+		// Retry with UserURLCallback if a third party wants to send the user to
+		// their browser.
 		updateOpts = append(updateOpts, tokens.WithUserURLCallback(uucb))
 	}
 
-	return t.Update(ctx, updateOpts...)
+	updated, err := t.Update(ctx, updateOpts...)
+
+	return updated || updatedParallel, err
 }
 
 // fetchOrgTokens checks that we macaroons for all orgs the user is a member of.

internal/config/tokens_test.go

@@ -3,9 +3,13 @@ package config
 import (
 	"context"
 	"errors"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"slices"
+	"sync"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/require"
 	"github.com/superfly/fly-go/tokens"
@@ -14,6 +18,7 @@ import (
 	"github.com/superfly/macaroon"
 	"github.com/superfly/macaroon/flyio"
 	"github.com/superfly/macaroon/resset"
+	"github.com/superfly/macaroon/tp"
 )
 
 func TestFetchOrgTokens(t *testing.T) {
@@ -200,3 +205,76 @@ func assertTokenOrgs(tb testing.TB, toks *tokens.Tokens, expectedOIDs ...uint64)
 	slices.Sort(actualOIDs)
 	require.Equal(tb, expectedOIDs, actualOIDs)
 }
+
+// TestRefreshDischargeTokensPartialSuccess covers a set of tickets where one
+// discharges on its own and another wants to send the user to their browser.
+// The parallel pass collects the first and fails on the second; the retry with
+// a callback then fails too. The discharge from the first pass is still a real
+// update and has to be reported, or our callers never write it to the config
+// file and every subsequent command fetches it again.
+func TestRefreshDischargeTokensPartialSuccess(t *testing.T) {
+	ctx := logger.NewContext(context.Background(), logger.New(os.Stdout, logger.Debug, true))
+
+	var (
+		thirdParty *tp.TP
+		m          sync.Mutex
+		inits      int
+	)
+
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch path := r.URL.EscapedPath(); path {
+		case tp.InitPath:
+			thirdParty.InitRequestMiddleware(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				m.Lock()
+				inits++
+				first := inits == 1
+				m.Unlock()
+
+				// the first ticket to arrive is discharged outright; anything
+				// after it needs the user.
+				if first {
+					thirdParty.RespondDischarge(w, r)
+
+					return
+				}
+
+				thirdParty.RespondUserInteractive(w, r)
+			})).ServeHTTP(w, r)
+		default:
+			t.Errorf("unexpected request to %s", path)
+		}
+	}))
+	t.Cleanup(server.Close)
+
+	store, err := tp.NewMemoryStore(tp.PrefixMunger("/user/"), 100)
+	require.NoError(t, err)
+
+	thirdParty = &tp.TP{
+		Location: server.URL,
+		Key:      macaroon.NewEncryptionKey(),
+		Store:    store,
+	}
+
+	toks := make([][]byte, 0, 2)
+	for _, oid := range []uint64{1, 2} {
+		perm := fakePermissionToken(t, &flyio.Organization{ID: oid, Mask: resset.ActionAll})
+		require.NoError(t, perm.Add3P(thirdParty.Key, thirdParty.Location))
+
+		tok, err := perm.Encode()
+		require.NoError(t, err)
+		toks = append(toks, tok)
+	}
+
+	var callbacks int
+	uucb := func(context.Context, string) error {
+		callbacks++
+
+		return errors.New("no browser here")
+	}
+
+	updated, err := refreshDischargeTokens(ctx, tokens.Parse(macaroon.ToAuthorizationHeader(toks...)), uucb, time.Minute)
+
+	require.Error(t, err)
+	require.Equal(t, 1, callbacks, "the interactive retry should have run")
+	require.True(t, updated, "the discharge from the parallel pass was dropped")
+}