Skip to content

feat(negotiation): complete proposal refinement API gaps - #38

Merged
bokelley merged 6 commits into
adcontextprotocol:mainfrom
garvitkaushik-123:feat/negotiation-gaps
Aug 30, 2026
Merged

feat(negotiation): complete proposal refinement API gaps#38
bokelley merged 6 commits into
adcontextprotocol:mainfrom
garvitkaushik-123:feat/negotiation-gaps

Conversation

@garvitkaushik-123

@garvitkaushik-123 garvitkaushik-123 commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Summary

Completes the Java/Kotlin client, seller, testing, and example surfaces for AdCP 3.2 proposal negotiation.

  • Maintained beta.9 request/response models with safely discriminated outcomes, all reason codes, typed constraints, product changes, alternatives, criteria, push configuration, and envelope fields
  • Capability-aware validation for supported dimensions, seller alternatives ceilings, the protocol maxima of 10 alternatives and 25 refinements, unique sources, and homogeneous finalize batches
  • RFC 8785 terms digests through the Java JCS reference implementation
  • Fail-closed response verification for ordering, outcome shapes, immutable lineage, fresh IDs, distinct alternatives, failure subsets and precedence, every typed constraint, product changes, and expiry/atomicity
  • AdcpServerBuilder.proposalHandler(...) registration with capability/preflight checks and an explicit atomic finalize transaction callback; response validation occurs before commit
  • Reactor, Mutiny, and Kotlin coroutine bridges preserving task errors and sealed per-proposal outcomes
  • Expanded adcp-testing fixtures/assertions and boundary/atomicity tests
  • Runnable constrained training-seller CLI and legacy-refinement compatibility guide

Test plan

  • ./gradlew build --no-configuration-cache (all eight modules)
  • JCS key-order, escaping, IEEE-754 number, digest, and tampering tests
  • All outcomes and all eight reason codes
  • 10-alternative and 25-refinement boundaries
  • Mixed-batch, unsupported-dimension, fail-closed constraints, partial alternatives, immutable successor, and replay assertions
  • Server preflight and validation-before-atomic-commit tests

Closes #36

garvitkaushik-123 and others added 3 commits August 16, 2026 01:22
Introduces the `negotiation` package across adcp, adcp-server, and
adcp-testing modules for the refine_proposals tool surface (AdCP 3.2).

Client (adcp module):
- Sealed RefinementResult with four outcomes (revised/partial/finalized/unable)
  supporting exhaustive pattern matching
- Capability-aware RefineProposalsRequest builder enforcing batch size
  ceilings, unique proposal IDs, and finalize-only homogeneity
- Three typed constraints from the 3.2 delta: CpmConstraint,
  ImpressionsConstraint, FlightConstraint
- TermsDigest utility implementing RFC 8785 JCS canonicalization for
  SHA-256 digest computation and verification
- ResponseVerifier checking result ordering, lineage (parent_proposal_id),
  finalize atomicity, outcome structural constraints, partial invariant,
  unique alternatives, and digest integrity
- AdcpClient.refineProposals() convenience method
- UnsupportedRefinementDetails for typed UNSUPPORTED_FEATURE error recovery
- RefinementCapability with product_changes dimension key (post-rename)

Server (adcp-server module):
- ProposalHandler interface with capability declaration, batch preflight
  hook, and refine callback — commercial decisions stay in the application

Testing (adcp-testing module):
- NegotiationFixtures with factory methods for common scenarios: single
  revise, batch finalize, constraint objects, draft/committed proposals

Covers the unsatisfied_constraints-is-open-strings design, reason code
precedence (constraint_unsatisfiable > commercially_declined), lineage
verification, and the hold_unavailable/batch_aborted reason codes from
the post-red-team protocol delta.

Ref: adcontextprotocol#36
- TermsDigest: remove unused MAPPER field and three unused imports
  (JsonProcessingException, Iterator, Map)
- TermsDigest: add @nullable to verify() digest parameter — the method
  handles null but the @NullMarked package default says non-null
- TermsDigest: fix JCS exponent formatting — strip ".0" before 'e' so
  1.0E-7 becomes "1e-7" per ES2015/RFC 8785 (was producing "1.0e-7")
- TermsDigest: replace per-character OutputStreamWriter allocation in
  writeCanonicalString with direct String.valueOf(c).getBytes()
- RefineProposalsRequest: remove unused Collections import and dead
  supportedDimensions builder field (stored but never validated)
- AdcpClient: fix import ordering — negotiation imports grouped with
  other org.adcontextprotocol imports, not after slf4j
- Add serialization round-trip test for RefinementResult
- Add test coverage for JCS exponent edge cases
Add missing typed constraints, alternatives validation, and server-side
successor utilities to close the proposal negotiation gaps tracked in
adcontextprotocol/adcp#6556.

- TotalBudgetConstraint: budget bounds (min/max/currency) per schema
- RefinementConstraints: composite of all four constraint dimensions
- ProposalRefinement: add constraints, productChanges, alternatives
  fields + builder; rename instructions→ask to match wire schema;
  enforce alternatives.count 2-10 (protocol max)
- RefinementCapability: add maxAlternatives field
- RefineProposalsRequest.Builder: validate alternatives against seller
  ceiling
