Feat/load tests #412
Feat/load tests #412
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughReplaces Turborepo README with a Docker-centric MetaState prototype guide; adds profile-driven Docker Compose, many multi-stage Dockerfiles and entrypoint, updates services/envs/ports, adds eName/X-ENAME propagation, DM deduplication, numerous UI Link/guard refactors, and a large Vitest end-to-end test harness. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant E2E as Vitest E2E
participant Factory as TestFactory
participant Registry as Registry / evault-core
participant Firebase as Firebase Admin
participant Blabsy as Blabsy (Firestore)
participant Pictique as Pictique (HTTP API)
participant Sync as Sync System
E2E->>Factory: createTestUsers(count)
Factory->>Registry: provisionEName()
Factory->>Firebase: createFirebaseUser()
Firebase-->>Factory: user created
Factory-->>E2E: TestUser list
par create platform data
E2E->>Blabsy: createPost/chat/message...
E2E->>Pictique: createPost/chat/message...
end
E2E->>Sync: waitForSyncWindow
Sync->>Blabsy: read/mapping
Sync->>Pictique: read/mapping
E2E->>E2E: compareAllData -> report
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Areas to focus review on:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
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: 32
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
platforms/blabsy/src/components/user/user-follow-stats.tsx (1)
45-96: Changes don't align with stated PR objectives.The PR objectives mention fixing the eID wallet to use the
X-ENAMEheader and adding e2e tests for various services. However, this file contains UI improvements for user follow statistics display that don't relate to those objectives. Consider keeping UI refactors separate from infrastructure/testing changes to maintain clear PR scope and simplify review.infrastructure/eid-wallet/src/lib/global/controllers/evault.ts (1)
113-117: Passvault.evaultinstead ofvault.enamefor the w3id parameter across all call sites.The Vault object has three distinct fields:
ename(user identifier like "[email protected]"),uri(endpoint), andevault(the W3ID identifier). All four call sites incorrectly passvault.enamefor both theenameandw3idparameters. Thew3idparameter should receivevault.evault, which is the actual W3ID used to resolve the eVault endpoint from the registry.Update lines 116, 320, and 333 to pass
vault.evaultas the third argument tocreateUserProfileInEVault(). Line 282-286 also has the same issue (resolveEndpoint should receivevault.evault).platforms/blabsy-w3ds-auth-api/src/web3adapter/watchers/firestoreWatcher.ts (1)
316-323: Remove debug logging.The excessive separator logging should be removed before merging to production. This will pollute logs and provides no value.
Apply this diff:
// If this is a message, fetch and attach the full chat details let enrichedData: DocumentData & { id: string; chat?: DocumentData } = { ...data, id: doc.id }; - console.log("----------------") - console.log("----------------") - console.log("----------------") - console.log("tableName", tableName) - console.log("----------------") - console.log("----------------") - console.log("----------------") - console.log("----------------") - if (tableName === "message" && data.chatId) {
♻️ Duplicate comments (4)
docker/Dockerfile.evault-core (1)
1-50: Same build reproducibility concern as pictique-api.This Dockerfile uses
--no-frozen-lockfileon lines 26 and 31, creating the same reproducibility concerns noted indocker/Dockerfile.pictique-api. The comments indicate this is a workaround for workspace packages not being included in the pruned lockfile.Refer to the review comment on
docker/Dockerfile.pictique-apifor suggested solutions. Consider establishing a consistent approach across all Dockerfiles in this PR to address the underlyingturbo prunelimitation.docker/Dockerfile.evoting-api (2)
36-36: Break the sequential build command into multiple statements for clarity.The same readability and debuggability concerns from the other Dockerfiles apply here.
Refer to the review comment on
docker/Dockerfile.blabsy-w3ds-auth-apiline 34 for the recommended approach.
29-29: Avoid--no-frozen-lockfilein production builds for reproducibility.The same reproducibility concerns from the other API Dockerfiles apply here. Using
--no-frozen-lockfileleads to non-deterministic builds.Refer to the review comment on
docker/Dockerfile.blabsy-w3ds-auth-apilines 27 and 32 for the recommended solutions.Also applies to: 34-34
docker/Dockerfile.group-charter-manager-api (1)
27-27: Avoid--no-frozen-lockfilein production builds for reproducibility.The same reproducibility concerns from
docker/Dockerfile.blabsy-w3ds-auth-apiapply here. Using--no-frozen-lockfileleads to non-deterministic builds.Refer to the review comment on
docker/Dockerfile.blabsy-w3ds-auth-apilines 27 and 32 for the recommended solutions.Also applies to: 32-32
🧹 Nitpick comments (37)
infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte (1)
1072-1074: Consider using an environment variable for the platform URL fallback.The hardcoded local network IP address (
192.168.0.235:7777) as a fallback is problematic for several reasons:
- It won't work outside of the specific local development environment
- It creates a silent failure mode if
signingData.platformUrlis missing in staging/production- The file already imports environment variables (e.g.,
PUBLIC_PROVISIONER_URL), suggesting this pattern is availableConsider refactoring to use an environment variable:
+import { PUBLIC_VOTING_PLATFORM_URL } from "$env/static/public"; + // ... // Extract platform URL from the data const platformUrl = signingData?.platformUrl || - "http://192.168.0.235:7777"; + PUBLIC_VOTING_PLATFORM_URL;Then add
PUBLIC_VOTING_PLATFORM_URLto your.envfiles for each environment.infrastructure/control-panel/src/lib/services/registry.ts (1)
65-90: Make fallback platform URLs environment-configurable to match baseUrl pattern.The fallback platform URLs use hard-coded private IPs (192.168.0.235) while the primary
PUBLIC_REGISTRY_URLis environment-configurable. This inconsistency limits portability—the code cannot be reused across dev/staging/prod environments without code changes.Recommended fix: Extract fallback host to an environment variable:
constructor() { this.baseUrl = env.PUBLIC_REGISTRY_URL || 'https://registry.staging.metastate.foundation'; this.fallbackHost = env.PUBLIC_FALLBACK_HOST || '192.168.0.235'; } private getFallbackPlatforms(): Platform[] { return [ { name: 'Blabsy', url: `http://${this.fallbackHost}:4444`, status: 'Active', uptime: '24h' }, { name: 'Pictique', url: `http://${this.fallbackHost}:1111`, status: 'Active', uptime: '24h' }, { name: 'Group Charter', url: `http://${this.fallbackHost}:5555`, status: 'Active', uptime: '24h' }, { name: 'Cerberus', url: `http://${this.fallbackHost}:6666`, status: 'Active', uptime: '24h' } ]; }This aligns configuration management across primary and fallback registry resolution.
platforms/blabsy/src/lib/firebase/utils.ts (1)
395-419: Consolidate duplicate DM lookup logic.This DM deduplication logic closely mirrors the existing
getOrCreateDirectChatfunction (lines 564-598). Consider refactoring to avoid duplication:
- Option 1: Have
createChatdelegate togetOrCreateDirectChatwhenisDMis true.- Option 2: Extract the common lookup logic into a shared helper function that both can use.
Additionally, the current implementation has a theoretical race condition—concurrent calls to
createChatfor the same two users could still create duplicate DMs between the check and the insert. If this is a concern under load, consider using a deterministic document ID (e.g., sorted participant IDs) or Firestore transactions.</parameter_end>
platforms/blabsy/src/components/user/user-follow-stats.tsx (2)
45-65: Consider extracting shared rendering logic to reduce duplication.The non-clickable stats rendering (lines 45-65) duplicates the structure and pluralization logic from the main render path (lines 74-96). Consider extracting a shared helper function for the stats display logic.
Example refactor:
const renderStat = (title: string, stats: number, index: number, isClickable: boolean) => { const displayTitle = index === 1 && stats > 1 ? `${title}s` : title; const content = ( <> <p className='font-bold text-light-primary dark:text-dark-primary'>{stats}</p> <p>{displayTitle}</p> </> ); return isClickable ? content : ( <div className='mt-0.5 mb-[3px] flex h-4 items-center gap-1 pointer-events-none'> {content} </div> ); };
77-80: Redundant CSS selectors.Both
[&>a>div]and[&>*>div]selectors are present. Since Next.jsLinkcomponents render as anchor tags, the[&>a>div]selector should be sufficient. The[&>*>div]selectors appear redundant unless this is transitional code for framework version compatibility.platforms/evoting-api/src/services/VoteService.ts (1)
697-706: Remove redundant type assertions.The type assertions on lines 697–698 are unnecessary. The
VoteDatainterface from the blindvote library (infrastructure/blindvote/src/core/types.ts) already correctly typescommitmentsandanchorsasRecord<string, Uint8Array>, so the casting is redundant and can be removed:// No assertions needed; use voteData directly for (const [optionId, commitment] of Object.entries(voteData.commitments)) { commitments[optionId] = Array.from(commitment).map(b => b.toString(16).padStart(2, '0')).join(''); } for (const [optionId, anchor] of Object.entries(voteData.anchors)) { anchors[optionId] = Array.from(anchor).map(b => b.toString(16).padStart(2, '0')).join(''); }infrastructure/eid-wallet/src/lib/global/controllers/evault.ts (2)
364-370: Consider using camelCase naming for getter methods.The methods
getclient()andgetendpoint()should follow TypeScript naming conventions and be renamed togetClient()andgetEndpoint()respectively.- getclient() { + getClient() { return this.#client; } - getendpoint() { + getEndpoint() { return this.#endpoint; }
25-26: Replace placeholder explanation in biome-ignore comment.The biome-ignore comment contains a placeholder
<explanation>instead of describing why theanytype is necessary for theparsedfield.- // biome-ignore lint/suspicious/noExplicitAny: <explanation> + // biome-ignore lint/suspicious/noExplicitAny: parsed can contain arbitrary JSON based on ontology schema parsed: any;docker/entrypoint.sh (1)
9-16: Consider dynamic directory creation to reduce maintenance burden.The script hardcodes six service-specific directory names. When new services are added to the platform, this script must be manually updated, which is error-prone and easy to forget.
Consider one of these approaches:
- Accept service names as environment variables
- Use a glob pattern or configuration file to determine which directories to create
- Allow services to create their own directories on startup (with proper error handling)
Example using environment variable:
-# Create SQLite mapping database directories if they don't exist -# This ensures directories exist even when volumes are mounted -mkdir -p /app/data/mapping-dbs/pictique -mkdir -p /app/data/mapping-dbs/evoting -mkdir -p /app/data/mapping-dbs/dreamsync -mkdir -p /app/data/mapping-dbs/cerberus -mkdir -p /app/data/mapping-dbs/group-charter -mkdir -p /app/data/mapping-dbs/blabsy +# Create SQLite mapping database directories if they don't exist +# SERVICE_NAMES should be a space-separated list of service names +for service in ${SERVICE_NAMES:-pictique evoting dreamsync cerberus group-charter blabsy}; do + mkdir -p "/app/data/mapping-dbs/$service" +doneplatforms/blabsy-w3ds-auth-api/dev.sh (1)
15-16: Consider safer signal handling.The trap command attempts to kill both processes without checking if they're still running, which could produce error messages. Consider using a more defensive approach.
Apply this diff:
-trap 'kill $TSC_PID $NODEMON_PID 2>/dev/null; exit' INT TERM +trap 'kill $TSC_PID $NODEMON_PID 2>/dev/null || true; exit' INT TERMOr even better, check process existence:
-trap 'kill $TSC_PID $NODEMON_PID 2>/dev/null; exit' INT TERM +cleanup() { + [ -n "$TSC_PID" ] && kill $TSC_PID 2>/dev/null || true + [ -n "$NODEMON_PID" ] && kill $NODEMON_PID 2>/dev/null || true + exit +} +trap cleanup INT TERMdocker/Dockerfile.eVoting (2)
7-8: Version specification inconsistency.The pnpm version uses an exact pin (
10.13.1) while turbo uses a caret range (^2). Consider using consistent versioning strategies. If reproducibility is important, pin both versions explicitly.Apply this diff for consistency:
-RUN npm install -g turbo@^2 +RUN npm install -g [email protected]Or verify the latest stable versions:
What are the latest stable versions of pnpm and turbo?
1-26: Consider adding non-root user for security.The container runs as root by default, which is a security risk. Consider adding a non-root user before the CMD instruction.
Add before the CMD instruction:
RUN addgroup -g 1001 -S nodejs && adduser -S nodejs -u 1001 RUN chown -R nodejs:nodejs /app USER nodejsplatforms/blabsy-w3ds-auth-api/src/controllers/WebhookController.ts (1)
167-189: Query may miss existing DMs if participant order differs.The query uses
array-containsonparticipants[0], but if the same two users create a DM from different initiating sides, the participant order might differ. The manual iteration (lines 170-189) checks both participants, but the initial query might not return the chat ifparticipants[0]is not in the existing chat's participants array.To ensure all existing DMs are found regardless of participant order, query both participants:
if (isDM) { // Query for existing chats with these participants - const existingChatsQuery = collection.where('participants', 'array-contains', participants[0]); + // Query for chats containing either participant + const existingChatsQuery1 = collection.where('participants', 'array-contains', participants[0]); + const existingChatsQuery2 = collection.where('participants', 'array-contains', participants[1]); + + const [snapshot1, snapshot2] = await Promise.all([ + existingChatsQuery1.get(), + existingChatsQuery2.get() + ]); + + // Combine results and deduplicate by document ID + const allDocs = new Map(); + [...snapshot1.docs, ...snapshot2.docs].forEach(doc => allDocs.set(doc.id, doc)); - const existingChatsSnapshot = await existingChatsQuery.get(); - - for (const doc of existingChatsSnapshot.docs) { + for (const doc of allDocs.values()) {However, note that Firestore doesn't support OR queries directly, so this approach runs two queries. Alternatively, use a composite key for DMs based on sorted participant IDs.
platforms/blabsy/src/components/modal/mobile-sidebar-modal.tsx (2)
139-170: Consider reducing code duplication in cover photo rendering.The cover photo rendering is duplicated between the Link and non-Link cases. The only difference is the wrapper element.
Extract the cover photo rendering into a reusable component or variable:
+ const coverPhotoContent = coverPhotoURL ? ( + <NextImage + useSkeleton + imgClassName='rounded-md' + src={coverPhotoURL} + alt={name} + layout='fill' + /> + ) : ( + <div className='h-full rounded-md bg-light-line-reply dark:bg-dark-line-reply' /> + ); + {username && userLink ? ( <Link href={userLink} className='blur-picture relative h-20 rounded-md' > - {coverPhotoURL ? ( - <NextImage - useSkeleton - imgClassName='rounded-md' - src={coverPhotoURL} - alt={name} - layout='fill' - /> - ) : ( - <div className='h-full rounded-md bg-light-line-reply dark:bg-dark-line-reply' /> - )} + {coverPhotoContent} </Link> ) : ( <div className='blur-picture relative h-20 rounded-md'> - {coverPhotoURL ? ( - <NextImage - useSkeleton - imgClassName='rounded-md' - src={coverPhotoURL} - alt={name} - layout='fill' - /> - ) : ( - <div className='h-full rounded-md bg-light-line-reply dark:bg-dark-line-reply' /> - )} + {coverPhotoContent} </div> )}
193-218: Stats rendering logic is correct but could be simplified.The conditional rendering of stats as either Links or divs is functionally correct. However, the repeated ternary logic could be extracted for clarity.
Consider simplifying with a wrapper component:
const StatItem = ({ id, label, stat, href }: { id: string; label: string; stat: number; href?: string }) => { const content = ( <> <p className='font-bold'>{stat}</p> <p className='text-light-secondary dark:text-dark-secondary'>{label}</p> </> ); if (!href) { return <div key={id} className='flex h-4 items-center gap-1 pointer-events-none'>{content}</div>; } return ( <Link href={href} key={id} className='hover-animation flex h-4 items-center gap-1 border-b border-b-transparent outline-none hover:border-b-light-primary focus-visible:border-b-light-primary dark:hover:border-b-dark-primary dark:focus-visible:border-b-dark-primary' > {content} </Link> ); }; // Then use it: <div className='text-secondary flex gap-4'> {allStats.map(([id, label, stat]) => ( <StatItem key={id} id={id} label={label} stat={stat} href={userLink ? `${userLink}/${id}` : undefined} /> ))} </div>docker/Dockerfile.dreamsync-api (1)
25-34: Using --no-frozen-lockfile reduces build reproducibility.Similar to other Dockerfiles in this PR, using
--no-frozen-lockfilesacrifices build reproducibility. This is a cross-cutting concern affecting multiple services.Consider addressing this at the monorepo level to ensure all service Dockerfiles use frozen lockfiles. See the comment on
docker/Dockerfile.cerberusfor detailed solutions.If this is a temporary workaround, please add a TODO comment and track it in an issue to ensure this gets addressed before production deployment.
# First install the dependencies (as they change less often) -# Use --no-frozen-lockfile because w3id dependencies aren't in the pruned lockfile +# TODO(#issue-number): Fix turbo prune to include workspace deps, then use --frozen-lockfile +# Use --no-frozen-lockfile as temporary workaround COPY --from=prepare /app/out/json/ . RUN pnpm install --no-frozen-lockfileplatforms/pictique-api/src/services/UserService.ts (1)
41-61: Input validation is optional defensive programming; database indexing cannot be verified from current codebase inspection.The
findByEnamemethod implementation is functionally correct. However, the review's suggestions are reasonable enhancements:
Input validation: While not strictly necessary if callers always provide valid ename values, adding validation aligns with the codebase pattern (see
searchUsersmethod which validates query length). The suggested check for empty/whitespace strings is defensive practice.Database indexing: The
enamecolumn in the User entity has no visible@Index()decorator. The OR query is valid but indexes would improve performance. However, migration files were not found in the codebase to verify if indexes are created at the database level.The method currently handles the intended purpose of normalizing ename with/without @ prefix and querying both formats correctly.
platforms/blabsy/src/components/user/user-nav-link.tsx (1)
16-28: Remove unnecessaryhover-animationclass from non-clickable variant.The non-clickable fallback correctly uses
pointer-events-noneto disable interaction, but thehover-animationclass on line 19 is unnecessary and could cause confusion. When an element is non-interactive, it shouldn't have hover-related classes.Apply this diff to remove the unnecessary class:
- <div className='hover-animation main-tab dark-bg-tab flex flex-1 justify-center pointer-events-none'> + <div className='main-tab dark-bg-tab flex flex-1 justify-center pointer-events-none'>tests/vitest.config.ts (1)
10-22: Consider enablinglogHeapUsagefor load testing.The 10-minute test timeout and 5-minute hook timeout are appropriate for the load tests mentioned in the PR objectives (~50 active users). The
singleFork: trueconfiguration correctly prevents test interference.However,
logHeapUsage: falseon line 20 may hinder debugging memory-related issues during load tests. Memory consumption is typically a critical concern when testing under load.Apply this diff to enable heap usage logging:
- logHeapUsage: false, + logHeapUsage: true,docker/Dockerfile.registry (3)
7-7: Consider extracting repeated corepack setup into a reusable stage.The
corepack enable && corepack prepare [email protected] --activatecommand is repeated in the prepare, builder, and runner stages. While this duplication ensures each stage is self-contained, you could reduce verbosity by creating a dedicated intermediate stage or using a script.Example approach:
FROM node:18-alpine AS base RUN apk update && apk add --no-cache libc6-compat +RUN corepack enable && corepack prepare [email protected] --activate WORKDIR /app # --- FROM base AS prepare -RUN corepack enable && corepack prepare [email protected] --activate RUN npm install -g turbo@^2Also applies to: 15-15, 26-26
8-8: Pin the exact turbo version for reproducible builds.Using
turbo@^2allows minor and patch version updates, which may introduce unexpected behavior or breaking changes.Apply this diff to pin the version:
-RUN npm install -g turbo@^2 +RUN npm install -g [email protected]
25-37: Consider running the container as a non-root user.The runner stage does not specify a USER directive, so the container runs as root by default. This increases the attack surface if the container is compromised.
Add a non-root user before the CMD:
WORKDIR /app/platforms/registry +RUN addgroup -g 1001 -S nodejs && adduser -S nodejs -u 1001 +RUN chown -R nodejs:nodejs /app +USER nodejs EXPOSE 4321 CMD ["pnpm", "start"]docker/Dockerfile.blabsy-w3ds-auth-api (1)
34-34: Break the sequential build command into multiple RUN statements for clarity.The single-line build command with multiple
&&operators is difficult to read and debug. Build failures won't clearly indicate which package failed.Apply this diff:
-RUN pnpm turbo build --filter=w3id && pnpm turbo build --filter=evault-core && pnpm turbo build --filter=web3-adapter && pnpm turbo build --filter=blabsy-w3ds-auth-api +# Build workspace dependencies in order +RUN pnpm turbo build --filter=w3id +RUN pnpm turbo build --filter=evault-core +RUN pnpm turbo build --filter=web3-adapter +RUN pnpm turbo build --filter=blabsy-w3ds-auth-apiAlternatively, use a single turbo command with multiple filters:
-RUN pnpm turbo build --filter=w3id && pnpm turbo build --filter=evault-core && pnpm turbo build --filter=web3-adapter && pnpm turbo build --filter=blabsy-w3ds-auth-api +RUN pnpm turbo build --filter=w3id --filter=evault-core --filter=web3-adapter --filter=blabsy-w3ds-auth-apitests/src/utils/user-cache.ts (2)
16-32: Add validation for cached TestUser objects.While the function validates that
cached.usersis an array with length > 0, it doesn't verify that each element conforms to the TestUser interface. Corrupted cache files or schema changes could cause downstream errors.Add type validation for cache entries:
export function loadCachedUsers(): CachedUsers | null { try { if (fs.existsSync(CACHE_FILE)) { const cacheData = fs.readFileSync(CACHE_FILE, 'utf-8'); const cached: CachedUsers = JSON.parse(cacheData); - // Validate cache structure - if (cached.users && Array.isArray(cached.users) && cached.users.length > 0) { + // Validate cache structure and user objects + if (cached.users && Array.isArray(cached.users) && cached.users.length > 0) { + // Validate that each user has required fields + const isValid = cached.users.every(user => + user.id && user.ename && user.email && user.username && user.name && user.firebaseUid + ); + if (!isValid) { + console.warn('Cache contains invalid user objects, ignoring cache'); + return null; + } return cached; } } } catch (error) { console.warn('Failed to load user cache:', error); } return null; }
5-5: Consider ESM compatibility for__dirnameusage.If this codebase migrates to ES modules,
__dirnamewill not be available. While it works fine for CommonJS, consider future-proofing.For ESM compatibility, you could use:
import { fileURLToPath } from 'url'; import { dirname } from 'path'; const __filename = fileURLToPath(import.meta.url); const __dirname = dirname(__filename);Or define the cache path relative to
process.cwd():-const CACHE_FILE = path.join(__dirname, '../../.test-users-cache.json'); +const CACHE_FILE = path.join(process.cwd(), 'tests/.test-users-cache.json');tests/src/utils/user-factory.ts (2)
35-49: Simplify path resolution for credentials.The path resolution on line 38 uses
..to navigate fromprocess.cwd()(assumed to betests/) to the project root. This is fragile and depends on where the tests are executed from.Use a more robust approach:
if (config.googleApplicationCredentials) { - // Resolve path relative to project root (where .env file is) - // When running from staging-load-tests directory, go up one level to project root - const credentialsPath = path.resolve(process.cwd(), '..', config.googleApplicationCredentials); + // Resolve credentials path - support both relative and absolute paths + const credentialsPath = path.isAbsolute(config.googleApplicationCredentials) + ? config.googleApplicationCredentials + : path.resolve(process.cwd(), config.googleApplicationCredentials); // Set environment variable for Firebase Admin SDK process.env.GOOGLE_APPLICATION_CREDENTIALS = credentialsPath;This way, if
GOOGLE_APPLICATION_CREDENTIALSis already an absolute path, it's used directly. If relative, it's resolved from the current working directory without assuming the directory structure.
44-45: Consider consistent import style across the module.The module uses
importstatements at the top but falls back torequire()on lines 44-45 and 256-257. While this works, it creates an inconsistent style.For line 44-45, you could use a top-level import with conditional logic:
+import type { ServiceAccount } from 'firebase-admin'; + export function initializeFirebase(): void { // ... if (config.googleApplicationCredentials) { // ... if (fs.existsSync(credentialsPath)) { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const serviceAccount = require(credentialsPath); + const serviceAccount: ServiceAccount = JSON.parse(fs.readFileSync(credentialsPath, 'utf-8')); credential = cert(serviceAccount);For line 256-257, the dynamic require is already imported at the top (line 9), so you can remove the redundant require:
export async function waitForUserSync(ename: string, maxWaitMs: number = 60000): Promise<boolean> { - // eslint-disable-next-line @typescript-eslint/no-require-imports - const axios = require('axios'); const startTime = Date.now();Also applies to: 256-257
neo4j-compose.yaml (1)
8-9: Consider using a stronger default password.The default password is set to
neo4j, which is a well-known default. For development environments, consider using a more secure default or at minimum document that users should change this in production.Consider updating the default:
environment: - - NEO4J_AUTH=${NEO4J_USER:-neo4j}/${NEO4J_PASSWORD:-neo4j} + - NEO4J_AUTH=${NEO4J_USER:-neo4j}/${NEO4J_PASSWORD:-changeme123}And document in the README that users should set
NEO4J_PASSWORDenvironment variable for production use.platforms/pictique-api/src/controllers/AuthController.ts (1)
76-78: Improve error response for better debugging.The generic "User not found" error makes it difficult to distinguish between:
- User never created via webhook
- eName normalization mismatch
- Database lookup failure
Consider including the normalized eName in the error response for better debugging.
Apply this diff:
if (!user) { - throw new Error("User not found"); + return res.status(404).json({ + error: "User not found", + message: "User must be created via eVault webhook before authentication", + ename: ename + }); }tests/src/populators/blabsy/messages.ts (1)
3-8: Consider consolidating CreatedMessage type definition.The
CreatedMessageinterface is defined in multiple locations:
tests/src/populators/blabsy/messages.ts(this file)tests/src/populators/pictique/messages.tstests/src/factories/test-social-user.tsThis duplication could lead to inconsistencies if the interface evolves. Consider defining it once in a shared location (e.g.,
tests/src/types/messages.ts) and importing it across all populators.#!/bin/bash # Description: Find all CreatedMessage definitions to verify duplication rg -n --type=ts 'interface CreatedMessage' tests/src/ -A5docker/Dockerfile.pictique (1)
21-21: Consider separating dev and production Dockerfiles.Using
pnpm install --frozen-lockfileand runningpnpm devsuggests this Dockerfile is for development. For production, you would typically want to:
- Run
pnpm install --prod --frozen-lockfile- Build the application (
pnpm build)- Serve the built assets with a production server
Consider creating separate Dockerfiles or using build args to differentiate dev and prod configurations.
platforms/evoting-api/src/services/UserService.ts (1)
24-46: SimplifyfindByEnameby removing the redundant OR condition.Evidence confirms that enames are consistently stored without the
@prefix. ThecreateBlankUsermethod (line 56) strips@before storing, andgetUserByEname(line 17) also strips@before lookup. This means theenameWithAtbranch in the OR query will never match any records in the database.The method can be simplified to match the pattern of
getUserByEname:async findByEname(ename: string): Promise<User | null> { - // Normalize the input: remove @ if present for comparison - const normalizedEname = ename.startsWith('@') ? ename.slice(1) : ename; - const enameWithAt = `@${normalizedEname}`; - - // Search for user where ename matches either with or without @ - const user = await this.userRepository - .createQueryBuilder("user") - .leftJoinAndSelect("user.polls", "polls") - .leftJoinAndSelect("user.votes", "votes") - .where("user.ename = :enameWithAt OR user.ename = :enameWithoutAt", { - enameWithAt, - enameWithoutAt: normalizedEname, - }) - .getOne(); - - return user; + // Strip @ prefix if present for database lookup + const cleanEname = this.stripEnamePrefix(ename); + return this.userRepository.findOne({ + where: { ename: cleanEname }, + relations: ["polls", "votes"], + }); }This removes dead code, improves clarity, and aligns with the existing
getUserByEnamepattern (which can be simplified similarly, or this method can consolidate the logic).platforms/blabsy/src/components/user/user-avatar.tsx (1)
22-55: Good refactor with image extraction and conditional interactivity.The extracted
imageconstant reduces duplication, and the conditional rendering properly handles missing usernames. Thepointer-events-nonestyling prevents interaction when the avatar shouldn't be clickable.Optional:
tabIndex={0}on Line 51 is redundant since Link elements are natively focusable. You may remove it:<Link href={`/user/${username}`} className={cn('blur-picture flex self-start', className)} - tabIndex={0} >README.md (1)
70-93: Add a language hint to this fenced block.
markdownlint(MD040) flags the project structure fence because it lacks a language spec. Please tag it with something like ```text so docs lint continues to pass.tests/src/populators/pictique/chats.ts (1)
3-7: Consolidate theCreatedChattype.
This interface is identical to the ones intests/src/factories/test-social-user.tsand the Blabsy populator. Please extract a single shared definition (e.g.,tests/src/types/chats.ts) and import it everywhere to keep the shape in sync. Based on learningsplatforms/blabsy-w3ds-auth-api/src/web3adapter/watchers/firestoreWatcher.ts (2)
29-34: Consider extracting magic numbers to configuration.The health monitoring constants (60000ms, 120000ms) are reasonable but could benefit from being configurable or at least documented with their rationale.
Example:
- private readonly healthCheckIntervalMs = 60000; // 1 minute - private readonly maxTimeWithoutSnapshot = 120000; // 2 minutes - if no snapshot in 2 min, reconnect + private readonly healthCheckIntervalMs = Number(process.env.FIRESTORE_HEALTH_CHECK_INTERVAL_MS) || 60000; // 1 minute + private readonly maxTimeWithoutSnapshot = Number(process.env.FIRESTORE_MAX_TIME_WITHOUT_SNAPSHOT_MS) || 120000; // 2 minutes
233-300: LGTM! Parallel processing is well-implemented.The parallel processing with Promise.all provides good performance while maintaining proper error isolation between documents. The try-finally pattern ensures cleanup happens reliably.
Minor optimization: Line 293's
this.processingIds.delete(docId)is redundant since the finally block (line 276) already handles it:} catch (error) { console.error( `Error processing ${change.type} for document ${docId}:`, error ); - // Remove from processing IDs on error - this.processingIds.delete(docId); // Continue processing other changes even if one fails }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (91)
README.md(1 hunks)dev-docker-compose.README.md(0 hunks)dev-docker-compose.yaml(18 hunks)docker/Dockerfile.blabsy(1 hunks)docker/Dockerfile.blabsy-w3ds-auth-api(1 hunks)docker/Dockerfile.cerberus(1 hunks)docker/Dockerfile.dreamsync-api(1 hunks)docker/Dockerfile.eVoting(1 hunks)docker/Dockerfile.ereputation(1 hunks)docker/Dockerfile.evault-core(1 hunks)docker/Dockerfile.evoting-api(1 hunks)docker/Dockerfile.group-charter-manager(1 hunks)docker/Dockerfile.group-charter-manager-api(1 hunks)docker/Dockerfile.marketplace(1 hunks)docker/Dockerfile.pictique(1 hunks)docker/Dockerfile.pictique-api(1 hunks)docker/Dockerfile.registry(1 hunks)docker/entrypoint.sh(1 hunks)infrastructure/control-panel/src/lib/services/registry.ts(1 hunks)infrastructure/eid-wallet/src/lib/global/controllers/evault.ts(2 hunks)infrastructure/eid-wallet/src/routes/(app)/scan-qr/+page.svelte(1 hunks)infrastructure/evault-core/package.json(2 hunks)infrastructure/evault-core/src/core/protocol/graphql-server.ts(1 hunks)infrastructure/evault-core/src/services/ProvisioningService.ts(4 hunks)infrastructure/web3-adapter/src/db/mapping.db.ts(2 hunks)neo4j-compose.yaml(1 hunks)package.json(1 hunks)platforms/blabsy-w3ds-auth-api/dev.sh(1 hunks)platforms/blabsy-w3ds-auth-api/src/controllers/WebhookController.ts(1 hunks)platforms/blabsy-w3ds-auth-api/src/index.ts(1 hunks)platforms/blabsy-w3ds-auth-api/src/web3adapter/watchers/firestoreWatcher.ts(9 hunks)platforms/blabsy/src/components/aside/aside-trends.tsx(1 hunks)platforms/blabsy/src/components/aside/suggestions.tsx(1 hunks)platforms/blabsy/src/components/input/input.tsx(2 hunks)platforms/blabsy/src/components/modal/mobile-sidebar-modal.tsx(5 hunks)platforms/blabsy/src/components/sidebar/menu-link.tsx(1 hunks)platforms/blabsy/src/components/sidebar/mobile-sidebar-link.tsx(1 hunks)platforms/blabsy/src/components/sidebar/sidebar-link.tsx(1 hunks)platforms/blabsy/src/components/sidebar/sidebar.tsx(1 hunks)platforms/blabsy/src/components/tweet/tweet-date.tsx(1 hunks)platforms/blabsy/src/components/tweet/tweet-share.tsx(1 hunks)platforms/blabsy/src/components/tweet/tweet.tsx(2 hunks)platforms/blabsy/src/components/user/user-avatar.tsx(1 hunks)platforms/blabsy/src/components/user/user-card.tsx(2 hunks)platforms/blabsy/src/components/user/user-details.tsx(1 hunks)platforms/blabsy/src/components/user/user-follow-stats.tsx(2 hunks)platforms/blabsy/src/components/user/user-name.tsx(1 hunks)platforms/blabsy/src/components/user/user-nav-link.tsx(1 hunks)platforms/blabsy/src/components/user/user-tooltip.tsx(3 hunks)platforms/blabsy/src/components/user/user-username.tsx(2 hunks)platforms/blabsy/src/components/view/view-tweet.tsx(1 hunks)platforms/blabsy/src/lib/firebase/utils.ts(1 hunks)platforms/dreamsync-api/src/controllers/AuthController.ts(1 hunks)platforms/dreamsync-api/src/services/UserService.ts(2 hunks)platforms/eReputation/vite.config.ts(2 hunks)platforms/evoting-api/src/controllers/AuthController.ts(1 hunks)platforms/evoting-api/src/services/UserService.ts(2 hunks)platforms/evoting-api/src/services/VoteService.ts(2 hunks)platforms/group-charter-manager-api/src/services/UserService.ts(1 hunks)platforms/marketplace/vite.config.ts(2 hunks)platforms/pictique-api/src/controllers/AuthController.ts(2 hunks)platforms/pictique-api/src/controllers/WebhookController.ts(5 hunks)platforms/pictique-api/src/services/UserService.ts(1 hunks)pnpm-workspace.yaml(1 hunks)tests/.gitignore(1 hunks)tests/README.md(1 hunks)tests/package.json(1 hunks)tests/src/config/env.ts(1 hunks)tests/src/factories/index.ts(1 hunks)tests/src/factories/platform.enum.ts(1 hunks)tests/src/factories/test-social-user-factory.ts(1 hunks)tests/src/factories/test-social-user.ts(1 hunks)tests/src/populators/blabsy/chats.ts(1 hunks)tests/src/populators/blabsy/comments.ts(1 hunks)tests/src/populators/blabsy/likes.ts(1 hunks)tests/src/populators/blabsy/messages.ts(1 hunks)tests/src/populators/blabsy/posts.ts(1 hunks)tests/src/populators/pictique/chats.ts(1 hunks)tests/src/populators/pictique/comments.ts(1 hunks)tests/src/populators/pictique/likes.ts(1 hunks)tests/src/populators/pictique/messages.ts(1 hunks)tests/src/populators/pictique/posts.ts(1 hunks)tests/src/sync-verification.test.ts(1 hunks)tests/src/utils/api-client.ts(1 hunks)tests/src/utils/data-comparator.ts(1 hunks)tests/src/utils/firebase-client.ts(1 hunks)tests/src/utils/sync-verifier.ts(1 hunks)tests/src/utils/user-cache.ts(1 hunks)tests/src/utils/user-factory.ts(1 hunks)tests/tsconfig.json(1 hunks)tests/vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- dev-docker-compose.README.md
🧰 Additional context used
🧠 Learnings (1)
📚 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:
README.md
🧬 Code graph analysis (26)
infrastructure/web3-adapter/src/db/mapping.db.ts (1)
infrastructure/eid-wallet/vitest.workspace.js (1)
dirname(8-11)
tests/src/factories/test-social-user-factory.ts (1)
tests/src/factories/test-social-user.ts (1)
TestSocialUser(53-327)
tests/src/populators/pictique/likes.ts (2)
tests/src/factories/test-social-user.ts (2)
CreatedLike(34-38)createLike(145-162)tests/src/populators/blabsy/likes.ts (2)
CreatedLike(3-7)createLike(12-20)
platforms/blabsy/src/components/sidebar/sidebar.tsx (2)
platforms/blabsy/src/components/ui/custom-icon.tsx (1)
CustomIcon(25-29)platforms/blabsy/src/components/sidebar/sidebar-link.tsx (1)
SidebarLink(12-56)
tests/src/populators/pictique/comments.ts (3)
tests/src/factories/test-social-user.ts (2)
CreatedComment(27-32)createComment(121-140)tests/src/populators/blabsy/comments.ts (2)
CreatedComment(3-8)createComment(13-26)tests/src/utils/api-client.ts (1)
createComment(258-277)
tests/src/populators/blabsy/chats.ts (3)
tests/src/factories/test-social-user.ts (2)
CreatedChat(40-44)createChat(168-185)tests/src/populators/pictique/chats.ts (2)
CreatedChat(3-7)createChat(13-35)tests/src/utils/firebase-client.ts (1)
createChat(192-241)
tests/src/populators/blabsy/posts.ts (3)
tests/src/factories/test-social-user.ts (2)
CreatedPost(21-25)createPost(99-116)tests/src/populators/pictique/posts.ts (2)
CreatedPost(3-7)createPost(12-20)tests/src/utils/firebase-client.ts (1)
createTweet(44-88)
tests/src/populators/pictique/messages.ts (2)
tests/src/factories/test-social-user.ts (2)
CreatedMessage(46-51)createMessage(190-197)tests/src/populators/blabsy/messages.ts (2)
CreatedMessage(3-8)createMessage(13-26)
tests/src/populators/pictique/posts.ts (3)
tests/src/factories/test-social-user.ts (2)
CreatedPost(21-25)createPost(99-116)tests/src/populators/blabsy/posts.ts (2)
CreatedPost(3-7)createPost(12-20)tests/src/utils/api-client.ts (1)
createPost(217-236)
tests/src/populators/pictique/chats.ts (2)
tests/src/factories/test-social-user.ts (2)
CreatedChat(40-44)createChat(168-185)tests/src/utils/api-client.ts (2)
createChat(295-314)searchUsers(181-187)
platforms/blabsy/src/components/user/user-card.tsx (5)
platforms/blabsy/src/components/ui/follow-button.tsx (1)
FollowButton(14-72)platforms/blabsy/src/components/user/user-tooltip.tsx (1)
UserTooltip(32-144)platforms/blabsy/src/components/user/user-avatar.tsx (1)
UserAvatar(13-56)platforms/blabsy/src/components/user/user-name.tsx (1)
UserName(14-67)platforms/blabsy/src/components/user/user-username.tsx (1)
UserUsername(10-42)
tests/src/utils/user-cache.ts (1)
tests/src/utils/user-factory.ts (1)
TestUser(14-21)
platforms/pictique-api/src/controllers/AuthController.ts (1)
platforms/pictique-api/src/utils/jwt.ts (1)
signToken(5-7)
tests/src/populators/blabsy/messages.ts (4)
tests/src/factories/test-social-user.ts (2)
CreatedMessage(46-51)createMessage(190-197)tests/src/factories/index.ts (1)
CreatedMessage(9-9)tests/src/populators/pictique/messages.ts (2)
CreatedMessage(3-8)createMessage(13-27)tests/src/utils/firebase-client.ts (1)
sendMessage(246-277)
tests/src/utils/user-factory.ts (2)
tests/src/config/env.ts (1)
config(32-45)tests/src/utils/user-cache.ts (3)
isCacheValid(69-77)getCachedUsers(82-95)saveCachedUsers(37-50)
tests/src/populators/blabsy/comments.ts (2)
tests/src/factories/test-social-user.ts (2)
CreatedComment(27-32)createComment(121-140)tests/src/utils/firebase-client.ts (1)
createReply(138-187)
tests/src/populators/blabsy/likes.ts (3)
tests/src/factories/test-social-user.ts (2)
CreatedLike(34-38)createLike(145-162)tests/src/populators/pictique/likes.ts (2)
CreatedLike(3-7)createLike(12-20)tests/src/utils/firebase-client.ts (1)
toggleLike(93-133)
tests/src/factories/test-social-user.ts (12)
tests/src/populators/pictique/posts.ts (1)
CreatedPost(3-7)tests/src/populators/blabsy/posts.ts (1)
CreatedPost(3-7)tests/src/populators/blabsy/comments.ts (1)
CreatedComment(3-8)tests/src/populators/pictique/comments.ts (1)
CreatedComment(3-8)tests/src/populators/blabsy/likes.ts (1)
CreatedLike(3-7)tests/src/populators/pictique/likes.ts (1)
CreatedLike(3-7)tests/src/populators/blabsy/chats.ts (1)
CreatedChat(3-7)tests/src/populators/pictique/chats.ts (1)
CreatedChat(3-7)tests/src/populators/blabsy/messages.ts (1)
CreatedMessage(3-8)tests/src/populators/pictique/messages.ts (1)
CreatedMessage(3-8)tests/src/utils/api-client.ts (6)
getAuthToken(45-150)getChat(332-340)getApiClient(15-26)getUserChats(319-327)getChatMessages(368-376)getPostComments(282-290)tests/src/utils/user-factory.ts (1)
initializeFirebase(26-61)
platforms/blabsy/src/components/tweet/tweet.tsx (8)
platforms/blabsy/src/components/user/user-tooltip.tsx (1)
UserTooltip(32-144)platforms/blabsy/src/components/user/user-avatar.tsx (1)
UserAvatar(13-56)platforms/blabsy/src/components/user/user-name.tsx (1)
UserName(14-67)platforms/blabsy/src/components/user/user-username.tsx (1)
UserUsername(10-42)platforms/blabsy/src/components/tweet/tweet-date.tsx (1)
TweetDate(12-39)platforms/blabsy/src/components/tweet/tweet-actions.tsx (1)
TweetActions(67-304)platforms/blabsy/src/components/input/image-preview.tsx (1)
ImagePreview(43-224)platforms/blabsy/src/components/tweet/tweet-stats.tsx (1)
TweetStats(23-163)
tests/src/utils/firebase-client.ts (3)
tests/src/utils/user-factory.ts (1)
initializeFirebase(26-61)platforms/blabsy/src/lib/firebase/utils.ts (3)
updateUserData(53-62)createChat(389-440)sendMessage(442-480)tests/src/utils/api-client.ts (5)
toggleLike(241-253)createChat(295-314)sendMessage(345-363)getChat(332-340)getChatMessages(368-376)
platforms/blabsy-w3ds-auth-api/src/web3adapter/watchers/firestoreWatcher.ts (1)
platforms/blabsy-w3ds-auth-api/src/controllers/WebhookController.ts (1)
adapter(73-78)
platforms/blabsy/src/components/modal/mobile-sidebar-modal.tsx (1)
platforms/blabsy/src/components/sidebar/mobile-sidebar-link.tsx (1)
MobileSidebarLink(11-38)
tests/src/sync-verification.test.ts (5)
tests/src/config/env.ts (1)
config(32-45)tests/src/utils/user-factory.ts (2)
TestUser(14-21)createTestUsers(177-240)tests/src/factories/test-social-user.ts (1)
TestSocialUser(53-327)tests/src/utils/user-cache.ts (2)
clearUserCache(55-64)isCacheValid(69-77)tests/src/factories/test-social-user-factory.ts (1)
TestSocialUserFactory(4-33)
tests/src/utils/api-client.ts (4)
tests/src/config/env.ts (1)
config(32-45)tests/src/factories/test-social-user.ts (3)
createPost(99-116)createComment(121-140)createChat(168-185)tests/src/populators/pictique/posts.ts (1)
createPost(12-20)tests/src/populators/pictique/chats.ts (1)
createChat(13-35)
platforms/blabsy/src/components/input/input.tsx (1)
platforms/blabsy/src/components/input/input-form.tsx (1)
fromTop(52-52)
tests/src/utils/sync-verifier.ts (5)
tests/src/config/env.ts (1)
config(32-45)platforms/dreamsync-api/src/services/UserService.ts (1)
searchUsers(70-77)tests/src/utils/api-client.ts (6)
searchUsers(181-187)getApiClient(15-26)getPostComments(282-290)getPost(381-400)getChatMessages(368-376)getChat(332-340)tests/src/utils/user-factory.ts (1)
initializeFirebase(26-61)tests/src/utils/firebase-client.ts (3)
getTweet(294-301)getChatMessages(318-329)getChat(306-313)
🪛 GitHub Actions: Tests [evault-core + web3-adapter Integration]
infrastructure/evault-core/src/services/ProvisioningService.ts
[error] 162-162: Provisioning failed: AxiosError: Request failed with status code 400. Response data: {"error":"Missing required fields"}. POST to http://localhost:4322/register during eVault provisioning.
🪛 markdownlint-cli2 (0.18.1)
README.md
70-70: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
Description of change
Issue Number
closes #396
closes #393
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
Documentation
New Features
Bug Fixes
Chores