test: reproduce #4813 state ownership regressions and restore - #29
Draft
enyst wants to merge 1 commit into
Draft
Conversation
Co-authored-by: openhands <openhands@all-hands.dev>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks 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 |
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
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.
HUMAN:
AGENT:
Why
This tests-only follow-up is for OpenHands/software-agent-sdk#4813, at
a4e7a74c0fb8f989a2c58ca1717c176f0aa1aaff. Its base,enyst:review-target/pr4813-a4e7a74, is an exact mirror of that head. Engel requested executable regressions for the five review findings and conversation-restore coverage undertests/cross.The state-ownership refactor fixes the original restart bug, but these tests reproduce five remaining regressions. The five failing tests are intentional ordinary assertions. They are not marked xfail or skipped. Please make them green by fixing the implementation in OpenHands#4813.
Summary
Adds
tests/cross/test_conversation_state_ownership.pyand shared fixtures intests/cross/conftest.py. No production code changes.test_acp_stream_masks_secrets_added_after_starttest_server_fork_preserves_request_secretstest_server_fork_preserves_guardrail_credentialsNone.test_encrypted_start_decrypts_guardrail_credentialstest_local_fork_keeps_guardrail_history_independentThe five passing restore cases cover:
meta.jsonfields not overriding canonical state on restore.Inference uses the SDK's
TestLLM. Its scripted response queue is reattached after serialization before resumed execution. The ACP test replaces the subprocess handshake while running the real bridge setup, incoming session update, and streaming callback. The restore cases use realConversationService/LocalConversationinstances and temporary encrypted files; no live provider credentials are needed.Issue Number
OpenHands#4810
How to Test
Verified against OpenHands#4813 at
a4e7a74: 5 failed, 5 passed. Every failure reaches the assertion for its corresponding review finding.Verified: 5 failed, 18 passed. All 13 existing restore tests and all five new restore cases pass.
Formatting, lint, type checking, import rules, and tool-registration checks pass.
Local validation used Python 3.13.15 and the existing SDK development environment, with
LITELLM_LOCAL_MODEL_COST_MAP=Trueto use the bundled model metadata.Video/Screenshots
Not applicable. Reproductions and expected assertion failures are listed above.
Design Doc
Not applicable; tests only.
Type
Notes
GitHub's connection returned
403: Resource not accessible by integrationwhen creating this PR directly againstBSmick6/software-agent-sdk:claude/github-issue-4810-51c120. This draft uses the exact mirrored base in the enyst fork instead; merging it here will not update the original PR.To apply the tests to the original PR branch:
Created by Codex for Engel's review. The branch starts directly at OpenHands#4813's reviewed head so it adds only these tests. The enyst fork's
mainwas independently fast-forwarded to upstream50080b58d35b4824fda25fca2345d80bcd08aeff.