feat: add shared payment runtime - #184
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6a4c272614
ℹ️ 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".
| try: | ||
| cleanup(self._bridge.cancel_pending) | ||
| if self._method_stack is not None: | ||
| cleanup(lambda: self._bridge.run(self._teardown_methods())) |
There was a problem hiding this comment.
Keep async context managers in their entering task context
When a factory returns an async context manager that uses task-local state, this submits _teardown_methods() as a new task even though __aenter__ ran in the earlier _initialize_methods() task. For example, an @asynccontextmanager that sets a ContextVar and resets its token in finally consistently raises ValueError: Token was created in a different Context during close(), potentially interrupting cleanup. Keep the exit stack alive in one supervisor task that performs both entry and exit rather than resubmitting teardown separately.
AGENTS.md reference: AGENTS.md:L68-L69
Useful? React with 👍 / 👎.
5d60b7d to
b5a4293
Compare
b5a4293 to
07d5dc8
Compare
07d5dc8 to
42cfb85
Compare
a9005de to
4aae448
Compare
8763962 to
3b8966f
Compare
3b8966f to
27fa696
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 27fa696319
ℹ️ 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".
| self.start() | ||
| if not any(candidate is method for candidate in self.methods): | ||
| raise ValueError("Method is not installed in this PaymentRuntime") | ||
| if challenge.method != method.name or challenge.intent not in _method_intents(method): |
There was a problem hiding this comment.
Honor name-only matches during credential creation
When match_challenge(..., allow_name_only=True) selects a legacy method for an intent not exposed by _method_intents, passing that returned pair into the documented create_credential flow always raises here because credential creation unconditionally reapplies intent validation. This makes allow_name_only unusable for the runtime's advertised match-then-create composition; credential creation needs a corresponding override, or matching must not return pairs it will reject.
AGENTS.md reference: AGENTS.md:L53-L60
Useful? React with 👍 / 👎.
| allow_name_only: bool = False, | ||
| ) -> tuple[Challenge, Method]: | ||
| """Return the first compatible challenge and method.""" | ||
| pairs = ( |
There was a problem hiding this comment.
Check closure before matching challenges
After close() or exiting either context manager, match_challenge() remains usable because it is the only public runtime operation that does not call start(). This contradicts close()'s promise to prevent new runtime operations and can either select a borrowed method for use after the runtime's lifetime or defer the closed-state error until the subsequent credential-creation step; check the closed state before matching.
Useful? React with 👍 / 👎.
Summary
PaymentRuntimethat runs on the caller's event loopMethodprotocol from the existing async transportReview boundary
This PR adds no HTTP policy, retry ledger, owned loop, thread bridge, sync transport, HTTPX patching, or MCP integration. Production delta is 154 lines.
Validation