on main help sign in
fix(agent): centralize 401 retry logic and fix notify-on-success bug

Add runWithUnauthorizedRetry helper that encapsulates the "run → 401 → refresh credentials → retry" pattern. Migrate all three call sites (runMessage, Summarize, sub-agent) to use it, preventing the retry logic from drifting between callers. Fix a bug in runMessage where the re-authenticate notification fired unconditionally whenever the original error was a 401, even if the retry succeeded. Now the notification only fires when the error is still unauthorized after the retry attempt.

commit90197756b670d743ad3c84528e4d69d2884a057a authorKieran Klukas <kieran@dunkirk.sh> dateFri 29 May 2026, 14:30 (5 months ago) parent3cd88ac
1 file changed+41−29
internal/agent/coordinator.go+41−29
internal/agent/coordinator.go+41−29
@@ -259,19 +259,21 @@ func (c *coordinator) Run(ctx context.Context, sessionID string, prompt string,
259259 })
260260 }
261261 beforeLoaded := c.skillTracker.LoadedNames()
262− result, originalErr := run()
262+ var result *fantasy.AgentResult
263+ var originalErr error = c.runWithUnauthorizedRetry(ctx, providerCfg, func() error {
264+ var err error
265+ result, err = run()
266+ return err
267+ })
263268 logTurnSkillUsage(sessionID, prompt, c.activeSkills, c.skillTracker, beforeLoaded)
264269
265− if c.isUnauthorized(originalErr) {
266− if err := c.retryAfterUnauthorized(ctx, providerCfg); err == nil {
267− result, originalErr = run()
268− }
269− if c.notify != nil && model.ModelCfg.Provider == hyper.Name {
270− c.notify.Publish(pubsub.CreatedEvent, notify.Notification{
271− Type: notify.TypeReAuthenticate,
272− ProviderID: model.ModelCfg.Provider,
273− })
274− }
270+ // Notify only if still unauthorized after retry — a successful
271+ // retry means the user doesn't need to re-authenticate.
272+ if originalErr != nil && c.isUnauthorized(originalErr) && c.notify != nil && model.ModelCfg.Provider == hyper.Name {
273+ c.notify.Publish(pubsub.CreatedEvent, notify.Notification{
274+ Type: notify.TypeReAuthenticate,
275+ ProviderID: model.ModelCfg.Provider,
276+ })
275277 }
276278
277279 if hasLatest && c.runComplete != nil {
@@ -1062,14 +1064,7 @@ func (c *coordinator) Summarize(ctx context.Context, sessionID string) error {
10621064 return c.currentAgent.Summarize(ctx, sessionID, getProviderOptions(c.currentAgent.Model(), providerCfg))
10631065 }
10641066
1065− err := summarize()
1066− if err != nil && c.isUnauthorized(err) {
1067− if retryErr := c.retryAfterUnauthorized(ctx, providerCfg); retryErr == nil {
1068− return summarize()
1069− }
1070− }
1071−
1072− return err
1067+ return c.runWithUnauthorizedRetry(ctx, providerCfg, summarize)
10731068}
10741069
10751070// refreshTokenIfExpired proactively refreshes the OAuth token if it has expired.
@@ -1081,6 +1076,21 @@ func (c *coordinator) refreshTokenIfExpired(ctx context.Context, providerCfg con
10811076 return c.refreshOAuth2Token(ctx, providerCfg)
10821077}
10831078
1079+// runWithUnauthorizedRetry executes fn. If fn returns a 401 error, it
1080+// attempts to refresh credentials and re-runs fn once. Returns the
1081+// final error: from the retry if a retry was attempted, otherwise from
1082+// the original run. Callers that need to notify the user on persistent
1083+// failure should check isUnauthorized on the returned error.
1084+func (c *coordinator) runWithUnauthorizedRetry(ctx context.Context, providerCfg config.ProviderConfig, fn func() error) error {
1085+ err := fn()
1086+ if err != nil && c.isUnauthorized(err) {
1087+ if retryErr := c.retryAfterUnauthorized(ctx, providerCfg); retryErr == nil {
1088+ return fn()
1089+ }
1090+ }
1091+ return err
1092+}
1093+
10841094// retryAfterUnauthorized attempts to refresh credentials after receiving a 401
10851095// and returns nil if retry should be attempted.
10861096func (c *coordinator) retryAfterUnauthorized(ctx context.Context, providerCfg config.ProviderConfig) error {
@@ -1184,16 +1194,18 @@ func (c *coordinator) runSubAgent(ctx context.Context, params subAgentParams) (f
11841194 NonInteractive: true,
11851195 })
11861196 }
1187− result, err := run()
1188− if err != nil && c.isUnauthorized(err) {
1189− if retryErr := c.retryAfterUnauthorized(ctx, providerCfg); retryErr == nil {
1190− result, err = run()
1191− } else if c.notify != nil && model.ModelCfg.Provider == hyper.Name {
1192− c.notify.Publish(pubsub.CreatedEvent, notify.Notification{
1193− Type: notify.TypeReAuthenticate,
1194− ProviderID: model.ModelCfg.Provider,
1195− })
1196− }
1197+ var result *fantasy.AgentResult
1198+ err = c.runWithUnauthorizedRetry(ctx, providerCfg, func() error {
1199+ var runErr error
1200+ result, runErr = run()
1201+ return runErr
1202+ })
1203+ // Notify only if still unauthorized after retry.
1204+ if err != nil && c.isUnauthorized(err) && c.notify != nil && model.ModelCfg.Provider == hyper.Name {
1205+ c.notify.Publish(pubsub.CreatedEvent, notify.Notification{
1206+ Type: notify.TypeReAuthenticate,
1207+ ProviderID: model.ModelCfg.Provider,
1208+ })
11971209 }
11981210 if err != nil {
11991211 return fantasy.NewTextErrorResponse(fmt.Sprintf("Failed to generate response: %s", err)), nil