Repository navigation
fix(compat): decide the compact response status from the served release's schema - #1761
Merged
Merged
Conversation
…se's schema #1754 stamps status on list_products and decline_proposals from 3.3.0-beta.3 by comparing version strings. That leaves three defects for a 3.2.x peer, and a fourth for the next 3.2.x release: - A typed ListProductsResponse dumps its defaulted `status: completed`. A server serving 3.2.x sent it to a closed root that forbids it, so strict response validation failed on a handler that returned the typed model. - Converting a synchronous decline_proposals response from 3.3 to 3.2 raised ADCPVersionConversionError: the 3.2 root permits `status` only as `submitted`, and the converter had no rule for the status that 3.3 requires. - Converting up from 3.2 to 3.3 did not supply the `status` 3.3 requires, so the converted list_products / decline_proposals payload failed 3.3 validation. - A 3.2.x release that composes the envelope (the backport) would still be treated as one that does not. uses_task_status_envelope(tool, version) now asks the served release's own response schema whether it composes core/protocol-envelope.json, as the TypeScript SDK does (adcontextprotocol/adcp-client#3493). The version comparison from #1754 stays as the answer for a release that ships no schema for the tool. The server drops only the typed default `completed` where the served release has none for the tool (a submitted arm keeps its status), and convert_payload supplies `completed` going up and drops it, recorded as a loss, going down. Nothing is stripped for any other tool or release. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
5 tasks done
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — compact-response status gating now reads the served release's own response schema (response_composes_protocol_envelope → root allOf composing core/protocol-envelope.json) rather than a brittle version-string compare, fixing three defects for a 3.2.x peer plus the backport case.
What I checked:
- Scope: Logic is tightly scoped to the two native-compact tools (
list_products,decline_proposals); the version-string path survives as a fallback for releases without a schema. - Minor-release pathway (DR-0032/DR-0033): The schema-driven gating is a Moves-style translation handled in both directions, with coverage.
- Semver signal:
fix(compat):is correct — no public-surface removal, rename, type-signature change, or required↔optional flip; buyers/sellers continue to deserialize. - Tests: 27 new tests cover both conversion directions, the server-strip path, and the schema-flip case.
- High-risk flag: True only because
src/adcp/compat/**andsrc/adcp/server/**were modified; both are presumed safe with no medium-or-higher findings.
No Critical/High/Medium findings. No gated paths, no no-auto-approve team match, no prior decision. Falls through to row 9.
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.
Summary
nextalready adopts the signed 3.3.0-beta.3 bundle (#1754) and stampsstatusonlist_products/decline_proposalsfrom that release on. That gate compares version strings. This PR makes the served release's own response schema decide, which is the contract in the task (and adcp-client#3493 for the TypeScript SDK), and closes three defects the string comparison leaves for a 3.2.x peer. Each is reproduced by a test that fails onnexttoday.nextListProductsResponsedumps its defaultedstatus: completed; a server serving 3.2.x sends it to a closed root that forbids it (strict response validation fails)._normalize_response_envelopedrops only that impliedcompletedwhere the served release has nostatusfor the tool. A submitted arm keeps its status.decline_proposalsresponse raisesADCPVersionConversionError(the 3.2 root permitsstatusonly assubmitted)._convert_compact_statusdropscompletedgoing down, as a recordedConversionLoss.status3.3 requires.completedgoing up.uses_task_status_envelope(tool, version)asks the release's schema (response_composes_protocol_envelope: does a rootallOfarm requirestatus/ referenceprotocol-envelope?). The #1754 version comparison stays as the answer for a release that ships no schema for the tool.Nothing is stripped for any other tool or release.
tests/test_response_status_gating.pyincludes a test that serves the composed schema as 3.2's and checks the answer flips with no code change.Context
This supersedes #1760, which adopted the same bundle in parallel. I opened it before #1754 landed; on rebase everything except the items above was already on
next(including the controllertask_idbound and the compact-response gating), so I closed it.Test plan
pytest tests: 20,880 passed, 0 failed, 2,833 skipped, 1 xfailedmake typecheckandruff check src/ packages/adcp-reporting-storage/srctests/test_response_status_gating.py; 4 fail onnextwithout this change🤖 Generated with Claude Code