fix(settings): restore MCP schema migration#4013
Conversation
Python API breakage checks — ✅ PASSEDResult: ✅ PASSED |
REST API breakage checks (OpenAPI) — ✅ PASSEDResult: ✅ PASSED |
Coverage Report •
|
|||||||||||||||||||||||||||||||||||
all-hands-bot
left a comment
all-hands-bot
left a comment
There was a problem hiding this comment.
✅ QA Report: PASS
The SDK settings API now migrates v4 MCP persisted payloads to schema version 5 while preserving SDK-native MCP server-map/auth data.
Does this PR achieve its stated goal?
Yes. I exercised the SDK settings validation path with realistic v4 persisted MCP payloads: an SDK-native OAuth server map and a legacy mcpServers wrapper. On origin/main, both payloads still validated/dumped as schema version 4; on PR commit 2404f41838a2f92fd504fde39f6a731d8fb6aa8d, the same inputs validated/dumped as schema version 5, the mcpServers wrapper was normalized away, and OAuth strategy/client/token state was preserved. That directly confirms the claimed v4 → v5 migration behavior works.
| Phase | Result |
|---|---|
| Environment Setup | ✅ make build completed successfully with uv sync --dev and pre-commit hook setup. |
| CI Status | PR Description Check) when checked; I did not rerun tests. |
| Functional Verification | ✅ Before/after SDK execution confirmed schema migration and MCP shape preservation. |
Functional Verification
Test 1: v4 persisted MCP settings migrate to v5 and keep SDK-native shape
Step 1 — Reproduce / establish baseline without the fix:
Ran git checkout origin/main && OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/qa_settings_migration_compare.py.
Relevant output:
CASE v4_sdk_native_oauth
{
"auth_strategy": "oauth2",
"dumped_schema_version": 4,
"has_mcpServers_wrapper": false,
"mcp_config_keys": [
"superhuman-mail"
],
"oauth_access_token": "**********",
"oauth_client_id": "client-id",
"settings_schema_version": 4
}
CASE v4_legacy_mcpServers_wrapper
{
"auth_strategy": null,
"dumped_schema_version": 4,
"has_mcpServers_wrapper": false,
"mcp_config_keys": [
"weather"
],
"oauth_access_token": null,
"oauth_client_id": null,
"settings_schema_version": 4
}This confirms the baseline problem: v4 MCP settings can normalize the MCP shape, but they remain persisted/validated as schema version 4, so there is no explicit migration to the new persisted schema version.
Step 2 — Apply the PR's changes:
Checked out PR commit 2404f41838a2f92fd504fde39f6a731d8fb6aa8d.
Step 3 — Re-run with the fix in place:
Ran OPENHANDS_SUPPRESS_BANNER=1 uv run python /tmp/qa_settings_migration_compare.py.
Relevant output:
CASE v4_sdk_native_oauth
{
"auth_strategy": "oauth2",
"dumped_schema_version": 5,
"has_mcpServers_wrapper": false,
"mcp_config_keys": [
"superhuman-mail"
],
"oauth_access_token": "**********",
"oauth_client_id": "client-id",
"settings_schema_version": 5
}
CASE v4_legacy_mcpServers_wrapper
{
"auth_strategy": null,
"dumped_schema_version": 5,
"has_mcpServers_wrapper": false,
"mcp_config_keys": [
"weather"
],
"oauth_access_token": null,
"oauth_client_id": null,
"settings_schema_version": 5
}This shows the fix works: the exact same v4 inputs now migrate to schema version 5, the legacy wrapper is not persisted, and the OAuth server-map auth fields remain present after validation/dump. The masked token is expected secret serialization behavior.
Issues Found
None from functional QA.
This review was generated by an AI agent (OpenHands) on behalf of the user.
Co-authored-by: openhands <openhands@all-hands.dev>
neubig
force-pushed
the
fix-settings-mcp-schema-migration
branch
from
2404f41 to
a5b9e19
Compare
Co-authored-by: openhands <openhands@all-hands.dev>
2 participants
Summary
This PR restores an explicit persisted settings migration for the MCP settings shape change from PR #3964 instead of relying on compatibility validators/shims.
Changes:
AGENT_SETTINGS_SCHEMA_VERSIONto 5.mcp_configbefore-validators that accepted legacy MCP wrappers at the current schema version. Current-version settings now reject legacymcpServerswrappers; only migrations accept old persisted shapes.event_compat.py) and its tests so event HTTP/WebSocket payloads are serialized directly rather than silently dropping newer event fields/tool kinds.Test Plan
uv run pytest -q tests/sdk/test_settings.py tests/sdk/test_apply_agent_settings_diff.py tests/cross/test_check_persisted_settings_compat.py tests/sdk/mcp/test_create_mcp_tool.py tests/agent_server/test_settings_router.py tests/sdk/plugin/test_plugin_loader.py tests/sdk/conversation/test_local_conversation_plugins.pyuv run pytest -q tests/sdk/test_settings.py tests/cross/test_check_persisted_settings_compat.py tests/agent_server/test_event_router.py tests/agent_server/test_event_router_websocket.py tests/agent_server/test_settings_router.pyuv run ruff format openhands-agent-server/openhands/agent_server/event_router.py openhands-agent-server/openhands/agent_server/sockets.py tests/agent_server/test_event_router.py tests/agent_server/test_event_router_websocket.py openhands-sdk/openhands/sdk/settings/model.py tests/sdk/test_settings.pyuv run ruff check openhands-sdk/openhands/sdk/settings/model.py tests/cross/test_check_persisted_settings_compat.py tests/sdk/test_settings.pyThis PR was created by an AI agent (OpenHands) on behalf of the user.
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:2b1a263-pythonRun
All tags pushed for this build
About Multi-Architecture Support
2b1a263-python) is a multi-arch manifest supporting both amd64 and arm642b1a263-python-amd64) are also available if needed