Skip to content

fix: Challenge selection ignores the offered intent - #212

Draft
mpp-agricola[bot] wants to merge 3 commits into
mainfrom
agricola/agr-2026-022
Draft

fix: Challenge selection ignores the offered intent#212
mpp-agricola[bot] wants to merge 3 commits into
mainfrom
agricola/agr-2026-022

Conversation

@mpp-agricola

@mpp-agricola mpp-agricola Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Motivation

Agricola noticed a discrepancy between tempoxyz/pympp and the canonical implementation: challenge selection ignores the offered intent.

The Agricola ticket #97 contains the audit evidence, affected SDKs, and remediation lifecycle.

Summary

  • Reconciles semantic:challenge-negotiation/select-supported-method-intent in the target SDK's idioms.
  • Adds implementation and regression coverage for AGR-2026-022.
  • Links the change to the Agricola ticket #97.

Key design considerations

  • Limits scope to the audited delta between wevm/mppx@b7ab48e38e3d and tempoxyz/pympp@7f7164afa55c.
  • Favors the target SDK's public API and conventions over a literal port.
  • Uses the stable agricola/agr-2026-022 automation branch.
  • Remains a draft until a maintainer reviews the generated changes.

@brendanjryan

Copy link
Copy Markdown
Collaborator

/ag fix can you add more test coverage

@brendanjryan
brendanjryan marked this pull request as ready for review August 9, 2026 03:22

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 22ce652262

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/mpp/runtime.py
for challenge in challenges:
method = self._methods.get(challenge.method)
if method is not None:
if method is not None and challenge.intent in method.intents:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve charge matching for client-only Stripe methods

When Stripe is configured exactly as the bundled client documentation shows (stripe(create_token=..., payment_method=..., intents={}) in src/mpp/methods/stripe/__init__.py), this condition rejects every charge challenge because method.intents is empty. PaymentTransport consequently returns the original 402 without invoking create_token, breaking the documented Stripe client flow; client-side charge capability needs a default or separate capability source rather than requiring a nonempty server-verifier registry.

Useful? React with 👍 / 👎.

Comment thread src/mpp/runtime.py
Comment on lines 49 to +50
method = self._methods.get(challenge.method)
if method is not None:
if method is not None and challenge.intent in method.intents:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Search every same-name method for a compatible intent

When two installed methods share a name but advertise disjoint intents, _methods has already discarded every instance except the last one, so a challenge supported by an earlier instance is incorrectly rejected. For example, PaymentRuntime([tempo_charge, tempo_session]) cannot match a charge challenge even though tempo_charge is installed; selection should preserve the existing last-method preference among compatible instances rather than consulting only the single collapsed entry.

Useful? React with 👍 / 👎.

Agricola-Request: revise:audit:AGR-2026-022:python:22ce65226239
Agricola-Summary: Expanded challenge negotiation coverage across runtime and HTTP transport paths; verification blocked by unavailable PyPI DNS.
@mpp-agricola
mpp-agricola Bot marked this pull request as draft August 9, 2026 04:03
@mpp-agricola

mpp-agricola Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor Author

Agricola updated this pull request from the latest review feedback and failing CI.

Summary of changes

  • Expanded challenge negotiation coverage across runtime and HTTP transport paths; verification blocked by unavailable PyPI DNS.

Changed files

 tests/test_client.py  | 40 ++++++++++++++++++++++++++++++++++++++++
 tests/test_runtime.py | 19 +++++++++++++++----
 2 files changed, 55 insertions(+), 4 deletions(-)

Requested from tempoxyz/mpp-tools#97.

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.

1 participant