Skip to content

Feat - Native Tokens (ETH/SOL/XLM) Payment support#47

Open
akbarsaputrait wants to merge 18 commits into
masterfrom
feat/native-token-payments
Open

Feat - Native Tokens (ETH/SOL/XLM) Payment support#47
akbarsaputrait wants to merge 18 commits into
masterfrom
feat/native-token-payments

Conversation

@akbarsaputrait

@akbarsaputrait akbarsaputrait commented Jul 9, 2026

Copy link
Copy Markdown
Member

Summary

Native-token payments (SOL, ETH, BNB, POL, XLM) as a payment source were broken across the SDK because the code assumed the source token was 1:1 with USD. This collapsed three distinct amounts into one:

Amount Example (SOL → 1.3 USDC) Field
USD value 1.3 required.usd
Source amount (what user pays) 0.016885 SOL required.amount
Destination payout (what recipient gets) 1.3 USDC destFinalCallTokenAmount

For USDC/USDT the three coincide, so the bug only surfaced on native tokens.

This PR also includes a full code-review pass (P0–P2 fixes), UX improvements across connection and loading states, and several mobile flow fixes.

Changes

Native token payments (core)

  • Transfers (payWithToken, Solana, Stellar): use required.amount (source-token units), not required.usd.
  • Amount helpers: tokenBaseAmountToDecimalString / tokenAmountToBaseUnits handle both integer base-units and decimal strings.
  • create-payment: new optional preferredAmountUnits carries the source amount; toUnits stays the destination payout.
  • Fee call: amount is type-aware — source-token amount for exactIn, USD/destination for exactOut.
  • formatPaymentResponseToHydratedOrder: usdValue / destFinalCallTokenAmount now come from the destination. Source amount surfaced via metadata.sourceAmountUnits.
  • Precision: Solana lamports/transferAmount stay bigint (no Number() truncation).
  • UI: native tokens display the chain icon instead of logoURI on SelectToken and pay pages (nativeAsChainIcon).

Code review fixes (P0/P1)

  • Typed metadata.sourceAmountUnits: added to zRozoPayOrderMetadata Zod schema — removes all (order.metadata as any)?.sourceAmountUnits casts.
  • Native fallback guard: for native deposit sources, throws "sourceAmountUnits missing" instead of silently falling back to usdValue (the wrong USD destination payout).
  • HydrateWalletOption type: narrower { required: { token, amount: string }, fees: { usd } } replaces double as any casts on the hydrateOrder deposit-address path.
  • store as any eliminated: narrowed store param type to { dispatch } — the only field actually used.
  • tokenBaseAmountToDecimalString: widened param from BigIntStr to string.
  • bridge-utils prefferedToken typo fixed.
  • rozoPaymentResponse undefined on hydrate path: PayWithSolanaToken falls back to hydratedOrder.metadata?.sourceAmountUnits.
  • Price NaN guard: WaitingDepositAddress uses Number.isFinite + logs stale price warning.

Dual-chain / mobile flow

  • dualChainConnect flag not cleared: restored setDualChainConnect(false) accidentally dropped in refactor — fixes Phantom mobile Solana-only → "Pay with another wallet" doing nothing.
  • setRoute atomic update: merged route + routeMeta into a single state object in RozoPayProvider. Eliminates two-render race where isExplicitBackNavigation check fired before routeMeta arrived, causing the auto-connect gate to re-navigate the user back to SELECT_TOKEN on Phantom mobile payId flows.
  • React Query migration: useWalletPaymentOptions, useSolanaPaymentOptions, useStellarPaymentOptions, useDepositAddressOptions migrated to @tanstack/react-query.
  • Dual-chain wallet connect: dualChainConnect state in PayContext/RozoPayProvider; mobile dual-chain wallet flow via ConnectorList.

UX improvements

  • "Pay to Address" loading state: option is disabled while deposit chains load; chain icons replaced with a spinner (Option.loading prop on OptionsList).
  • ConnectorStellar error recovery: "Connection Failed" now shows Try Again + Cancel instead of a dead-end message.
  • ConnectorSolana error recovery: no error handling existed — spinner hung forever on rejection/expiry. Added adapter error event listener + same Try Again / Cancel buttons.

Infrastructure

  • Stellar SEP-0007 deep links: generateStellarDeepLink; WaitingDepositAddress generates them for Stellar deposit addresses.
  • Chain-aware token normalization: normalizeTokenAddress(chainId, token) replaces ad-hoc .toLowerCase().
  • Workspace deps: workspace:* in packages/connectkit and examples/nextjs-app.
  • Tooling: .oxfmtrc.json; AGENTS.md updated; CHANGELOG.md created.

Tested

  • pnpm build: 0 TypeScript errors (connectkit + pay-common).
  • pnpm lint: 0 errors on both packages.
  • pnpm --filter @rozoai/intent-common test: 41/41 pass.
  • Manual: Bridge SOL → 1.3 USDC; native ETH/BNB/POL deposit address flows; dual-chain Phantom mobile.

Follow-ups (not blocking)

  • WaitingWallet: no cancel path when wallet app doesn't open.
  • WaitingExternal: no timeout/cancel for abandoned exchange flows.
  • SelectToken empty state: no guidance on why no tokens are available.

…end-to-end

Native sources were treated as 1:1 USD-pegged, so amounts were wrong across
  the transfer, fee, create-payment, and display paths. Separate the three
  distinct amounts (USD value / source-token amount / destination payout) and
  thread them correctly.

  - payWithToken/Solana/Stellar transfers: use required.amount (source-token
    units) instead of required.usd
  - add decimal-tolerant helpers tokenBaseAmountToDecimalString /
    tokenAmountToBaseUnits (fixes "Cannot convert 0.0129… to a BigInt")
  - createPayment payload: send source.amount via new preferredAmountUnits,
    keep destination payout in toUnits (no longer conflated)
  - getFee amount is now type-aware: source-token amount for exactIn,
    USD/destination for exactOut
  - formatPaymentResponseToHydratedOrder: derive usdValue/destFinalCallTokenAmount
    from destination, not source.amount (fixes cancel → $0.02 + corrupted
    usdRequired); surface source.amount via metadata for deposit flow
  - default preferredSymbol now includes natives so they aren't filtered out
  - keep Solana lamports/transferAmount as bigint (no Number() precision loss)
  - tsconfig moduleResolution node → bundler (drops false ox@0.14.30 type error)
  - native tokens render chain icon instead of logoURI on SelectToken + pay pages
