Commit f681e1

2026-03-19 21:34:15 Claude (MCP): [api] Edit: Design/Server_Consolidation
Design/Server_Consolidation.md ..
@@ 166,9 166,59 @@
7. Remove old `auth_server.py` and `api_server.py`
8. Resume E2E test implementation with simplified fixtures
- ## Risks
+ ## Risks and Review Findings
- - **Merge complexity**: The two app factories have different initialization patterns. Reconciling them requires care but isn't architecturally novel.
- - **Template path changes**: Every `render_template()` call needs updating. Mechanical but easy to miss one.
- - **Existing unit tests**: Tests that import `auth_server.create_app()` or `api_server._create_flask_app()` need updating. Many tests — but the change is the same for each.
- - **Deployment window**: The Ansible change removes one service and modifies Caddy. Brief downtime for auth routes during deploy. Mitigate by deploying the combined service first (both ports), then removing the old auth service.
+ Plan reviewed 2026-03-19. Core approach confirmed sound. Key findings:
+
+ ### Template migration (important)
+
+ Not just `render_template()` calls — the `{% extends %}` directives inside templates must also update:
+ - Auth templates: `{% extends 'base.html' %}` → `{% extends 'auth/base.html' %}`
+ - Management templates: `{% extends "layout.html" %}` → `{% extends "management/layout.html" %}`
+
+ 6 auth + 4 management template files affected.
+
+ ### App factory interface (important)
+
+ Expose two functions:
+ - `create_app()` — Flask-only factory for unit tests (no ManagementMiddleware)
+ - `build_app()` — Full WSGI app with ManagementMiddleware for production/Gunicorn
+
+ Currently `test_management_ui.py` uses `_create_flask_app()` and `test_auth_server.py` uses `create_app()`. Both should map to the new `create_app()`. 3 test files, ~46 import callsites (mechanical).
+
+ ### Error handlers (important)
+
+ Both apps define `errorhandler(429)`, `errorhandler(500)`, `errorhandler(400)`. Flask allows only one handler per status code. Simplest fix: shared error template in `app/templates/error.html`.
+
+ ### Deployment strategy (important)
+
+ Correct zero-downtime approach:
+ 1. Deploy `robot-platform` on port 8002 (new service)
+ 2. Keep `robot-auth` on 8003 running temporarily
+ 3. Verify platform handles `/app/*` and `/api/*`
+ 4. Update Caddy to route `/auth/*` to 8002
+ 5. Stop and disable `robot-auth`
+
+ ### Caddyfile management (important)
+
+ The Caddyfile is NOT in `ansible/roles/deploy/templates/`. Must find which role manages it and update there. All VPS changes must be in Ansible.
+
+ ### Hardcoded service references (important)
+
+ - `admin_stats()` hardcodes `["robot-otterwiki", "robot-mcp", "robot-api", "robot-auth"]` for systemctl/journalctl — must update
+ - `smoke-test.sh` hardcodes port 8003 checks — must update
+ - `healthcheck` role defaults reference port 8003 — must update
+
+ ### Environment variables (important)
+
+ `CLIENT_JWK_PATH` is only in `robot-auth.service` env. Must propagate to consolidated service.
+
+ ### ProxyFix (minor)
+
+ Apply once at outermost WSGI layer. Don't accidentally apply twice.
+
+ ### Confirmed correct
+
+ - ManagementMiddleware passes through `/auth/*` (line 162-165)
+ - Flask-Limiter + WSGIRateLimiter coexist without conflict
+ - Static file serving has no conflicts
0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9