fix: align relay payment polling contract with tx-schemas#95
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
x402-api-production | 6df332b | Apr 03 2026, 07:13 PM |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddd7824d25
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
Aligns relay payment polling / retry responses with @aibtc/tx-schemas so middleware classification, retry logic, logs, and public docs all share the same canonical parsing and surfaced fields.
Changes:
- Added canonical-first payment status adapter + retry-decision helpers (with legacy compat shims) and corresponding unit tests.
- Updated x402 middleware to classify retry behavior based on canonical states/terminal reasons and to emit structured payment observability fields.
- Refactored test harness retry flow to reuse a shared helper; updated OpenAPI/docs to document canonical
402retry bodies + lifecycle metadata.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/payment-status.unit.test.ts | Adds unit coverage for canonical/compat payment-status parsing and retry context helpers. |
| tests/payment-observability.unit.test.ts | Adds unit coverage for structured payment log fields and instability classification. |
| tests/_shared_utils.ts | Enhances retry parsing/shaping to use canonical retry context; consolidates retry flow. |
| tests/_run_all_tests.ts | Switches endpoint lifecycle tests to the shared retry helper and removes duplicated signing logic. |
| src/utils/x402-schema.ts | Adds lifecycle metadata to the generated x402 manifest. |
| src/utils/payment-status.ts | Introduces canonical-first payment status adapter backed by @aibtc/tx-schemas. |
| src/utils/payment-observability.ts | Adds shared structured payment log helpers + instability derivation. |
| src/utils/payment-contract.ts | Centralizes public payment lifecycle metadata/constants used across docs/manifests. |
| src/services/pricing.ts | Hardens multimodal message token estimation by normalizing message content handling. |
| src/middleware/x402.ts | Prefers canonical status/terminalReason for classification, shapes paid-retry errors, and emits structured payment observability logs. |
| src/index.ts | Surfaces lifecycle metadata in / response + docs text alignment. |
| src/endpoints/schema.ts | Updates shared OpenAPI 402 schema to document unpaid challenge vs canonical paid-retry error bodies. |
| src/endpoints/inference/openrouter/chat.ts | Reuses shared response402 schema in endpoint docs. |
| src/endpoints/inference/cloudflare/chat.ts | Reuses shared tokenTypeParam and shared response402 schema in endpoint docs. |
| src/endpoints/ax-discovery.ts | Updates discovery docs/examples to match x402 v2 structures + canonical lifecycle and retry fields. |
| package.json | Adds @aibtc/tx-schemas dependency for canonical parsing. |
| package-lock.json | Locks @aibtc/tx-schemas and updates zod resolution accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
biwasxyz
left a comment
There was a problem hiding this comment.
Phase 6 rollout review
Clean and well-aligned. All 7 review comments were addressed in the follow-up commit, and the canonical vocabulary is used consistently throughout.
What looks good
HttpPaymentStatusResponseSchema,TrackedPaymentStateSchema,TerminalReasonSchemaall imported from@aibtc/tx-schemasterminalReasonpreserved as the normalized semantic signal in error responses, logs, and discovery docspaymentIdrelay-owned throughout, extracted viaextractPaymentId()checkStatusUrladditive when present, never required- Immediate pay-per-call behavior correctly preserved as the documented service-level exception
STATUS_ALIASESmap (submitted→queued,queued_with_warning→queued) is appropriate local compat logicPAYMENT_REPO_VERSIONnow sourced frompackage.jsoninstead of hardcoded- All review feedback resolved in code
Verdict
Ready to merge. No open items.
arc0btc
left a comment
There was a problem hiding this comment.
Centralizes canonical payment status parsing from @aibtc/tx-schemas into dedicated utility modules, replacing ad-hoc error-string matching with schema-validated extraction and a structured compat shim for legacy relay responses.
What works well:
payment-status.tsis well-composed —firstExtracted,noneExtracted,ExtractedField<T>make the compat-shim tracking explicit and testable without leaking stateisInFlightPaymentStatedelegates toInFlightPaymentStateSchema.safeParse()rather than reimplementing string matching — schema stays the single source of truth- Canonical-first ordering in
classifyPaymentErroris correct: checkcanonical.statusbefore falling back to combined-string heuristics, which prevents misclassification when the relay does return structured fields - The priority reordering of fallback checks (relay-specific broadcast/pending before generic nonce/expired) is the right call — relay errors are more precise and shouldn't be shadowed by broad string patterns
- Adding
@aibtc/tx-schemasas a direct dependency locks the contract boundary instead of inferring it
[question] Double log on failure path (src/middleware/x402.ts:554–635)
When settleResult fails, the code emits either payment.poll or payment.finalized, then always emits payment.retry_decision after. That's two events per failure — intentional for structured retry observability, or will this double the noise on dashboards? Worth confirming the log consumer expects both events.
[suggestion] getRetryDecisionContext is defined but not wired (src/utils/payment-status.ts)
getRetryDecisionContext is exported from payment-status.ts but x402.ts uses the inline getRetryAction function instead. Either wire getRetryDecisionContext into getRetryAction to keep the logic in one place, or remove getRetryDecisionContext to avoid a dead export that could confuse future callers.
[nit] pricing.ts type cast (src/services/pricing.ts:184)
const content = msg?.content as unknown is a workaround to avoid a TypeScript narrowing error on the union type. The behavior is correct, but the cast is worth a comment explaining why it's needed, or fixing the upstream type if ChatCompletionRequest["messages"] can be narrowed more precisely.
Code quality notes:
- The test harness deduplication (routing through
makeX402RequestWithRetryinstead of the inline retry loop) is a real simplification — the removed loop was ~100 lines and the shared helper handles nonce-conflict semantics consistently - Removing
EXPECTED_ASSETSfrom_run_all_tests.tsand consolidating into_shared_utilsis the right move; asset constants had no business living in the orchestrator
Operational context:
We run x402 payments through this relay in production with effectiveCapacity=1 and have watched ghost nonces (sender_nonce_duplicate, sender_nonce_stale) cascade into opaque 402s. This PR gives clients terminalReason and checkStatusUrl on those failures — exactly what's needed for informed retry decisions instead of blind backoff. isSenderRebuildTerminalReason covers all three nonce terminal reasons we've seen in the wild.
- Add comment on getRetryDecisionContext explaining it's used by test utilities, not production middleware (not dead code) - Add comment on dual payment.poll/payment.retry_decision emission explaining dashboard vs alerting consumer split - Add comment on `as unknown` cast in estimateInputTokens explaining OpenRouter SDK type narrowing mismatch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
@aibtc/tx-schemasso canonical parsing is shared across middleware and test retry logicpaymentId,status,terminalReason,retryable, and additivecheckStatusUrlon paid retry errors when knownsubmittedcollapsed toqueuedand confirmed-only delivery as the defaultContract Alignment Completed
@aibtc/tx-schemasfor canonical-first parsing and compatibility-only inferencecheckStatusUrlpassthroughpaymentId,status,terminalReason,checkStatusUrl_present, and compat-shim markers/,x402.json, AX discovery docs, and shared OpenAPI402response docs so public docs match runtime fieldsSimplifications
/,x402.json, and docs instead of repeating the state list inlineCompatibility / Rollout Notes
checkStatusUrlremains additive and depends on upstream relay responses surfacing it; older relay responses still work through the compat pathpaymentIdas relay-owned andterminalReasonas the normalized terminal signalVerification
npm run checkbun test tests/openrouter-validation.unit.test.ts tests/cloudflare-ai-fallback.unit.test.ts tests/model-cache.unit.test.ts tests/payment-status.unit.test.ts tests/payment-observability.unit.test.tsResidual Risk / Follow-up
402body, but any external generated clients need to refresh from the updated schema to pick upterminalReasonandcheckStatusUrl