From 2bf39dee647f0d5db6d8b6b76f7cb83ca9953834 Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Mon, 18 May 2026 04:27:21 -0700 Subject: [PATCH] studio/frontend: hide Current password input on first boot (#5545) * studio/frontend: hide Current password input on first boot PR #5490 added a third Current password input to the change-password form so the admin-forced must_change_password reset path could supply a current password (the bootstrap is empty in that path). The side effect is that the dominant first-boot UX, which has window.__UNSLOTH_BOOTSTRAP__ present and silently fed into currentPassword, now shows three visible inputs instead of the two it had before. Render the Current password input only when window.__UNSLOTH_BOOTSTRAP__ is absent. The loadBootstrap effect already seeds the password state from the bootstrap and currentPassword keeps the bootstrap fallback, so handleSubmit sees the same value as before. On admin-forced resets where the bootstrap is undefined, the Current password input still appears so the user can type their actual current password. Verified end-to-end against a local install via UNSLOTH_STUDIO_HOME + install.sh --local with Playwright driving the page: bootstrap present renders two inputs (New, Confirm) and completes change-password into /chat; bootstrap suppressed via a non-configurable property descriptor init script renders the three inputs (Current, New, Confirm) and keeps the #5490 fix intact. * studio/frontend: add deterministic input-count tests for auth-form Pure-source pytest covering the change-password JSX contract. No browser, no Studio boot, no JS toolchain -- runs on any CI runner. Complements the Playwright probe in tests/studio/playwright_chat_ui.py which exercises the same contract end to end. Pins seven invariants with explicit failure reasons: 1. hasBootstrapPassword is derived from window.__UNSLOTH_BOOTSTRAP__ so a future swap to a localStorage flag or prop cannot silently drift from the backend's _inject_bootstrap contract in studio/backend/main.py. 2. Exactly one !hasBootstrapPassword conditional exists; multiple would split rendering into branches these tests cannot reason about. 3. The Current password input sits inside that conditional, so it never renders on first boot (the regression PR #5490 introduced and that this fix reverses). 4. The New password input sits outside it, so it always renders in change-password mode (admin-forced reset still works). 5. Confirm password: same as New. 6. The change-password JSX subtree declares exactly current / new / confirm; a fourth password input would almost certainly break the 2-input first-boot contract. 7. The login JSX subtree declares exactly one password input. Verified the tests fail loudly on the pre-fix auth-form.tsx at c4575ca0 (5/7 fail with descriptive reasons) and pass on the fixed version (7/7). * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../features/auth/components/auth-form.tsx | 65 +++--- tests/studio/test_auth_form_input_count.py | 190 ++++++++++++++++++ 2 files changed, 223 insertions(+), 32 deletions(-) create mode 100644 tests/studio/test_auth_form_input_count.py diff --git a/studio/frontend/src/features/auth/components/auth-form.tsx b/studio/frontend/src/features/auth/components/auth-form.tsx index de0a1df995..a10c77e9fa 100644 --- a/studio/frontend/src/features/auth/components/auth-form.tsx +++ b/studio/frontend/src/features/auth/components/auth-form.tsx @@ -183,6 +183,10 @@ export function AuthForm({ mode }: AuthFormProps): ReactElement | null { const switchLinkTo = "/login"; const switchLinkText = "Back to login"; const currentPassword = password || window.__UNSLOTH_BOOTSTRAP__?.password || ""; + // On first boot the backend injects __UNSLOTH_BOOTSTRAP__ and we silently + // reuse that password; the Current password input is only rendered for the + // admin-forced must_change_password path where no bootstrap is available. + const hasBootstrapPassword = Boolean(window.__UNSLOTH_BOOTSTRAP__?.password); const invalidChangePasswordForm = !isLoginMode && (newPassword.length < 8 || newPassword !== confirmPassword || currentPassword === newPassword); @@ -337,39 +341,36 @@ export function AuthForm({ mode }: AuthFormProps): ReactElement | null { {!isLoginMode && ( <> -
- -
- setPassword(event.target.value)} - minLength={8} - required - placeholder={ - window.__UNSLOTH_BOOTSTRAP__?.password - ? "Pre-filled with first-boot password" - : undefined - } - /> - + {!hasBootstrapPassword && ( +
+ +
+ setPassword(event.target.value)} + minLength={8} + required + /> + +
-
+ )}
diff --git a/tests/studio/test_auth_form_input_count.py b/tests/studio/test_auth_form_input_count.py new file mode 100644 index 0000000000..559fb7e524 --- /dev/null +++ b/tests/studio/test_auth_form_input_count.py @@ -0,0 +1,190 @@ +# SPDX-License-Identifier: AGPL-3.0-only +# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. + +"""Pin the auth-form input-count contract on the change-password page. + +PR #5490 added a third visible "Current password" input so the +admin-forced must_change_password reset path (where no bootstrap +script is injected) could supply a current password. The side +effect was that the dominant first-boot UX, where the backend +injects window.__UNSLOTH_BOOTSTRAP__ and the form silently reuses +that password, now showed three visible inputs instead of the two +it had before. PR #5545 restores the two-input first-boot UX by +rendering the Current password input only when +window.__UNSLOTH_BOOTSTRAP__ is absent. + +These tests inspect the auth-form source file directly. They never +boot Studio, never spawn a browser, and have no network or device +dependencies, so they are fully deterministic and run on any CI +runner without a JS toolchain. The companion Playwright probe lives +in tests/studio/playwright_chat_ui.py and covers the runtime side. +""" + +from __future__ import annotations + +import re +from pathlib import Path + +AUTH_FORM = ( + Path(__file__).resolve().parents[2] + / "studio/frontend/src/features/auth/components/auth-form.tsx" +) + +CONDITIONAL_OPENER = "{!hasBootstrapPassword && (" + + +def _conditional_extent(src: str) -> tuple[int, int]: + """Return the (start, end) char offsets of the + `{!hasBootstrapPassword && (...)}` JSX block. ``start`` points + at the opening `{`; ``end`` points one past the matching `)}`.""" + start = src.find(CONDITIONAL_OPENER) + assert start != -1, ( + "the {!hasBootstrapPassword && (...)} JSX block that hides the " + "Current password input on first boot is missing -- PR #5545 has " + "been reverted or the conditional was inlined as a ternary" + ) + depth = 1 + i = start + len(CONDITIONAL_OPENER) + while i < len(src): + c = src[i] + if c == "(": + depth += 1 + elif c == ")": + depth -= 1 + if depth == 0: + return start, i + 1 + i += 1 + raise AssertionError("unterminated !hasBootstrapPassword JSX block") + + +def test_hasbootstrappassword_constant_is_derived_from_bootstrap_window_value(): + """The conditional guard must read from window.__UNSLOTH_BOOTSTRAP__. + A future refactor that swaps the source (e.g. a localStorage flag, + a prop) would silently drift from the backend's bootstrap-injection + contract in studio/backend/main.py::_inject_bootstrap.""" + src = AUTH_FORM.read_text() + assert ( + "const hasBootstrapPassword = Boolean(window.__UNSLOTH_BOOTSTRAP__?.password);" + in src + ), ( + "hasBootstrapPassword constant missing or its derivation drifted; " + "this is the gate that hides the Current password input on first boot" + ) + + +def test_exactly_one_hasBootstrapPassword_conditional_exists(): + """Only one `!hasBootstrapPassword` JSX check is allowed. A second + one would split the form rendering into branches that the rest of + these structural tests cannot reason about, and would almost + certainly hide or duplicate one of the New / Confirm inputs.""" + src = AUTH_FORM.read_text() + count = src.count("!hasBootstrapPassword") + assert count == 1, ( + f"expected exactly one !hasBootstrapPassword usage, found {count}; " + "extra conditionals can hide or duplicate the always-on inputs" + ) + + +def test_current_password_input_is_inside_the_hasBootstrapPassword_conditional(): + """`id="current-password"` MUST sit inside `{!hasBootstrapPassword && (...)}`. + Otherwise the input renders on first boot too, regressing the + pre-#5490 two-input UX that PR #5545 restores.""" + src = AUTH_FORM.read_text() + s, e = _conditional_extent(src) + idx = src.find('id="current-password"') + assert idx != -1, "the Current password input was removed entirely" + assert s < idx < e, ( + "Current password input is rendered unconditionally; this is the " + "PR #5490 regression -- on first boot the bootstrap-derived " + "password is reused silently and only New + Confirm should render" + ) + + +def test_new_password_input_is_outside_the_hasBootstrapPassword_conditional(): + """`id="new-password"` MUST sit outside `{!hasBootstrapPassword && (...)}`. + Otherwise it disappears on admin-forced resets, regressing PR #5490.""" + src = AUTH_FORM.read_text() + s, e = _conditional_extent(src) + idx = src.find('id="new-password"') + assert idx != -1, "the New password input was removed entirely" + assert not (s < idx < e), ( + "New password is wrapped in !hasBootstrapPassword; that would " + "hide the field on admin-forced resets, regressing PR #5490. " + "New password must always render in change-password mode." + ) + + +def test_confirm_password_input_is_outside_the_hasBootstrapPassword_conditional(): + """Same as New password, for `id="confirm-password"`.""" + src = AUTH_FORM.read_text() + s, e = _conditional_extent(src) + idx = src.find('id="confirm-password"') + assert idx != -1, "the Confirm password input was removed entirely" + assert not (s < idx < e), ( + "Confirm password is wrapped in !hasBootstrapPassword; same " + "regression as New password -- it must always render in " + "change-password mode." + ) + + +def test_change_password_jsx_declares_exactly_three_password_inputs(): + """The change-password JSX block (`{!isLoginMode && (...)}`) must + declare exactly the three known password inputs -- current, new, + confirm. A fourth would almost certainly break the 2-input + first-boot contract because the conditional only hides the + Current input, not any new one a future PR might add.""" + src = AUTH_FORM.read_text() + start = src.find("{!isLoginMode && (") + assert start != -1, ( + "the change-password JSX subtree marker {!isLoginMode && (...)} " + "is missing; the file's structure has drifted" + ) + # Match the corresponding `)}` for {!isLoginMode && (...)}. + depth = 1 + i = start + len("{!isLoginMode && (") + while i < len(src) and depth > 0: + c = src[i] + if c == "(": + depth += 1 + elif c == ")": + depth -= 1 + i += 1 + subtree = src[start:i] + ids = sorted(re.findall(r'id="([a-z-]+-password)"', subtree)) + assert ids == [ + "confirm-password", + "current-password", + "new-password", + ], ( + "change-password JSX must declare exactly current-password, " + f"new-password, confirm-password; found {ids!r}. A fourth " + "password input would almost certainly break the 2-input " + "first-boot contract." + ) + + +def test_login_jsx_declares_exactly_one_password_input(): + """The login JSX block (`isLoginMode && (...)`) must declare + exactly one password input -- the bootstrap password the user + pastes from the CLI. Adding a second here would break the + matrix that the per-mode tests assume.""" + src = AUTH_FORM.read_text() + start = src.find("{isLoginMode && (") + assert start != -1, "the login JSX subtree marker is missing" + depth = 1 + i = start + len("{isLoginMode && (") + while i < len(src) and depth > 0: + c = src[i] + if c == "(": + depth += 1 + elif c == ")": + depth -= 1 + i += 1 + subtree = src[start:i] + ids = re.findall(r'id="([a-z-]+)"', subtree) + # The login subtree currently uses id="password". Lock the count + # rather than the spelling so a rename does not falsely fail. + pw_ids = [x for x in ids if "password" in x] + assert len(pw_ids) == 1, ( + f"login JSX must declare exactly one password-typed input; " f"found {pw_ids!r}" + )