- ResponseVerifier: add verifyConstraintSatisfaction checking budget
  bounds and CPM ceiling on revised outcomes
- ProposalSuccessor (server): stamp immutable successors with lineage
  and recomputed terms_digest
- 15 new tests across constraints, alternatives, verification, and
  successor stamping

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@aao-ipr-bot

aao-ipr-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

This is an automated message from the Argus AI review workflow.

@bokelley

Copy link
Copy Markdown
Contributor

This is a useful core start, but it does not yet meet #36's RC acceptance criteria. I found four concrete blockers in the current diff:

  1. RefineProposalsRequest.Builder.maxBatchSize(int) accepts values above the protocol maximum of 25. That lets capability input weaken the protocol ceiling; clamp/reject above 25 and add the boundary test.
  2. ProposalSuccessor.stamp preserves an existing proposal_id, and the test explicitly blesses that behavior. The 3.2 contract says every refinement result receives a new ID and the source remains unchanged. The helper must at least reject proposal_id == sourceProposalId; preferably make fresh successor identity the safe default and test the invariant.
  3. TermsDigest hand-rolls RFC 8785 number serialization even though feat(negotiation): add first-class buyer and seller proposal APIs #36 explicitly calls this the risky part and points to the reference implementation. Basic local number tests are not enough to establish cross-language JCS parity. Please use a maintained JCS implementation or run the complete shared/cross-language canonicalization vectors and byte-parity tests before this is an acceptance boundary.
  4. The PR changes only adcp, adcp-server, and adcp-testing. feat(negotiation): add first-class buyer and seller proposal APIs #36 also requires the reactive/Kotlin surfaces, a runnable buyer/server example compatible with the training seller, and legacy-refinement compatibility documentation. Those are absent from the file list. The request model also omits the schema's optional push_notification_config; if version fields are framework-injected, that should be documented, but the remaining canonical request fields should not silently disappear.

Please also map the adcp-testing fixtures/assertions to #36's full matrix (all outcomes/reason codes, 10/25 limits, fail-closed constraints, partial alternatives, idempotency, and finalization atomicity) so the issue can be closed on evidence rather than just compilation.

@bokelley

Copy link
Copy Markdown
Contributor

Maintainer takeover update pushed in a6389fe; this addresses the four blockers and audits the branch against the final 3.2 beta.9 contract:

  • Enforces the 25-refinement protocol ceiling in both builders and canonical construction, seller capabilities, homogeneous finalize batches, unique source IDs, and the 2–10 alternatives range.
  • Replaces the hand-written canonicalizer with the Java JCS reference implementation and parity-oriented number/key-order tests.
  • Updates maintained request/result models to the beta.9 plural proposal shapes, all reason codes, typed product actions/alternatives, version/context/push/ext fields, and complete constraints.
  • Makes verification fail closed across ordering, outcome/cardinality, fresh global IDs, lineage, digest/distinctness, budget, CPM, impressions, flight, products, failure subsets/precedence, and hold expiry/atomicity.
  • Wires ProposalHandler into AdcpServerBuilder, including capability validation and preflight. Finalize uses an explicit transaction callback whose response is verified before commit; exact-replay results are reverified.
  • Adds Project Reactor, Mutiny, and Kotlin coroutine surfaces without collapsing task errors into per-proposal outcomes.
  • Extends adcp-testing fixtures/assertions across outcomes, reason codes, cardinality boundaries, replay, and atomic verification.
  • Adds a runnable constrained training-seller CLI plus legacy compatibility documentation.
  • ProposalSuccessor now always assigns a fresh ID, requires commercial terms, preserves lineage, and recomputes the digest.

Verification: ./gradlew build --no-configuration-cache passes all eight modules (the existing non-fatal Javadoc diagnostics remain unchanged). CI is now running on the pushed head.

@aao-ipr-bot

aao-ipr-bot Bot commented Aug 30, 2026

Copy link
Copy Markdown

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

This is an automated message from the Argus AI review workflow.

bokelley
bokelley previously approved these changes Aug 30, 2026

@bokelley bokelley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer review complete after the takeover commit. The implementation now satisfies the issue acceptance criteria: beta.9 model shapes, reference JCS canonicalization, fail-closed response verification, fresh successor identities, atomic finalize execution, server/client integrations, testing fixtures, CLI exercise, and documentation. Full multi-module Gradle build passes locally.

bokelley
bokelley previously approved these changes Aug 30, 2026

@bokelley bokelley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review complete after the CI metadata fix. The new private workspace manifests only register the eight Gradle artifacts with Changesets; npx changeset status --since=origin/main now identifies the seven affected artifacts and the full Gradle build remains green.

@aao-ipr-bot

aao-ipr-bot Bot commented Aug 30, 2026

Copy link
Copy Markdown

⚠️ Argus review could not complete

The automated review encountered an issue (possibly reached max turns, timed out, or failed to post the final gh pr review). A human reviewer should take this PR.

View workflow run

This is an automated message from the Argus AI review workflow.

@bokelley bokelley left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final re-review complete. Clean-install validation now passes: npm ci, npx changeset status --since=origin/main (seven minor artifact entries), the full Gradle build, and git diff --check are all green.

@bokelley
bokelley merged commit 159702e into adcontextprotocol:main Aug 30, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(negotiation): add first-class buyer and seller proposal APIs

2 participants