fix(cli): make useLogger follow the active session ID after /clear#27907
fix(cli): make useLogger follow the active session ID after /clear#27907i-anubhav-anand wants to merge 1 commit into
Conversation
useLogger() created the Logger from config.getSessionId() but its effect depended only on [config]. /clear mints a new session ID on the same config object, so the dependency never changed and the logger kept writing under the old session ID — making logs disagree with the current chat session and misleading when debugging. Depend on the session ID value as well, so the logger is re-created with the new ID whenever it changes. Fixes google-gemini#27280
Summary of ChangesHello, 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 a bug where the logger failed to track session changes after a Highlights
Using Gemini Code AssistThe 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
Customization To customize the 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 Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. 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. Footnotes
|
|
📊 PR Size: size/S
|
There was a problem hiding this comment.
Code Review
This pull request updates the useLogger hook in packages/cli to extract and depend on the sessionId from the configuration object. This ensures that the logger is re-created when the session ID changes (such as after a /clear command), preventing logs from being written under a stale session ID. A corresponding unit test has been added to verify this behavior. I have no feedback to provide as there are no review comments.
Summary
Fixes #27280.
useLogger()builds theLoggerfromconfig.getSessionId(), but its effect dependency array is only[config]./clearmints a new session ID on the sameconfigobject (resetNewSessionState), so the dependency never changes and the effect never re-runs. The logger keeps writing under the old session ID, so the log files disagree with the current chat session — misleading when debugging.Fix
Read the session ID into a value and include it in the dependency array, so the logger is re-created with the new ID whenever the active session changes (after
/clear, or any future session reset). Minimal change; the happy path (no session change) is unaffected.Testing
useLogger.test.tsxwith a test that changes the session ID and re-renders, asserting the logger is re-created with the new ID. Verified it fails before this change (logger keeps the stale ID) and passes after.npx vitest run src/ui/hooks/useLogger.test.tsx→ 2 passed.eslint(incl. react-hooks/exhaustive-deps) andnpm run typecheck(packages/cli) clean.