Skip to content

x402 settlement: post-merge cleanup from PR #281 review (naming, edge cases, secret hygiene) #283

Description

@vedanshujain

Non-blocking cleanup items surfaced during PR #281's (INC-C5, email dispatch and x402 settlement in-process) three rounds of review. None of these blocked merge; tracking them here so they don't get lost.

  • .changeset/in-process-email-and-x402-settlement.md names the edge-token kv key as settings:edgeToken; the real key is settings:otta-wh-token (see packages/plugin/src/payment-secrets.ts). Fix the changeset/changelog-facing name.
  • Check the still-unreleased .changeset/payment-secrets-write-only-kv.md — it may still document the old settings:x402FacilitatorSecret key as live; it was renamed to settings:x402FacilitatorApiKey in PR [Plugin][Domain][Adapters] Email dispatch and x402 settlement in-process #281 and should be corrected if so.
  • Stale symbol names after the kv key rename: PaymentSecrets.x402FacilitatorSecret, x402FacilitatorSecretFromKv, and in packages/plugin/src/admin/settings-form.ts: fieldId: "x402FacilitatorSecret" / actionId: "save-x402-facilitator-secret" all still say "Secret" even though the field now holds an API key sent as a Bearer token. The fieldId/actionId have real churn cost (pinned by a test matrix) so a rename needs care, but a reader grepping for "FacilitatorSecret" currently lands on live code, cutting against the rename's purpose.
  • A fail-open edge case in the new RECEIPT_REBOUND guard exists only on the store-emdash tier: in packages/domain/src/orders/settle-order.ts, if dedupe(...) returns false but orderForDedupeKey(...) returns null, the code proceeds and settles — impossible on the SQL stores (single transaction/snapshot) but theoretically possible via two separate reads on store-emdash. Reviewers judged this unreachable today because dedupe's own implementation in packages/store-emdash/src/emdash-payment-event-store.ts already refuses a null read-back (returns CAS_RETRY instead), so the inconsistency is self-correcting in the same file — but it's fragile and worth hardening, e.g. by having dedupe hand back the bound order directly instead of a bare boolean.
  • The legacy settings:x402FacilitatorSecret kv key is only deleted when an operator next saves the facilitator-secret admin field; an operator who provisions the new key by other means and never touches that form again leaves the orphaned old secret sitting in kv indefinitely (harmless since nothing reads it, but worth a cleanup pass or a startup migration).
  • No concurrent (as opposed to sequential) variant of the cross-order-replay test exists yet — packages/store-postgres/test/order-flow.dialects.test.ts's new test settles two orders against the same dedupe key sequentially. The fix depends on Postgres's INSERT ... ON CONFLICT DO NOTHING blocking semantics under real concurrency; CLAUDE.md's "no oversell under concurrency" non-negotiable would ideally have a genuinely concurrent variant pinning this.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CQbJYWWm8tf8owshm7XRp8

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    follow-upDeferred non-blocking item from a phase review gate

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions