Skip to content

fix(studio): prevent auth monitor reload loop - #7118

Merged
danielhanchen merged 1 commit into
unslothai:mainfrom
Lyxot:fix/studio-auth-monitor-loop
Jul 14, 2026
Merged

danielhanchen merged 1 commit into
unslothai:mainfrom
Lyxot:fix/studio-auth-monitor-loop

Conversation

@Lyxot

@Lyxot Lyxot commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Fixes #7114

Summary

  • Mark /login, /change-password, and onboarding as authentication-flow routes and do not mount the global settings UI or persisted System monitor on those routes.
  • Close transient settings-dialog state when entering an authentication flow and ignore the settings shortcut there, while preserving the persisted monitor preference for the next authenticated route.
  • Make redirectToAuth() skip assigning window.location.href when the browser is already at the resolved /login or /change-password target, and release the redirect latch when navigation is skipped.
  • Add focused redirect tests and extend the existing Studio UI smoke flow to verify that monitor polling stops on /login and resumes after authentication.

Root cause

SettingsDialog was mounted globally and always mounted FloatingMonitor. When the persisted monitor state was open, authentication routes immediately polled protected /api/system. A 401 called redirectToAuth(), which assigned /login even when already on /login; the reload restored the open monitor and repeated the same request and redirect indefinitely.

Fast reproduction

On an unfixed web build, open DevTools, enable Preserve log in the Network panel, and run:

localStorage.setItem(
  "unsloth_monitor_overlay",
  JSON.stringify({ state: { isOpen: true, isMinimized: false }, version: 0 }),
);
localStorage.removeItem("unsloth_auth_token");
localStorage.removeItem("unsloth_auth_refresh_token");
location.assign("/login");

Before this patch, /login repeatedly reloads after /api/system returns 401. With this patch, /login remains stable and does not request /api/system; after a successful login, the persisted monitor resumes polling normally.

Validation

python -m pytest tests/studio/test_auth_form_input_count.py -q
npx --yes --package=node@22 -- python -m pytest tests/studio/test_auth_form_input_count.py -q
python -m ruff check tests/studio/test_auth_form_input_count.py tests/studio/playwright_chat_ui.py
python -m py_compile tests/studio/test_auth_form_input_count.py tests/studio/playwright_chat_ui.py
cd studio/frontend
npm run typecheck
npm run build
  • Focused auth tests: 9 passed on the default runtime and Node 22.
  • Node 20: 8 passed and the unsupported strip-types runtime test skipped as intended.
  • Ruff, Python compilation, frontend type checking, and frontend production build passed.
  • The existing cross-platform Playwright smoke flow now covers dormant monitor polling on /login, settings-shortcut cleanup, and polling resumption after login.

Risk

The boundary is explicit route metadata, so any future guest authentication-flow route must opt in with isAuthFlow: true. Token refresh and Tauri automatic authentication paths are unchanged.

@Lyxot
Lyxot requested a review from danielhanchen as a code owner July 14, 2026 07:08
Copilot AI review requested due to automatic review settings July 14, 2026 07:08

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

Copy link
Copy Markdown
Contributor

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 changes to prevent the global settings dialog from mounting or being opened via shortcuts during authentication flows (login, change password, onboarding). It also ensures that the persisted system monitor stays dormant on the login page and resumes polling only after successful authentication. Corresponding Playwright and unit tests have been added to verify these behaviors. The reviewer feedback suggests explicitly specifying encoding='utf-8' when reading and writing files in Python to avoid platform-dependent encoding issues.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread tests/studio/test_auth_form_input_count.py
Comment thread tests/studio/test_auth_form_input_count.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an infinite reload loop on Studio authentication routes caused by the persisted System monitor polling protected /api/system while /login (or /change-password) is active, repeatedly triggering 401 redirects. It introduces explicit “auth-flow route” metadata to prevent mounting global settings/monitor UI on those routes, and makes auth redirects idempotent when already at the target route.

Changes:

  • Add staticData.isAuthFlow route metadata and use it in the root layout to avoid mounting SettingsDialog and to disable/close settings-dialog behavior during auth flows.
  • Update redirectToAuth() to avoid reassigning window.location.href when already on the resolved auth target, releasing the redirect latch when navigation is skipped.
  • Add/extend tests to pin the auth-flow boundary behavior and to verify monitor polling is dormant on /login and resumes after authentication.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
studio/frontend/src/app/routes/__root.tsx Introduces isAuthFlow route static data and uses it to gate Settings dialog mounting and shortcuts during auth flows.
studio/frontend/src/features/auth/api.ts Makes redirectToAuth() idempotent when already on /login or /change-password, preventing reload loops.
studio/frontend/src/app/routes/login.tsx Marks /login as an auth-flow route via staticData.isAuthFlow.
studio/frontend/src/app/routes/change-password.tsx Marks /change-password as an auth-flow route via staticData.isAuthFlow.
studio/frontend/src/app/routes/onboarding.tsx Marks /onboarding as an auth-flow route via staticData.isAuthFlow.
tests/studio/test_auth_form_input_count.py Adds focused source/runtime contracts for auth-flow boundaries and redirect idempotence/concurrency.
tests/studio/playwright_chat_ui.py Extends the existing smoke flow to assert /api/system polling is dormant on /login and resumes after login, and that settings-dialog state doesn’t leak across auth.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@danielhanchen

Copy link
Copy Markdown
Member

@codex review

1 similar comment
@danielhanchen

Copy link
Copy Markdown
Member

@codex review

danielhanchen added a commit to danielhanchen/unsloth-staging-2 that referenced this pull request Jul 14, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🎉

Reviewed commit: d9476381d3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danielhanchen
danielhanchen merged commit c80e7d3 into unslothai:main Jul 14, 2026
36 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.

[Bug] [studio] Infinite reload on Unsloth Studio login page

3 participants