feat: wallet connection and holder session - #28
Conversation
…are and verify pages
|
@sotoJ24 is attempting to deploy a commit to the ACTA Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR adds a Stellar wallet session layer ( ChangesWallet Session, Credential Source, and UI
Sequence Diagram(s)sequenceDiagram
participant User
participant WalletButton
participant SessionProvider
participant WalletConnector
participant localStorage
User->>WalletButton: clicks "Connect wallet"
WalletButton->>SessionProvider: connect()
SessionProvider->>SessionProvider: status = "connecting"
SessionProvider->>WalletConnector: getWalletConnector().connect()
WalletConnector-->>SessionProvider: { address: "G...xyz" }
SessionProvider->>localStorage: setItem("acta:session:address", address)
SessionProvider->>SessionProvider: status = "connected", derive DID
SessionProvider-->>WalletButton: re-render with address + DID
User->>WalletButton: clicks "Disconnect"
WalletButton->>SessionProvider: disconnect()
SessionProvider->>WalletConnector: getWalletConnector().disconnect()
SessionProvider->>localStorage: removeItem("acta:session:address")
SessionProvider->>SessionProvider: status = "disconnected", address = null
SessionProvider-->>WalletButton: re-render with Connect button
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 5
🧹 Nitpick comments (1)
apps/credit-history/src/session/wallet-connector.ts (1)
53-55: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse a stable connector instance for session lifecycle calls.
Creating a new connector on every call can break real wallet adapters that track connection state/listeners per instance. Return one shared connector instance from the factory.
♻️ Proposed refactor
class MockWalletConnector implements WalletConnector { @@ } +const walletConnector: WalletConnector = new MockWalletConnector(); + // ─── Seam factory ───────────────────────────────────────────────────────────── @@ export function getWalletConnector(): WalletConnector { - return new MockWalletConnector(); + return walletConnector; }🤖 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 `@apps/credit-history/src/session/wallet-connector.ts` around lines 53 - 55, The getWalletConnector() function creates a new MockWalletConnector instance on every call, which breaks wallet adapters that track connection state and listeners per instance. Instead, create a single shared connector instance at the module level and cache it, then return that same cached instance from getWalletConnector() on all subsequent calls rather than instantiating a new MockWalletConnector each time.
🤖 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 `@apps/credit-history/src/providers/acta-provider.tsx`:
- Around line 7-20: Remove the declare const statements for
NEXT_PUBLIC_STELLAR_NETWORK and NEXT_PUBLIC_ACTA_API_KEY at the top of the file.
Update the resolveNetwork function to reference
process.env.NEXT_PUBLIC_STELLAR_NETWORK directly instead of using the declared
constant, which will enable Next.js to properly inline these environment
variables at build time through its static analysis instead of leaving them
undefined at runtime.
In `@apps/credit-history/src/session/session-provider.tsx`:
- Around line 98-105: The disconnect function has the try-catch block in the
wrong position. The connector.disconnect() call is not wrapped in the try-catch,
so if it fails, the subsequent setAddress, setStatus, and localStorage cleanup
are never executed. Restructure the disconnect useCallback function to wrap the
connector.disconnect() call in a try-catch block, ensuring that
setAddress(null), setStatus('disconnected'), and the
localStorage.removeItem(SESSION_STORAGE_KEY) are executed in a finally block or
are guaranteed to run regardless of whether the connector.disconnect() call
succeeds or throws an error.
- Around line 25-34: The declare const pattern for NEXT_PUBLIC_STELLAR_NETWORK
prevents Next.js from inlining the environment variable at build time, causing
it to always be undefined at runtime and fall back to 'testnet'. Remove the
declare const statement and refactor the resolveNetwork function to directly
access process.env.NEXT_PUBLIC_STELLAR_NETWORK with a nullish coalescing
operator and 'testnet' fallback instead of using the typeof check and type
casting. Apply the same fix to the acta-provider.tsx file which uses the same
broken pattern.
In `@packages/acta/src/mock.ts`:
- Around line 196-212: The listCredentials() and getCredential() methods expose
shared references to objects in the FIXTURES array, allowing downstream
mutations to corrupt the mock state. While listCredentials() spreads the array,
the objects within are still references to the originals. The getCredential()
method returns a direct reference to the found object. Fix both methods by
creating deep copies of the returned objects before returning them so that
mutations by consumers do not affect the shared FIXTURES state.
- Around line 224-237: Remove the ambient declarations for NEXT_PUBLIC_MOCK_MODE
and NEXT_PUBLIC_DATA_SOURCE at the top of the file. In the resolveMockMode
function, replace the reference to NEXT_PUBLIC_MOCK_MODE with
process.env.NEXT_PUBLIC_MOCK_MODE. In the getCredentialSource function, replace
the reference to NEXT_PUBLIC_DATA_SOURCE with
process.env.NEXT_PUBLIC_DATA_SOURCE. This ensures Next.js's static replacement
mechanism works correctly by accessing environment variables through the
standard process.env object.
---
Nitpick comments:
In `@apps/credit-history/src/session/wallet-connector.ts`:
- Around line 53-55: The getWalletConnector() function creates a new
MockWalletConnector instance on every call, which breaks wallet adapters that
track connection state and listeners per instance. Instead, create a single
shared connector instance at the module level and cache it, then return that
same cached instance from getWalletConnector() on all subsequent calls rather
than instantiating a new MockWalletConnector each time.
🪄 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
Run ID: c419e209-7a96-4306-8a22-7709ebf5567d
📒 Files selected for processing (12)
.gitignoreapps/credit-history/src/app/layout.tsxapps/credit-history/src/app/share/page.tsxapps/credit-history/src/app/verify/[token]/page.tsxapps/credit-history/src/components/header.tsxapps/credit-history/src/components/wallet-button.tsxapps/credit-history/src/i18n/messages/en.jsonapps/credit-history/src/i18n/messages/es.jsonapps/credit-history/src/providers/acta-provider.tsxapps/credit-history/src/session/session-provider.tsxapps/credit-history/src/session/wallet-connector.tspackages/acta/src/mock.ts
| // Next.js inlines NEXT_PUBLIC_* at build time — declare so TypeScript resolves. | ||
| declare const NEXT_PUBLIC_STELLAR_NETWORK: string | undefined; | ||
| declare const NEXT_PUBLIC_ACTA_API_KEY: string | undefined; | ||
|
|
||
| function resolveBaseURL(network: StellarNetwork): baseURL { | ||
| return network === 'mainnet' ? mainNet : testNet; | ||
| } | ||
|
|
||
| function resolveNetwork(): StellarNetwork { | ||
| const raw = | ||
| typeof NEXT_PUBLIC_STELLAR_NETWORK !== 'undefined' | ||
| ? (NEXT_PUBLIC_STELLAR_NETWORK as string) | ||
| : 'testnet'; | ||
| return raw === 'mainnet' ? 'mainnet' : 'testnet'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find NEXT_PUBLIC usage patterns =="
rg -n -C2 'NEXT_PUBLIC_(STELLAR_NETWORK|ACTA_API_KEY)|process\.env\.NEXT_PUBLIC_(STELLAR_NETWORK|ACTA_API_KEY)'
echo
echo "== Check for explicit global injections of bare NEXT_PUBLIC_* =="
rg -n -C2 'globalThis\.NEXT_PUBLIC_|window\.NEXT_PUBLIC_|declare const NEXT_PUBLIC_'
echo
echo "== Inspect Next config for custom env/define wiring =="
fd -i 'next.config.*' -x sh -c 'echo "--- {} ---"; sed -n "1,240p" "{}"'
rg -n -C3 'DefinePlugin|webpack\(|env\s*:' --iglob '*next.config.*'Repository: ACTA-Team/products-acta
Length of output: 5470
🌐 Web query:
Next.js NEXT_PUBLIC environment variables process.env vs declare const globals
💡 Result:
In Next.js, the NEXT_PUBLIC_ prefix and manual constant declarations (or configuration-based variables) serve different purposes regarding accessibility, security, and the build process. NEXT_PUBLIC_ Environment Variables By prefixing an environment variable with NEXT_PUBLIC_ (e.g., NEXT_PUBLIC_API_URL), you instruct Next.js to inline the value directly into the JavaScript bundle at build time [1][2][3]. This makes the variable accessible to both server-side code and client-side browser code [1][4][5]. Because these values are baked into the static JavaScript files, they are publicly visible to anyone who inspects the browser source code [6][7]. Consequently, you must never store sensitive secrets (such as API private keys or database passwords) using the NEXT_PUBLIC_ prefix [3][6][5]. Furthermore, because they are inlined at build time, any changes to these variables require a full application rebuild; they cannot be updated at runtime without redeploying the app [1][3][6]. Process.env vs. Global Constants Next.js relies on static analysis (via tools like Webpack's DefinePlugin or Turbopack) to perform the replacement of process.env.NEXT_PUBLIC_ variables [8][3][9]. For this to work correctly, you must use the full, explicit syntax: process.env.NEXT_PUBLIC_VAR_NAME [8][3]. Do not attempt to destructure process.env (e.g., const { NEXT_PUBLIC_VAR } = process.env) or use dynamic access (e.g., process.env[key]), as these patterns bypass the build-time replacement mechanism and will return undefined in the browser bundle [3][9]. Similarly, accessing global.process.env instead of process.env is considered dynamic and will also fail to inline correctly [8]. Server-Only Variables Any environment variable defined in your .env files that does not start with NEXT_PUBLIC_ is strictly server-only [1][10][11]. These variables are accessible in Node.js environments—such as API routes, Server Components, and middleware—but are completely stripped from the client-side JavaScript bundle [3][5][12]. This is the secure, default method for handling sensitive credentials [3][11]. Summary of Best Practices 1. Sensitive Secrets: Use standard environment variables (no prefix) and access them only on the server [5][7]. 2. Public Configuration: Use the NEXT_PUBLIC_ prefix only for non-sensitive values like public API base URLs or feature flags [3][7]. 3. Avoid Destructuring: Always reference variables using the full process.env.VARIABLE_NAME syntax [3][9]. 4. Startup Validation: Use a library like Zod to validate your environment variables at application startup, which helps catch missing or misconfigured variables immediately [6][5][12]. 5. No Runtime Updates: Remember that NEXT_PUBLIC_ values are frozen at build time; for runtime-configurable values, you must expose them through an API endpoint or another dynamic delivery mechanism [1][2].
Citations:
- 1: https://nextjs.im/docs/app/guides/environment-variables/
- 2: https://nextjs.im/docs/13/app/building-your-application/configuring/environment-variables/
- 3: https://env.dev/guides/nextjs-env-variables
- 4: [RFC] .env environment support vercel/next.js#11106
- 5: https://dev.to/whoffagents/nextjs-environment-variables-nextpublic-server-only-secrets-and-startup-validation-5f1d
- 6: https://www.nextcraft.agency/resources/insights/nextjs-environment-variables
- 7: https://www.hashbuilds.com/articles/next-js-environment-variables-complete-security-guide-2025
- 8: Difference between `global.process.env` and `process.env` vercel/next.js#82272
- 9: https://nextjs.org/docs/app/api-reference/config/next-config-js/env
- 10: https://nextjs.org/docs/pages/guides/environment-variables
- 11: https://blog.logrocket.com/configure-environment-variables-next-js/
- 12: https://dev.to/whoffagents/nextjs-environment-variables-the-complete-guide-to-avoiding-common-mistakes-4mmk
🌐 Web query:
Next.js how are NEXT_PUBLIC_ variables exposed to browser client code
💡 Result:
In Next.js, environment variables prefixed with NEXT_PUBLIC_ are exposed to the browser by being statically "inlined" into your JavaScript bundles during the build process [1][2]. When you run the build command (e.g., next build), the Next.js bundler (such as webpack or Turbopack) scans your code for any occurrences of process.env.NEXT_PUBLIC_VARIABLE_NAME and replaces these references with the actual string value defined in your environment at that time [1][3][4]. Key implications of this mechanism include: 1. Build-Time Freezing: Because these variables are hard-coded into your JavaScript bundles at build time, they are "frozen" [1][5]. If you change the value of a NEXT_PUBLIC_ variable in your environment (such as in your.env file or a hosting provider's dashboard) after the build is complete, your application will not reflect that change until you rebuild the project [1][3][5]. 2. Client-Side Exposure: Any variable prefixed with NEXT_PUBLIC_ is intentionally exposed to the browser [1][4]. Consequently, these values are visible to anyone who inspects the client-side JavaScript or the source code of your page [6][5]. You must never use this prefix for sensitive secrets, such as API private keys, database credentials, or internal configuration values [6][3][5]. 3. Default Behavior for Non-Prefixed Variables: Environment variables that do not use the NEXT_PUBLIC_ prefix are kept server-only [1][3]. If you attempt to access a non-prefixed environment variable (like process.env.DATABASE_URL) in client-side code, it will resolve to undefined [7][3][5]. 4. Strict Pattern Matching: The replacement process performs literal text matching [4]. It requires that you use the full expression, such as process.env.NEXT_PUBLIC_VAR, directly in your code [8][4]. Destructuring process.env or using dynamic keys will prevent the bundler from correctly identifying and replacing the variable, likely resulting in undefined at runtime [8][4]. For values that need to be dynamic or secret, the recommended approach is to keep those variables on the server and access them only through server-side code or by creating a dedicated API route that the client can call to fetch necessary configuration at runtime [1][6].
Citations:
- 1: https://nextjs.org/docs/app/guides/environment-variables
- 2: https://nextjs.org/docs/pages/guides/environment-variables
- 3: https://envtools.dev/guides/next-public-env-variables
- 4: https://env.dev/guides/nextjs-env-variables
- 5: https://www.nextcraft.agency/resources/insights/nextjs-environment-variables
- 6: https://stackoverflow.com/questions/73039310/is-using-next-public-to-expose-environmental-variables-to-the-browser-safe
- 7: Environment variables exposed on the client vercel/next.js#23980
- 8: https://nextjs.org/docs/app/api-reference/config/next-config-js/env
Replace bare NEXT_PUBLIC_* declarations with process.env.NEXT_PUBLIC_* to enable proper environment variable inlining.
Next.js uses static analysis to replace process.env.NEXT_PUBLIC_* references at build time. The current approach using declare const bypasses this mechanism entirely, leaving these variables undefined at runtime and forcing fallback to 'testnet' and the placeholder API key. Use the standard Next.js pattern: process.env.NEXT_PUBLIC_STELLAR_NETWORK and process.env.NEXT_PUBLIC_ACTA_API_KEY.
Suggested fix
-'use client';
+'use client';
import { ActaConfig, mainNet, testNet } from '`@acta-products/acta`';
import type { baseURL } from '`@acta-products/acta`';
import type { StellarNetwork } from '`@acta-products/acta/did`';
-
-// Next.js inlines NEXT_PUBLIC_* at build time — declare so TypeScript resolves.
-declare const NEXT_PUBLIC_STELLAR_NETWORK: string | undefined;
-declare const NEXT_PUBLIC_ACTA_API_KEY: string | undefined;
function resolveBaseURL(network: StellarNetwork): baseURL {
return network === 'mainnet' ? mainNet : testNet;
}
function resolveNetwork(): StellarNetwork {
- const raw =
- typeof NEXT_PUBLIC_STELLAR_NETWORK !== 'undefined'
- ? (NEXT_PUBLIC_STELLAR_NETWORK as string)
- : 'testnet';
+ const raw = process.env.NEXT_PUBLIC_STELLAR_NETWORK ?? 'testnet';
return raw === 'mainnet' ? 'mainnet' : 'testnet';
}
@@
export function ActaProvider({ children }: ActaProviderProps) {
const network = resolveNetwork();
const baseURL = resolveBaseURL(network);
- const apiKey =
- (typeof NEXT_PUBLIC_ACTA_API_KEY !== 'undefined' &&
- (NEXT_PUBLIC_ACTA_API_KEY as string).length > 0)
- ? (NEXT_PUBLIC_ACTA_API_KEY as string)
- : 'placeholder-batch1-no-api-calls';
+ const apiKey =
+ process.env.NEXT_PUBLIC_ACTA_API_KEY?.trim().length
+ ? process.env.NEXT_PUBLIC_ACTA_API_KEY
+ : 'placeholder-batch1-no-api-calls';🤖 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 `@apps/credit-history/src/providers/acta-provider.tsx` around lines 7 - 20,
Remove the declare const statements for NEXT_PUBLIC_STELLAR_NETWORK and
NEXT_PUBLIC_ACTA_API_KEY at the top of the file. Update the resolveNetwork
function to reference process.env.NEXT_PUBLIC_STELLAR_NETWORK directly instead
of using the declared constant, which will enable Next.js to properly inline
these environment variables at build time through its static analysis instead of
leaving them undefined at runtime.
| // Next.js inlines NEXT_PUBLIC_* at build time. | ||
| declare const NEXT_PUBLIC_STELLAR_NETWORK: string | undefined; | ||
|
|
||
| function resolveNetwork(): StellarNetwork { | ||
| const raw = | ||
| typeof NEXT_PUBLIC_STELLAR_NETWORK !== 'undefined' | ||
| ? (NEXT_PUBLIC_STELLAR_NETWORK as string) | ||
| : 'testnet'; | ||
| return raw === 'mainnet' ? 'mainnet' : 'testnet'; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify how NEXT_PUBLIC_STELLAR_NETWORK is consumed across session/config layers.
rg -n "NEXT_PUBLIC_STELLAR_NETWORK|process\\.env" \
apps/credit-history/src/session/session-provider.tsx \
apps/credit-history/src/providers/acta-provider.tsxRepository: ACTA-Team/products-acta
Length of output: 862
🏁 Script executed:
sed -n '100,120p' apps/credit-history/src/session/session-provider.tsxRepository: ACTA-Team/products-acta
Length of output: 575
🏁 Script executed:
sed -n '1,50p' apps/credit-history/src/providers/acta-provider.tsxRepository: ACTA-Team/products-acta
Length of output: 1344
🌐 Web query:
Next.js NEXT_PUBLIC_ environment variables declare const vs process.env
💡 Result:
In Next.js, the NEXT_PUBLIC_ prefix and process.env are fundamental to how the framework handles environment variables. Understanding their behavior is critical for both security and functionality [1][2][3]. NEXT_PUBLIC_ Prefix and Browser Exposure The NEXT_PUBLIC_ prefix is the mechanism used to explicitly expose an environment variable to the browser [4][1]. When you prefix a variable with NEXT_PUBLIC_ (e.g., NEXT_PUBLIC_API_URL), Next.js automatically inlines the value of that variable into the JavaScript bundle during the build process (next build) [4][2][5]. Because these variables are inlined as hard-coded strings, they become part of your client-side source code [4][6]. Consequently: - Security: You must never use the NEXT_PUBLIC_ prefix for secrets, private keys, or credentials [1][2][5]. Anything prefixed this way can be easily viewed by any user inspecting your website's source code [1][5]. - Build Time: The values are frozen at build time [4][2][5]. If you change the value in your environment after the app has been built, the browser will continue to use the old, inlined value until you rebuild your application [4][5]. process.env Usage process.env is the standard Node.js object used to access all loaded environment variables [7]. - Server-Side Access: Any environment variable (whether it has the NEXT_PUBLIC_ prefix or not) is available to your server-side code (e.g., in Server Components, API Routes, or Server Actions) via process.env [1][8][3]. - Client-Side Access: Only variables prefixed with NEXT_PUBLIC_ can be accessed via process.env in client-side code (e.g., Client Components) [4][1][7]. Critical Implementation Rules To ensure environment variables work correctly in Next.js, you must follow these specific patterns: - Do Not Destructure: Never use object destructuring (e.g., const { API_KEY } = process.env) [2][9]. Because Next.js performs a static search-and-replace to inline these values at build time, it requires the full, explicit reference (e.g., process.env.API_KEY) to function [2][6][9]. - Do Not Use Dynamic Keys: Avoid using dynamic keys (e.g., process.env[variableName]), as this bypasses the static replacement mechanism and will result in the variable being undefined in the client bundle [2]. - Validate Variables: Because process.env types are inherently broad (string | undefined), it is recommended to validate environment variables at application startup (e.g., using a library like Zod) to catch configuration errors early [1][5]. In summary, use the NEXT_PUBLIC_ prefix only for values that are safe to expose to the public, always reference variables using the full process.env.VARIABLE_NAME syntax, and keep all sensitive secrets server-only by omitting the prefix [1][2][3].
Citations:
- 1: https://www.matthewswong.com/en/blog/environment-variables-secrets-nextjs/
- 2: https://env.dev/guides/nextjs-env-variables
- 3: https://quarterly.dev/article/next-js-environment-files-how-loading-priority-and-browser-exposure-actually-work
- 4: https://nextjs.im/docs/app/guides/environment-variables/
- 5: https://www.nextcraft.agency/resources/insights/nextjs-environment-variables
- 6: https://nextjs.org/docs/app/api-reference/config/next-config-js/env
- 7: https://nextjs.org/docs/app/guides/environment-variables
- 8: https://stackoverflow.com/questions/77089879/next-js-is-exposing-my-environment-variables-to-the-client
- 9: Docs: Environment variables documentation implies they're always read at build time vercel/next.js#39299
Use process.env to allow Next.js to inline NEXT_PUBLIC_STELLAR_NETWORK at build time.
The declare const pattern prevents Next.js from performing static replacement on the NEXT_PUBLIC_STELLAR_NETWORK variable. This causes it to be undefined at runtime, forcing fallback to 'testnet'. If the actual environment variable is set to 'mainnet', the DID generated at line 110 will be inconsistent with the network configuration used elsewhere. Replace with process.env.NEXT_PUBLIC_STELLAR_NETWORK ?? 'testnet' to allow Next.js to inline the value correctly at build time.
This also applies to acta-provider.tsx (line 8), which uses the same broken pattern.
✅ Suggested fix
-// Next.js inlines NEXT_PUBLIC_* at build time.
-declare const NEXT_PUBLIC_STELLAR_NETWORK: string | undefined;
-
function resolveNetwork(): StellarNetwork {
- const raw =
- typeof NEXT_PUBLIC_STELLAR_NETWORK !== 'undefined'
- ? (NEXT_PUBLIC_STELLAR_NETWORK as string)
- : 'testnet';
+ const raw = process.env.NEXT_PUBLIC_STELLAR_NETWORK ?? 'testnet';
return raw === 'mainnet' ? 'mainnet' : 'testnet';
}📝 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.
| // Next.js inlines NEXT_PUBLIC_* at build time. | |
| declare const NEXT_PUBLIC_STELLAR_NETWORK: string | undefined; | |
| function resolveNetwork(): StellarNetwork { | |
| const raw = | |
| typeof NEXT_PUBLIC_STELLAR_NETWORK !== 'undefined' | |
| ? (NEXT_PUBLIC_STELLAR_NETWORK as string) | |
| : 'testnet'; | |
| return raw === 'mainnet' ? 'mainnet' : 'testnet'; | |
| } | |
| function resolveNetwork(): StellarNetwork { | |
| const raw = process.env.NEXT_PUBLIC_STELLAR_NETWORK ?? 'testnet'; | |
| return raw === 'mainnet' ? 'mainnet' : 'testnet'; | |
| } |
🤖 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 `@apps/credit-history/src/session/session-provider.tsx` around lines 25 - 34,
The declare const pattern for NEXT_PUBLIC_STELLAR_NETWORK prevents Next.js from
inlining the environment variable at build time, causing it to always be
undefined at runtime and fall back to 'testnet'. Remove the declare const
statement and refactor the resolveNetwork function to directly access
process.env.NEXT_PUBLIC_STELLAR_NETWORK with a nullish coalescing operator and
'testnet' fallback instead of using the typeof check and type casting. Apply the
same fix to the acta-provider.tsx file which uses the same broken pattern.
| const disconnect = useCallback(async () => { | ||
| const connector = getWalletConnector(); | ||
| await connector.disconnect(); | ||
| setAddress(null); | ||
| setStatus('disconnected'); | ||
| try { | ||
| localStorage.removeItem(SESSION_STORAGE_KEY); | ||
| } catch { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Always clear local session state even when wallet disconnect fails.
If connector.disconnect() throws, address/status/localStorage cleanup is skipped, leaving a stale connected session in UI.
🛠️ Suggested fix
const disconnect = useCallback(async () => {
- const connector = getWalletConnector();
- await connector.disconnect();
- setAddress(null);
- setStatus('disconnected');
- try {
- localStorage.removeItem(SESSION_STORAGE_KEY);
- } catch {
- // Non-fatal.
- }
+ try {
+ const connector = getWalletConnector();
+ await connector.disconnect();
+ } finally {
+ setAddress(null);
+ setStatus('disconnected');
+ try {
+ localStorage.removeItem(SESSION_STORAGE_KEY);
+ } catch {
+ // Non-fatal.
+ }
+ }
}, []);📝 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 disconnect = useCallback(async () => { | |
| const connector = getWalletConnector(); | |
| await connector.disconnect(); | |
| setAddress(null); | |
| setStatus('disconnected'); | |
| try { | |
| localStorage.removeItem(SESSION_STORAGE_KEY); | |
| } catch { | |
| const disconnect = useCallback(async () => { | |
| try { | |
| const connector = getWalletConnector(); | |
| await connector.disconnect(); | |
| } finally { | |
| setAddress(null); | |
| setStatus('disconnected'); | |
| try { | |
| localStorage.removeItem(SESSION_STORAGE_KEY); | |
| } catch { | |
| // Non-fatal. | |
| } | |
| } | |
| }, []); |
🤖 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 `@apps/credit-history/src/session/session-provider.tsx` around lines 98 - 105,
The disconnect function has the try-catch block in the wrong position. The
connector.disconnect() call is not wrapped in the try-catch, so if it fails, the
subsequent setAddress, setStatus, and localStorage cleanup are never executed.
Restructure the disconnect useCallback function to wrap the
connector.disconnect() call in a try-catch block, ensuring that
setAddress(null), setStatus('disconnected'), and the
localStorage.removeItem(SESSION_STORAGE_KEY) are executed in a finally block or
are guaranteed to run regardless of whether the connector.disconnect() call
succeeds or throws an error.
| async listCredentials(): Promise<CreditCredential[]> { | ||
| // Simulate slight API delay for fidelity | ||
| return new Promise((resolve) => { | ||
| setTimeout(() => resolve(mockCreditCredentials), 100); | ||
| }); | ||
| await this.delay(); | ||
| if (this.mode === 'error') { | ||
| throw new Error('[MockCredentialSource] Simulated network error in listCredentials()'); | ||
| } | ||
| if (this.mode === 'empty') return []; | ||
| return [...FIXTURES]; | ||
| } | ||
|
|
||
| async getCredential(id: string): Promise<CreditCredential | null> { | ||
| return new Promise((resolve) => { | ||
| setTimeout(() => { | ||
| const cred = mockCreditCredentials.find((c) => c.id === id); | ||
| resolve(cred || null); | ||
| }, 50); | ||
| }); | ||
| await this.delay(); | ||
| if (this.mode === 'error') { | ||
| throw new Error(`[MockCredentialSource] Simulated network error in getCredential(${id})`); | ||
| } | ||
| if (this.mode === 'empty') return null; | ||
| return FIXTURES.find((c) => c.id === id) ?? null; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Return defensive copies from listCredentials() and getCredential().
These methods currently expose shared FIXTURES object references. Any downstream mutation can bleed into future reads and corrupt mock state across flows.
Proposed fix
+function cloneCredential(c: CreditCredential): CreditCredential {
+ return {
+ ...c,
+ claims: { ...c.claims },
+ };
+}
+
class MockCredentialSource implements CreditCredentialSource {
@@
async listCredentials(): Promise<CreditCredential[]> {
@@
- return [...FIXTURES];
+ return FIXTURES.map(cloneCredential);
}
@@
async getCredential(id: string): Promise<CreditCredential | null> {
@@
- return FIXTURES.find((c) => c.id === id) ?? null;
+ const credential = FIXTURES.find((c) => c.id === id);
+ return credential ? cloneCredential(credential) : null;
}📝 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.
| async listCredentials(): Promise<CreditCredential[]> { | |
| // Simulate slight API delay for fidelity | |
| return new Promise((resolve) => { | |
| setTimeout(() => resolve(mockCreditCredentials), 100); | |
| }); | |
| await this.delay(); | |
| if (this.mode === 'error') { | |
| throw new Error('[MockCredentialSource] Simulated network error in listCredentials()'); | |
| } | |
| if (this.mode === 'empty') return []; | |
| return [...FIXTURES]; | |
| } | |
| async getCredential(id: string): Promise<CreditCredential | null> { | |
| return new Promise((resolve) => { | |
| setTimeout(() => { | |
| const cred = mockCreditCredentials.find((c) => c.id === id); | |
| resolve(cred || null); | |
| }, 50); | |
| }); | |
| await this.delay(); | |
| if (this.mode === 'error') { | |
| throw new Error(`[MockCredentialSource] Simulated network error in getCredential(${id})`); | |
| } | |
| if (this.mode === 'empty') return null; | |
| return FIXTURES.find((c) => c.id === id) ?? null; | |
| } | |
| function cloneCredential(c: CreditCredential): CreditCredential { | |
| return { | |
| ...c, | |
| claims: { ...c.claims }, | |
| }; | |
| } | |
| class MockCredentialSource implements CreditCredentialSource { | |
| async listCredentials(): Promise<CreditCredential[]> { | |
| await this.delay(); | |
| if (this.mode === 'error') { | |
| throw new Error('[MockCredentialSource] Simulated network error in listCredentials()'); | |
| } | |
| if (this.mode === 'empty') return []; | |
| return FIXTURES.map(cloneCredential); | |
| } | |
| async getCredential(id: string): Promise<CreditCredential | null> { | |
| await this.delay(); | |
| if (this.mode === 'error') { | |
| throw new Error(`[MockCredentialSource] Simulated network error in getCredential(${id})`); | |
| } | |
| if (this.mode === 'empty') return null; | |
| const credential = FIXTURES.find((c) => c.id === id); | |
| return credential ? cloneCredential(credential) : 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 `@packages/acta/src/mock.ts` around lines 196 - 212, The listCredentials() and
getCredential() methods expose shared references to objects in the FIXTURES
array, allowing downstream mutations to corrupt the mock state. While
listCredentials() spreads the array, the objects within are still references to
the originals. The getCredential() method returns a direct reference to the
found object. Fix both methods by creating deep copies of the returned objects
before returning them so that mutations by consumers do not affect the shared
FIXTURES state.
| // Inlined by Next.js build — declared here so TypeScript resolves the names. | ||
| declare const NEXT_PUBLIC_MOCK_MODE: string | undefined; | ||
| declare const NEXT_PUBLIC_DATA_SOURCE: string | undefined; | ||
|
|
||
| function resolveMockMode(): MockMode { | ||
| const envMode = | ||
| typeof NEXT_PUBLIC_MOCK_MODE !== 'undefined' ? (NEXT_PUBLIC_MOCK_MODE as string) : ''; | ||
| if (envMode === 'empty' || envMode === 'error') return envMode as MockMode; | ||
| return 'normal'; | ||
| } | ||
|
|
||
| export function getCredentialSource(options?: MockSourceOptions): CreditCredentialSource { | ||
| const dataSource = | ||
| typeof NEXT_PUBLIC_DATA_SOURCE !== 'undefined' ? (NEXT_PUBLIC_DATA_SOURCE as string) : 'mock'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify env access patterns around credential source configuration.
rg -n -C2 'declare const NEXT_PUBLIC_(MOCK_MODE|DATA_SOURCE)' packages/acta/src/mock.ts
rg -n -C2 'NEXT_PUBLIC_(MOCK_MODE|DATA_SOURCE)' packages/acta/src/mock.ts
rg -n -C2 'process\.env\.NEXT_PUBLIC_(MOCK_MODE|DATA_SOURCE)' packages/acta/src/mock.ts apps packagesRepository: ACTA-Team/products-acta
Length of output: 1363
🌐 Web query:
Next.js NEXT_PUBLIC environment variables access pattern best practices
💡 Result:
In Next.js, the NEXT_PUBLIC_ prefix is the designated mechanism for exposing environment variables to the browser [1][2]. When a variable is prefixed with NEXT_PUBLIC_, Next.js automatically "inlines" its value into the JavaScript bundle at build time, replacing all references to process.env.NEXT_PUBLIC_... with the actual value [1][3]. Best Practices and Critical Considerations: 1. Never Expose Secrets: Variables prefixed with NEXT_PUBLIC_ are visible to anyone who inspects the client-side JavaScript bundle or page source [4][5]. Never store sensitive information such as database credentials, API secret keys, or private tokens using this prefix [4][6]. 2. Build-Time Freezing: Because these variables are inlined during next build, they are "frozen" at that moment [1][2]. If you change the value of a NEXT_PUBLIC_ variable, the application must be rebuilt and redeployed for the change to take effect [2][5]. They cannot be dynamically updated at runtime [3]. 3. Use Proper Variable Categorization: - Server-Only: Use variables without the NEXT_PUBLIC_ prefix for any sensitive data. These remain accessible only in Node.js environments (e.g., API routes, Server Components, Server Actions) and are never included in the client-side bundle [4][2]. - Client-Accessible: Only use NEXT_PUBLIC_ for non-sensitive, public configuration, such as analytics IDs, public API base URLs, or feature flags [4][5]. 4. Implementation Patterns: - Always access variables using the full process.env.NEXT_PUBLIC_VAR_NAME syntax [2]. Avoid destructuring process.env (e.g., const { VAR } = process.env), as this prevents Next.js from performing the static replacement required for inlining [2][7]. - Avoid dynamic key access (e.g., process.env[key]), as this will not be inlined and will return undefined in the browser [2][6]. 5. Validation: Use schema validation libraries (such as Zod) to validate environment variables at application startup [2][5]. This ensures the application fails early with a clear error if required variables are missing, rather than experiencing unexpected runtime behavior [5]. 6. File Management: Maintain an .env.local file for local development (which must be included in .gitignore to prevent committing secrets) and use your hosting platform's environment variable management (e.g., Vercel's dashboard) for production settings [4][2][5]. Document required variables in a committed .env.example file [6].
Citations:
- 1: https://nextjs.org/docs/pages/guides/environment-variables
- 2: https://env.dev/guides/nextjs-env-variables
- 3: https://nextjs.im/docs/app/guides/environment-variables/
- 4: https://www.matthewswong.com/en/blog/environment-variables-secrets-nextjs/
- 5: https://www.nextcraft.agency/resources/insights/nextjs-environment-variables
- 6: https://vercel.com/academy/nextjs-foundations/env-and-security
- 7: https://nextjs.org/docs/app/api-reference/config/next-config-js/env
🏁 Script executed:
# Check if there are other patterns in the codebase for accessing public env vars
rg -n 'process\.env\.NEXT_PUBLIC' packages/acta/src --type ts
rg -n 'NEXT_PUBLIC' packages/acta/src --type ts | head -30Repository: ACTA-Team/products-acta
Length of output: 610
🏁 Script executed:
# Check if this pattern is used elsewhere or if there's configuration guidance
fd -e md -e ts -e js . packages/acta | xargs rg -l 'NEXT_PUBLIC' | head -10Repository: ACTA-Team/products-acta
Length of output: 93
Use process.env.NEXT_PUBLIC_* for proper Next.js environment variable inlining.
The current code declares ambient NEXT_PUBLIC_MOCK_MODE and NEXT_PUBLIC_DATA_SOURCE constants and accesses them as bare globals. This bypasses Next.js's static replacement mechanism, which requires using process.env.NEXT_PUBLIC_* directly to inline values at build time. The bare globals approach is non-standard and may result in undefined values at runtime.
Replace the ambient declarations and bare global reads with process.env.NEXT_PUBLIC_MOCK_MODE and process.env.NEXT_PUBLIC_DATA_SOURCE to align with Next.js best practices:
Proposed fix
-// Inlined by Next.js build — declared here so TypeScript resolves the names.
-declare const NEXT_PUBLIC_MOCK_MODE: string | undefined;
-declare const NEXT_PUBLIC_DATA_SOURCE: string | undefined;
-
function resolveMockMode(): MockMode {
- const envMode =
- typeof NEXT_PUBLIC_MOCK_MODE !== 'undefined' ? (NEXT_PUBLIC_MOCK_MODE as string) : '';
+ const envMode =
+ typeof process !== 'undefined' ? (process.env.NEXT_PUBLIC_MOCK_MODE ?? '') : '';
if (envMode === 'empty' || envMode === 'error') return envMode as MockMode;
return 'normal';
}
export function getCredentialSource(options?: MockSourceOptions): CreditCredentialSource {
const dataSource =
- typeof NEXT_PUBLIC_DATA_SOURCE !== 'undefined' ? (NEXT_PUBLIC_DATA_SOURCE as string) : 'mock';
+ typeof process !== 'undefined' ? (process.env.NEXT_PUBLIC_DATA_SOURCE ?? 'mock') : 'mock';📝 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.
| // Inlined by Next.js build — declared here so TypeScript resolves the names. | |
| declare const NEXT_PUBLIC_MOCK_MODE: string | undefined; | |
| declare const NEXT_PUBLIC_DATA_SOURCE: string | undefined; | |
| function resolveMockMode(): MockMode { | |
| const envMode = | |
| typeof NEXT_PUBLIC_MOCK_MODE !== 'undefined' ? (NEXT_PUBLIC_MOCK_MODE as string) : ''; | |
| if (envMode === 'empty' || envMode === 'error') return envMode as MockMode; | |
| return 'normal'; | |
| } | |
| export function getCredentialSource(options?: MockSourceOptions): CreditCredentialSource { | |
| const dataSource = | |
| typeof NEXT_PUBLIC_DATA_SOURCE !== 'undefined' ? (NEXT_PUBLIC_DATA_SOURCE as string) : 'mock'; | |
| function resolveMockMode(): MockMode { | |
| const envMode = | |
| typeof process !== 'undefined' ? (process.env.NEXT_PUBLIC_MOCK_MODE ?? '') : ''; | |
| if (envMode === 'empty' || envMode === 'error') return envMode as MockMode; | |
| return 'normal'; | |
| } | |
| export function getCredentialSource(options?: MockSourceOptions): CreditCredentialSource { | |
| const dataSource = | |
| typeof process !== 'undefined' ? (process.env.NEXT_PUBLIC_DATA_SOURCE ?? 'mock') : 'mock'; |
🤖 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 `@packages/acta/src/mock.ts` around lines 224 - 237, Remove the ambient
declarations for NEXT_PUBLIC_MOCK_MODE and NEXT_PUBLIC_DATA_SOURCE at the top of
the file. In the resolveMockMode function, replace the reference to
NEXT_PUBLIC_MOCK_MODE with process.env.NEXT_PUBLIC_MOCK_MODE. In the
getCredentialSource function, replace the reference to NEXT_PUBLIC_DATA_SOURCE
with process.env.NEXT_PUBLIC_DATA_SOURCE. This ensures Next.js's static
replacement mechanism works correctly by accessing environment variables through
the standard process.env object.
2 participants
Summary
Implements the wallet connection and holder session layer for the
credit-historyapp (issue #4). This is the identity foundation thatfeeds the share flow (#7) and all routes that require a holder session.
New files
src/providers/acta-provider.tsx'use client'wrapper aroundActaConfigfrom@acta-products/actaNEXT_PUBLIC_STELLAR_NETWORKto select between the SDK'smainNet/
testNetURL constants (baseURLis a string literal union, not ageneric string)
NEXT_PUBLIC_ACTA_API_KEY; falls back to a placeholder in Batch 1because no real API calls are made yet (mock data layer from Mock data layer + SDK integration seam #5 handles
all reads)
NextIntlClientProviderinlayout.tsx— as low aspossible per the spec
src/session/wallet-connector.tsWalletConnectorinterface:connect(): Promise<{ address: string }>,disconnect(): Promise<void>MockWalletConnector: returns a fixed brand-book G... address after 800 mssimulated latency; no signing, no real wallet
getWalletConnector()— single swap-point factory with documented seamcomment explaining what Freighter / Stellar Wallets Kit replaces and that
the UI does not change
src/session/session-provider.tsxSessionProvider+useSession()Client Componentstatus('disconnected' | 'connecting' | 'connected'),address,did,connect(),disconnect()didPkhStellar(network, address)— never by handlocalStorageunder keyacta:session:address(see persistence decision below)
src/components/wallet-button.tsxdisconnected→ "Connect wallet" button with wallet iconconnecting→ disabled button with spinner + i18n labelconnected→ truncatedGABC...7KQ4+ truncated DID infont-monouseTranslations('session')— zero hardcoded stringsResults:
Modified files
src/components/header.tsx<WalletButton />alongside<MobileMenu />valid in Next.js App Router
src/app/layout.tsx<ActaProvider><SessionProvider>insideNextIntlClientProvidersrc/i18n/messages/en.json+es.jsonsessionnamespace:connect,connecting,disconnect,connectedAs,notConnected,connectCta,connectButton.env.exampleNEXT_PUBLIC_STELLAR_NETWORKandNEXT_PUBLIC_ACTA_API_KEYPersistence decision
localStorage(keyacta:session:address) was chosen oversessionStoragebecausesessionStoragedoes not survive a page refresh,which breaks the acceptance criterion "session persists during navigation
(and ideally survives refresh)". In a later batch, when Freighter is
wired, the connector itself handles persistence (Freighter remembers the
permission grant) and the
localStorageentry becomes redundant and canbe removed.
How to wire the real wallet (future batch)
FreighterWalletConnector implements WalletConnectorusingFreighter / Stellar Wallets Kit — provides
address(G...) andsignTransactionreturn new MockWalletConnector()ingetWalletConnector()SessionProvider,useSession,WalletButton) does not changeVerification checklist
G...+ DID in exact formatdid:pkh:stellar:{network}:{G...}appears in headerlocalStorageNEXT_PUBLIC_STELLAR_NETWORK=mainnet→ DID network segment changesto
mainnet(proof it is not hardcoded)sessioni18n namespaceActaConfigmounted as'use client', as low as possible in treegetWalletConnector()pnpm lint && pnpm typecheck && pnpm buildpassEnvironment variables
NEXT_PUBLIC_STELLAR_NETWORKtestnetNEXT_PUBLIC_ACTA_API_KEYCloses #4
Summary by CodeRabbit
New Features
Documentation