Chore/fix check code #404
Chore/fix check code #404
Conversation
WalkthroughThis PR encompasses multi-module refactoring and targeted improvements: removes a linting CI workflow, switches eVault data fetching from details to metrics, reorganizes imports across eID wallet modules, refactors web3-adapter with database schema updates and function modernization, enhances error handling in DreamSync, updates eReputation service signatures from string to numeric IDs, and adds new environment variables to Pictique. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrastructure/control-panel/src/routes/evaults/[namespace]/[pod]/+page.svelte (1)
27-33: Critical: This change breaks the "Details" tab functionality.The change from
getEVaultDetailstogetEVaultMetricson line 29 means thedetailsvariable now contains metrics data instead of details data. However, the "Details" tab (lines 126-133) attempts to displaydetails?.podInfo, which would have been present in the details response but not in the metrics response. This will cause the details tab to always show "No details available".To support all three tabs (Logs, Details, Metrics), you need to fetch both details and metrics data separately.
Apply this diff to fetch both details and metrics:
+ let metrics: any = null; + const fetchEVaultDetails = async () => { if (!namespace || !podName) { error = 'Invalid namespace or pod name'; return; } try { isLoading = true; error = null; - const [logsData, detailsData] = await Promise.all([ + const [logsData, detailsData, metricsData] = await Promise.all([ EVaultService.getEVaultLogs(namespace, podName), + EVaultService.getEVaultDetails(namespace, podName), EVaultService.getEVaultMetrics(namespace, podName) ]); logs = logsData; details = detailsData; + metrics = metricsData; } catch (err) {Then update the metrics tab to use the
metricsvariable:{:else if selectedTab === 'metrics'} <div class="rounded-lg bg-white p-6 shadow"> <h3 class="mb-4 text-lg font-semibold text-gray-900">Pod Metrics</h3> - {#if details?.metrics} + {#if metrics?.metrics} <div class="rounded-lg bg-gray-50 p-4"> - <pre class="text-sm">{details.metrics}</pre> + <pre class="text-sm">{metrics.metrics}</pre> </div> {:else}
🧹 Nitpick comments (9)
platforms/dreamSync/client/src/components/auth/login-screen.tsx (1)
24-32: Consider usingaxios.isAxiosError()for cleaner type narrowing.While the current error handling works correctly, Axios provides
isAxiosError()which offers more idiomatic type narrowing and cleaner code.Apply this diff to simplify the error handling:
} catch (error) { console.error("❌ Failed to get auth offer:", error); - if (error && typeof error === 'object' && 'response' in error) { - const axiosError = error as { response?: { data?: unknown; status?: number } }; - console.error("❌ Error details:", axiosError.response?.data); - console.error("❌ Error status:", axiosError.response?.status); + if (axios.isAxiosError(error)) { + console.error("❌ Error details:", error.response?.data); + console.error("❌ Error status:", error.response?.status); } setIsLoading(false); }infrastructure/control-panel/src/routes/evaults/[namespace]/[pod]/+page.svelte (1)
17-17: Update function name and error message to reflect actual behavior.The function name
fetchEVaultDetailsand error message "Failed to fetch eVault details" are misleading after the change. If you implement the fix above to fetch both details and metrics, consider renaming tofetchEVaultDataand updating the error message to "Failed to fetch eVault data" for accuracy.Also applies to: 35-35
platforms/dreamSync/client/src/components/group-card.tsx (1)
196-200: Optional chaining fix is correct; consider cleaning up dead code.The change to
suggestion?.reasonon line 198 properly resolves the TypeScript error. Since the&& falsein the condition prevents type narrowing, the optional chaining is necessary even though line 196 checks the same property.However, this entire block (and similar blocks at lines 147-152 and 168-178) will never render due to the
&& falseguards. If these are disabled features, consider removing them to reduce code clutter, or document why they're retained.- {showSuggestionInfo && suggestion?.reason && false && ( - <p className="text-xs text-purple-600 mb-3 italic line-clamp-1 flex-grow"> - {suggestion?.reason} - </p> - )}platforms/pictique/src/routes/(auth)/auth/+page.svelte (1)
26-37: Consider using the imported constants for app store URLs.The constants
PUBLIC_APP_STORE_EID_WALLETandPUBLIC_PLAY_STORE_EID_WALLETare imported (lines 4-6) but not used. Replace the hardcoded URLs in lines 29, 33, and 36 with these constants for better maintainability.Example:
getAppStoreLink = () => { const userAgent = navigator.userAgent || navigator.vendor || (window as { opera?: string }).opera || ''; if (/android/i.test(userAgent)) { - return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet'; + return PUBLIC_PLAY_STORE_EID_WALLET; } if (/iPad|iPhone|iPod/.test(userAgent) && !('MSStream' in window)) { - return 'https://apps.apple.com/in/app/eid-for-w3ds/id6747748667'; + return PUBLIC_APP_STORE_EID_WALLET; } - return 'https://play.google.com/store/apps/details?id=foundation.metastate.eid_wallet'; + return PUBLIC_PLAY_STORE_EID_WALLET; };platforms/eReputation/vite.config.ts (1)
6-9: Consider using proper plugin typing.The
any[]type bypasses type safety. Consider using Vite's plugin type instead.Apply this diff:
-const plugins: any[] = [ +const plugins = [ react(), runtimeErrorOverlay(), ];TypeScript will infer the correct type automatically, or you can explicitly use
import('vite').PluginOption[]if needed.platforms/dreamSync/client/src/components/user-card.tsx (1)
114-114: Optional: Remove redundant type annotation.The explicit type annotation
hobby: { id: string; name: string }is redundant since TypeScript infers the type fromtopHobbies. Consider simplifying to:-{topHobbies.map((hobby: { id: string; name: string }) => ( +{topHobbies.map((hobby) => (platforms/eReputation/server/routes.ts (1)
116-134: Consider consolidating variables normalization logic.The variables array normalization at lines 116-119 is duplicated in
ReputationService.calculateReputation(lines 17-20 in reputation.ts). Since the service performs the same normalization, the preprocessing here is redundant.Additionally, the minimum variables validation at line 122 remains commented out, meaning no enforcement of the "at least 3 variables" requirement mentioned in the comment.
Apply this diff to remove the redundant normalization:
- // Convert variables to string array if needed - const variablesArray = Array.isArray(variables) - ? variables as string[] - : (variables ? [String(variables)] : []); - // Skip the variables check for now to isolate the validation issue - // if (!Array.isArray(variablesArray) || variablesArray.length < 3) { + // if (!Array.isArray(variables) || variables.length < 3) { // return res.status(400).json({ // message: "Please select at least 3 variables for accurate analysis" // }); // } const calculation = await ReputationService.calculateReputation( userId, targetType, targetId || undefined, targetName || undefined, - variablesArray + variables );Is the commented-out validation intentionally disabled, or should it be re-enabled?
platforms/eReputation/server/services/reputation.ts (1)
5-7: Remove unused helper function.The
nullToUndefinedhelper is defined but never called in the file.Apply this diff to remove the dead code:
-// Helper to convert null to undefined -const nullToUndefined = <T>(value: T | null | undefined): T | undefined => - value === null ? undefined : value; - export class ReputationService {infrastructure/web3-adapter/src/index.ts (1)
142-242: Consider using the logger utility and making schema ID configurable.The exponential backoff retry mechanism is well-implemented, but there are a couple of refinements to consider:
The function uses
console.logandconsole.error(lines 173, 219, 222, 228, 236) instead of the importedloggerutility (line 7), which would provide consistent structured logging.The GroupManifest schema ID
"550e8400-e29b-41d4-a716-446655440001"(line 211) is hardcoded. If this ID needs to change or vary by environment, it would require code changes.Optional: Use structured logging
- console.log( - `Attempting to create GroupManifest in eVault (attempt ${attempt}/${maxRetries})`, - ); + logger.info(`Attempting to create GroupManifest in eVault (attempt ${attempt}/${maxRetries})`);Optional: Make schema ID configurable
Consider accepting the schema ID as a parameter with a default value if this might need to vary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (48)
.github/workflows/check-lint.yml(0 hunks)infrastructure/control-panel/src/routes/evaults/[namespace]/[pod]/+page.svelte(1 hunks)infrastructure/eid-wallet/src/lib/crypto/HardwareKeyManager.ts(1 hunks)infrastructure/eid-wallet/src/lib/crypto/KeyManagerFactory.ts(1 hunks)infrastructure/eid-wallet/src/lib/global/controllers/evault.ts(1 hunks)infrastructure/eid-wallet/src/lib/global/state.ts(1 hunks)infrastructure/eid-wallet/src/lib/services/NotificationService.ts(1 hunks)infrastructure/eid-wallet/src/routes/(app)/+layout.svelte(1 hunks)infrastructure/eid-wallet/src/routes/(app)/settings/+page.svelte(1 hunks)infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte(1 hunks)infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte(2 hunks)infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte(1 hunks)infrastructure/eid-wallet/src/routes/+layout.svelte(1 hunks)infrastructure/web3-adapter/biome.json(1 hunks)infrastructure/web3-adapter/src/__tests__/adapter.test.ts(1 hunks)infrastructure/web3-adapter/src/__tests__/evault.test.ts(5 hunks)infrastructure/web3-adapter/src/adapter.ts(1 hunks)infrastructure/web3-adapter/src/db/index.js(2 hunks)infrastructure/web3-adapter/src/db/mapping.db.js(1 hunks)infrastructure/web3-adapter/src/db/mapping.db.ts(2 hunks)infrastructure/web3-adapter/src/evault/evault.js(0 hunks)infrastructure/web3-adapter/src/evault/evault.ts(2 hunks)infrastructure/web3-adapter/src/index.d.ts(1 hunks)infrastructure/web3-adapter/src/index.js(3 hunks)infrastructure/web3-adapter/src/index.ts(3 hunks)infrastructure/web3-adapter/src/logging/index.js(2 hunks)infrastructure/web3-adapter/src/logging/logger.d.ts(1 hunks)infrastructure/web3-adapter/src/logging/logger.js(1 hunks)infrastructure/web3-adapter/src/logging/transport.js(1 hunks)infrastructure/web3-adapter/src/logging/transport.ts(1 hunks)infrastructure/web3-adapter/src/mapper/mapper.js(0 hunks)infrastructure/web3-adapter/src/mapper/mapper.ts(2 hunks)infrastructure/web3-adapter/src/mapper/mapper.types.js(0 hunks)infrastructure/web3-adapter/src/mapper/mapper.types.ts(1 hunks)platforms/dreamSync/client/src/components/auth/login-screen.tsx(1 hunks)platforms/dreamSync/client/src/components/group-card.tsx(1 hunks)platforms/dreamSync/client/src/components/user-card.tsx(1 hunks)platforms/dreamSync/client/src/lib/apiClient.ts(2 hunks)platforms/dreamSync/client/src/lib/auth-context.tsx(1 hunks)platforms/dreamSync/client/src/pages/wishlist-editor.tsx(1 hunks)platforms/dreamSync/shared/schema.ts(2 hunks)platforms/eReputation/server/auth.ts(1 hunks)platforms/eReputation/server/routes.ts(2 hunks)platforms/eReputation/server/services/reputation.ts(3 hunks)platforms/eReputation/shared/entities.ts(1 hunks)platforms/eReputation/vite.config.ts(1 hunks)platforms/pictique/src/env.d.ts(1 hunks)platforms/pictique/src/routes/(auth)/auth/+page.svelte(1 hunks)
💤 Files with no reviewable changes (4)
- infrastructure/web3-adapter/src/evault/evault.js
- infrastructure/web3-adapter/src/mapper/mapper.types.js
- .github/workflows/check-lint.yml
- infrastructure/web3-adapter/src/mapper/mapper.js
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-04-16T18:04:28.983Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 99
File: infrastructure/w3id/tests/w3id.test.ts:48-57
Timestamp: 2025-04-16T18:04:28.983Z
Learning: In test files, suggestions to add defensive coding patterns (like extra null checks) are often unnecessary, as tests are designed to fail explicitly when expectations aren't met. The test in `infrastructure/w3id/tests/w3id.test.ts` validates that `id.logs` is an instance of `IDLogManager` before accessing its repository property.
Applied to files:
infrastructure/web3-adapter/src/__tests__/adapter.test.tsinfrastructure/web3-adapter/src/__tests__/evault.test.ts
📚 Learning: 2025-06-07T04:59:24.520Z
Learnt from: Sahil2004
Repo: MetaState-Prototype-Project/prototype PR: 193
File: platforms/metagram/src/lib/store/store.svelte.ts:0-0
Timestamp: 2025-06-07T04:59:24.520Z
Learning: In the MetaState prototype project, prefer using centralized type definitions from `$lib/types` over importing interfaces from component files for better type organization and to avoid circular dependencies.
Applied to files:
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelteinfrastructure/eid-wallet/src/lib/global/state.ts
📚 Learning: 2025-05-08T08:40:51.710Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/lib/ui/Button/Button.svelte:83-83
Timestamp: 2025-05-08T08:40:51.710Z
Learning: In Svelte 5, event handlers should use lowercase DOM-style attributes (e.g., `onclick={handler}`) rather than the Svelte 3/4 syntax with the `on:` prefix (e.g., `on:click={handler}`). The `on:` syntax is deprecated in Svelte 5 though still supported for backward compatibility.
Applied to files:
infrastructure/eid-wallet/src/routes/(app)/settings/+page.svelte
📚 Learning: 2025-05-08T08:40:51.710Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/lib/ui/Button/Button.svelte:83-83
Timestamp: 2025-05-08T08:40:51.710Z
Learning: In Svelte 5, event handlers can be written using the lowercase DOM-style attributes (e.g., `onclick`) rather than the Svelte 3/4 syntax with the `on:` prefix (e.g., `on:click`). Both syntaxes are supported, but `onclick` is the newer approach introduced in Svelte 5.
Applied to files:
infrastructure/eid-wallet/src/routes/(app)/settings/+page.svelte
🧬 Code graph analysis (9)
platforms/dreamSync/client/src/lib/auth-context.tsx (1)
platforms/dreamSync/shared/schema.ts (2)
User(279-279)UserWithProfile(304-311)
infrastructure/web3-adapter/src/__tests__/adapter.test.ts (1)
infrastructure/web3-adapter/src/adapter.ts (1)
Web3Adapter(7-59)
infrastructure/web3-adapter/src/__tests__/evault.test.ts (1)
infrastructure/web3-adapter/src/adapter.ts (1)
Web3Adapter(7-59)
platforms/eReputation/server/services/reputation.ts (3)
platforms/eReputation/shared/entities.ts (1)
InsertReputationCalculation(193-193)platforms/eReputation/shared/schema.ts (1)
InsertReputationCalculation(111-111)platforms/eReputation/shared/schema-backup.ts (1)
InsertReputationCalculation(108-108)
platforms/eReputation/shared/entities.ts (1)
platforms/eReputation/shared/schema.ts (2)
InsertFileUpload(115-115)FileUpload(114-114)
infrastructure/web3-adapter/src/mapper/mapper.types.ts (1)
infrastructure/web3-adapter/src/db/mapping.db.ts (1)
MappingDatabase(5-176)
infrastructure/web3-adapter/src/logging/transport.ts (1)
infrastructure/web3-adapter/src/logging/transport.js (1)
envPath(9-9)
infrastructure/web3-adapter/src/evault/evault.ts (1)
infrastructure/web3-adapter/src/evault/evault.js (4)
CONFIG(6-16)STORE_META_ENVELOPE(17-27)FETCH_META_ENVELOPE(28-36)UPDATE_META_ENVELOPE(37-53)
infrastructure/web3-adapter/src/index.ts (5)
infrastructure/web3-adapter/src/mapper/mapper.types.ts (1)
IMapping(3-42)infrastructure/web3-adapter/src/db/mapping.db.ts (1)
MappingDatabase(5-176)infrastructure/web3-adapter/src/evault/evault.ts (1)
EVaultClient(127-616)infrastructure/web3-adapter/src/mapper/mapper.ts (2)
toGlobal(238-356)fromGlobal(123-208)infrastructure/web3-adapter/src/logging/logger.ts (1)
logger(4-4)
🪛 ast-grep (0.39.7)
infrastructure/web3-adapter/src/mapper/mapper.ts
[warning] 224-224: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b${token.replace(".", "\\.")}\\b, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint
🔇 Additional comments (50)
platforms/dreamSync/client/src/lib/apiClient.ts (2)
1-1: LGTM! Correct Axios type import for v1.0+.The change from
AxiosRequestConfigtoInternalAxiosRequestConfigis appropriate for Axios v1.0+ and fixes TypeScript compatibility issues in request interceptors.
20-31: Correct typing for request interceptor.The parameter type update to
InternalAxiosRequestConfigis consistent with the import change and properly types the interceptor callback for Axios v1.0+.platforms/pictique/src/routes/(auth)/auth/+page.svelte (2)
27-27: LGTM - Type-safe access to window.opera.The type cast correctly handles the absence of the
operaproperty on the window object, preventing TypeScript compilation errors.
32-32: LGTM - Proper property existence check.Using the
inoperator is the correct approach for checking whetherMSStreamexists in the window object, avoiding potential errors from direct property access.platforms/dreamSync/client/src/pages/wishlist-editor.tsx (1)
116-116: LGTM! Improved fallback logic for user display.The change from
user?.nameto a fallback chain (user?.firstName || user?.email || 'User') is well-structured and provides graceful degradation. This likely fixes a TypeScript error where thenameproperty doesn't exist on the user type. The fallback logic ensures the welcome message always displays something meaningful.infrastructure/web3-adapter/src/logging/transport.ts (2)
9-21: Formatting updates to transport config—no functional changes.The indentation and structure of the transport configuration remain semantically identical. The pino-loki setup with environment variable references (LOKI_URL, username, password) is correct.
1-2: Import consolidation is correct—Node version compatibility confirmed.The project's
package.jsondeclaresengines.node: ">=18", and CI workflows use Node 22.x. Since thenode:pathsyntax requires Node 16+, your project fully supports this modern import style. The consolidation of imports at the top is idiomatic and correct.infrastructure/eid-wallet/src/lib/global/controllers/evault.ts (1)
6-6: LGTM! Type-only import optimization.Converting to a type-only import is a good TypeScript practice that clarifies the import is used solely for type annotations, potentially enabling better tree-shaking.
infrastructure/eid-wallet/src/lib/crypto/HardwareKeyManager.ts (1)
8-9: LGTM! Import organization.The import reordering maintains functionality while improving code organization consistency across the codebase.
infrastructure/eid-wallet/src/routes/(app)/settings/+page.svelte (1)
125-125: LGTM! Svelte 5 syntax adoption.The change from
on:clicktoonclickadopts the modern Svelte 5 DOM-style event handler syntax. This is the recommended approach for Svelte 5 while maintaining identical behavior.Based on learnings
infrastructure/eid-wallet/src/lib/services/NotificationService.ts (1)
1-7: LGTM! Import reorganization.The import restructuring improves code organization by consolidating related imports at the top level while maintaining all functionality.
infrastructure/eid-wallet/src/lib/global/state.ts (1)
2-2: LGTM! Import consolidation.Moving the NotificationService import to the top-level import region improves code organization and readability.
infrastructure/eid-wallet/src/lib/crypto/KeyManagerFactory.ts (1)
3-3: LGTM! Type import organization.Reorganizing type imports maintains functionality while improving consistency with the codebase's import ordering conventions.
infrastructure/eid-wallet/src/routes/(auth)/verify/+page.svelte (2)
7-7: LGTM! Type import addition.Adding the type-only imports for KeyManager and KeyManagerFactory improves type safety and code clarity.
20-28: LGTM! Import reorganization.The import statement reorganization and addition of store exports (verifStep, verificaitonId) maintains functionality while improving code organization.
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (1)
7-7: LGTM! Import organization.Reorganizing the crypto module import to place type imports consistently maintains functionality while improving code organization.
infrastructure/eid-wallet/src/routes/+layout.svelte (1)
3-5: LGTM! Import reordering for linting compliance.The import statements have been reordered for consistency, which aligns with the PR's objective to fix CI errors. No functional changes.
infrastructure/eid-wallet/src/routes/(auth)/verify/steps/passport.svelte (1)
7-14: LGTM! Import organization and missing import fix.The import block has been reorganized, and notably,
documentTypehas been added to the imports. Since$documentTypeis used throughout the component (lines 139, 230, 265, 291, 323), this addition fixes a missing import that could have caused runtime issues.infrastructure/eid-wallet/src/routes/(app)/+layout.svelte (1)
3-7: LGTM! Import reordering for consistency.The import statements have been reorganized, grouping regular and type imports more consistently. This is a formatting change with no functional impact, aligning with the PR's goal to fix CI linting errors.
infrastructure/web3-adapter/src/mapper/mapper.types.ts (1)
3-42: Documentation and formatting enhancements look good.The updated JSDoc comments on
IMapping.ownerEnamePathprovide clear guidance on path syntax options (direct fields, nested references via table lookups, and fallback patterns). The interface structure remains unchanged and well-typed.infrastructure/web3-adapter/src/mapper/mapper.ts (6)
7-32:getValueByPathlogic is sound.The function correctly handles both array mappings (with
[]syntax) and nested object paths. Proper null-check on line 29 prevents accessing undefined intermediate values.
46-95:extractOwnerEvaultfallback logic is well-structured.The function properly splits fallback paths on
||, validates format, and iterates through options with appropriate logging. The delegation toextractOwnerEvaultSinglePathkeeps concerns separated cleanly.
101-121:extractOwnerEvaultSinglePathhandles table references and edge cases correctly.The function safely checks for parentheses before splitting, handles array returns on line 112, and includes type-checking for strings before calling
.includes()on line 116 to prevent runtime errors.
123-208:fromGlobalconversion logic is comprehensive.The function handles internal functions (
__date,__calc), multiple Firebase timestamp formats (v8/v9 and Timestamp objects), table references, and array mappings. The mapping-store integration for ID translation is correct.
210-236:evaluateCalcExpressionperforms safe arithmetic evaluation.The function tokenizes expressions, resolves path values, and uses
Function()constructor in strict mode to evaluate. Thetry/catchensures undefined results on evaluation errors rather than throwing.However, see the separate ReDoS security concern flagged for line 224.
238-356:toGlobalmapping logic is robust and complete.The function handles array element mapping, internal functions, relation references (with
@prefix syntax), table references, and ID translation via mappingStore. The final call toextractOwnerEvaultproperly determines data ownership. All conditional branches are handled.platforms/dreamSync/shared/schema.ts (1)
283-283: LGTM: Type inference consistency improved.This change aligns
InsertWishwith the established pattern used for other insert types (e.g.,InsertUser,InsertProfile,InsertSkill), making the codebase more consistent.platforms/dreamSync/client/src/lib/auth-context.tsx (1)
6-8: LGTM: Type consolidation improves consistency.Replacing the local
Userinterface with the sharedUserWithProfiletype eliminates duplication and ensures type consistency across the DreamSync platform.platforms/eReputation/server/auth.ts (1)
8-17: LGTM! Proper session typing added.The module augmentation correctly types the session user data, aligning with the runtime usage throughout the authentication flow (lines 61, 95-100, 136-141).
platforms/eReputation/shared/entities.ts (1)
190-195: Inconsistency between summary and code.The AI-generated summary indicates that multiple export types were removed, leaving only
InsertFileUploadas the sole public export. However, the code shows all type exports (lines 190-195) are still present:UpsertUser,InsertUser,LoginUser,InsertReputationCalculation,InsertReference, andInsertFileUpload.This is confirmed by
platforms/eReputation/server/auth.tsline 6, which importsLoginUserandInsertUserfrom this module.platforms/eReputation/server/services/reputation.ts (2)
10-20: Signature changes align with routes.ts.The updated signature correctly accepts
userIdas a number and allowsundefinedfor optionaltargetId/targetNameparameters, matching howroutes.tsnow invokes this method (lines 128-134).Note: Variables normalization at lines 17-20 duplicates the same logic already performed in
routes.ts(lines 116-119), as mentioned in the routes.ts review.
57-107: LGTM! Method signatures updated consistently.Both
getActivityHistoryandgetUserStatsnow correctly acceptuserIdas anumber, maintaining consistency with thecalculateReputationsignature change.infrastructure/web3-adapter/src/adapter.ts (1)
1-59: LGTM! Formatting changes only.The formatting and indentation adjustments improve code consistency without altering behavior.
infrastructure/web3-adapter/src/logging/transport.js (1)
1-24: LGTM! Syntactic modernization.The conversion of
__importDefaultto an arrow function preserves identical behavior while modernizing the syntax.infrastructure/web3-adapter/src/db/index.js (1)
1-36: LGTM! Module boilerplate modernization.The conversion of internal helpers to arrow functions aligns with the project-wide refactoring while preserving runtime behavior.
infrastructure/web3-adapter/src/logging/index.js (1)
1-37: LGTM! Consistent modernization.The arrow function conversions match the pattern used across other modules in this PR with no behavioral impact.
infrastructure/web3-adapter/src/logging/logger.js (1)
1-9: LGTM! Consistent with other module helpers.The arrow function conversion maintains identical import behavior.
infrastructure/web3-adapter/src/index.js (1)
1-409: LGTM! Comprehensive module boilerplate modernization.The conversion of multiple helper functions to arrow syntax modernizes the codebase without affecting the public API or runtime behavior.
infrastructure/web3-adapter/src/evault/evault.ts (3)
69-114: LGTM! Interface restructuring for clarity.The
StoreMetaEnvelopeResponseinterface has been flattened to provide clearer separation betweenstoreMetaEnvelopeandupdateMetaEnvelopeByIdresponse shapes. This improves type clarity without affecting runtime behavior.
127-616: LGTM! Class reorganization with preserved functionality.The class implementation has been modernized with explicit visibility modifiers and better organization. The core functionality (token management, endpoint resolution, health checks, GraphQL operations) remains intact.
21-21: Confirm whether aggressive health check settings are intentional and environment-appropriate.The review comment's core claims are factually accurate—
MAX_HEALTH_CHECK_FAILURES: 1(down from 3) andHEALTH_CHECK_TIMEOUT: 2000ms(down from 5000ms) are indeed very aggressive settings. A single health check failure or timeout will immediately mark an endpoint unhealthy and trigger re-resolution.However, the code explicitly labels this as "much more aggressive!" and git history shows
chore: fix web3-adapter timeoutas a deliberate change, suggesting this was intentional.Clarify:
- What problem were these aggressive settings intended to fix?
- Are these settings appropriate for all environments (production, CI, etc.), or should they be configurable?
- Should this behavior be validated with explicit tests documenting the intended sensitivity?
If these aggressive settings are intentional and necessary for your use case, consider adding code comments explaining the rationale. If they're environment-specific, consider making them configurable rather than hardcoded constants.
infrastructure/web3-adapter/biome.json (1)
4-4: Ignore pattern is appropriate for this TypeScript project.The
.jsfiles ininfrastructure/web3-adapter/src/are TypeScript compiler outputs, not source files. Since all source code is TypeScript (.ts), ignoring**/*.jscorrectly prevents Biome from processing generated CommonJS artifacts. The configuration is intentional and correct.infrastructure/web3-adapter/src/__tests__/adapter.test.ts (1)
1-73: LGTM! Formatting changes improve code consistency.The indentation and whitespace adjustments enhance readability while preserving all test logic and assertions.
infrastructure/web3-adapter/src/db/mapping.db.js (1)
1-3: LGTM! Generated code change.The conversion of
__importDefaultto an arrow function is functionally equivalent and appears to be a transpiler output update.infrastructure/web3-adapter/src/logging/logger.d.ts (1)
1-1: LGTM! Best practice for declaration files.Using
import typeclarifies that this is a type-only import, which is appropriate for.d.tsfiles and can improve build performance.infrastructure/web3-adapter/src/index.d.ts (1)
1-2: LGTM! Type-only imports align with best practices.Converting to
import typeforMappingDatabaseandEVaultClientis appropriate since these are only used for type annotations in this declaration file.infrastructure/web3-adapter/src/__tests__/evault.test.ts (1)
1-249: LGTM! Formatting changes maintain test integrity.The indentation and whitespace updates improve code consistency while preserving all test logic, GraphQL queries, and assertions.
infrastructure/web3-adapter/src/db/mapping.db.ts (1)
33-46: LGTM! Schema improvements enhance database structure.The additions improve the database design:
created_attimestamp provides audit capabilityPRIMARY KEYonglobal_idenforces uniqueness- Index on
local_idoptimizes lookup performance forgetGlobalIdqueriesinfrastructure/web3-adapter/src/index.ts (2)
90-137: LGTM! Two-step process with retry improves reliability.The refactoring to first spin up the eVault and then create the manifest with retry logic enhances resilience and makes the process more explicit.
234-237: Exponential backoff calculation is correct.The delay formula
Math.min(1000 * 2 ** (attempt - 1), 10000)correctly implements exponential backoff with a 10-second cap, providing a good balance between retry frequency and total wait time.
| constructor(dbPath: string) { | ||
| // Ensure the directory exists | ||
| const fullPath = join(dbPath, "mappings.db"); | ||
| this.db = new sqlite3.Database(fullPath); | ||
|
|
||
| // Promisify database methods | ||
| this.runAsync = promisify(this.db.run.bind(this.db)); | ||
| this.getAsync = promisify(this.db.get.bind(this.db)); | ||
| this.allAsync = promisify(this.db.all.bind(this.db)); | ||
|
|
||
| // Initialize the database with the required tables | ||
| this.initialize(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify initialization timing for immediate usage.
The constructor calls this.initialize() without awaiting (line 30), which means the constructor returns before database tables are created. If MappingDatabase methods are called immediately after construction, there could be race conditions.
Verify that callers wait for initialization to complete before using the database methods:
🏁 Script executed:
#!/bin/bash
# Description: Check how MappingDatabase is instantiated and used to verify if there's a delay before first use
# Search for MappingDatabase instantiation patterns
rg -n -A 5 'new MappingDatabase\(' --type=ts
# Look for any await patterns or initialization guards
rg -n 'mappingDb.*initialize|await.*MappingDatabase' --type=tsLength of output: 461
---
🏁 Script executed:
```shell
#!/bin/bash
# Get full context of the MappingDatabase instantiation
rg -n -B 10 -A 10 'this.mappingDb = new MappingDatabase' infrastructure/web3-adapter/src/index.ts
Length of output: 702
🏁 Script executed:
#!/bin/bash
# Check if MappingDatabase methods have any initialization checks or guards
rg -n -A 15 '(storeMapping|getGlobalId|getAllMappings)\s*\(' infrastructure/web3-adapter/src/db/mapping.db.tsLength of output: 1851
🏁 Script executed:
#!/bin/bash
# Look for any await or initialization patterns in the class using MappingDatabase
rg -n 'await.*readPaths|readPaths.*await|initialize' infrastructure/web3-adapter/src/index.tsLength of output: 0
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the class name containing the constructor
rg -n -B 30 'constructor\(' infrastructure/web3-adapter/src/index.ts | grep -E '(class|export)' | tail -1
Length of output: 30
🏁 Script executed:
#!/bin/bash
# Check how this service/class is instantiated and if there's any delay before first DB access
rg -n -A 10 'new.*Web3Adapter|new.*MappingDatabase.*|storeMapping\(|getGlobalId\(' infrastructure/web3-adapter/src/index.ts | head -30Length of output: 991
🏁 Script executed:
#!/bin/bash
# Search across the codebase for any callers of this service to see usage patterns
find infrastructure/web3-adapter/src -name "*.ts" -type f -exec grep -l "new Web3Adapter\|mappingDb\." {} \;Length of output: 155
---
🏁 Script executed:
```shell
#!/bin/bash
# Get context of lines around 296 and 359 to see which methods they're in
rg -n -B 15 'this.mappingDb.getGlobalId' infrastructure/web3-adapter/src/index.ts | grep -E '(async |public |private )' | tail -2
Length of output: 32
🏁 Script executed:
#!/bin/bash
# Get full context of the method containing line 296
sed -n '280,305p' infrastructure/web3-adapter/src/index.tsLength of output: 669
🏁 Script executed:
#!/bin/bash
# Get full context of the method containing line 359
sed -n '340,370p' infrastructure/web3-adapter/src/index.tsLength of output: 786
🏁 Script executed:
#!/bin/bash
# Check test files to see if they await initialization or call methods immediately after construction
rg -n -A 5 'new Web3Adapter' infrastructure/web3-adapter/src/__tests__/adapter.test.tsLength of output: 195
---
🏁 Script executed:
```shell
#!/bin/bash
# Find the registerMapping method implementation
rg -n -A 15 'registerMapping\s*\(' infrastructure/web3-adapter/src/index.ts
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check the initialize method to see what tables it creates
rg -n -A 20 'private.*initialize\s*\(|initialize\s*\(\s*\)\s*{' infrastructure/web3-adapter/src/db/mapping.db.tsLength of output: 161
---
🏁 Script executed:
```shell
#!/bin/bash
# Find registerMapping - simpler pattern
rg -n 'registerMapping' infrastructure/web3-adapter/src/index.ts
Length of output: 0
🏁 Script executed:
#!/bin/bash
# Find initialize method - simpler pattern
rg -n -A 20 'initialize.*{' infrastructure/web3-adapter/src/db/mapping.db.ts | head -40Length of output: 89
🏁 Script executed:
#!/bin/bash
# Get the full mapping.db.ts file to understand the initialization
cat infrastructure/web3-adapter/src/db/mapping.db.tsLength of output: 4135
Await database initialization before methods can be called.
The constructor calls this.initialize() without awaiting (line 30), and Web3Adapter's constructor is not async (meaning it also cannot await). Methods like getGlobalId() and storeMapping() can be invoked immediately after instantiation, potentially executing SQL before the id_mappings table is created. While CREATE TABLE IF NOT EXISTS provides some protection, queries can still fail if executed during the initialization window.
To fix this, either:
- Make the Web3Adapter constructor async and await
this.mappingDbinitialization - Provide an async factory method or separate
initialize()method that must be awaited before using the adapter - Ensure callers await initialization before calling adapter methods
🤖 Prompt for AI Agents
In infrastructure/web3-adapter/src/db/mapping.db.ts around lines 19 to 31, the
constructor invokes this.initialize() without awaiting so callers can call DB
methods before the schema exists; change initialization to be async and ensure
callers await it: make initialize return a Promise and remove side-effectful
async work from the constructor, then add a static async create(dbPath: string)
factory that constructs the instance, awaits instance.initialize(), and returns
the ready-to-use instance (or alternatively expose an async init() method that
callers must await); update usages to call await MappingDb.create(...) (or await
instance.init()) so all queries run only after the id_mappings table is created.
| REQUEST_TIMEOUT: 5000, // 5 seconds | ||
| CONNECTION_TIMEOUT: 5000, // 5 seconds |
There was a problem hiding this comment.
Reduced timeouts may cause premature failures.
REQUEST_TIMEOUT and CONNECTION_TIMEOUT are both set to 5 seconds. While this may help catch failures faster in CI, it could cause issues with:
- Slower network conditions
- High-latency connections to remote services
- Services under load
Ensure these timeout values have been tested under realistic load conditions and network latency scenarios.
🤖 Prompt for AI Agents
in infrastructure/web3-adapter/src/evault/evault.ts around lines 14-15,
REQUEST_TIMEOUT and CONNECTION_TIMEOUT are hard-coded to 5000ms which can cause
premature failures under real network latency; make these timeouts configurable
via environment variables (e.g., EVault_REQUEST_TIMEOUT,
EVault_CONNECTION_TIMEOUT) with sensible larger defaults (for example 15000ms
for request and 10000ms for connection) and fallback to the current values if
env vars are absent, and add a short comment explaining why larger defaults are
used and recommend running load/latency tests to validate the chosen values.
| const tokens = expr | ||
| .split(/[^\w.]+/) | ||
| .map((t) => t.trim()) | ||
| .filter(Boolean); | ||
|
|
||
| let resolvedExpr = expr; | ||
| for (const token of tokens) { | ||
| const value = getValueByPath(context, token); | ||
| if (typeof value !== "undefined") { | ||
| resolvedExpr = resolvedExpr.replace( | ||
| new RegExp(`\\b${token.replace(".", "\\.")}\\b`, "g"), | ||
| value, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Potential ReDoS vulnerability in dynamic regex construction.
The regex on line 224 is constructed from variable input (token). Although tokens are derived from splitting on non-word characters (which limits them to \w and .), only the dot is escaped. If the token source logic ever changes or if unexpected characters appear, this could enable a ReDoS attack.
Harden the escape logic by comprehensively escaping all regex metacharacters or use a built-in escaping utility.
Apply this diff to fix the vulnerability:
+ // Escape all regex special characters in the token
+ const escapedToken = token.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
resolvedExpr = resolvedExpr.replace(
- new RegExp(`\\b${token.replace(".", "\\.")}\\b`, "g"),
+ new RegExp(`\\b${escapedToken}\\b`, "g"),
value,
);Alternatively, use a library like escape-string-regexp for safety:
+ import escapeStringRegexp from "escape-string-regexp";
resolvedExpr = resolvedExpr.replace(
- new RegExp(`\\b${token.replace(".", "\\.")}\\b`, "g"),
+ new RegExp(`\\b${escapeStringRegexp(token)}\\b`, "g"),
value,
);🧰 Tools
🪛 ast-grep (0.39.7)
[warning] 224-224: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b${token.replace(".", "\\.")}\\b, "g")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🤖 Prompt for AI Agents
in infrastructure/web3-adapter/src/mapper/mapper.ts around lines 215 to 228, the
code builds a RegExp from the variable token and only escapes dots, creating a
potential ReDoS or incorrect matching if tokens include other regex
metacharacters; fix by fully escaping all regex metacharacters before creating
the RegExp (or use a trusted utility like escape-string-regexp), then use the
escaped token when constructing the RegExp so only literal token instances are
replaced; keep the replacement logic and global/word-boundary intent intact.
| hobbies?: Interest[]; // Alias for interests for backward compatibility | ||
| habits?: Interest[]; // Alias for interests for backward compatibility |
There was a problem hiding this comment.
🧩 Analysis chain
Verify runtime handling of backward-compatibility aliases.
The hobbies and habits aliases are TypeScript-only declarations that won't automatically populate at runtime. Ensure that the API or data layer explicitly maps interests to these fields when constructing UserWithProfile objects, or consumers may encounter undefined when accessing these properties.
Run the following script to verify if runtime code handles these aliases:
🏁 Script executed:
#!/bin/bash
# Description: Check if interests are mapped to hobbies/habits at runtime
# Search for assignments or mappings to hobbies/habits fields
rg -nP --type=ts -C3 '\b(hobbies|habits)\s*[:=]' platforms/dreamSync/
# Search for spread operations or object construction with UserWithProfile
ast-grep --pattern $'const $_ = {
$$$
interests: $_,
$$$
}'Length of output: 0
I need to run additional verification scripts to understand the codebase structure better, since the initial patterns didn't match.
🏁 Script executed:
#!/bin/bash
# Verify the file exists and examine the schema structure
echo "=== Checking if schema.ts exists ==="
find . -name "schema.ts" -type f | head -20
echo -e "\n=== Examining the schema.ts file around lines 308-309 ==="
if [ -f "platforms/dreamSync/shared/schema.ts" ]; then
sed -n '300,320p' "platforms/dreamSync/shared/schema.ts"
fi
echo -e "\n=== Searching for UserWithProfile interface/type ==="
rg -nP --type=ts "interface UserWithProfile|type UserWithProfile" -A 20
echo -e "\n=== Searching for any interests field usage ==="
rg -nP --type=ts "\binterests\b" platforms/dreamSync/ -C 2 | head -50
echo -e "\n=== Searching for hobbies/habits mentions anywhere ==="
rg -nP --type=ts "\b(hobbies|habits)\b" platforms/dreamSync/ -C 2Length of output: 8944
Need to verify where UserWithProfile objects are constructed and returned from the API/backend to confirm if any mapping happens.
🏁 Script executed:
#!/bin/bash
# Search for API endpoints or server-side code that returns UserWithProfile
echo "=== Searching for UserWithProfile usage/construction ==="
rg -nP --type=ts "UserWithProfile" platforms/dreamSync/ -C 3 | head -100
echo -e "\n=== Searching for API endpoints or handlers returning user data ==="
rg -nP --type=ts "export.*function|export.*const.*=.*\(" platforms/dreamSync/server/ -A 5 -B 1 | grep -A 5 -B 1 "user\|User" | head -80
echo -e "\n=== Searching for data transformation/mapping functions ==="
rg -nP --type=ts "toJSON|serialize|transform|map.*user\|hobbies.*=\|habits.*=" platforms/dreamSync/ -C 2Length of output: 16712
Remove the backward-compatibility aliases—they're unused at runtime and cause undefined access bugs.
The TypeScript type aliases hobbies and habits in the UserWithProfile type are not populated at runtime. Client code in user-card.tsx (line 77), profile-completion.tsx (lines 46, 54) directly accesses these properties expecting data, but they will be undefined since only the interests field is ever populated by the API. Remove these aliases to eliminate this type-safety and runtime bug, or implement actual runtime mapping in the API layer if backward compatibility is genuinely required.
🤖 Prompt for AI Agents
platforms/dreamSync/shared/schema.ts lines 308-309: the TypeScript aliases
`hobbies?: Interest[]` and `habits?: Interest[]` are misleading and cause
undefined-access bugs because only `interests` is populated at runtime; either
remove these two alias properties from the UserWithProfile type and update all
consumers (e.g., user-card.tsx line 77 and profile-completion.tsx lines 46,54)
to read from `interests`, or if backward compatibility is required, implement
runtime mapping in the API layer that sets `hobbies` and `habits` to the same
array as `interests` before returning the object and keep the type in sync;
choose one approach and apply it consistently (remove aliases + fix usages OR
add runtime mapping + keep types).
| // Create initial calculation record - ensure null/undefined is handled | ||
| const baseData = { | ||
| userId, | ||
| targetType, | ||
| targetId, | ||
| targetName, | ||
| variables, | ||
| status: "processing", | ||
| variables: variablesArray as any, // jsonb accepts array | ||
| status: "processing" as const, | ||
| }; | ||
|
|
||
| const optionalFields: Partial<Pick<InsertReputationCalculation, 'targetId' | 'targetName'>> = {}; | ||
| if (targetId != null) optionalFields.targetId = targetId as string; | ||
| if (targetName != null) optionalFields.targetName = targetName as string; | ||
|
|
||
| const calculationData = { | ||
| ...baseData, | ||
| ...optionalFields, | ||
| } as unknown as InsertReputationCalculation; | ||
|
|
||
| // @ts-expect-error - TypeScript incorrectly infers null in optionalFields, but runtime value is correct | ||
| const calculation = await storage.createReputationCalculation(calculationData); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix the type issue instead of suppressing it.
The ts-expect-error comment at line 39 indicates a TypeScript type mismatch that should be properly resolved rather than suppressed. The issue likely stems from the type assertion at line 37 (as unknown as InsertReputationCalculation).
The explicit string casts at lines 31-32 (as string) are unnecessary since the != null check already ensures the values are defined.
Apply this diff to address the type handling properly:
const optionalFields: Partial<Pick<InsertReputationCalculation, 'targetId' | 'targetName'>> = {};
- if (targetId != null) optionalFields.targetId = targetId as string;
- if (targetName != null) optionalFields.targetName = targetName as string;
+ if (targetId != null) optionalFields.targetId = targetId;
+ if (targetName != null) optionalFields.targetName = targetName;
const calculationData = {
...baseData,
...optionalFields,
- } as unknown as InsertReputationCalculation;
+ } satisfies InsertReputationCalculation;
- // @ts-expect-error - TypeScript incorrectly infers null in optionalFields, but runtime value is correct
const calculation = await storage.createReputationCalculation(calculationData);Using satisfies provides type checking while preserving the inferred type, avoiding the need for type assertions.
📝 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.
| // Create initial calculation record - ensure null/undefined is handled | |
| const baseData = { | |
| userId, | |
| targetType, | |
| targetId, | |
| targetName, | |
| variables, | |
| status: "processing", | |
| variables: variablesArray as any, // jsonb accepts array | |
| status: "processing" as const, | |
| }; | |
| const optionalFields: Partial<Pick<InsertReputationCalculation, 'targetId' | 'targetName'>> = {}; | |
| if (targetId != null) optionalFields.targetId = targetId as string; | |
| if (targetName != null) optionalFields.targetName = targetName as string; | |
| const calculationData = { | |
| ...baseData, | |
| ...optionalFields, | |
| } as unknown as InsertReputationCalculation; | |
| // @ts-expect-error - TypeScript incorrectly infers null in optionalFields, but runtime value is correct | |
| const calculation = await storage.createReputationCalculation(calculationData); | |
| // Create initial calculation record - ensure null/undefined is handled | |
| const baseData = { | |
| userId, | |
| targetType, | |
| variables: variablesArray as any, // jsonb accepts array | |
| status: "processing" as const, | |
| }; | |
| const optionalFields: Partial<Pick<InsertReputationCalculation, 'targetId' | 'targetName'>> = {}; | |
| if (targetId != null) optionalFields.targetId = targetId; | |
| if (targetName != null) optionalFields.targetName = targetName; | |
| const calculationData = { | |
| ...baseData, | |
| ...optionalFields, | |
| } satisfies InsertReputationCalculation; | |
| const calculation = await storage.createReputationCalculation(calculationData); |
🤖 Prompt for AI Agents
In platforms/eReputation/server/services/reputation.ts around lines 22-40,
remove the ts-expect-error and the double type assertion on calculationData;
stop using unnecessary `as string` casts on targetId/targetName (the `!= null`
checks guarantee definedness). Build calculationData as a plain object that
merges baseData and optionalFields and use the `satisfies
InsertReputationCalculation` operator so TypeScript checks the shape without
forcing casts; ensure variablesArray has the correct type for JSONB before
merging. This eliminates the type suppression and lets the compiler validate the
final payload passed to storage.createReputationCalculation.
| // Conditionally add cartographer plugin if in development and REPL_ID is set | ||
| if (process.env.NODE_ENV !== "production" && process.env.REPL_ID !== undefined) { | ||
| // Use dynamic import at runtime, but handle it synchronously for type checking | ||
| // This will be resolved at runtime | ||
| plugins.push( | ||
| import("@replit/vite-plugin-cartographer").then((m) => m.cartographer()) as any | ||
| ); | ||
| } |
There was a problem hiding this comment.
Critical: Pushing a Promise instead of the resolved plugin object.
Lines 15-17 push a Promise (the result of import().then()) into the plugins array, not the actual plugin object. Vite expects resolved plugin objects, not Promises. This will cause the cartographer plugin to fail to load.
The config must be async to properly handle dynamic imports. Apply this diff:
-const plugins: any[] = [
- react(),
- runtimeErrorOverlay(),
-];
-
-// Conditionally add cartographer plugin if in development and REPL_ID is set
-if (process.env.NODE_ENV !== "production" && process.env.REPL_ID !== undefined) {
- // Use dynamic import at runtime, but handle it synchronously for type checking
- // This will be resolved at runtime
- plugins.push(
- import("@replit/vite-plugin-cartographer").then((m) => m.cartographer()) as any
- );
-}
-
-export default defineConfig({
- plugins,
+export default defineConfig(async () => {
+ const plugins = [
+ react(),
+ runtimeErrorOverlay(),
+ ];
+
+ // Conditionally add cartographer plugin if in development and REPL_ID is set
+ if (process.env.NODE_ENV !== "production" && process.env.REPL_ID !== undefined) {
+ const { cartographer } = await import("@replit/vite-plugin-cartographer");
+ plugins.push(cartographer());
+ }
+
+ return {
+ plugins,And update the closing:
},
- });
+ };
+});Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In platforms/eReputation/vite.config.ts around lines 11 to 18, the code pushes a
Promise (from import(...).then(...)) into the plugins array instead of the
resolved plugin object; change the config to be async and await the dynamic
import so the actual plugin instance is added: make the exported config function
async (or wrap config creation in an async IIFE), await
import("@replit/vite-plugin-cartographer"), call the cartographer() factory to
get the plugin and push that object into plugins, and ensure the exported
default returns the fully resolved config object (update any closing export
accordingly).
| export const PUBLIC_REGISTRY_URL: string; | ||
| export const PUBLIC_APP_STORE_EID_WALLET: string; | ||
| export const PUBLIC_PLAY_STORE_EID_WALLET: string; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that these environment variables are defined and used.
Ensure these new environment variables are:
- Actually used in the Pictique codebase
- Defined in the appropriate environment configuration files (e.g.,
.env.example) - Correctly typed as required (
string) rather than optional (string | undefined)
Run the following scripts to verify:
🏁 Script executed:
#!/bin/bash
# Description: Check if the new environment variables are used in the Pictique codebase
echo "=== Checking usage of PUBLIC_REGISTRY_URL ==="
rg -n --type=ts --type=js -C3 'PUBLIC_REGISTRY_URL' platforms/pictique/
echo ""
echo "=== Checking usage of PUBLIC_APP_STORE_EID_WALLET ==="
rg -n --type=ts --type=js -C3 'PUBLIC_APP_STORE_EID_WALLET' platforms/pictique/
echo ""
echo "=== Checking usage of PUBLIC_PLAY_STORE_EID_WALLET ==="
rg -n --type=ts --type=js -C3 'PUBLIC_PLAY_STORE_EID_WALLET' platforms/pictique/Length of output: 1436
🏁 Script executed:
#!/bin/bash
# Description: Check if the new environment variables are defined in config files
echo "=== Checking for environment variable definitions in config files ==="
fd -e env -e example -e config platforms/pictique/ --exec cat {} \; --exec echo "--- File: {} ---" \;
echo ""
echo "=== Checking .env.example specifically ==="
cat platforms/pictique/.env.example 2>/dev/null || echo ".env.example not found"Length of output: 680
Need to verify the complete .env.example file and check if there are other related files or usages:
Add missing environment variables to .env.example and verify they are actually used in the code.
The PR declares three new PUBLIC environment variables in env.d.ts, but verification reveals:
-
Missing from
.env.example:PUBLIC_APP_STORE_EID_WALLETandPUBLIC_PLAY_STORE_EID_WALLETare not defined in the configuration file. OnlyPUBLIC_REGISTRY_URLis present. -
No usages found: These variables do not appear to be used anywhere in the Pictique codebase (only in the type declaration file).
Add the missing variables to platforms/pictique/.env.example with placeholder values, or remove the unused type declarations if these variables aren't needed yet.
Description of change
fixes CI errors
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactoring
Chores