Conversation
MdTanwer
force-pushed
the
feat/4973-typed-acp-contracts
branch
from
September 24, 2026 07:13
4199ab5 to
9abf1ea
Compare
MdTanwer
force-pushed
the
feat/4973-typed-acp-contracts
branch
from
September 24, 2026 07:14
9abf1ea to
2ff7ef3
Compare
- Introduce openhands.sdk.agent.acp_contracts with typed dataclasses, protocols, and boundary normalizers - Replace dynamic attribute probing (getattr, setattr, hasattr) across acp_agent.py, acp_models.py, acp_file_credentials.py, and acp_tracing.py - Unroll task cleanup loop in ACPAgent to use explicit attribute access - Shrink forbidden dynamic attributes baseline by eliminating all 23 ACP entries - Add comprehensive unit tests in test_acp_contracts.py Resolves OpenHands#4973 Co-authored-by: openhands <openhands@all-hands.dev>
MdTanwer
force-pushed
the
feat/4973-typed-acp-contracts
branch
from
September 24, 2026 07:17
2ff7ef3 to
ae6a849
Compare
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
ACP integration code previously used 23 dynamic attribute calls (
getattr,setattr,hasattr) across 4 files (acp_agent.py,acp_models.py,acp_file_credentials.py,acp_tracing.py) to probe objects from external ACP libraries due to protocol version differences (0.8.xRootModelvs 0.10.x union), UNSTABLE extensions (models), error data payloads, credential revisions, and background task slots.Per #4973 (parent #4905), ACP integration code should express these capabilities and options through explicit typed models or protocols, with compatibility normalization strictly confined to the external-library boundary.
Summary
openhands.sdk.agent.acp_contractsdefining typed dataclasses (ACPAuthMethod,ACPConfigSelectOption,ACPSessionModelState,ACPMcpCapabilities,ACPErrorInfo), runtime-checkable protocols, and boundary normalization helpers (extract_model_config_option,extract_session_models,normalize_auth_method,normalize_mcp_capabilities,normalize_acp_error).acp_agent.py,acp_models.py,acp_file_credentials.py, andacp_tracing.pyto use typed direct attribute access, eliminating all 23 dynamic calls and unrolling the background task cleanup loop.scripts/forbidden_dynamic_attributes_baseline.jsonby removing all 23 ACP entries (-115 lines, 0 ACP dynamic violations remaining).Issue Number
Resolves #4973
Parent: #4905
How to Test
Run the forbidden dynamic attribute checker to verify no new or stale violations exist:
Run the unit tests covering the new contracts and boundary normalizers:
Check linting and formatting:
Video/Screenshots
Command output evidence:
Design Doc
Type
Notes
scripts/forbidden_dynamic_attributes_baseline.jsonhave been eliminated.scripts/check_forbidden_dynamic_attributes.pypath handling cross-platform (posix normalization and UTF-8 encoding).