Establish test suite (Vitest/Playwright/CI) + native Stellar & Aztec wallet integration - #191
Establish test suite (Vitest/Playwright/CI) + native Stellar & Aztec wallet integration#191Dannyorji wants to merge 2 commits into
Conversation
Adds Vitest + React Testing Library for component/unit coverage of the checkout (TicketInfo) and search/filter (MainContent) flows, a Playwright E2E spec covering the ticket purchase journey, and a GitHub Actions workflow that runs lint, type-check, unit tests, and E2E on every PR. Also adds missing aria-labels to the quantity stepper buttons in TicketInfo so they're addressable by both assistive tech and tests.
Replaces the mock_tx_... random-string wallet SDK with real chain-specific adapters. Stellar wallets (Freighter, Lobstr, WalletConnect, xBull) connect through @creit.tech/stellar-wallets-kit, sign real transactions, and submit them to Horizon; the transaction status API now looks up genuine Stellar tx hashes on Horizon instead of only simulating confirmation. Aztec support is scaffolded via an Azguard-compatible injected-provider adapter, clearly flagged where Aztec's wallet-connection RPC surface is still unstable. lib/walletSdk.ts stays the stable facade both checkout (TicketInfo) and the organizer payout prompt (ConnectWalletPrompt) already depended on, now lazy-loading the right chain adapter and exposing the connected account so the UI can show a real wallet name/address instead of "Connected".
❌ Deploy Preview for zicket failed.
|
|
@Dannyorji Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThe PR adds Stellar and Aztec wallet adapters, chain-aware wallet session state, on-chain Stellar transaction status checks, checkout updates, component tests, Playwright tests, and GitHub Actions CI jobs. ChangesWallet integration and testing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Attendee
participant TicketInfo
participant walletSdk
participant StellarWallet
participant Horizon
Attendee->>TicketInfo: Select tickets and confirm privacy
TicketInfo->>walletSdk: Sign transaction
walletSdk->>StellarWallet: Request wallet signature
StellarWallet-->>walletSdk: Return transaction hash
walletSdk-->>TicketInfo: Return signed transaction result
TicketInfo->>Horizon: Request transaction status
Horizon-->>TicketInfo: Return confirmed, failed, or pending status
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
lib/walletSdk.ts (2)
101-106: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the duplicated failure cleanup.
getOrLoadAdapteralready deletes the cache entry on rejection at lines 55-57. The block here repeats that logic. Keep only thecatchthat swallows the rejection so no unhandled rejection is reported.♻️ Proposed simplification
export function preloadWalletSDK(chain: WalletChain = DEFAULT_CHAIN): void { - const current = getOrLoadAdapter(chain); - current.catch(() => { - if (adapterLoadPromises.get(chain) === current) adapterLoadPromises.delete(chain); - }); + // getOrLoadAdapter already resets the cache entry on failure. + void getOrLoadAdapter(chain).catch(() => {}); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/walletSdk.ts` around lines 101 - 106, Remove the duplicated cache cleanup from preloadWalletSDK. In the current.catch handler, retain only rejection swallowing and rely on getOrLoadAdapter for deleting the adapterLoadPromises entry; preserve the preload behavior and avoid introducing additional error handling.
71-77: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winDe-duplicate concurrent connect calls.
Two concurrent
loadWalletSDKcalls for the same chain both observeisConnected() === false, so both calladapter.connect(). For Stellar this opens the wallet picker twice. Cache the in-flight connect promise per chain, in the same way the module load is cached.♻️ Proposed in-flight connect cache
+const connectPromises = new Map<WalletChain, Promise<unknown>>(); + export async function loadWalletSDK(chain: WalletChain = DEFAULT_CHAIN): Promise<WalletAdapter> { const adapter = await getOrLoadAdapter(chain); - if (!adapter.isConnected()) { - await adapter.connect(); - } + if (!adapter.isConnected()) { + const pending = + connectPromises.get(chain) ?? + adapter.connect().finally(() => connectPromises.delete(chain)); + connectPromises.set(chain, pending); + await pending; + } return adapter; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/walletSdk.ts` around lines 71 - 77, Update loadWalletSDK to cache in-flight adapter.connect() promises keyed by chain, reusing the existing module-load caching pattern. Before connecting, check the per-chain cache; store the promise when starting a connection and clear it after completion so later calls observe the connected adapter without duplicate wallet prompts.lib/wallet/stellarAdapter.ts (2)
38-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winValidate
NEXT_PUBLIC_STELLAR_NETWORKinstead of casting it.The cast accepts any value. A typo such as
mainnetsilently selects the testnet passphrase and testnet Horizon URL, so a production deployment would submit to testnet without any signal. Normalize the value and fail fast on unknown input.♻️ Proposed validation
-const NETWORK = (process.env.NEXT_PUBLIC_STELLAR_NETWORK ?? "testnet") as "testnet" | "public"; +const RAW_NETWORK = process.env.NEXT_PUBLIC_STELLAR_NETWORK ?? "testnet"; +if (RAW_NETWORK !== "testnet" && RAW_NETWORK !== "public") { + throw new Error( + `Invalid NEXT_PUBLIC_STELLAR_NETWORK "${RAW_NETWORK}". Use "testnet" or "public".` + ); +} +const NETWORK: "testnet" | "public" = RAW_NETWORK;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/wallet/stellarAdapter.ts` around lines 38 - 42, Replace the unsafe cast in the NETWORK configuration with explicit normalization and validation of NEXT_PUBLIC_STELLAR_NETWORK. Accept only the supported testnet/public values, preserve their existing passphrase and Horizon URL mappings, and fail fast with a clear error for unknown values so invalid configuration cannot default to testnet.
135-147: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMap Horizon submission failures to actionable messages.
server.submitTransactionandserver.loadAccounthave no timeout and no error translation. Two consequences:
- A missing or unfunded account makes
loadAccountreject with a 404 error. The user sees a generic message instead of a funding hint.- Horizon answers
504when a submission is still pending. The current code treats that as a failure, although the transaction may still confirm.Wrap the Horizon calls and surface the
extras.result_codesdetail, and add an explicit request timeout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/wallet/stellarAdapter.ts` around lines 135 - 147, Update the Horizon interactions in the adapter’s account-loading and transaction-submission flows to use an explicit request timeout and catch failures. Translate 404 account-load errors into an actionable funding message, treat submission HTTP 504 responses as potentially pending rather than a definitive failure, and include Horizon’s extras.result_codes details in other surfaced errors.lib/user-session-sync.ts (1)
46-48: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winValidate the restored wallet fields.
parseStatetrusts the persisted JSON. Any value passes aswalletChain, so the declaredWalletChaintype can be wrong at runtime. A non-stringwalletAddressalso reachesshortenAddressinapp/components/WalletConnectionIndicator.tsx, where.lengthand.slicewould throw during render.Narrow the values while parsing.
♻️ Proposed validation
+const WALLET_CHAINS: WalletChain[] = ['stellar', 'aztec']; +const asString = (value: unknown): string | null => + typeof value === 'string' && value.length > 0 ? value : null; + const parseState = (value: string | null): SessionState | null => {- walletAddress: parsed.walletAddress ?? null, - walletName: parsed.walletName ?? null, - walletChain: parsed.walletChain ?? null, + walletAddress: asString(parsed.walletAddress), + walletName: asString(parsed.walletName), + walletChain: WALLET_CHAINS.includes(parsed.walletChain as WalletChain) + ? (parsed.walletChain as WalletChain) + : null,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/user-session-sync.ts` around lines 46 - 48, Update parseState to validate restored walletAddress, walletName, and walletChain values before assigning them: retain only strings, and for walletChain additionally restrict values to the supported WalletChain members; otherwise use null. Ensure walletAddress cannot pass a non-string value to shortenAddress while preserving valid persisted wallet data.lib/wallet/aztecAdapter.ts (1)
14-14: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winImport
AztecAddressandWalletfrom the narrower Aztec JS entry points.At
lib/wallet/aztecAdapter.ts:14andlib/wallet/aztecAdapter.ts:49, importing from the package root pulls in the large@aztec/aztec.jsentrypoint just for address validation and wallet provider access. Use@aztec/aztec.js/addressesforAztecAddress.fromString, and importWalletfrom its specific entrypoint if available for your installed Aztec version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/wallet/aztecAdapter.ts` at line 14, Update the imports in aztecAdapter.ts to use narrower Aztec JS entry points: import AztecAddress from `@aztec/aztec.js/addresses` for address validation, and import Wallet from its version-appropriate specific entrypoint instead of the `@aztec/aztec.js` package root. Preserve the existing AztecAddress.fromString and Wallet usage.e2e/ticket-purchase.spec.ts (1)
49-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the chain-specific wallet pickers.
This test stays on Stellar and only checks Freighter/Lobstr/WalletConnect. Add coverage for the other supported wallets: xBull on Stellar, and Azguard on Aztec. If chain selection gates the picker contents, switch to Aztec and assert the Aztec wallet option is available.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/ticket-purchase.spec.ts` around lines 49 - 55, Extend the wallet picker coverage in the test around the existing walletModal assertions: verify xBull is available for Stellar, then switch the selected chain to Aztec when picker contents are chain-specific, reopen or inspect the picker, and assert that Azguard is available there. Preserve the existing Freighter, Lobstr, and WalletConnect checks.app/api/transactions/[txHash]/status/route.ts (1)
33-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the transaction-status handler in CI.
The component test replaces
useTransactionStatus, so it does not execute the Horizon and simulation split. The V8 include list also omitsapp/api, so CI cannot expose zero coverage for this route.
app/api/transactions/[txHash]/status/route.ts#L33-L43: Add isolated handler tests for successful records, unsuccessful records, lookup errors, and non-Stellar hashes.vitest.config.ts#L17-L21: Addapp/api/**/*.{ts,tsx}to the V8 coverage include list.Proposed coverage include
include: [ "app/components/explore/**/*.{ts,tsx}", + "app/api/**/*.{ts,tsx}", "lib/**/*.{ts,tsx}", "hooks/**/*.{ts,tsx}", ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/transactions/`[txHash]/status/route.ts around lines 33 - 43, Add isolated handler tests for app/api/transactions/[txHash]/status/route.ts covering successful records, unsuccessful records, Horizon lookup errors, and non-Stellar hashes; update vitest.config.ts coverage includes to add app/api/**/*.{ts,tsx} so the route is measured in CI.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 14: Update both actions/checkout steps in .github/workflows/ci.yml at
lines 14-14 and 46-46 to set persist-credentials to false, preserving the
existing checkout behavior and avoiding credential persistence in PR-triggered
jobs.
In `@app/api/transactions/`[txHash]/status/route.ts:
- Around line 35-42: Update getRealTxStatus to use an explicit timeout for
Horizon reads, configuring Horizon.Config.setTimeout or reusing a shared
Horizon.Server backed by an HTTP-client timeout before
transactions().transaction(hash).call(). Preserve the existing confirmed,
failed, and pending status handling.
In `@app/components/organizer/ConnectWalletPrompt.tsx`:
- Around line 80-100: Complete the radiogroup keyboard interaction in the
CHAIN_OPTIONS map within ConnectWalletPrompt: implement roving tabIndex with
only the selected option at 0 and all others at -1, and handle ArrowLeft,
ArrowRight, ArrowUp, and ArrowDown to move selection and focus between options
with wraparound. Preserve the existing click selection and loading-state
behavior.
In `@e2e/ticket-purchase.spec.ts`:
- Around line 20-23: Update the wallet mock’s signTransaction flow in the e2e
ticket purchase setup so it bypasses or mocks the getKit().signTransaction(),
TransactionBuilder.fromXDR(), and Horizon submission boundary. Return a valid
synthetic signedPayload/txHash combination, ensuring the purchase reaches the
/api/transactions/*/status route without attempting to parse the placeholder
XDR.
In `@lib/wallet/aztecAdapter.ts`:
- Around line 71-76: Validate the result from provider.request in the
transaction flow before returning from the adapter method containing
send_transaction. Reject the response when txHash is missing or not a valid
string, ensuring the malformed response throws a wallet error instead of
returning undefined; preserve the existing successful return shape for valid
responses.
In `@lib/wallet/stellarAdapter.ts`:
- Around line 1-14: Update the documentation comment above the Stellar wallet
adapter to list only the wallet modules registered by buildModules(): Freighter,
Lobstr, xBull, and WalletConnect. Remove Albedo, Rabet, and Hana from the
comment unless buildModules() is also updated to register them.
In `@playwright.config.ts`:
- Line 12: Update the reporter configuration in Playwright’s config so CI runs
both the GitHub reporter and the HTML reporter, preserving the existing
HTML-only behavior outside CI and ensuring the playwright-report directory is
generated during CI runs.
---
Nitpick comments:
In `@app/api/transactions/`[txHash]/status/route.ts:
- Around line 33-43: Add isolated handler tests for
app/api/transactions/[txHash]/status/route.ts covering successful records,
unsuccessful records, Horizon lookup errors, and non-Stellar hashes; update
vitest.config.ts coverage includes to add app/api/**/*.{ts,tsx} so the route is
measured in CI.
In `@e2e/ticket-purchase.spec.ts`:
- Around line 49-55: Extend the wallet picker coverage in the test around the
existing walletModal assertions: verify xBull is available for Stellar, then
switch the selected chain to Aztec when picker contents are chain-specific,
reopen or inspect the picker, and assert that Azguard is available there.
Preserve the existing Freighter, Lobstr, and WalletConnect checks.
In `@lib/user-session-sync.ts`:
- Around line 46-48: Update parseState to validate restored walletAddress,
walletName, and walletChain values before assigning them: retain only strings,
and for walletChain additionally restrict values to the supported WalletChain
members; otherwise use null. Ensure walletAddress cannot pass a non-string value
to shortenAddress while preserving valid persisted wallet data.
In `@lib/wallet/aztecAdapter.ts`:
- Line 14: Update the imports in aztecAdapter.ts to use narrower Aztec JS entry
points: import AztecAddress from `@aztec/aztec.js/addresses` for address
validation, and import Wallet from its version-appropriate specific entrypoint
instead of the `@aztec/aztec.js` package root. Preserve the existing
AztecAddress.fromString and Wallet usage.
In `@lib/wallet/stellarAdapter.ts`:
- Around line 38-42: Replace the unsafe cast in the NETWORK configuration with
explicit normalization and validation of NEXT_PUBLIC_STELLAR_NETWORK. Accept
only the supported testnet/public values, preserve their existing passphrase and
Horizon URL mappings, and fail fast with a clear error for unknown values so
invalid configuration cannot default to testnet.
- Around line 135-147: Update the Horizon interactions in the adapter’s
account-loading and transaction-submission flows to use an explicit request
timeout and catch failures. Translate 404 account-load errors into an actionable
funding message, treat submission HTTP 504 responses as potentially pending
rather than a definitive failure, and include Horizon’s extras.result_codes
details in other surfaced errors.
In `@lib/walletSdk.ts`:
- Around line 101-106: Remove the duplicated cache cleanup from
preloadWalletSDK. In the current.catch handler, retain only rejection swallowing
and rely on getOrLoadAdapter for deleting the adapterLoadPromises entry;
preserve the preload behavior and avoid introducing additional error handling.
- Around line 71-77: Update loadWalletSDK to cache in-flight adapter.connect()
promises keyed by chain, reusing the existing module-load caching pattern.
Before connecting, check the per-chain cache; store the promise when starting a
connection and clear it after completion so later calls observe the connected
adapter without duplicate wallet prompts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e713612a-d44e-4c02-a772-3a9d9721b16d
📒 Files selected for processing (19)
.env.example.github/workflows/ci.yml.gitignoreapp/api/transactions/[txHash]/status/route.tsapp/components/WalletConnectionIndicator.tsxapp/components/explore/EventCheckout/TicketInfo.test.tsxapp/components/explore/EventCheckout/TicketInfo.tsxapp/components/explore/MainContent.test.tsxapp/components/organizer/ConnectWalletPrompt.tsxe2e/ticket-purchase.spec.tslib/user-session-sync.tslib/wallet/aztecAdapter.tslib/wallet/stellarAdapter.tslib/wallet/types.tslib/walletSdk.tspackage.jsonplaywright.config.tsvitest.config.tsvitest.setup.ts
| name: Lint & unit tests | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 3 'actions/checkout@v4|persist-credentials|run: npm' .github/workflows/ci.yml
jq '.scripts' package.jsonRepository: BuidlZone-Labs/zicket-web
Length of output: 1402
🌐 Web query:
actions/checkout v4 README default persist-credentials true README.md
💡 Result:
In the actions/checkout action version 4 (v4), the default value for the persist-credentials input is true [1][2][3]. When set to true (the default), the action configures the authentication token or SSH key in the local git configuration [1][2]. This allows subsequent steps in the same GitHub Actions job to execute authenticated git commands [1][4]. The token is automatically removed during the post-job cleanup [1]. To opt out of this behavior and prevent the credentials from being persisted in the local git configuration, you must explicitly set persist-credentials to false in your workflow file [1][4][5]: - name: Checkout code uses: actions/checkout@v4 with: persist-credentials: false While there has been ongoing community discussion and requests to change this default to false to improve security posture [6][5][7], as of August 2026, the default remains true [2][3].
Citations:
- 1: https://github.com/actions/checkout/blob/v4/README.md
- 2: https://github.com/actions/checkout/blob/v4/action.yml
- 3: https://github.com/actions/checkout/blob/v4.1.1/action.yml
- 4: https://github.com/actions/checkout
- 5: Remove
persist-credentialsor change the default tofalseactions/checkout#485 - 6: [security] Escalate concerning default
persist-credentials=trueplease actions/checkout#2312 - 7: Change the default value of persist-credentials to false actions/checkout#1687
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n .github/workflows/ci.yml | sed -n '1,80p'Repository: BuidlZone-Labs/zicket-web
Length of output: 2062
🌐 Web query:
GitHub Actions pull_request token contents permissions default read contents
💡 Result:
In GitHub Actions, the GITHUB_TOKEN used in workflows is subject to specific permission rules depending on the workflow trigger and the repository configuration [1][2]. When a workflow is triggered by a pull request from a forked repository, the GITHUB_TOKEN is automatically restricted to read-only access for all scopes [1][2]. This is a security measure to prevent unauthorized write operations from external contributors [2]. This restriction applies to all pull request-related events (such as pull_request or pull_request_review), except for the pull_request_target event [3]. Beyond this automatic restriction, the default permissions for the GITHUB_TOKEN in your repository are determined by your organization or repository settings [1]. Administrators can configure these to be either: - Permissive: Grants read/write access to most scopes [1][2]. - Restricted: Limits permissions (typically defaulting to read access for contents) [1][2]. You can explicitly override these defaults by using the permissions key within your workflow file or at the job level [1][4]. If you do not define specific permissions, the token will inherit the default settings configured for the repository or organization [1]. Even if you specify higher permissions, they will still be downgraded to read-only when the workflow is triggered by a pull request from a fork, unless the pull_request_target event is used [2][3].
Citations:
- 1: https://docs.github.com/en/enterprise-server@3.2/actions/reference/authentication-in-a-workflow
- 2: https://github.blog/changelog/2021-04-20-github-actions-control-permissions-for-github_token/
- 3: Clarify Permissions Calculation for Workflow Jobs in Documentation github/docs#33566
- 4: https://docs.github.com/en/actions/tutorials/authenticate-with-github_token
Sensitive Data Exposure (CWE-522): Insufficiently Protected Credentials
Reachability: External
Do not persist checkout credentials in PR-triggered CI.
actions/checkout@v4 defaults persist-credentials to true, leaving the workflow token available to npm scripts in the current job. These PR workflows install and execute PR-controlled code, so set persist-credentials: false unless authenticated Git operations are needed afterward.
Proposed fix
- uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - uses: actions/checkout@v4 | |
| - uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 14-14: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
📍 Affects 1 file
.github/workflows/ci.yml#L14-L14(this comment).github/workflows/ci.yml#L46-L46
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/ci.yml at line 14, Update both actions/checkout steps in
.github/workflows/ci.yml at lines 14-14 and 46-46 to set persist-credentials to
false, preserving the existing checkout behavior and avoiding credential
persistence in PR-triggered jobs.
Source: Linters/SAST tools
| const record = await getHorizonServer().transactions().transaction(hash).call(); | ||
| return record.successful | ||
| ? { status: "confirmed" } | ||
| : { status: "failed", error: "Transaction failed on-chain." }; | ||
| } catch (err) { | ||
| // Horizon 404s until the transaction lands in a ledger — treat that (and | ||
| // any transient network error) as still pending rather than failed. | ||
| return { status: "pending", error: err instanceof Error ? err.message : undefined }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Expect a finite timeout that reaches the SDK HTTP client before Horizon calls.
rg -nP -C 4 --glob '*.{ts,tsx,js,mjs,cjs}' \
'(?:Horizon\.AxiosClient|AxiosClient\.defaults\.timeout|\.defaults\.timeout\s*=|new\s+Horizon\.Server|\.transactions\(\)\.transaction\([^)]*\)\.call\(\))' .Repository: BuidlZone-Labs/zicket-web
Length of output: 2192
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package files =="
git ls-files | rg '(^|/)(package(-lock)?\.json|pnpm-lock\.yaml|yarn\.lock)$' || true
echo
echo "== stellar-sdk versions =="
python3 - <<'PY'
import json, pathlib
for p in pathlib.Path('.').glob('**/package-lock.json'):
if '.next' in p.parts or 'node_modules' in p.parts:
continue
data=json.loads(p.read_text())
deps=data.get('packages',{})
for name in ['stellar-sdk','`@stellar/stellar-sdk`']:
if name in deps:
print(p)
print(deps[name].get('version'))
for p in pathlib.Path('.').glob('**/package.json'):
if '.next' in p.parts or 'node_modules' in p.parts:
continue
data=json.loads(p.read_text())
for key in ('dependencies','devDependencies','peerDependencies'):
deps=data.get(key,{})
for name in ["stellar-sdk","`@stellar/stellar-sdk`","axios"]:
if name in deps:
print(f"{p}:{key}:{name}={deps[name]}")
PY
echo
echo "== stellar adapter relevant section =="
cat -n lib/wallet/stellarAdapter.ts | sed -n '1,120p'
echo
echo "== api status route relevant section =="
cat -n 'app/api/transactions/[txHash]/status/route.ts' | sed -n '1,120p'
echo
echo "== timeout / axios / Horizon imports/usages broad =="
rg -n --glob '*.{ts,tsx,js,mjs,cjs}' 'axios|AxiosClient|new Horizon\.Server|Horizon\.Server|\.call\(\)|timeout|AbortController|fetch\(' .Repository: BuidlZone-Labs/zicket-web
Length of output: 9967
🌐 Web query:
@stellar/stellar-sdk 13.1.0 horizon CallBuilder call request options timeout source
💡 Result:
In @stellar/stellar-sdk 13.1.0, the CallBuilder does not support passing a per-request timeout directly through the call() method [1][2]. Instead, request timeouts are managed globally through the Config object [3][4]. You can set a global timeout (in milliseconds) that applies to all HTTP requests made by the SDK [3][4]. To set this globally: import { Config } from '@stellar/stellar-sdk'; Config.setTimeout(5000); // Sets the global timeout to 5000ms (5 seconds) If you are using the stream() method, there is a specific reconnectTimeout option available within the options object, which defaults to 15 seconds [5][1][2]. This is distinct from the HTTP request timeout and applies specifically to the EventSource stream connection [1][2]. Note that for transaction-specific operations, the SDK may handle retries and time bounds independently [6]. As of version 13.1.0, the default timeout for transaction calls was increased to 300 seconds [6].
Citations:
- 1: https://cdn.jsdelivr.net/npm/stellar-sdk@13.1.0/lib/minimal/horizon/call_builder.d.ts
- 2: https://stellar.github.io/js-stellar-sdk/lib_horizon_call_builder.d.ts.html
- 3: https://cdn.jsdelivr.net/npm/stellar-sdk@13.1.0/lib/config.d.ts
- 4: https://stellar.github.io/js-stellar-sdk/reference/cross-cutting/
- 5: https://stellar.github.io/js-stellar-sdk/CallBuilder.html
- 6: https://cdn.jsdelivr.net/npm/stellar-sdk@13.1.0/CHANGELOG.md
Set an explicit horizon read timeout.
getRealTxStatus calls Horizon without a deadline; this route has no local timeout and @stellar/stellar-sdk v13.1.0 does not pass per-request options through CallBuilder.call(). Call Horizon.Config.setTimeout(...) or create a shared Horizon.Server with an HTTP-client timeout so stalled Horizon reads do not keep the API request open indefinitely.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/api/transactions/`[txHash]/status/route.ts around lines 35 - 42, Update
getRealTxStatus to use an explicit timeout for Horizon reads, configuring
Horizon.Config.setTimeout or reusing a shared Horizon.Server backed by an
HTTP-client timeout before transactions().transaction(hash).call(). Preserve the
existing confirmed, failed, and pending status handling.
| {!walletConnected && ( | ||
| <div className="flex gap-2" role="radiogroup" aria-label="Wallet network"> | ||
| {CHAIN_OPTIONS.map((option) => ( | ||
| <button | ||
| key={option.id} | ||
| type="button" | ||
| role="radio" | ||
| aria-checked={chain === option.id} | ||
| onClick={() => setChain(option.id)} | ||
| disabled={walletState.isLoading} | ||
| className={`px-4 py-1.5 rounded-full text-sm font-medium border transition disabled:opacity-60 disabled:cursor-not-allowed ${ | ||
| chain === option.id | ||
| ? "bg-[#6917AF] text-white border-[#6917AF]" | ||
| : "bg-white text-[#475467] border-[#E3E3E3] hover:border-[#6917AF]" | ||
| }`} | ||
| > | ||
| {option.label} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete the radiogroup keyboard pattern.
The buttons declare role="radio" inside a role="radiogroup", but the group has no arrow-key navigation and every option stays in the tab order. Assistive-technology users are told this is a radio group, then get button-style keyboard behavior.
Choose one of two options:
- Add a roving
tabIndex(0for the selected option,-1for the others) and handleArrowLeft/ArrowRight/ArrowUp/ArrowDown. - Drop the ARIA roles and use native
<input type="radio">elements inside a<fieldset>with a<legend>.
♻️ Option 1: roving tabindex and arrow keys
{CHAIN_OPTIONS.map((option) => (
<button
key={option.id}
type="button"
role="radio"
aria-checked={chain === option.id}
+ tabIndex={chain === option.id ? 0 : -1}
onClick={() => setChain(option.id)}
+ onKeyDown={(event) => {
+ if (!["ArrowLeft", "ArrowUp", "ArrowRight", "ArrowDown"].includes(event.key))
+ return;
+ event.preventDefault();
+ const step = event.key === "ArrowLeft" || event.key === "ArrowUp" ? -1 : 1;
+ const index = CHAIN_OPTIONS.findIndex((o) => o.id === chain);
+ const next =
+ CHAIN_OPTIONS[(index + step + CHAIN_OPTIONS.length) % CHAIN_OPTIONS.length];
+ setChain(next.id);
+ }}
disabled={walletState.isLoading}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/components/organizer/ConnectWalletPrompt.tsx` around lines 80 - 100,
Complete the radiogroup keyboard interaction in the CHAIN_OPTIONS map within
ConnectWalletPrompt: implement roving tabIndex with only the selected option at
0 and all others at -1, and handle ArrowLeft, ArrowRight, ArrowUp, and ArrowDown
to move selection and focus between options with wraparound. Preserve the
existing click selection and loading-state behavior.
| requestAccess: async () => ({ address: "GATESTPUBLICKEYFORE2ETESTINGXXXXXXXXXXXXXXXXXXXXXXXXXX" }), | ||
| getAddress: async () => ({ address: "GATESTPUBLICKEYFORE2ETESTINGXXXXXXXXXXXXXXXXXXXXXXXXXX" }), | ||
| getNetwork: async () => ({ network: "TESTNET", networkPassphrase: "Test SDF Network ; September 2015" }), | ||
| signTransaction: async () => ({ signedTxXdr: "AAAAAgAAAABE2ETESTSIGNEDXDR", signerAddress: "GATESTPUBLICKEYFORE2ETESTINGXXXXXXXXXXXXXXXXXXXXXXXXXX" }), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline lib/wallet/stellarAdapter.ts --items all
rg -n -C 5 --glob '*.ts' --glob '*.tsx' \
'signedTxXdr|submitTransaction|/transactions|fromXDR|TransactionEnvelope' \
lib/wallet app e2eRepository: BuidlZone-Labs/zicket-web
Length of output: 4379
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '--- stellarAdapter outline ---\n'
ast-grep outline lib/wallet/stellarAdapter.ts --view expanded || true
printf '\n--- stellarAdapter relevant sections ---\n'
cat -n lib/wallet/stellarAdapter.ts | sed -n '1,220p'
printf '\n--- ticket purchase spec relevant sections ---\n'
cat -n e2e/ticket-purchase.spec.ts | sed -n '1,120p'
printf '\n--- stellar transaction status/fetch usages ---\n'
rg -n -C 4 'status|fetch|transactions' e2e lib app --glob '*.ts' --glob '*.tsx' | head -n 240
printf '\n--- package deps stellar-sdk/axios/fetch ---\n'
if [ -f package.json ]; then jq '.dependencies, .devDependencies' package.json; fiRepository: BuidlZone-Labs/zicket-web
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- stellarAdapter outline ---'
ast-grep outline lib/wallet/stellarAdapter.ts --view expanded || true
echo
echo '--- stellarAdapter relevant sections ---'
cat -n lib/wallet/stellarAdapter.ts | sed -n '1,220p'
echo
echo '--- ticket purchase spec relevant sections ---'
cat -n e2e/ticket-purchase.spec.ts | sed -n '1,120p'
echo
echo '--- stellar transaction status/fetch usages ---'
rg -n -C 4 'status|fetch|transactions' e2e lib app --glob '*.ts' --glob '*.tsx' | head -n 240
echo
echo '--- package deps stellar-sdk/axios/fetch ---'
if [ -f package.json ]; then jq '.dependencies, .devDependencies' package.json; fiRepository: BuidlZone-Labs/zicket-web
Length of output: 30284
Mock the end-to-end Stellar submission boundary.
signTransaction() now calls getKit().signTransaction(), then parses the returned XDR with TransactionBuilder.fromXDR(), then sends it through Horizon. Returning "AAAAAgAAAABE2ETESTSIGNEDXDR" cannot parse as a signed TransactionEnvelope, so the purchase can fail before the /api/transactions/*/status route is reached. Mock the adapter or Horizon submission path, and return a valid synthetic signedPayload/txHash.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@e2e/ticket-purchase.spec.ts` around lines 20 - 23, Update the wallet mock’s
signTransaction flow in the e2e ticket purchase setup so it bypasses or mocks
the getKit().signTransaction(), TransactionBuilder.fromXDR(), and Horizon
submission boundary. Return a valid synthetic signedPayload/txHash combination,
ensuring the purchase reaches the /api/transactions/*/status route without
attempting to parse the placeholder XDR.
| const result = await provider.request<{ txHash: string }>({ | ||
| method: "send_transaction", | ||
| params: [payload ?? { memo: "zicket-ticket" }], | ||
| }); | ||
|
|
||
| return { txHash: result.txHash, signedPayload: payload ?? "" }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Validate the provider response before returning txHash.
provider.request is typed as { txHash: string }, but the injected provider returns unchecked data. If the response omits txHash, this adapter returns undefined as the hash. lib/walletSdk.ts then resolves successfully, and TicketInfo.runPurchase calls startTracking(undefined). The purchase UI stays in a pending state and the catch branch never runs.
Reject a malformed response here so the failure surfaces as a wallet error.
🐛 Proposed fix to validate the response
- const result = await provider.request<{ txHash: string }>({
+ const result = await provider.request<{ txHash?: unknown }>({
method: "send_transaction",
params: [payload ?? { memo: "zicket-ticket" }],
});
- return { txHash: result.txHash, signedPayload: payload ?? "" };
+ if (typeof result?.txHash !== "string" || result.txHash.length === 0) {
+ throw new Error("Azguard did not return a transaction hash.");
+ }
+
+ return { txHash: result.txHash, signedPayload: payload ?? "" };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const result = await provider.request<{ txHash: string }>({ | |
| method: "send_transaction", | |
| params: [payload ?? { memo: "zicket-ticket" }], | |
| }); | |
| return { txHash: result.txHash, signedPayload: payload ?? "" }; | |
| const result = await provider.request<{ txHash?: unknown }>({ | |
| method: "send_transaction", | |
| params: [payload ?? { memo: "zicket-ticket" }], | |
| }); | |
| if (typeof result?.txHash !== "string" || result.txHash.length === 0) { | |
| throw new Error("Azguard did not return a transaction hash."); | |
| } | |
| return { txHash: result.txHash, signedPayload: payload ?? "" }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/wallet/aztecAdapter.ts` around lines 71 - 76, Validate the result from
provider.request in the transaction flow before returning from the adapter
method containing send_transaction. Reject the response when txHash is missing
or not a valid string, ensuring the malformed response throws a wallet error
instead of returning undefined; preserve the existing successful return shape
for valid responses.
| /** | ||
| * Stellar wallet adapter — connects real browser wallets (Freighter, Lobstr, | ||
| * WalletConnect, xBull, Albedo, Rabet, Hana) through `@creit.tech/stellar-wallets-kit`, | ||
| * which owns wallet discovery/selection and delegates signing to whichever | ||
| * wallet the user picks. This replaces the old `mock_tx_...` random string | ||
| * generator with a real signed transaction submitted to Horizon. | ||
| * | ||
| * NOTE: There is no organizer payout address in the data model yet, so the | ||
| * default (no-argument) `signTransaction()` call signs a minimal self-payment | ||
| * (1 stroop, memo "zicket-ticket") purely to produce a real, verifiable | ||
| * on-chain transaction for the checkout flow. Once ticket purchases have a | ||
| * real destination (organizer wallet + priced asset), build that XDR | ||
| * server-side and pass it into `signTransaction(xdr)` instead. | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the wallet list in the doc comment.
The comment names Albedo, Rabet, and Hana. buildModules() registers only Freighter, Lobstr, xBull, and WalletConnect. Either register the extra modules or remove them from the comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/wallet/stellarAdapter.ts` around lines 1 - 14, Update the documentation
comment above the Stellar wallet adapter to list only the wallet modules
registered by buildModules(): Freighter, Lobstr, xBull, and WalletConnect.
Remove Albedo, Rabet, and Hana from the comment unless buildModules() is also
updated to register them.
| forbidOnly: !!process.env.CI, | ||
| retries: process.env.CI ? 2 : 0, | ||
| workers: process.env.CI ? 1 : undefined, | ||
| reporter: process.env.CI ? "github" : "html", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For @playwright/testversion 1.49.1, verify that a reporter array containinggithubandhtmlpreserves GitHub annotations and creates the defaultplaywright-report folder.
💡 Result:
In @playwright/test version 1.49.1, using a reporter array containing both 'github' and 'html' is fully supported and does not interfere with the functionality of either reporter [1][2]. 1. GitHub Annotations: Including 'github' in the reporter array ensures that Playwright generates automatic failure annotations when running in GitHub Actions [3][1]. This functionality operates independently of other reporters, meaning the 'html' reporter will continue to generate its report alongside these annotations [1][4]. 2. Default HTML Folder: The 'html' reporter will continue to create the default 'playwright-report' folder during the test run [5][1]. By default, Playwright overwrites the contents of this directory on each run [6]. If you need to change this behavior, you can specify a custom directory using the outputFolder option in the reporter configuration: reporter: [ ['github'], ['html', { outputFolder: 'playwright-report' }] ] Note that when using TypeScript, you may need to explicitly type the reporter array as ReporterDescription[] to avoid validation errors, as shown below [7]: import { ReporterDescription } from '@playwright/test'; export default { reporter: [ ['github'], ['html'] ] as ReporterDescription[] };
Citations:
- 1: https://playwright.dev/docs/test-reporters
- 2: https://playwright.dev/docs/api/class-testconfig
- 3: https://github.com/microsoft/playwright/blob/4d289016/docs/src/test-reporters-js.md
- 4: https://github.com/microsoft/playwright/blob/5790370e/packages/playwright/src/runner/reporters.ts
- 5: https://github.com/microsoft/playwright/blob/c0cc9802/packages/playwright/src/reporters/html.ts
- 6: https://gaffer.sh/blog/playwright-reports-guide/
- 7: [Bug]: Using array of reporter types fails typescript validation: "Type 'string[]' is not assignable to type 'ReporterDescription'" microsoft/playwright#33708
Emit the CI-uploaded reporter alongside GitHub annotations.
In CI, only the GitHub reporter runs, so the uploaded playwright-report directory is not generated by the configured Playwright run. Keep the GitHub reporter and add the HTML reporter for CI output.
Proposed fix
- reporter: process.env.CI ? "github" : "html",
+ reporter: process.env.CI
+ ? [["github"], ["html", { open: "never" }]]
+ : "html",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| reporter: process.env.CI ? "github" : "html", | |
| reporter: process.env.CI | |
| ? [["github"], ["html", { open: "never" }]] | |
| : "html", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@playwright.config.ts` at line 12, Update the reporter configuration in
Playwright’s config so CI runs both the GitHub reporter and the HTML reporter,
preserving the existing HTML-only behavior outside CI and ensuring the
playwright-report directory is generated during CI runs.
1 participant
Summary
Testing infrastructure
npm run test,test:watch, andtest:coveragescripts.TicketInfo.test.tsx: ticket selection, quantity stepper, sold-out state, wallet-connect purchase flow, anonymous free-event flow, wallet error banner) and the search/filter flow (MainContent.test.tsx: price/location filtering, empty state, deep-link initial query).aria-labels to the previously unlabeled quantity stepper buttons (accessibility gap + needed for reliable test queries).npm run test:e2e.e2e/ticket-purchase.spec.tscovers ticket/quantity selection through the wallet connection modal, plus a full purchase run against a stubbed Freighter provider..github/workflows/ci.yml: lint, type-check, unit tests (with coverage upload), and Playwright E2E on every PR tomain.Native Stellar & Aztec wallet integration
mock_tx_...random-string wallet SDK inlib/walletSdk.tswith a chain-agnostic facade backed by real adapters inlib/wallet/.lib/wallet/stellarAdapter.ts: connects Freighter, Lobstr, WalletConnect, and xBull via@creit.tech/stellar-wallets-kit; signs and submits real transactions to Horizon; returns genuine on-chain tx hashes.lib/wallet/aztecAdapter.ts: Azguard-compatible injected-provider adapter for Aztec, scaffolded against@aztec/aztec.js— flagged in comments since Aztec's wallet-connection RPC surface is still evolving.app/api/transactions/[txHash]/status/route.tsnow looks up real Stellar transaction hashes on Horizon, falling back to the existing simulated status only for non-Stellar-shaped (demo/free-flow) hashes.ConnectWalletPrompt.tsxgets a Stellar/Aztec chain selector and shows the connected wallet's name/address;WalletConnectionIndicator.tsxnow surfaces the real wallet name + shortened address instead of just "Connected".lib/user-session-sync.tsto persistwalletAddress/walletName/walletChainalongside the existing connected flag.Notes for reviewers
package.jsonbutnpm installwas not run — no lockfile update yet.signTransaction()call signs a minimal real self-payment (1 stroop, memozicket-ticket) purely to produce a verifiable on-chain transaction. Once ticket purchases have a real destination (organizer wallet + priced asset), build that XDR and pass it intosignTransaction(xdr).lib/wallet/aztecAdapter.tsare a best-effort based on the injected-provider convention other wallets use — verify against current Azguard/@aztec/aztec.jsdocs before shipping.NEXT_PUBLIC_STELLAR_NETWORK,NEXT_PUBLIC_STELLAR_HORIZON_URL, andNEXT_PUBLIC_WALLETCONNECT_PROJECT_IDare documented in.env.example.Closes #186
Closes #187
Summary by CodeRabbit
New Features
Bug Fixes
Tests