Skip to content

fix: Preserve AuthMode and fix scope corruption on OAuth token refresh#42

Open
FelixLisczyk wants to merge 1 commit intojoa23:mainfrom
FelixLisczyk:tl-356
Open

fix: Preserve AuthMode and fix scope corruption on OAuth token refresh#42
FelixLisczyk wants to merge 1 commit intojoa23:mainfrom
FelixLisczyk:tl-356

Conversation

@FelixLisczyk
Copy link
Copy Markdown
Contributor

Summary

This PR addresses two remaining gaps in the OAuth token refresh flow after PR #8.

Bug 1 — AuthMode lost after first token refresh

refreshTokenLocked() already preserves RefreshToken from the existing token
when the OAuth server's response omits it. However, the same preservation was missing
for AuthMode. Since the OAuth protocol never includes auth_mode in a refresh
response, the field is silently dropped on the first refresh. The saved token file
then has an empty auth_mode, which breaks --assignee me resolution for users
who authenticated in agent mode.

Fix: apply the same "preserve if absent" guard to AuthMode in refresher.go,
mirroring the existing RefreshToken logic.

Bug 2 — SanitizeToken corrupts JSON string values containing spaces

LoadTokenData() called SanitizeToken() on the entire raw file content before
parsing it as JSON. SanitizeToken strips all whitespace characters, which corrupts
any JSON string value that contains a space — for example a scope field of
"read write" becomes "readwrite". The JSON structure remained parseable, so the
corruption was silent.

SaveTokenData already sanitizes the individual AccessToken and RefreshToken
fields before writing, so sanitizing the whole document on load is redundant for the
JSON path and actively harmful. The fix moves SanitizeToken to the legacy
plain-string fallback path only, where it is still needed.

Test plan

  • TestRefresher_PreservesAuthModeOnRefresh — verifies AuthMode is carried over
    when the refresh response does not include it
  • TestRefresher_DoesNotOverwriteAuthModeWhenPresent — verifies a new AuthMode
    in the response wins over the old one
  • TestRefresher_PreservesRefreshTokenWhenAbsent — regression guard for the
    existing RefreshToken preservation logic
  • "load token data JSON with spaces in scope" — directly reproduces the
    space-stripping bug and verifies the fix
  • make test passes (all existing tests remain green)

🤖 Generated with Claude Code

Preserve AuthMode across token refreshes — the OAuth server never
returns
it, so without explicit preservation it was silently dropped after the
first
refresh, breaking --assignee me for agent-mode users. Mirrors the
existing
RefreshToken preservation pattern in refreshTokenLocked.

Remove the SanitizeToken call on the raw JSON document in LoadTokenData.
Sanitizing the whole document stripped spaces from JSON string values
(e.g. scope "read write" → "readwrite"). SaveTokenData already sanitizes
individual token fields on write, so the document-level call was
redundant
and harmful. The legacy plain-string path retains sanitization.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

1 participant