Fix runtime status error on conversation resume#12718
Merged
Conversation
Contributor
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
malhotra5
reviewed
Feb 2, 2026
Contributor
|
Can this behavior change be reflected in either |
Tests cover the behavior when a sandbox is marked as RUNNING but has no execution_status (indicating the server may still be starting): - Server responds with uptime within grace period -> shows STARTING - Server responds with uptime past grace period -> shows RUNNING - Server is unresponsive -> shows STARTING - Sandbox with execution_status set -> skips server check - Non-RUNNING sandbox -> skips server check Co-authored-by: openhands <openhands@all-hands.dev>
malhotra5
reviewed
Feb 2, 2026
|
|
||
|
|
||
| @pytest.mark.asyncio | ||
| async def test_get_conversation_running_sandbox_no_execution_status_server_responds_within_grace_period(): |
Collaborator
There was a problem hiding this comment.
NIT: maybe we can parameterize these tests
Consolidate 5 separate test functions into a single parameterized test for brevity, as suggested in code review. Co-authored-by: openhands <openhands@all-hands.dev>
|
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Summary of PR
This PR fixes an issue where resumed conversations would incorrectly show an error status during startup. The problem occurs because the runtime API marks sandboxes as
RUNNINGbefore they are actually fully started, particularly affecting resumed runtimes.Changes in
openhands/server/routes/manage_conversations.py:get_conversation()to handle the case where a sandbox is marked asRUNNINGbut has no execution status/server_infoendpoint_RESUME_GRACE_PERIOD), the sandbox status is set toSTARTINGinstead of potentially showing an errorThis is a temporary workaround for a bug in the runtime API that marks servers as RUNNING before they are actually started.
Demo Screenshots/Videos
When we resume a conversation...
** We have an initial starting state **
Before changes - We get an error state during startup
After Changes - The status displays as
STARTINGuntil the value is actually runningFinally it is ready
Change Type
Checklist
Fixes
Resolves #(issue)
Release Notes
Fixed an issue where resumed conversations would incorrectly display an error status during startup. The system now properly detects when a sandbox is still initializing and shows a "starting" status instead of an error.
To run this PR locally, use the following command:
GUI with Docker: