Skip to content

fix: suppress token refresh write-back loop and reduce log noise#1943

Open
YoungMoneyInvestments wants to merge 1 commit intorouter-for-me:mainfrom
YoungMoneyInvestments:fix/refresh-writeback-loop
Open

fix: suppress token refresh write-back loop and reduce log noise#1943
YoungMoneyInvestments wants to merge 1 commit intorouter-for-me:mainfrom
YoungMoneyInvestments:fix/refresh-writeback-loop

Conversation

@YoungMoneyInvestments
Copy link

Summary

  • conductor.go: Use WithSkipPersist in refreshAuth to prevent disk write-back after auto-refresh. The refresh cycle writes the updated token to the JSON file, triggering the fsnotify watcher → reloadCallbackUpdateClients, logging "server clients and configuration updated" every ~1 minute per token. Since the in-memory state is already current, persisting is unnecessary and creates a feedback loop.

  • server.go: Change UpdateClients fmt.Printf to log.Debugf so configuration reload messages respect the debug flag and don't unconditionally fill stdout logs.

Problem

With 2+ OAuth tokens (e.g., Claude Max accounts), each refreshing within a 4-hour lead window with a 1-minute backoff, the log accumulates ~2 messages/minute continuously. Over 12 days this produced 438 redundant "configuration updated" lines — pure noise that obscures meaningful log entries and contributes to log file growth.

Test plan

  • go build ./... passes
  • go test ./sdk/cliproxy/auth/... -run "SkipPersist|Refresh" passes
  • Verify no "configuration updated" messages in stdout when debug: false
  • Verify messages appear when debug: true
  • Confirm token refresh still works (check token expiry updates in auth JSON files)

🤖 Generated with Claude Code

Two changes to reduce excessive "server clients and configuration updated"
log messages that occur every ~1 minute per auth token:

1. conductor.go: Use WithSkipPersist when updating auth after auto-refresh.
   The refreshAuth function writes the refreshed token back to disk via
   m.Update(), which triggers the fsnotify file watcher, which calls
   reloadCallback -> UpdateClients, logging the message. Since the
   in-memory state is already updated by Update(), persisting to disk
   is unnecessary here and creates a feedback loop.

2. server.go: Change UpdateClients fmt.Printf to log.Debugf.
   Configuration reload messages should use the structured logger at
   debug level rather than unconditional stdout, so they respect the
   debug flag and don't fill logs during normal operation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses an issue where token refresh operations were generating excessive log noise due to a feedback loop involving disk persistence and file system watchers. The changes prevent redundant disk writes during token refreshes and convert a high-frequency informational log message into a debug-level message, significantly reducing log verbosity and improving log clarity for meaningful events.

Highlights

  • Token Refresh Loop Suppression: Implemented WithSkipPersist during token refresh to prevent unnecessary disk write-backs, thereby breaking a feedback loop that caused excessive log entries.
  • Log Noise Reduction: Converted a fmt.Printf statement in UpdateClients to log.Debugf, ensuring that configuration update messages only appear when debug logging is enabled.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog
  • internal/api/server.go
    • Changed the fmt.Printf call for "server clients and configuration updated" messages to log.Debugf to respect debug logging settings.
  • sdk/cliproxy/auth/conductor.go
    • Modified the m.Update call within refreshAuth to use WithSkipPersist(ctx), preventing the refreshed token from being written back to disk and thus avoiding a log-spamming feedback loop.
Activity
  • The author has outlined a comprehensive test plan, including go build and go test checks, and specific verification steps for log behavior with debug flags and token expiry updates.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces two targeted fixes to reduce log noise and prevent a write-back loop. In conductor.go, the token auto-refresh logic now skips persisting the updated token to disk, effectively breaking a feedback loop with the file watcher that was causing excessive logging. In server.go, a log message for client and configuration updates has been changed from an unconditional fmt.Printf to a log.Debugf, ensuring it only appears when debug logging is enabled. Both changes are well-implemented and address the issues described in the pull request description.

Copy link

@xkonjin xkonjin left a comment

Choose a reason for hiding this comment

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

Quick review pass:

  • Main risk area here is auth/session state and stale credential handling.
  • I didn’t see targeted regression coverage in the diff; please add or point CI at a focused test for the changed path in server.go, conductor.go.
  • Before merge, I’d smoke-test the behavior touched by server.go, conductor.go with malformed input / retry / rollback cases, since that’s where this class of change usually breaks.

Copy link
Collaborator

@luispater luispater left a comment

Choose a reason for hiding this comment

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

Summary: Thanks for tackling the reload-log noise. The logging change in server.go looks reasonable, but the new WithSkipPersist call in refreshAuth introduces a correctness regression.

Blocking:

  • WithSkipPersist is documented for watcher-driven updates where disk is already the source of truth. In refreshAuth, the provider has just returned new credentials, so skipping persistence means Manager.Load will restore stale auth state after restart.
  • This is especially risky because refresh paths like Claude and Codex can rotate refresh_token values, and those updated tokens would now exist only in memory.

Suggested fix:

  • Keep persisting refreshed auth data, and narrow the reload/log suppression to the watcher/reload path instead.
  • Please add a regression test proving that auto-refresh still persists updated tokens.

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.

3 participants