Skip to content

fix: flagsmith init fails with invalid_grant on an expired session - #110

Merged
khvn26 merged 2 commits into
mainfrom
fix/expired-session-login
Oct 15, 2026
Merged

khvn26 merged 2 commits into
mainfrom
fix/expired-session-login

Conversation

@khvn26

@khvn26 khvn26 commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

A fresh flagsmith init with an old login still in the keychain fails instead of logging in:

$ flagsmith init
Error: refreshing session failed: invalid_grant
Run `flagsmith login` to re-authenticate.

In this PR, we classify an invalid_grant response to a refresh as auth.ErrSessionExpired, which matches auth.ErrNotLoggedIn. This way:

  • flagsmith init takes its existing inline-login path for an expired session, same as for no session at all.
  • Other commands get the regular login hint instead of the re-login one, and the error is no longer reported as unexpected.
  • Other refresh failures (unreachable or failing token endpoint) still surface as ErrRefreshFailed, so e.g. flaky network doesn't send anyone through a browser login.

@khvn26
khvn26 requested a review from a team as a code owner October 15, 2026 10:07
@khvn26
khvn26 requested review from emyller and removed request for a team October 15, 2026 10:07
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

📝 Walkthrough

When a refresh request receives invalid_grant, EnsureFresh now returns ErrSessionExpired, which wraps ErrNotLoggedIn. Other refresh errors continue to use ErrRefreshFailed. Tests cover this distinction and command behaviour for expired sessions, including interactive login and auth status.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~12 minutes

Merge Risk: 🔵 Low · up to 1d44c

Merge Risk: 🔵 Low · up to 1d44c

The expired-session behavior is mergeable with a bounded test gap: the new status test would not catch the wrong login hint.

- [ ] Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 40dd1550-a868-4a30-95c0-e1257cfde444
📥 Commits

Reviewing files that changed from the base of the PR and between 476ba5d and 0f8300d.

📒 Files selected for processing: 4
  • internal/auth/oauth.go
  • internal/auth/oauth_test.go
  • internal/cmd/cmd_test.go
  • internal/cmd/sentinel_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Comment thread internal/cmd/cmd_test.go Outdated
@khvn26
khvn26 requested a review from Zaimwa9 October 15, 2026 10:13
@khvn26
khvn26 merged commit 1bef28c into main Oct 15, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants