Add ServerProvider for test restart scenarios #833
+522
−133
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BREAKING CHANGES:
- Room creation is now separate from joining:
- POST /api/rooms creates a new room (returns 201)
- POST /api/rooms/{id}/join joins existing rooms only (returns 404 if not exists)
- Password hashing upgraded from SHA-256 to Argon2id (existing passwords invalidated)
- JWT tokens now have 7-day expiration
Key changes:
- Add ensure_user_exists() to UserService for idempotent user creation
- Users are created on socket connect from JWT claims, not during login
- JWT is now the single source of truth for identity
- Simplified login endpoint - guests get JWT without Redis entry until socket connect
- Added argon2-cffi dependency for secure password hashing
- Frontend: Removed redundant role state, use getRoleFromToken() only
- Updated all tests to use create-then-join pattern
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This refactoring enforces strict single responsibility per endpoint: - /api/user/register: ONLY place for user creation (guest or registered) - /api/login: ONLY authenticates users (never creates) - Socket connect and room join validate user exists Key changes: - Add password validation (minimum 8 characters) - Add get_authoritative_role() for Redis-based role checks - Add is_admin_username() to AdminService for proper error messages - Replace deprecated datetime.utcnow() with time utilities - Remove redundant USERNAME_KEY from frontend localStorage - Update Python client to use register-then-login flow All users must now register before login. Guest flow: 1. POST /api/user/register (creates user) 2. POST /api/login (authenticates, returns JWT) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Security fixes: - Unify all auth error messages to "Authentication failed" to prevent username enumeration attacks - Add username validation (1-64 chars, alphanumeric + underscore/hyphen) to prevent path injection, XSS, and DoS attacks Bug fixes: - Fix 409 handling in frontend auth.ts - now properly handles case where registerData.userName is undefined on conflict response Code cleanup: - Remove unused get_authoritative_role() function (YAGNI) - Update get_current_user_role() docstring to reference AdminService Test updates: - Update tests to expect generic "Authentication failed" message - Fix test usernames to comply with new validation (John Doe → john-doe) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Upgrade argon2-cffi from >=23.1.0 to >=25.1.0 (current stable) - Use datetime.timezone.utc instead of datetime.UTC for Python 3.11.x compatibility - Replace deprecated datetime.utcnow() with utc_now() utility in tests - Add clarifying comment for username regex pattern (1 + 0-63 = 1-64 chars) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Sanitize worker_id to valid username by replacing ':' with '-'
(celery:{uuid} format violates username pattern)
- Remove unused 'public' parameter from _get_extension_class (YAGNI)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Move PasswordValidationError import to module level - Remove redundant exception object from log.exception calls - Add specific error message for 409 Conflict in room creation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Change PID file from single `~/.zndraw/server.pid` to port-specific
`~/.zndraw/server-{port}.pid` files
- Remove `--force-new-server` flag (no longer needed with port-based detection)
- Add `find_running_server(port)` function for auto-discovery:
- If port specified, only checks that port
- If port is None, checks default port (5000) first, then smallest port
- Update all dependent code to use new API:
- cli.py: Use find_running_server for server detection
- zndraw.py: Use find_running_server for auto-discovery
- zndraw-mcp: Use find_running_server for status checks
- tests: Remove --force-new-server, pass port to remove_server_info
This enables running multiple ZnDraw servers simultaneously on different
ports, each with their own PID file for proper lifecycle management.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix redundant exception handling: remove ConnectionError (subclass of RequestException) - Skip default port in find_running_server loop to avoid double-checking - Update docstring for get_current_state with proper return type docs - Improve error messages with actionable guidance Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add unit tests for server_manager.py (22 tests): - PID file path generation (port-specific paths) - Read/write/remove server info operations - list_all_pid_files discovery - find_running_server auto-discovery logic - get_server_status with stale file cleanup - Add CLI tests for --status and --shutdown flags (14 tests): - Mock-based tests for CLI behavior - Port-specific vs auto-discovery behavior - Integration tests with real server fixture Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
- Add tests/server_provider.py with ServerProvider class for managing server lifecycle in tests (start/stop/restart/fresh_restart) - Add flush_redis() and fresh_restart() for complete server resets - Add server_provider fixture to conftest.py - Add config singleton reset to _create_server_process for test isolation - Refactor test_zndraw_admin.py to use conftest fixtures (removes 116 lines of duplicate code) - Add tests for ServerProvider (12 tests) and extension persistence (2 tests) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
for more information, see https://pre-commit.ci
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #833 +/- ##
==========================================
+ Coverage 79.33% 79.52% +0.19%
==========================================
Files 156 159 +3
Lines 19075 19221 +146
==========================================
+ Hits 15133 15286 +153
+ Misses 3942 3935 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
server lifecycle in tests (start/stop/restart/fresh_restart)
lines of duplicate code)
Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com
TODO