Repository navigation
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Coverage Report •
|
||||||||||||||||||||
|
@OpenHands /codereview and post your feedback as review with smolpaws token; feel free to post inline findings if any but it's okay if not |
|
I'm on it! enyst can track my progress at all-hands.dev |
smolpaws
left a comment
There was a problem hiding this comment.
🟢 Good taste — The persisted catalog is now clearly authoritative, read-time repair of private runtime drift is gone, and startup publishes the catalog record before exposing the live runtime.
Focused validation at 35b60a75:
uv run pytest -q tests/agent_server/test_conversation_service.py tests/cross/test_conversation_lease_behavior.py -x— 98 passedgit diff --check pr-4122-base HEAD— passed
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
The change is localized to conversation catalog/runtime lifecycle handling, adds no dependency or public API changes, and has focused coverage for persisted round trips, unloaded deletion, lease contention, waiting hydration, and record-before-runtime publication.
VERDICT:
✅ Worth merging: The core invariant is simpler and the implementation is sound.
KEY INSIGHT:
Making the persisted catalog the single authority removes hidden repair behavior and makes catalog/runtime ownership easier to reason about.
This review was generated by an OpenHands AI agent on behalf of the requester and posted through the smolpaws account.
|
OpenHands encountered an error: **Failed to fetch final response from agent server: HTTP 503 error: no available server See the conversation for more information. |
|
[Automatic Post]: It has been a while since there was any activity on this PR. @enyst, are you still working on it? If so, please go ahead, if not then please request review, close it, or request that someone else follow up. This comment was created by an AI agent (OpenHands) on behalf of the user. |
|
This PR is stale because it has been open for 40 days with no activity. Remove the stale label or leave a comment, otherwise it will be closed in 10 days. |
|
This PR was closed because it had no activity for 50 days. If you feel this was closed in error, and you would like to continue the PR, please resubmit or let us know. |
Summary
Follow-up to #4100 and the simplification feedback in #4100 (comment).
_reconcile_active_records()and missing-record repair pathsValidation
uv run pre-commit run --files openhands-agent-server/openhands/agent_server/conversation_service.py tests/agent_server/test_conversation_service.py— passeduv run pytest -q tests/agent_server/test_conversation_service.py tests/cross/test_conversation_lease_behavior.py -x— 98 passedgit diff --check— passedBaseline note
tests/agent_server/test_sockets_service_getters.py::test_events_socket_uses_app_state_conversation_servicecurrently fails because anAsyncMockreachesjson.loads(). The same failure reproduces from clean merged-main commitd06ff0b3; this PR does not modify the socket implementation or that test.This pull request was created by an AI agent (OpenHands) on behalf of the requester.
@enyst can click here to continue refining the PR
Agent Server images for this PR
• GHCR package: https://github.com/OpenHands/agent-sdk/pkgs/container/agent-server
Variants & Base Images
eclipse-temurin:17-jdknikolaik/python-nodejs:python3.13-nodejs22-slimgolang:1.21-bookwormPull (multi-arch manifest)
# Each variant is a multi-arch manifest supporting both amd64 and arm64 docker pull ghcr.io/openhands/agent-server:35b60a7-pythonRun
All tags pushed for this build
About Multi-Architecture Support
35b60a7-python) is a multi-arch manifest supporting both amd64 and arm6435b60a7-python-amd64) are also available if needed