fix(runtime): use lowercase wire role for v0.3 JSON-RPC compat layer - #2187
Merged
hongmingwang-moleculeai merged 1 commit intoApr 27, 2026
Merged
Conversation
Manual-test failure surfaced what was hidden behind the MCP-path bug:
once delegate_task could actually fire, every cross-workspace call
came back as JSON-RPC -32600 "Invalid Request" with the underlying
pydantic ValidationError:
params.message.role
Input should be 'agent' or 'user' [type=enum,
input_value='ROLE_USER', input_type=str]
PR #2184's a2a-sdk 1.x migration sweep over-corrected: it changed
every `"role": "user"` literal in JSON-RPC payload construction to
`"role": "ROLE_USER"` to match the protobuf enum names of the 1.x
native types (a2a.types.Role.ROLE_USER / ROLE_AGENT). That was
correct for in-process Message construction (which the SDK
serialises before wire transmission) but WRONG for the 8 sites that
hand-build JSON-RPC payloads. The workspace's own a2a-sdk runs
inbound requests through the v0.3 compat adapter
(/usr/local/lib/python3.11/site-packages/a2a/compat/v0_3/) because
main.py sets enable_v0_3_compat=True for backwards compatibility,
and that adapter validates against the v0.3 Pydantic Role enum
(`agent` | `user` lowercase). The protobuf-style names blow it up.
Reverted the 8 wire-payload sites to lowercase:
- workspace/a2a_client.py:74
- workspace/a2a_cli.py:74, 111
- workspace/heartbeat.py:378
- workspace/main.py:464, 563
- workspace/builtin_tools/a2a_tools.py:60
- workspace/builtin_tools/delegation.py:272
Native-type usage at workspace/a2a_executor.py:471 (`Role.ROLE_AGENT`)
stays — that's an in-process Message construction; the SDK handles
wire serialisation correctly.
Updated the misleading comment at main.py:255-257 (which said
"outbound payloads are now 1.x-shaped (ROLE_USER)") to spell out
the actual rule: outbound JSON-RPC wire payloads MUST use v0.3
shape, native types are only for in-process construction.
New regression test test_jsonrpc_wire_role_format.py greps the 6
wire-payload-emitting files for any "ROLE_USER" / "ROLE_AGENT"
string literal and fails loud — cheapest possible drift detector.
Why E2E missed it: the priority-runtimes harness sends a single
message canvas → workspace, but the canvas already used lowercase
"user" (it never went through the migration sweep). The bug only
surfaces on workspace → workspace delegation, which the harness
doesn't exercise. Same gap as #131 (extend smoke to call main()
against a stub).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
requested a review
from hongmingwang-moleculeai
as a code owner
April 27, 2026 19:40
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…t into ensureDefaultConfig (provider field) — fixes canvas moonshot/kimi-k2.6 NOT_CONFIGURED' (#2187) from fix/ensure-default-config-stamp-derived-provider into main
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…t/kimi NOT_CONFIGURED The moonshot/kimi incident: a canvas-created claude-code workspace with provider=Platform + model=moonshot/kimi-k2.6 booted NOT_CONFIGURED in prod because the generated config.yaml lacked the manifest-derived `provider:` key, so the adapter slash-split "moonshot/..." -> unregistered provider. Fixed by #2187 (ensureDefaultConfig stamps DeriveProvider->provider:platform) + #2188 (canvas). Unit tests passed; the REAL boot path was the gap. This adds comprehensive regression coverage so the CLASS cannot reship: Deterministic (no live infra, runs in the normal unit suite): workspace-server/internal/handlers/workspace_provision_platform_boot_test.go - TestEnsureDefaultConfig_StampsProviderForEverySSOTPlatformModel: enumerates the claude-code `platform` arm from the providers SSOT (providers.LoadManifest) and asserts ensureDefaultConfig stamps provider:platform (top-level AND runtime_config) for EVERY offered platform model — not just the single moonshot/kimi pin #2187 shipped. A newly-offered platform model gets a case for free and only passes if actually stamped (closes the offered-but-not-stamped divergence the bug rode in on). Mutation-verified: disabling the stamp fails the test. - TestPlatformModelDeriveProvider_SSOTConsistency: the upstream half — DeriveProvider maps every SSOT platform model to provider Name "platform". Real-boot (staging; I will run it): Extends the existing staging harness (no new harness) with a platform-managed path: E2E_LLM_PATH=platform pin-selects moonshot/kimi-k2.6, sends NO tenant key, and reuses the harness's online-wait + completion assertions to prove the workspace reaches status=online (not not_configured) and a completion returns 200. The BYOK branches never exercised the platform arm — the exact arm the bug shipped on. - tests/e2e/lib/model_slug.sh: platform path + override semantics - tests/e2e/test_model_slug.sh: 4 new pinned cases (16/16 green) - tests/e2e/test_staging_full_saas.sh: empty-secrets platform branch - .gitea/workflows/e2e-staging-saas.yml: new `E2E Staging Platform Boot` job (continue-on-error during de-flake; bp-required: pending #2187), + providers.yaml/model_slug.sh added to the path triggers. Coverage-audit theme: mc#1982 (continue-on-error masks; de-flake-then-gate). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
HongmingWang-Rabbit
pushed a commit
that referenced
this pull request
Jun 12, 2026
…+ local-provision port-squatter flake - reduced-motion.test.ts: the connection-status pulse dot moved from SidePanel.tsx into the extracted WorkspacePanelTabs.tsx; retarget the motion-safe:animate-pulse assertion to where the guarded indicator now lives (was the only red in CI / Canvas -> gates CI / all-required). - e2e-staging-saas.yml: add bp directives to the 4 new concierge jobs the Tier-2g lint flagged — bp-required: pending #2430 for the three real push-time staging e2e jobs (creates-workspace / platform / user-tasks, aspiring gates sharing the cp#245 de-flake surface), bp-exempt for the PR-time compile-only job. #2187 (the sibling's tracker) is closed/unrelated. - local-provision-e2e.yml (no-flakes RCA): the :8080 kill-step only matched procs *named* platform-server, so a differently-named squatter survived, our bind went FATAL, and the /health loop false-positived against the squatter. Free :8080 from ANY holder (fuser/lsof) and verify our own PID owns the port BEFORE trusting /health, in both the stub and real jobs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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
Restores cross-workspace delegation. Once fix(runtime): legacy /app/ path leaks across MCP server + agent prompts + docstrings #2186 fixed the MCP path so `delegate_task` could actually fire, every cross-workspace call came back as JSON-RPC `-32600` "Invalid Request" with the underlying pydantic ValidationError:
```
params.message.role
Input should be 'agent' or 'user' [type=enum,
input_value='ROLE_USER', input_type=str]
```
PR fix: pass rpc_url='/' to create_jsonrpc_routes (a2a-sdk 1.x) #2184's a2a-sdk 1.x sweep over-corrected: it changed every `"role": "user"` literal in JSON-RPC payload construction to `"role": "ROLE_USER"` to match the protobuf enum names of the 1.x native types. That was correct for in-process `Message` construction (which the SDK serialises before the wire) but wrong for the 8 sites that hand-build JSON-RPC payloads. The workspace's own a2a-sdk runs inbound requests through the v0.3 compat adapter (`enable_v0_3_compat=True` in main.py), and that adapter validates against the v0.3 Pydantic Role enum (`agent`|`user` lowercase). The protobuf-style names blow it up.
Reverted the 8 wire-payload sites to lowercase `"user"`. Native-type usage at `a2a_executor.py:471` (`Role.ROLE_AGENT`) stays — that's an in-process `Message` construction; the SDK handles wire serialisation correctly. Updated the misleading comment at `main.py:255-257`.
Regression test: `test_jsonrpc_wire_role_format.py` greps the 6 wire-payload-emitting files for any `"ROLE_USER"` / `"ROLE_AGENT"` string literal and fails loud — cheapest possible drift detector.
Why E2E missed it
The priority-runtimes harness sends a single message canvas → workspace, but the canvas already used lowercase `"user"` (it never went through the migration sweep). The bug only surfaces on workspace → workspace delegation, which the harness doesn't exercise. Same gap as #131.
Test plan
🤖 Generated with Claude Code