Skip to content

codex/supervaizer-registration-refresh - #32

Merged
alain-sv merged 4 commits into
developfrom
codex/supervaizer-registration-refresh
May 4, 2026
Merged

codex/supervaizer-registration-refresh#32
alain-sv merged 4 commits into
developfrom
codex/supervaizer-registration-refresh

Conversation

@alain-sv

@alain-sv alain-sv commented May 4, 2026

Copy link
Copy Markdown
Contributor
  • Add a new registration refresh (feat: add registration refresh endpoint)
  • Add tests documentation entry to CHANGELOG (feat(changelog): summary to CHANGELOG)

alain-sv added 2 commits May 4, 2026 19:54
…docs/CHANGE.md that records the test suite for the current release. entrydocuments the `just test` command includes a small table withstatus (passed skipped, failed and total runtime.

This improves release transparency surfacing CI/testresults directly in theelog so readers can quicklytest health for the.
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Add registration refresh endpoint with comprehensive tests

✨ Enhancement 🧪 Tests

Grey Divider

Walkthroughs

Description
• Add registration refresh endpoint for asynchronous server re-registration
• Endpoint requires write-scope API key and supervisor account configuration
• Comprehensive test coverage for authentication, authorization, and functionality
• Update CHANGELOG with endpoint details and test suite results
Diagram
flowchart LR
  A["Studio Request"] -->|POST /registration/refresh| B["refresh_controller_registration"]
  B -->|validate supervisor account| C{Account exists?}
  C -->|yes| D["Background Task"]
  C -->|no| E["503 Service Unavailable"]
  D -->|_send_registration_refresh| F["register_server"]
  F -->|log result| G["Completion"]
Loading

Grey Divider

File Changes

1. src/supervaizer/contracts.py ✨ Enhancement +2/-0

Add registration refresh endpoint contract definition

• Add POST_CONTROLLER_REGISTRATION_REFRESH enum value to ControllerEndpoint
• Map endpoint to /api/supervaizer/registration/refresh route in CONTROLLER_ENDPOINTS dictionary

src/supervaizer/contracts.py


2. src/supervaizer/routes.py ✨ Enhancement +44/-0

Implement registration refresh endpoint and request model

• Add RegistrationRefreshRequest model with optional reason and requested_at fields
• Implement _send_registration_refresh helper function to handle asynchronous registration
• Add POST /registration/refresh route with 202 Accepted response, write-scope authentication, and
 background task scheduling
• Route validates supervisor account availability and returns error if not configured

src/supervaizer/routes.py


3. tests/test_contracts.py 🧪 Tests +4/-0

Add contract test for registration refresh endpoint

• Add assertion to verify POST_CONTROLLER_REGISTRATION_REFRESH endpoint maps to correct API path

tests/test_contracts.py


View more (2)
4. tests/test_routes.py 🧪 Tests +82/-0

Add comprehensive tests for registration refresh endpoint

• Add import for StringIO and log module
• Add import for get_server dependency function
• Add test for successful registration refresh with background task execution and logging
 verification
• Add test verifying API key authentication requirement
• Add test verifying supervisor account configuration requirement with 503 response

tests/test_routes.py


5. docs/CHANGELOG.md 📝 Documentation +15/-0

Document registration refresh endpoint and test results

• Add "Added" section documenting the new POST /api/supervaizer/registration/refresh endpoint with
 authentication and functionality details
• Add "Tests" section with test suite results table showing 563 passed tests, 0 skipped, 0 failed,
 and 69s runtime

docs/CHANGELOG.md


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented May 4, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Unhandled refresh task exceptions🐞 Bug ☼ Reliability
Description
_send_registration_refresh() awaits server.supervisor_account.register_server() without catching
exceptions; failures from the underlying HTTP call will bubble out of the FastAPI background task,
creating unhandled server errors even though the endpoint already returned 202 Accepted. This can
make refresh requests appear accepted while the registration refresh actually fails and only
manifests as runtime exceptions/log noise.
Code

src/supervaizer/routes.py[R136-149]

+async def _send_registration_refresh(
+    server: "Server", request_data: RegistrationRefreshRequest
+) -> None:
+    """Ask the configured supervisor account to process normal server registration."""
+    if not getattr(server, "supervisor_account", None):
+        log.warning("Registration refresh requested but no supervisor account is configured")
+        return
+    result = await server.supervisor_account.register_server(server=server)
+    log.info(
+        "Registration refresh completed with result={} reason={} requested_at={}",
+        result.__class__.__name__,
+        request_data.reason,
+        request_data.requested_at,
+    )
Evidence
The new background task calls Account.register_server() directly and does not wrap it in any
try/except. The registration flow ultimately calls account_service.send_event(), which
explicitly re-raises connection/timeouts and raises on HTTP errors, so exceptions are expected
during outages and will propagate out of the background task unhandled.

src/supervaizer/routes.py[136-149]
src/supervaizer/account.py[204-223]
src/supervaizer/account_service.py[109-152]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`_send_registration_refresh()` awaits `server.supervisor_account.register_server()` without exception handling. The registration flow can raise `httpx` errors (connect/timeout/HTTP), which will escape the FastAPI background task and become unhandled server-side errors even though the client already received `202 Accepted`.
### Issue Context
- `Account.register_server()` delegates to `send_event()`.
- `send_event()` can re-raise connection/timeouts and raises on other HTTP errors.
- This endpoint is meant to be an asynchronous trigger; failures should be logged clearly and not surface as unhandled task exceptions.
### Fix Focus Areas
- src/supervaizer/routes.py[136-149]
### Suggested change
- Wrap the `await server.supervisor_account.register_server(...)` call in `try/except Exception`.
- On exception, log with `log.exception(...)` (include `reason`/`requested_at`) and return.
- Optionally include retry/backoff or convert the refresh to a tracked job if you need failure visibility beyond logs.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/supervaizer/routes.py
alain-sv added 2 commits May 4, 2026 23:02
…_registration_refresh returns early when no supervisor account is configured, avoiding attribute access errors.

- Wrap supervisor call in try/except catch and log any exceptions from register_server (including http connection errors)
 and prevent background propagation.
- Add detailed log message on failure reason and requested_at for observability.
- Update tests: import asyncio and http, expose RegistrationRefreshRequest and __registration_refresh for testing, and add a test that simulates register_server raising httpx.Connect to assert the failure is logged and not propagate.

Motivation: make background registration refresh resilient to missingsupervisor configuration and network/HTTP errors, and add coverage toprevent regressions.
…s and to Python3.+ styleand native collection types to simplify type hints and reduceredundant typing imports.

- Replace typing.List/Dict/Optional/AsyncGenerator built-in listict/Type | None and P604 union syntax (X | None).
- Use dict[str, str] queues and listeners and update annotated return types accordingly.
- Normalize OptionalRequest] -> Request | None across helper APIs.
- unused AsyncGenerator import and collections.abc import for AsyncGenerator where needed- Keep runtime behavior unchanged; changes are purely type annotation modernizations to improve readability and align with Python conventions.
@alain-sv
alain-sv merged commit 2c1c105 into develop May 4, 2026
6 checks passed
@alain-sv
alain-sv deleted the codex/supervaizer-registration-refresh branch May 13, 2026 13:18
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.

1 participant