Conversation
…#3215) `_build_payment_required` passed no `scheme=` to the SDK helper, so every 402 — and every verify and settle built from the same helper — advertised the SDK default `nvm:erc4337`. The facilitator is POSTed that requirements document verbatim, so `accepts[0].scheme` is the only channel the scheme travels through: a fiat (card) plan's token was always checked against crypto requirements and rejected. Card plans were unpayable. - `PlanScheme(scheme, network)` + `default_plan_scheme(env)`; the erc4337 environment map stays an explicit frozen literal (a payments-py bump must not move a live agent's network, #3216). - `_build_payment_required(..., plan_scheme=None)` — `None` is the pre-#3215 document, so crypto bytes are unchanged per environment (golden test). - `scheme_from_token` reads `accepted.{scheme,network,planId}` off the presented token, allow-listed against the SDK's scheme/network vocabularies and checked against this agent's plan id. This is what verify and settle use: it removes every network call from the money path and makes a settle re-driven hours later byte-stable with its own verify. - `resolve_plan_scheme` (402 and `/info` only) parses `plans.get_plan` with the SDK's own keys — parity-tested against `resolve_scheme`/`resolve_network` — behind a per-(env, plan_id) cache, single-flight futures, its own 2-slot gate (never a facilitator slot, so an anonymous 402 flood cannot starve paying traffic), stale-while-revalidate and a WARN once per negative window. The SDK swallows these failures at DEBUG, which is how a card plan silently stayed crypto with nothing in Trinity's logs. 61 new tests; the 196-test ent#679/#3185 baseline stays green unchanged. Mutations proven red: dropping `scheme=` (5 red), swapping settle's token-derived scheme for a plan lookup (2 red). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…3215) AC5. The x402 402's `resource.url` is what a buyer's token is minted and verified against, and the facilitator compares origin+path — so one wrong scheme character mints a token for a URL the client never calls. Both payment doors built it from `request.base_url`, which is http on a standard Trinity deployment for two independent reasons: - `docker-compose.prod.yml` / `docker-compose.hosted.yml` override the image `command:` and drop the Dockerfile CMD's `--proxy-headers --forwarded-allow-ips=*`, so uvicorn does not trust `X-Forwarded-*` at all and `request.url.scheme` stays http behind ANY proxy. Restoring those flags is a trust change (it re-prices `request.client.host` for every per-IP limiter, and agents on the agent network reach `backend:8000` directly), so it stays deferred — which is why `utils/public_url.py` reads the RAW header and never `request.url.scheme`. - the frontend nginx forwarded `X-Forwarded-Proto $scheme`, and `$scheme` is http on that hop (TLS terminates upstream), clobbering the real https before the backend saw it. Now a `$fwd_proto` map preserves an upstream https and otherwise falls back to this hop's scheme, on all three proxied locations (`/api/`, `/a2a/`, `/mcp`) — a door that lied about the scheme on one of them would mint tokens for the wrong origin just the same. `public_base_url(request, configured=, frontend_url=)` owns the precedence: the operator's declared origin (Settings `public_chat_url` → `PUBLIC_CHAT_URL` → `FRONTEND_URL`) when it is the host the caller actually used, else the request host with an https upgrade — never a downgrade — from the raw header. The same-host restriction keeps a caller that reached a private host from being sent to a public one whose narrow tunnel may not route that path. One helper now serves the agent card, the paid door and the A2A door, so a single request yields a single origin; `_base_url_from_request` also starts honouring the Settings row, as the Telegram/WhatsApp webhook URLs already do. `GET /api/paid/{name}/info` — the card's `paymentInfoUrl`, from which a payment-aware client pays on its first request — now loads the config WITH the key so it can resolve the plan's real scheme, and emits an absolute `resource.url`. The key is used server-side only; a test pins that it never reaches the body. 34 new tests; 291 green across the #3215 files and the ent#679/#3185 baseline. Mutation proven red: removing the header upgrade → 9 red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lus docs (#3215) Operator scope-add (2026-10-04T11:48:59Z): a settled `message/send` returned a Task with `metadata: null` while `a2a-protocol.md` promised "a normal A2A Task whose metadata carries the payment status and a receipt". The metadata WAS built and placed — on `status.message.metadata`, which is the spec location (the provider SDK's own `X402A2AUtils` reads exactly there) — but top-level `Task.metadata` was never set, so a typed `a2a-sdk` client rendered a charged turn as `metadata: null`. `_task_object` now mirrors it from the same assignment, the same dict object, so the two locations cannot drift. The docs name `status.message.metadata` as the primary location and the mirror as the compatibility half. The free path still emits no `metadata` key at all, so an unpaid Task's bytes and every idempotency snapshot built from them are unchanged. Docs: - `user-docs/integrations/a2a-protocol.md` — names both locations precisely. - `user-docs/integrations/nevermined-payments.md` — crypto vs card plans and the scheme/network each advertises; why the 402's `resource.url` origin matters and which knob fixes it. - `user-docs/guides/deploying/public-access.md` — the A2A door was published by #3213 and the narrow tunnel table had no row for it. Added with ANCHORED rules (`^/a2a/[^/]+$`, `^/a2a/[^/]+/\.well-known/agent-card\.json$`): an unanchored `/a2a/*` would also match `/api/agents/{name}/a2a/call`, Trinity's authenticated OUTBOUND caller, which has no business on a public hostname. - `memory/requirements/public-access.md` §23.2–23.3, `memory/requirements/mcp.md` FR-4 + new FR-4a, `memory/feature-flows/nevermined-payments.md` (two new sections + the two new knobs), `memory/architecture/backend.md` one-liners for `nevermined_payment_service.py`, `paid.py` and `a2a.py`. 11 new tests; 382 green across the #3215 files and every payments/A2A neighbour. Mutation proven red: removing the mirror → 6 red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…st (#3215 review I3) #3215 made ONE origin rule serve the agent card and both payment 402s. The doors needed same-host — an x402 token is minted and facilitator-verified against `resource.url`, so a 402 quoting another origin is unpayable. But the card is the opposite kind of document, and same-host regressed it: the `get_agent_a2a_card` MCP tool proxies the card route from `backend:8000`, so a card fetched there advertised `http://backend:8000/a2a/{name}` to every external buyer. That is dev's behaviour broken on an existing surface, and the plan's own T5 text named configured-first as "keep the card as is". Still one module and one upgrade rule, now one keyword apart: * `public_base_url(..., configured_wins=False)` — default unchanged, so every door's bytes are identical. `True` returns the declared origin whatever host the caller used, and falls through to the SAME request-host + upgrade-only raw `X-Forwarded-Proto` logic when nothing is configured. * `routers/a2a._card_base_url` wraps it for the two card routes only (the authenticated per-agent card and the public well-known card). The A2A JSON-RPC door, the paid door and `/info` keep T5 exactly as built. Safe because the two only differ where it cannot matter: a buyer that follows the card arrives on the configured host, so the door's own same-host rule mints that 402 for that very origin — the 402 is byte-identical to the URL the card sent it to (pinned by a test). With nothing configured the two are the same function. Tests: 14 new in `test_3215_public_base_url.py`, 10 red before the fix — `TestConfiguredWins` at the helper, `TestCardAndDoorOverTheRealRoutes` over the real routes (both card surfaces and the anonymous 402 on one internal host, which is the only layer that can catch a call site passing the wrong precedence), and `TestOneOriginPerRequest` adjusted to the new rule rather than trimmed. The route-level fixture imports `dependencies` at collection time on purpose: `tests/unit/conftest.py` pops it from `sys.modules` between tests, so a fixture-local import makes `dependency_overrides` stop matching after the first test. 405 green in the #3215/#3185/ent#679 batch (391 before). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…3215 review I4) The single-flight leader's `finally` answered its followers either way, but with the WRONG answer when it had nothing: a cancelled leader (its caller disconnected, its request shut down) completed the shared future with `default_plan_scheme(...)`. Every follower then returned crypto for a card plan with no WARN, no negative window opened, and last-known-good never consulted — the exact silent mis-advertisement #3215 exists to stop, reintroduced at the one place the service is not looking. A leader that resolved nothing now sets `PlanLookupLeaderLost` on the future instead. Each follower already catches it, so it falls into its existing `_record_plan_lookup_failure` branch: the 402 still gets an answer (fail-open is the accepted design), but it is last-known-good when there is one, the negative window opens, and the WARN names the reason. The leader's own `CancelledError` still propagates unchanged — `finally` swallows nothing. The future's exception is marked retrieved, so a cancelled 402 with no followers does not log "Future exception was never retrieved" at GC. Tests: 4 new in `test_3215_plan_scheme.py::TestCancelledLeaderFailsFollowersLoudly` — the WARN and the negative window, last-known-good preferred over the default, no asyncio noise with zero followers, and the successful-leader path proved untouched. 2 go red with the fix reverted (verified against a scratch copy, then restored byte-identical); the other 2 are the no-regression arms. 409 green in the #3215/#3185/ent#679 batch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
5 tasks
This branch has not been deployed
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.
Fixes #3215 (draft: merges on the operator's call).
What
Card (fiat) Nevermined plans could not be paid. The 402, verify and settle always used the
nvm:erc4337scheme. This branch:services/nevermined_payment_service.py): resolves each plan's x402 scheme at runtime,nvm:erc4337for crypto plans andnvm:card-delegationfor card plans. It uses its own parser ofget_plan, with an SDK parity test. Results are cached with stale-while-revalidate, bounded, and single-flighted. One rule decides the scheme on the money path (402 → verify → settle). No schema change.utils/public_url.py,routers/a2a.py,routers/paid.py,src/frontend/nginx.conf):resource.urlin both 402 doors (the paid door and the A2A JSON-RPC door) reflects the origin the caller actually used. The scheme is upgrade-only from the rawX-Forwarded-Proto, independent of uvicorn's forwarded-ip trust. nginx passes$fwd_protoon the backend locations that carry x402 traffic.routers/a2a.py,services/a2a_payment_gate.py): a settledmessage/sendnow mirrors the payment status and receipt ontoTask.metadata. The reply now matches whatintegrations/a2a-protocol.mdpromises (scope addition from the operator's 2026-10-04 comment).Rulings carried
Task.metadataand make the docs precise.X-Forwarded-Proto, upgrade-only, not uvicorn trust.--forwarded-allow-ipsstays deferred.get_agent_a2a_cardproxies through the internal backend host, and would otherwise have advertised an internal URL.Review + security
/review(range1279475f..b3e742c5): MERGEABLE, 0 critical, 6 informational.eb7495f9and8caf4a79.Retry-After; see Handoffs./mcpnginx$fwd_protoline is inert today. It was kept for parity./cso --diff: 0 findings at the 8/10 gate. The run was PARTIAL (no docker, no scanners). One Host-derived hypothesis came in at confidence 4; it belongs to a pre-existing class.dev(1279475f) there is one head, and the merged tree equals this branch's tree..claudeandsrc/backend/enterpriseare untouched in the range.Tests
urlsuites gives 465 passed.test_3215_plan_scheme.py,test_3215_public_base_url.py,test_3215_task_metadata.py.tests/unit/conftest.pypopsdependenciesbetween tests. A fixture-localimport dependenciestherefore gets a differentget_authorized_agent_by_namethan the router captured, anddependency_overridessilently stops matching from the second test on. Import at collection time when mounting an authed route in a new test file.Before merge
/infoshowsnvm:card-delegation. This needs live Nevermined, so only the mechanism is unit-guarded here.get_plancarriesregistry.price.isCrypto=falseplusmetadata.plan.fiatPaymentProvider. This is an SDK-source assumption, covered by the parity test only.$fwd_protomap ships.routers/a2a.py) and feat(pull): /chat on the durable queue for pull pilots (#3127) #3145 (routers/paid.py) already conflict withdevon files this branch also edits, so whichever merges second rebases over this branch's hunks too.Handoffs (not in this PR)
schedules._build_webhook_urlstill derives fromrequest.base_url(separate bug).--forwarded-allow-ipswith a proxy-subnet allow-list in the prod/hosted compose. This changes trust and needs its own security review.Retry-Afterinstead of failing open (review I2).🤖 Generated with Claude Code