Repository navigation
Don't drop discharges fetched before the interactive retry - #5123
Merged
Merged
Conversation
refreshDischargeTokens discharges in two passes when it has a way to open the user's browser: a parallel one for everything that needs no interaction, then, if a third party asked for the user, a sequential retry with the callback attached. It returned only the second pass's updated flag, so a run where the first pass discharged some tokens and the second failed reported no update at all. Both callers gate persistence on that flag, so those discharges were never written to the config file. They stayed in memory for the current command and were fetched again by the next one. That is the common shape for a user with several orgs where only one needs an interactive login: everything else is re-discharged on every invocation. Report an update if either pass produced one, and recognise the third party's request for the user with errors.Is against the sentinel macaroon now exports, rather than by matching the text of its error message.
dangra
force-pushed
the
discharge-poll-deadline
branch
from
August 24, 2026 23:32
13fdaaf to
5de1c7e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
refreshDischargeTokensreported no update when its first pass discharged tokens and its second pass failed, so those discharges were never persisted and the next command fetched them again.Context
The function discharges in two passes whenever it has a way to open the user's browser (
internal/config/tokens.go:186). The first runs without aUserURLCallback, which is what makes the macaroon client fetch every ticket in parallel; if a third party comes back asking for the user, it retries sequentially with the callback attached so the resulting session can carry the remaining tickets.It returned only the second pass's
updatedflag. Both callers,MonitorTokensandkeepConfigTokensFresh, decide whether to write the config file from that value, so a run where the parallel pass discharged something and the interactive retry then failed persisted nothing. The tokens survived in memory for the command in flight and were re-fetched by the next one. It is the ordinary shape for a user with several orgs where one of them needs an interactive login: every other org's discharge is thrown away and re-fetched on every invocation, for as long as that one org keeps failing.Details
The retry condition moves from
strings.Contains(err.Error(), "missing user-url callback")toerrors.Is(err, tp.ErrMissingUserURLCallback), the sentinel exported by superfly/macaroon#47 and released in v0.3.2, which this bumps to. The old match was against an inlineerrors.Newin the library: rewording it there would have silently disabled the interactive retry, leaving no browser opened and a debug log as the only trace.errors.Issees through the joined per-ticket error the same way.The test stands up a real third party that discharges the first ticket outright and asks for the user on the second, with a callback that refuses, and asserts the discharge from the parallel pass is still reported.