fix: resolve dashboard login hidden-state bug and rename auth routes#3
fix: resolve dashboard login hidden-state bug and rename auth routes#3dxu104 wants to merge 1 commit intowillynikes2:masterfrom
Conversation
…to session
- Add global [hidden] { display: none !important } to fix login screen staying
visible after successful sign-in (CSS display rule was overriding hidden attr)
- Rename /api/auth/* endpoints to /api/session/* for clarity
- Upgrade better-sqlite3 from v11 to v12
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
willynikes2
left a comment
There was a problem hiding this comment.
Multi-Agent Code Review (Claude + Codex + Gemini)
Hey @dxu104 — thanks for the PR! The CSS fix is solid and we'd like to get it merged. A few things need addressing first:
What's good
- The
[hidden] { display: none !important }fix correctly solves the login screen bug. The root cause is.login-screen { display: flex }overriding the browser's default[hidden]styling — your fix is the right approach. - No security issues found with the route rename.
authMiddlewarestill protects the password endpoint, and session cookies remainHttpOnly+SameSite=Strict.
Requested changes
1. Node engine mismatch (blocker)
package.json declares "engines": { "node": ">=18.0.0" } but better-sqlite3@12 requires node: "20.x || 22.x || 23.x || 24.x || 25.x". This will break installs on Node 18/19. Please either:
- Bump the engine field to
>=20.0.0, or - Keep
better-sqlite3@11if Node 18 support is needed
2. Inconsistent route naming
/api/login and /api/logout stay at the old paths while /api/auth/check and /api/auth/password moved to /api/session/*. This creates a split API surface. Please either:
- Move all dashboard auth endpoints under
/api/session/*(/api/session/login,/api/session/logout,/api/session/check,/api/session/password), or - Revert to
/api/auth/*for consistency
3. Rename the file
src/routes/auth-routes.js now serves /api/session/* routes — please rename to session-routes.js to match.
4. Docs check
Please verify the OpenAPI spec (src/routes/openapi.js) and README don't reference the old /api/auth/check or /api/auth/password endpoints.
The CSS fix alone is worth merging if you'd prefer to split this into two PRs (one for the CSS fix, one for the route rename + dep bump). Let us know which approach you prefer!
🤖 Review powered by Claude Code + OpenAI Codex + Google Gemini
Summary
[hidden] { display: none !important; }(CSS display rule was overriding the hidden attribute)/api/auth/*endpoints to/api/session/*for claritybetter-sqlite3from v11 to v12Test plan
kb startand openlocalhost:3838/api/session/checkand/api/session/passwordendpoints work correctly🤖 Generated with Claude Code