@vercel

vercel Bot commented Jul 9, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
intent-example Error Error Jul 17, 2026 10:08am

Request Review

@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

return await sendTransactionAsync({
to: getAddress(destinationAddress),
value: paymentAmount,
});

P1 Badge Switch to the selected native chain before sending

When the user selects a native EVM token on a chain different from the wallet's current chain, this branch sends without either switchChainAsync({ chainId: required.token.chainId }) or a chainId on sendTransactionAsync. Unlike the ERC-20 branch below, the wallet can therefore submit the value transfer on the currently selected chain while the backend expects the payment on required.token.chainId, which can make native ETH/BNB/POL payments fail or land on the wrong chain.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread package.json
…ment transaction logic

- Removed redundant imports for token formatting utilities.
- Consolidated token chain ID checks and data suffix handling for improved clarity and efficiency.
- Enhanced deep link generation for EVM payments by simplifying amount unit parsing.
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI review: 0 P0 / 6 P1 / 4 P2 — no blocking issues found

Labeled ai-review-passed. This is advisory; a human still merges.

P0 (blocking)

None.

P1 (should fix)

  • examples/nextjs-app/package.json:43-44 — dependency versions bumped to pre-release beta tags (@rozoai/intent-common@0.1.22-beta.1, @rozoai/intent-pay@0.1.35-beta.1). Shipping an example app pinned to beta packages risks pulling unstable/未-published SDK behavior into what is presumably a reference implementation for external developers. Confirm this is intentional and not a leftover from local testing before merge.
  • package.json:19-24dev:example:16 script was removed and replaced with start:example, but the dev script at line 20 still references pnpm run dev:example:16 ("dev": "pnpm run dev:common & pnpm run dev:pay & pnpm run dev:example:16"). This will break pnpm dev since the referenced script no longer exists.
  • package.json:18 — new build:example step added to the root build script. If examples/nextjs-app fails to build (e.g., due to the beta dependency bump above), this now hard-fails the whole monorepo build/release pipeline, where previously the example app wasn't part of the release-critical path. Worth confirming this coupling is intended.
  • examples/nextjs-app/components/BridgeMode.tsx:56-59 — behavior change: preferredSymbol now returns undefined instead of [TokenSymbol.USDC, TokenSymbol.USDT] for the non-EURC case. This silently changes what tokens are preferred/filtered for bridge payments in the example app with no accompanying test or comment explaining why undefined is now equivalent/better than an explicit list. Since this affects payment flow logic, it deserves at least a one-line justification or a test to confirm it doesn't regress the demo behavior.
  • Large bundle-analysis.html diff — this is a generated build artifact being committed into version control and diffed. It adds significant noise to the PR and has no reason to be tracked; if this is committed on every build, it will generate large, unreviewable diffs going forward. Recommend adding packages/connectkit/bundle-analysis.html to .gitignore.
  • Several large planning/spec docs were deleted from docs/superpowers/plans/ and docs/superpowers/specs/ in this diff, while docs/superpowers/ was simultaneously added to .gitignore. If these plans documented work that's still in-flight or not yet fully implemented (e.g., the auto-connect gate and payin poll fallback plans reference source files like useAutoConnectGate.ts and usePayinPolling.ts that appear in the bundle analysis, implying they were implemented), deleting the design docs removes the paper trail for why that logic exists, with no replacement reference. Consider linking to an ADR or keeping a lightweight changelog entry instead of a hard delete + gitignore.

P2 (nice to have)

  • examples/nextjs-app/components/BridgeMode.tsx:17-19 — the hook-call reordering (useRozoPayUI() before useSharedConfig()) plus the inserted blank line is a no-op change that adds diff noise without functional benefit; consider reverting for a cleaner diff, unless there's a deliberate reason (e.g., hook ordering lint rule) worth calling out in the PR description.
  • docs/NEXTJS_BEST_PRACTICES.md is a large, well-written new doc — good addition, no issues, but consider cross-linking it from the main README so it's discoverable.
  • .gitignore:9 — missing trailing newline retained from original file; minor style nit, not blocking.
  • package.json script naming: start:example vs. the removed dev:example:16 — naming convention across scripts (dev:example, dev:example:16, start:example) is inconsistent; consider a consistent <verb>:<target>[:variant] scheme for maintainability.

ai-pr-review · claude-sonnet-5

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

ai-pr-review swept this head (6314758).

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI review: 3 P0 (blocking) / 6 P1 / 4 P2

P0 findings must be addressed before merge. This is advisory — no auto-approve, no auto-block; a human decides.

P0 (blocking)

  • package.json:18-25dev:example:16 script (used by root dev command: "dev": "pnpm run dev:common & pnpm run dev:pay & pnpm run dev:example:16") is removed but the dev script still references it. Running pnpm dev from root will fail (dev:example:16 not found). Same for the new start:example — it's added but nothing wires it in; if intended to replace dev:example:16 in dev, the dev script was not updated to match.
  • examples/nextjs-app/package.json:43-44 — dependency versions bumped to pre-release/beta tags (@rozoai/intent-common: 0.1.22-beta.2, @rozoai/intent-pay: 0.1.35-beta.2). Shipping a beta SDK dependency in an example that's part of the build pipeline (build:example now runs in root build) risks pulling untested/unstable payment logic into CI/release artifacts with no indication of what changed in the beta, and no test coverage added for the new feeType/payment-affecting fields.
  • examples/nextjs-app/components/BridgeMode.tsx:57-59 and CheckoutMode.tsx, DepositMode.tsxfeeType is now threaded into live payment paths (resetPayment, createPayment, RozoPayButton.Custom) with zero test coverage anywhere in the diff. This changes payment/fee behavior in example flows that presumably mirror production integration guidance, but there's no verification that FeeType.ExactIn/ExactOut produce correct fee behavior end-to-end.

P1 (should fix)

  • examples/nextjs-app/components/BridgeMode.tsx:56-60preferredSymbol logic changed from isDestinationEURC ? [EURC] : [USDC, USDT] to isDestinationEURC ? [EURC] : undefined. This silently drops the explicit USDC/USDT preference for all non-EURC destinations, which could change which token is preselected/defaulted in the picker. Verify this is an intentional behavior change and not a regression — no comment or changelog note explains why undefined is now equivalent/preferred.
  • examples/nextjs-app/components/BridgeMode.tsx:8-10 — moved const { resetPayment } = useRozoPayUI() above useSharedConfig() with a stray blank line inserted; pure churn with no functional purpose, adds noise to the diff.
  • examples/nextjs-app/components/BridgeMode.tsx, CheckoutMode.tsx, DepositMode.tsxFeeType imported from @rozoai/intent-common but never referenced directly in these files (only config.feeType typed as FeeType | undefined is passed through) — check whether the import is actually used or just adds an unused import lint warning (in CheckoutMode.tsx and BridgeMode.tsx/DepositMode.tsx the FeeType symbol itself doesn't appear to be used besides the type occurrences already covered via ParamFormValues/SharedConfig). If unused, oxlint/tsc should flag it — verify build actually passes.
  • examples/nextjs-app/components/ParamForm.tsx:206-221 — new Fee Type selector defaults to FeeType.ExactIn via values.feeType ?? FeeType.ExactIn in the value prop only; the underlying values.feeType in state is never defaulted, so generated code snippets or submitted configs could carry undefined while the UI displays "Exact In," causing a mismatch between what's shown and what's sent (unless DEFAULTS.feeType in useSharedConfig.ts covers it — confirm all three mode components consistently rely on that default and don't construct configs bypassing it).
  • docs/NEXTJS_BEST_PRACTICES.md — new 167-line doc file; not wrong, but no reference to feeType/fee configuration despite this PR adding feeType support to the SDK's example integration — consider whether provider docs should mention the new prop for consistency, or that's intentionally out of scope.
  • General — the diff mixes a large deletion of planning/spec docs (docs/superpowers/plans/*, docs/superpowers/specs/*) with unrelated functional SDK/example changes (feeType support, dependency bumps, package.json script changes) in one PR. This makes review harder and increases risk that the doc deletions (which look like routine cleanup) mask the more consequential dependency/script changes. Consider splitting.

P2 (nice to have)

  • .gitignore — adding docs/superpowers/ is reasonable given the deletion of tracked files under that path, but combined with the deletion diff, it's unclear if this is meant to stop tracking these plan/spec docs going forward (untracking) vs. genuinely deleting historical records. Consider a short commit message/PR note clarifying intent for future contributors.
  • examples/nextjs-app/components/ParamForm.tsx:207 — Fee Type label text could clarify what "Exact In" vs "Exact Out" mean for someone unfamiliar with the SDK (brief inline help text or tooltip).
  • package.json script naming — start:example was added but dev:example:16 was removed; naming asymmetry (:16 suffix vs none) suggests leftover naming convention drift from a prior Next.js version pin; consider consistent naming across dev:example, start:example.
  • docs/NEXTJS_BEST_PRACTICES.md is a large addition with good structure, but duplicates a fair amount of content already described as living in PROVIDER_SETUP.md — consider tightening cross-references or merging rather than maintaining two parallel guidance docs.

ai-pr-review · claude-sonnet-5

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

AI review: 3 P0 (blocking) / 6 P1 / 5 P2

P0 findings must be addressed before merge. This is advisory — no auto-approve, no auto-block; a human decides.

P0 (blocking)

  • packages/connectkit/bundle-analysis.html is a large generated build artifact being committed directly into the diff. This is not a secret leak, but it's a checked-in build output that will bloat the repo and drift from source on every build — should be gitignored, not committed. If this file is regenerated automatically as part of CI/build and always diffed into PRs, that's a repo hygiene problem that should block merge until addressed (either gitignore it or move it out of version control).
  • package.json:18 — root build script changed from build:common && build:pay to also run build:example (examples/nextjs-app). If examples/nextjs-app build failure now blocks the release script (release: build && ... publish), a broken example app could block publishing @rozoai/intent-pay/@rozoai/intent-common to npm. Given this pipeline directly gates package releases (which is payment-critical infrastructure), a hard coupling between an example/demo app's build and the release of production packages is risky and should be verified/fixed before merge.
  • examples/nextjs-app/package.json bumps @rozoai/intent-common to 0.1.22-beta.2 and @rozoai/intent-pay to 0.1.35-beta.2 — pinning the example (and by extension anyone copying its snippets) to pre-release/beta SDK versions. If this example is customer/developer-facing documentation/reference code, shipping beta dependencies as the canonical example is a payment-integration risk (beta APIs can change/break before stable release). Should be confirmed intentional and time-boxed.

P1 (should fix)

  • examples/nextjs-app/components/BridgeMode.tsx:57-60preferredSymbol behavior changed from always specifying [USDC, USDT] fallback to undefined when not EURC. This changes the SDK's preferred-token behavior when the destination isn't EURC; no explanation in the diff for why USDC/USDT preference was dropped. Could unintentionally widen or narrow payment method eligibility. Needs a test or rationale in the PR description.
  • feeType prop plumbed through BridgeMode, CheckoutMode, DepositMode, ParamForm, and useSharedConfig with zero tests added for either FeeType path (ExactIn/ExactOut). This affects the actual fee/payment amount charged to users — a change to payment-affecting logic with no test coverage. While this is example/demo code (lower stakes than SDK core), if this pattern is meant to demonstrate correct integration to third-party developers, incorrect fee semantics propagate directly to their production doubling as documentation.
  • examples/nextjs-app/hooks/useSharedConfig.ts defaults feeType: FeeType.ExactIn — but there's no migration/versioning handling for existing localStorage blobs that predate this field; loadFromStorage isn't shown in the diff, so it's unclear whether stale stored configs missing feeType will silently fall back correctly or produce undefined behavior downstream. Worth verifying.
  • package.json:24dev:example:16 removed and replaced with start:example, but dev script (line 20) still references dev:example:16 ("dev": "pnpm run dev:common & pnpm run dev:pay & pnpm run dev:example:16"). This is a dangling reference to a now-deleted script — pnpm dev will fail.
  • Several deleted planning/spec docs (docs/superpowers/plans/*, docs/superpowers/specs/*) reference real code changes (e.g., useAutoConnectGate.ts, usePayinPolling.ts, WaitingDepositAddress fallback) that appear to have already been implemented per the bundle analysis file (which lists useAutoConnectGate.js and usePayinPolling.js as existing modules). These plans are being deleted as part of this diff with no corresponding diff hunks shown for the actual implementation files — if those implementations are not part of this PR (already merged separately), this is fine as pure docs cleanup; but if this PR is meant to also land those payment-flow changes, the diff excerpt provided doesn't show the actual WaitingDepositAddress/RozoPayModal/usePayinPolling code changes for this reviewer to verify test coverage or correctness — flagging as a gap in reviewable scope.
  • .gitignore adds docs/superpowers/ but the diff simultaneously deletes files under docs/superpowers/plans/ and docs/superpowers/specs/ in the same commit — if this directory was previously tracked, ignoring it going forward without confirming all previously-committed files are removed could leave stale tracked files invisible to future git status, causing silent drift.

P2 (nice to have)

  • examples/nextjs-app/components/BridgeMode.tsx:18-19 — added blank line after destructuring useRozoPayUI before useSharedConfig(); pure style, but the reordering (moving resetPayment hook call above useSharedConfig) has no functional justification shown — minor readability/ordering nit.
  • docs/NEXTJS_BEST_PRACTICES.md is a solid, well-written guide — no functional issues, but consider adding a short "Troubleshooting" anchor link consistency check since it references PROVIDER_SETUP.md anchors (#alternative-pattern--dynamic-with-ssr-false, #props-reference, etc.) that aren't verifiable in this diff — worth confirming those anchors actually exist in PROVIDER_SETUP.md to avoid dead links.
  • ParamForm.tsx new Fee Type <Select> defaults display value to FeeType.ExactIn via values.feeType ?? FeeType.ExactIn but the underlying config default is also ExactIn — slightly redundant fallback logic in two places (useSharedConfig defaults and ParamForm render); could centralize to one source of truth.
  • Unused import risk: FeeType is imported in BridgeMode.tsx, DepositMode.tsx, and CheckoutMode.tsx but only used as a type annotation via config.feeType (already typed through SharedConfig) — double check these imports are actually used (e.g., FeeType enum members referenced) and not flagged as unused by lint, since the diff only shows the import line changing, not clear usage of the FeeType symbol itself in those files.
  • Commit-message conventions and mechanical, highly verbose planning docs (thousands of lines) being deleted in bulk — consider squashing this cleanup into a separate, smaller PR from the feature work (feeType plumbing) for easier review isolation.

ai-pr-review · claude-sonnet-5

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

ai-pr-review swept this head (99f2577).

- Add getTokenPrices API (pay-common) for live USD conversion
- WaitingDepositAddress: convert USD → source-token units via live price
  for native sources (SOL/ETH/XLM); getFee uses actual feeType (no
  ExactOut override)
- buildCreatePaymentPayload: destination stays at toUnits — fee is on
  source side, not destination (was: ExactOut branch subtracted fee)
- usePaymentState.payWithDepositAddress: source amount from fees response
  (fee-inclusive) for both native and stablecoin; was bare toUnits
- fetchApi: surface BE structured error message, not raw JSON envelope
- PayWithToken/{Solana,Stellar}: scope native ExactOut override to
  fee-quote only (createPayment keeps payParams.feeType)
- WaitingDepositAddress QR: nativeAsChainIcon for native tokens (single
  chain badge, no redundant token+chain pair)
- Bump @rozoai/intent-common → 0.1.22-beta.3, @rozoai/intent-pay → 0.1.35-beta.3
- Add evm-native / solana-native / stellar-native e2e specs
- Wire them into playwright.config.ts (sequential deps on existing
  merchant suite; reroute deposit-stellar-to-evm dep)
- Extract APP_ID to app/const.ts (shared by Bridge/Checkout/DepositMode)
- snippets.ts: import reorder + prettier semi-colons
@github-actions

Copy link
Copy Markdown

AI review: 2 P0 (blocking) / 8 P1 / 5 P2

P0 findings must be addressed before merge. This is advisory — no auto-approve, no auto-block; a human decides.

P0 (blocking)

  • examples/nextjs-app/e2e/payment-flows/merchant/evm-native.spec.ts, solana-native.spec.ts, stellar-native.spec.ts: these new E2E specs "move real money" (per their own doc comments) against mainnet, driven by real seed phrases/secrets (E2E_EVM_SEED_PHRASE, E2E_SOLANA_SEED_PHRASE, E2E_STELLAR_SECRET) and a merchant appId. Secrets are only referenced via env vars in these files (not literal leaks), but committing tests that transact real funds with production merchant accounts as part of a CI-triggerable pipeline is high risk — confirm these are excluded from any automatic/CI run (they rely on .env.e2e being unset to skip, which is a soft guard, not a hard gate) and that no .env.e2e file or real secret material is included in this diff. If .env.e2e or equivalent secrets file is anywhere in the actual full diff (truncated in this view), that's a P0 leaked-secret finding — please confirm it's git-ignored and not added here.
  • examples/nextjs-app/package.json: bumps @rozoai/intent-common to 0.1.22-beta.3 and @rozoai/intent-pay to 0.1.35-beta.3 — shipping a beta/pre-release SDK version as the example app's (and by extension, documented "quick start") dependency. If this example is used as the reference for integrators or is part of a release train, pinning to beta versions with zero indication of stability risk in the PR is a payment-path-adjacent risk (these packages contain the payment button / bridge / checkout logic) with no test coverage shown for the beta delta itself.

P1 (should fix)

  • examples/nextjs-app/lib/snippets.ts / components/BridgeMode.tsx: APP_ID was moved to app/const.ts but hardcoded to "rozoBridgeStellar" — this is a demo/example app id, but worth confirming it isn't accidentally a real production merchant appId being made public in example docs/snippets that developers will copy-paste.
  • components/BridgeMode.tsx:53-58: preferredSymbol changed from [TokenSymbol.USDC, TokenSymbol.USDT] to undefined for the non-EURC branch. This changes default payment-method filtering behavior for all bridge payments in the example app with no explanation in the diff and no test coverage — worth confirming this is intentional and matches the SDK's new default-preference semantics rather than an oversight.
  • components/ParamForm.tsx: new feeType select defaults via values.feeType ?? FeeType.ExactIn in the value prop, but SharedConfig's DEFAULTS.feeType is already FeeType.ExactIn — redundant but harmless; however, there's no validation that FeeType.ExactOut is meaningfully supported end-to-end (e.g., in generateBridgeSnippet/generateCheckoutSnippet/generateDepositSnippet) — the snippet generators in lib/snippets.ts were not updated to include feeType in generated code, so the "copy this code" snippets will silently omit the fee type the user configured in the form, producing incorrect example code for users who select ExactOut.
  • examples/nextjs-app/e2e/playwright.config.ts: new merchant-evm-native / merchant-solana-native / merchant-stellar-native projects are chained via dependencies to existing merchant-* projects and now gate deposit-stellar-to-evm's dependency chain (changed from merchant-stellar to merchant-stellar-native). This lengthens the sequential real-funds dependency chain — a failure in any earlier real-money test now blocks strictly more downstream tests than before, increasing flakiness/blast radius of the (already real-funds) E2E suite with no error/retry hardening added (retries: 0 everywhere).
  • examples/nextjs-app/e2e/payment-flows/merchant/stellar-native.spec.ts: comment notes payInWithStellarHeadless hardcodes a USDC text filter, so the XLM option click is inlined here rather than mutating the shared helper — this is a known code-duplication/drift risk flagged by the author but not fixed; the inlined selector logic (rozopay-option-${XLM_SOURCE_OPTION_ID}) could silently break if the shared helper changes without updating this copy.
  • package.json (root): build now runs build:example (which runs next build for examples/nextjs-app) as part of the standard build pipeline. If examples/nextjs-app's build requires env vars (API keys, RPC URLs) that aren't present in all environments running pnpm build, this could break builds that previously succeeded (e.g., library-only CI/publish flows) — worth confirming build:pay/build:common publish paths don't now indirectly depend on the example app building successfully.
  • package.json (root): dev:example:16 script removed and replaced by start:example, but dev script still references pnpm run dev:example:16 ("dev": "pnpm run dev:common & pnpm run dev:pay & pnpm run dev:example:16" is unchanged per diff context) — this now points to a deleted script, which will break pnpm dev. Please verify/fix.
  • docs/superpowers/plans/*.md and docs/superpowers/specs/*.md: five large planning/spec documents are deleted while .gitignore adds docs/superpowers/ — if these plans documented decisions/rationale still relevant to shipped code (e.g., the wallet auto-connect gate, payin poll fallback — both of which appear to have been implemented, given usePayinPolling, useAutoConnectGate in the bundle-analysis tree), losing this design history from version control removes an audit trail for why payment-critical logic (auto-connect routing, payin detection fallback) was built the way it was. Consider preserving these outside the ignored path or in release notes.

P2 (nice to have)

  • docs/NEXTJS_BEST_PRACTICES.md: solid, well-written guide; minor nit — the RootLayout example in §3 uses async function RootLayout with await headers(), but §1's example uses a sync RootLayout. Consider a one-line callout that the cookie-based pattern requires switching to an async layout, since that's an easy detail to miss when copy-pasting.
  • lib/snippets.ts: large diff is almost entirely whitespace/formatting (added semicolons, import reordering) mixed with the actual APP_ID extraction — makes the functional change harder to review; consider separating formatting-only changes into their own commit next time.
  • hooks/useSharedConfig.ts: feeType?: FeeType is optional in the interface but DEFAULTS always sets it — consider making it required (non-optional) for consistency now that a default always exists, simplifying downstream config.feeType optional-chaining/fallbacks in BridgeMode.tsx/CheckoutMode.tsx/DepositMode.tsx.
  • examples/nextjs-app/e2e/payment-flows/merchant/*.spec.ts: repeated boilerplate (payId capture + afterEach reporting) across all three merchant native specs — consider extracting a shared test.describe wrapper or fixture to reduce duplication, especially since the pattern will likely be copied again for future chains.
  • .gitignore: trailing newline still missing after the added docs/superpowers/ line (\ No newline at end of file persists) — minor style nit.

ai-pr-review · claude-sonnet-5

- add AGENTS.md
- pay-common: safe getChainName fallback, chain-aware normalizeTokenAddress
- connectkit: dualChainConnect state + fallback QueryClientProvider
- connectkit: migrate payment option hooks to @tanstack/react-query
- connectkit: add generateStellarDeepLink helper
- connectkit: remove refreshUtils.ts
- point @rozoai/intent-common/pay to workspace:* in package.json files
- update pnpm-lock.yaml for workspace dependency resolution
- add packages/connectkit/.oxfmtrc.json
@github-actions

Copy link
Copy Markdown

AI review: 3 P0 (blocking) / 9 P1 / 4 P2

P0 findings must be addressed before merge. This is advisory — no auto-approve, no auto-block; a human decides.

P0 (blocking)

  • Committed build artifact / generated file: packages/connectkit/bundle-analysis.html is a huge generated rollup-plugin-visualizer output checked into the diff. This is not source and should never be committed — it bloats the repo, is impossible to review meaningfully, and typically indicates a build step accidentally ran and git add -A'd output into the PR. Should be removed and gitignored.

  • Merchant E2E tests move real mainnet funds with weak/no safety net: examples/nextjs-app/e2e/payment-flows/merchant/{evm,solana,stellar}-native.spec.ts explicitly state "THIS TEST MOVES REAL MONEY" and depend only on env-var presence (E2E_MERCHANT_APP_ID, seed phrases/secrets) to skip. There is no evidence of amount caps, an allow-list of destination addresses, or a check that E2E.merchant.appId/destination isn't accidentally pointed at a production merchant in CI. If CI secrets are ever populated (e.g., a misconfigured environment or accidental E2E_* var leak into a shared CI runner), this silently sends real ETH/SOL/XLM. This needs a hard safety gate (e.g., require an explicit E2E_ALLOW_REAL_FUNDS=1 flag in addition to the credentials, and/or a testnet-first default) before merge, given the "payout/withdrawal logic + zero/weak test coverage guard" concern extends to test code that can trigger real payouts.

  • Stellar secret key handling in test helpers: stellar-native.spec.ts reads E2E.stellar.secret! and passes it directly into useStellarSigner(page, ...), which (per usage) likely gets evaluated/injected into the browser page context. If this secret is ever logged (e.g., via Playwright trace/video capture on failure, or console.log in useStellarSigner), it could leak a live wallet secret into CI artifacts. Confirm useStellarSigner never logs the raw secret, and that Playwright trace capture doesn't record page-injected globals containing it — otherwise this is a leaked-secret risk in CI logs/artifacts.

P1 (should fix)

  • examples/nextjs-app/lib/snippets.ts APP_ID moved but still hardcoded to a fixed string (app/const.ts"rozoBridgeStellar"): the doc/checklist elsewhere emphasizes swapping the demo appId before production, but here the constant literally changed from rozoDemo to rozoBridgeStellar with no indication of what this ID is tied to (real merchant? sandbox?). Worth confirming this isn't accidentally pointing example code at a non-sandbox/production appId.

  • BridgeMode.tsx preferredSymbol regression: preferredSymbol changed from isDestinationEURC ? [EURC] : [USDC, USDT] to isDestinationEURC ? [EURC] : undefined. This silently drops the explicit USDC/USDT preference filter for all non-EURC destinations. If undefined doesn't map to "all tokens allowed" the same way the SDK defaults expect, this could change which source tokens are offered to the user without a corresponding test verifying equivalence.

  • New feeType prop plumbed through Bridge/Checkout/Deposit modes without any test coverage: feeType (via FeeType.ExactIn/FeeType.ExactOut) is a new fee-behavior toggle passed into resetPayment, createPayment, and RozoPayButton.Custom across three modes (BridgeMode.tsx, CheckoutMode.tsx, DepositMode.tsx, ParamForm.tsx, useSharedConfig.ts). Fee computation affects the actual amount charged/paid — this is payment-adjacent logic with zero unit/E2E coverage added in this diff. Given fee type directly affects settlement amounts, at least a snippet-generation test or an E2E assertion that feeType propagates correctly to the SDK call would be expected.

  • Dependency version bump to beta packages in a shipped example app: examples/nextjs-app/package.json bumps @rozoai/intent-common to 0.1.22-beta.4 and @rozoai/intent-pay to 0.1.35-beta.4. Shipping a demo/reference app pinned to beta SDK versions risks demonstrating unstable behavior to external developers evaluating the SDK, and could break if the beta is later yanked/republished. Should be pinned to a released version before merge, or clearly flagged as intentional pre-release testing.

  • Root package.json build script now also builds the example app (build:example added to the build composite script). If examples/nextjs-app's build has any dependency on env vars, API keys, or network calls (e.g., static generation hitting a live API) that aren't available in all CI/build contexts, this could break the core SDK build pipeline for reasons unrelated to the SDK itself. Worth confirming the example build is hermetic/fast and doesn't introduce flakiness to pnpm build.

  • dev:example:16 script removed/renamed to start:example, and examples/nextjs-16 presumably now unused/undocumented: the root dev script still references dev:example:16 conceptually via pnpm run dev:example:16 — wait, actually the diff shows dev still calls pnpm run dev:example:16 but that script was deleted and replaced by start:example. This is a broken script: "dev": "pnpm run dev:common & pnpm run dev:pay & pnpm run dev:example:16" still references the now-deleted dev:example:16 script name. Running pnpm dev from root will fail.

  • Playwright merchant-stellar-native test hardcodes a workaround comment ("ponytail") noting payInWithStellarHeadless hardcodes a USDC text filter, so the test inlines its own click logic instead of fixing the shared helper. This is technical debt — the shared helper (payInWithStellarHeadless per the comment, though this file doesn't even use it) has a known bug/limitation baked in and is being routed around rather than fixed, which will bite future XLM-related tests too.

  • ParamForm.tsx fee type default silently coerces undefined to FeeType.ExactIn in the <Select value={values.feeType ?? FeeType.ExactIn}> line, but the parent state (values.feeType) may remain undefined since onChange only fires on user interaction — meaning the visually-selected default may not match what's actually sent to the SDK if a consumer reads config.feeType directly before any interaction. Minor state/UI mismatch risk.

  • New E2E spec files use a magic "ponytail" comment convention without explanation (e.g., evm-native.spec.ts, solana-native.spec.ts, solana-native.spec.ts): these appear to be shorthand notes for future maintainers referencing internal source constants, but the term isn't defined anywhere in the diff — likely fine internally, but confusing to external contributors reading test code.

P2 (nice to have)

  • docs/superpowers/ deletion en masse: multiple large planning/spec docs under docs/superpowers/plans/ and docs/superpowers/specs/ are deleted while .gitignore is updated to ignore docs/superpowers/ going forward. This is a reasonable cleanup (moving ephemeral planning docs out of version control) but the PR bundles a large amount of unrelated doc churn together with actual code changes (feeType, snippets, e2e tests), making review harder. Consider splitting doc/gitignore cleanup from functional changes in future PRs.

  • lib/snippets.ts reformatted with semicolons added throughout as an unrelated stylistic change bundled with the feeType/import reordering work — increases diff noise and makes it harder to spot the actual logic delta (e.g., the preferredSymbol change in BridgeMode.tsx is much easier to spot than it would be here because that file wasn't reformatted).

  • AGENTS.md and docs/NEXTJS_BEST_PRACTICES.md are large new documentation files; reasonable additions, but worth confirming they don't duplicate/conflict with existing CLAUDE.md guidance (the diff itself references CLAUDE.md as the "real" source of trutruth for architecture).

  • New .oxfmtrc.json with empty ignorePatterns: harmless but adds an empty-effect config file; could be omitted until there's an actual need to ignore patterns, or a comment could clarify why it exists if it's there for tooling detection.

ai-pr-review · claude-sonnet-5

- Add missing native specs: checkout/solana-native, deposit/evm-native,
  deposit/solana-native, merchant/polygon-native
- Add pay-to-address payment-flows mode (merchant-evm, merchant-evm-native,
  merchant-solana fixme) with correct DepositAddressPaymentOptions testIds
- Replace 30+ test:e2e:* npm scripts with single e2e/run.cjs runner
  (pnpm test:e2e -- <project>, --list, --mocked, --ui)
- Update playwright.config.ts with all new projects in dependency chain
- Update README with full E2E usage guide and maintenance contract

fix(sdk): usdRequired=0 shows wrong deposit-address chain options

- useDepositAddressOptions: guard enabled with usdRequired > 0 to prevent
  querying with zero value before order loads
- startMerchantDepositAddressCheckout: wait for order sentinel before
  returning so usdRequired is populated when pay-to-address is clicked
- getDepositAddressInfo: wait for data-value attributes to be non-empty
  before reading (async API population race)

fix(sdk): native source amount corrupts usdValue in formatPaymentResponseToHydratedOrder

- bridge-utils: detect native source token via address sentinel set
  (ethAddress/zeroAddress/SOL system program/XLM) instead of fiatISO;
  use destination.amount for native sources, source.amount for stablecoins

refactor(pay-common): expose isNativeToken from pay-common, remove duplicate

- token.ts: export NATIVE_TOKEN_ADDRESSES + isNativeToken(string) with
  zeroAddress added to sentinel set
- bridge-utils: import isNativeToken from same package, drop inline set
- connectkit/utils/token.ts: re-export isNativeToken from @rozoai/intent-common,
  drop local implementation
- Update all isNativeToken callers to pass address string instead of Token object
@github-actions

Copy link
Copy Markdown

AI review: 3 P0 (blocking) / 8 P1 / 6 P2

P0 findings must be addressed before merge. This is advisory — no auto-approve, no auto-block; a human decides.

P0 (blocking)

  • examples/nextjs-app/e2e/payment-flows/checkout/stellar-native.spec.ts: test.fixme is used with an async callback that contains real logic (calling payInWithStellarHeadless, waitForPayoutCompleted, etc.) instead of being skipped cleanly. While test.fixme should prevent execution, mixing a documented "currently disabled/rejected by SDK" test with a fully-implemented body (that moves real funds if test.fixme semantics are ever misconfigured, e.g. run via --grep bypassing fixme status in some Playwright versions) is risky for a spec explicitly labeled "THIS TEST MOVES REAL MONEY." Confirm test.fixme truly can't execute the body under any CLI/config combination used in this repo (e.g., test:e2e -- checkout-stellar-native --headed forcing it).
  • Payment/fee logic changes shipped with no updated test coverage: ParamForm.tsx, BridgeMode.tsx, CheckoutMode.tsx, and DepositMode.tsx add a new feeType (FeeType.ExactIn / FeeType.ExactOut) parameter that flows directly into resetPayment, createPayment, and RozoPayButton.Custom — i.e., payment-affecting logic (fee calculation mode). No unit/e2e test exercises the new feeType param or its effect on quoted/settled amounts. Per project policy this is payment logic with zero test coverage.
  • preferredSymbol behavior silently weakened: In BridgeMode.tsx and DepositMode.tsx, the non-EURC branch changed from [TokenSymbol.USDC, TokenSymbol.USDT] to undefined. This changes real payment routing — previously the SDK was constrained to stablecoins; now (for non-EURC destinations) any token symbol is allowed as preferred, which could route through unintended/volatile assets. This is a payment-logic change with no test coverage and no explanation in the diff/commit for why the constraint was dropped.

P1 (should fix)

  • docs/superpowers/** deleted but also newly .gitignored (.gitignore diff adds docs/superpowers/) — the directory is being removed from version control entirely mid-PR along with its content. If these plan/spec docs are meant to be historical record (they document exactly the auto-connect gate and payin-poll-fallback features touched by this same PR), deleting them removes the design rationale trail for future readers. Worth confirming intentional archival vs. accidental loss.
  • examples/nextjs-app/e2e/helpers.tssetupPaymentIdCapture: The response listener does response.json() unconditionally inside a .then() chain without checking content-type; non-JSON 2xx responses (e.g., redirects, static assets matching the URL substring intentapiv4.rozo.ai) will throw inside the promise, caught by .catch(() => {}), silently swallowing errors — acceptable for test helper but could mask real capture bugs since there's no logging on failure.
  • UUID regex matching in setupPaymentIdCapture: UUID_RE is not anchored (^...$) and not scoped to the id field structure strictly — if any other UUID-shaped field appears earlier in a differently-shaped response body (e.g. merchantId), a wrong ID could be captured since the code only checks body?.id and item?.result?.data?.id, which is fine, but worth double-checking all API responses hitting intentapiv4.rozo.ai don't have false-positive shapes.
  • payInWithStellarHeadless — swallowed try/catch around "Pay with another method": The catch {} around anotherMethod.click({ timeout: 5_000 }) blindly assumes any failure means "method list already visible," but it could also mean the button existence check failed for unrelated reasons (detached DOM, navigation). This could mask real UI regressions in CI as false test passes.
  • ParamForm.tsx new Fee Type select uses FeeType.ExactIn as an unconditional default (values.feeType ?? FeeType.ExactIn) without persisting that default back into values — if a parent reads config.feeType before the user touches the selector, it will get undefined even though the UI displays "Exact In," causing behavior/UI mismatch (e.g. feeType: c.feeType sent to resetPayment will be undefined, not FeeType.ExactIn, unless the user explicitly interacts).
  • examples/nextjs-app/app/const.ts APP_ID = "rozoBridgeStellar": Previously per-component consts were "rozoDemo"; now hard-coded to "rozoBridgeStellar" shared across Bridge/Checkout/Deposit modes. If this appId is tied to a specific merchant/environment config in the backend, using it for all three demo modes could produce confusing analytics/misrouted webhooks. Worth a comment explaining the naming and scope.
  • getDepositAddressInfo helper trims data-value attribute via ?.trim() — no validation that amount/address are non-empty strings before use downstream in tests; a truthy-but-empty attribute would pass the toHaveAttribute(..., /.+/) regex check (since /.+/ requires at least one char) so this is likely fine, but worth double-checking the regex is applied to the attribute value and not to presence only.
  • Docs (AGENTS.md, NEXTJS_BEST_PRACTICES.md) reference appId="rozoSandbox" as the "swap before launch" example, while the example app's actual const file now hardcodes "rozoBridgeStellar" — minor doc/code drift that could confuse new integrators about which is the "safe sandbox" ID.

P2 (nice to have)

  • examples/nextjs-app/e2e/helpers.ts: createMerchantPayId/openMerchantModal refactor (splitting startMerchantCheckout into smaller functions) is a good readability improvement — no issues, just noting it's clean.
  • Trailing newline removed then re-added inconsistently in .gitignore (\ No newline at out of file markers) — purely cosmetic, but worth normalizing EOF newlines repo-wide.
  • docs/NEXTJS_BEST_PRACTICES.md: Very thorough, but duplicates a lot of content already in PROVIDER_SETUP.md per its own admission ("For the full walkthrough... see PROVIDER_SETUP.md"). Consider trimming to reduce doc-maintenance surface area (two docs to keep in sync).
  • New Bridge/Checkout/Deposit "-native" e2e specs (evm-native.spec.ts, solana-native.spec.ts, stellar-native.spec.ts) are heavily copy-pasted across bridge/checkout/deposit with near-identical bodies aside from the helper function called (startBridgePayment vs startCheckoutPayment vs startDepositPayment). Consider extracting a shared parametrized test generator to reduce duplication and future drift risk (e.g., minimum amount comments repeated verbatim in every file).
  • Magic sentinel addresses duplicated across many spec files (ETH_SOURCE_OPTION_ID = "8453-0xEeee...E9E", SOL_SOURCE_OPTION_ID = "501-So111...") — worth hoisting into a shared constants module (e.g., e2e/constants.ts) instead of redefining the same literal + comment in every spec file.
  • README.md rewrite is a solid improvement in clarity; minor nit: the "Common commands" table mixes pnpm test:e2e:mocked and pnpm test:e2e -- <project> styles — consider standardizing on one invocation style for discoverability.

ai-pr-review · claude-sonnet-5

- Type metadata.sourceAmountUnits in zRozoPayOrderMetadata Zod schema
- Remove as any casts: HydrateWalletOption type, store param narrowed to { dispatch }
- Native deposit source throws on missing sourceAmountUnits instead of wrong usdValue fallback
- Fix rozoPaymentResponse undefined fallback in PayWithSolanaToken hydrate path
- Price NaN guard + stale flag warning in WaitingDepositAddress
- Fix tokenBaseAmountToDecimalString param: BigIntStr -> string
- Fix bridge-utils prefferedToken typo
- Fix dualChainConnect flag not cleared in second useEffect — fixes Phantom mobile Solana-only pay-with-another-wallet doing nothing
- Atomic setRoute: merge route+routeMeta into single state in RozoPayProvider — fixes two-render race on Phantom mobile payId mode causing isExplicitBackNavigation to miss and re-navigate user to SELECT_TOKEN
- Pay to Address loading: disabled option + spinner replaces chain icons (Option.loading prop)
- ConnectorStellar: add Try Again + Cancel buttons on connection error
- ConnectorSolana: add error state via adapter error event + Try Again / Cancel buttons
- Add CHANGELOG.md to connectkit
- RozoPayModal: auto-trigger injected connector connect() when inside
  Phantom in-app browser with Solana-only auto-connected (deeplink case)
- SelectMethod: extract disconnectAll() using Promise.allSettled with
  guards (isSolanaConnected/isStellarConnected) to prevent TypeError
  when disconnect is undefined on uninitialized adapter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant