refactor:control-panel #469
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughReplaces pod/kubectl discovery with registry-driven EVault discovery, adds Loki logging and env entries, introduces DB init and token helper scripts, rewrites control-panel APIs/UI to use evaultId/registry data, and updates dev tooling and configs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as Control Panel UI
participant API as Control-Panel API
participant Registry as RegistryService
participant Health as eVault /health
participant Loki as LokiService
participant EVault as eVault Instance
User->>UI: Request eVault list
UI->>API: GET /api/evaults
API->>Registry: getEVaults()
Registry-->>API: RegistryVault[]
loop per evault
API->>Health: GET {evault.uri}/health (2s timeout)
Health-->>API: Healthy / Unhealthy / Unreachable
end
API-->>UI: EVault[] (id, name, ename, uri, status)
User->>UI: Open details for evaultId
UI->>API: GET /api/evaults/{evaultId}/details
API->>Registry: getEVaults() -> find evault
API-->>UI: details (ename, evault, uri, healthStatus)
User->>UI: Request logs for evaultId
UI->>API: GET /api/evaults/{evaultId}/logs?tail=100
API->>Registry: getEVaults() -> find evault
API->>Loki: getEVaultLogs(evaultId, ename, limit, start, end)
Loki->>Loki: run query patterns, dedupe, sort, limit
Loki-->>API: logs[]
API-->>UI: logs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
| // Return fallback platforms if registry is unavailable | ||
| return [ | ||
| { | ||
| name: "Blabsy", | ||
| url: "http://192.168.0.235:4444", | ||
| status: "Active", | ||
| uptime: "24h", | ||
| }, | ||
| { | ||
| name: "Pictique", | ||
| url: "http://192.168.0.235:1111", | ||
| status: "Active", | ||
| uptime: "24h", | ||
| }, | ||
| { | ||
| name: "Group Charter", | ||
| url: "http://192.168.0.235:5555", | ||
| status: "Active", | ||
| uptime: "24h", | ||
| }, | ||
| { | ||
| name: "Cerberus", | ||
| url: "http://192.168.0.235:6666", | ||
| status: "Active", | ||
| uptime: "24h", | ||
| }, | ||
| ]; | ||
| } |
There was a problem hiding this comment.
Bad, coz these URL's are 99.9% wrong. We should just fail.
There was a problem hiding this comment.
Ya I saw it as the platform loaded these for me and i was like wtf, but I have just preserved them cuz they were already there and i have no idea why @coodos had them :P
But I wouldnt mind handling it separately outside this pr's scope
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte (1)
201-212: Remove or conditionally gate debug logs containing sensitive data before production release.Lines 201 and 212 contain unconditional console.log statements that will execute in all environments, including production. Line 212 specifically logs
provisionRes.data, which contains security-sensitive identity tokens (uri, w3id) and should never be exposed to browser console in production. Line 201 logsregistryEntropy, which is a security token.The vite.config.js has no log stripping or environment-based filtering configured, meaning these logs will persist in production builds.
Option 1: Remove the logs entirely
const registryEntropy = entropyRes.data.token; - console.log("Registry entropy:", registryEntropy); const provisionRes = await axios.post( new URL("/provision", PUBLIC_PROVISIONER_URL).toString(), @@ -210,7 +209,6 @@ publicKey: await getApplicationPublicKey(), }, ); - console.log("Provision response:", provisionRes.data); if (!provisionRes.data?.success) {Option 2: Conditionally gate on debug flag
+ const DEBUG = false; // or read from environment const registryEntropy = entropyRes.data.token; - console.log("Registry entropy:", registryEntropy); + if (DEBUG) console.log("Registry entropy:", registryEntropy); const provisionRes = await axios.post( new URL("/provision", PUBLIC_PROVISIONER_URL).toString(), @@ -210,7 +208,7 @@ publicKey: await getApplicationPublicKey(), }, ); - console.log("Provision response:", provisionRes.data); + if (DEBUG) console.log("Provision response:", provisionRes.data); if (!provisionRes.data?.success) {platforms/registry/src/index.ts (1)
34-39: Fix CORS misconfiguration:origin: "*"withcredentials: true.Browsers block requests when CORS is configured with both
origin: "*"andcredentials: true. This combination is invalid per the CORS specification and will cause authentication/cookie-based requests to fail.Apply this diff to fix the configuration:
server.register(cors, { - origin: "*", + origin: process.env.ALLOWED_ORIGINS?.split(",") || ["http://localhost:3000"], methods: ["GET", "POST", "OPTIONS"], allowedHeaders: ["Content-Type", "Authorization"], credentials: true, });Alternatively, if you don't need credentials, remove the
credentialsoption:server.register(cors, { origin: "*", methods: ["GET", "POST", "OPTIONS"], allowedHeaders: ["Content-Type", "Authorization"], - credentials: true, });infrastructure/control-panel/src/routes/+page.svelte (3)
200-206: Bug:evaultIdproperty does not exist onEVaulttype.The
EVaultinterface definesid,evault, andenamefields, butevaultIdis not a valid property. This will cause the sessionStorage to storeundefinedvalues.Apply this diff to fix the property reference:
// Store selections immediately in sessionStorage const selectedEVaultData = selectedEVaults - .map((id) => evaults.find((e) => e.evaultId === id)) + .map((id) => evaults.find((e) => (e.evault || e.ename || e.id) === id)) .filter(Boolean); sessionStorage.setItem('selectedEVaults', JSON.stringify(selectedEVaultData));
249-256: Bug: SameevaultIdproperty issue ingoToMonitoring.Consistent with the previous issue, this references the non-existent
evaultIdproperty.Apply this diff:
function goToMonitoring() { const selectedEVaultData = selectedEVaults - .map((id) => evaults.find((e) => e.evaultId === id)) + .map((id) => evaults.find((e) => (e.evault || e.ename || e.id) === id)) .filter(Boolean);
286-311: Dead code:mappedDatauses obsolete EVault fields.This mapping block references
evault.ageandevault.ip, which no longer exist in theEVaultinterface. ThemappedDatavariable appears to be unused—the table usesmappedEVaultsData()instead.Consider removing this dead code block entirely, or update it to use the correct fields if it's needed elsewhere:
try { - const mapped = evaults.map((evault) => { - const mapped = { - eName: { - type: 'text', - value: evault.evault || evault.ename || evault.id, - className: - 'cursor-pointer text-blue-600 hover:text-blue-800 hover:underline' - }, - Uptime: { - type: 'text', - value: evault.age - }, - IP: { - type: 'text', - value: evault.ip - }, - URI: { - type: 'link', - value: evault.serviceUrl || 'N/A', - link: evault.serviceUrl || '#', - external: true - } - }; - return mapped; - }); - - mappedData = mapped; - console.log('✅ Data mapped successfully:', mappedData.length, 'items'); + console.log('✅ eVaults loaded successfully:', evaults.length, 'items');
🧹 Nitpick comments (16)
db/init-multiple-databases.sh (1)
20-20: Unconventional whitespace trimming; consider Bash parameter expansion instead.Using
echo "$db" | xargsfor trimming is non-idiomatic and introduces a subprocess. Bash offers built-in parameter expansion that is more efficient and doesn't require external tools.- db=$(echo "$db" | xargs) + db="${db##*( )}" # Remove leading whitespace + db="${db%%*( )}" # Remove trailing whitespaceOr use a more portable approach with sed or awk if targeting older Bash versions. This also improves robustness in minimal container environments where
xargsmay not be available.platforms/registry/src/services/VaultService.ts (1)
7-32: CRUD methods remain straightforward repository wrappers
create,findAll,findOne,findByEname,update, anddeleteall delegate directly toserviceRepositorywith clear, conventional TypeORM usage. Theupdate→findOnepattern is appropriate when you need the updated entity, and there are no obvious edge‑case or correctness issues here.If your project style allows, you could optionally simplify
return await this.serviceRepository...to justreturn this.serviceRepository...in these methods, since they immediately return the promise, but this is purely stylistic and fine as-is.platforms/registry/src/services/HealthCheckService.ts (3)
9-38: Confirm health vs reachability semantics inisUriHealthyWith
validateStatus: (status) => status < 500, this method returnstruefor any HTTP status below 500 (including 3xx and 4xx); only 5xx or network/timeout errors producefalse. If you intend this to model reachability that’s fine, but the nameisUriHealthyand the log message “Health check passed” suggest stricter semantics (e.g., only 2xx/3xx). Consider either:
- tightening the success condition (e.g., require 2xx) and logging pass/fail accordingly, or
- renaming/logging it as a reachability check to avoid confusion.
45-68:parseUriis intentionally narrow; consider robustness only if future callers need itThe protocol stripping and
cleanUri.split(":")withparts.length === 2match the documented supported formats (IP:PORT,http://IP:PORT,https://IP:PORT). This will deliberately reject URIs that include paths, queries, or hostnames. That’s fine given the comment, but if future inputs may include/whoisor DNS hostnames, consider switching to the built‑inURLparser for more robust extraction of host and port.
73-78: IPv4-only IP validation is acceptable but limitedThe regex correctly validates dotted‑quad IPv4 addresses. Just be aware this will reject IPv6 literals and DNS hostnames; if those ever need to be supported, you’ll need a broader validation strategy.
infrastructure/evault-core/src/core/http/server.ts (2)
23-28: Consider stronger typing for theevaultparameter.The
evaultparameter is typed asany. While this works, a more specific type would improve maintainability and catch potential issues at compile time.export async function registerHttpRoutes( server: FastifyInstance, - evault: any, // EVault instance to access publicKey + evault: { publicKey?: string }, // EVault instance - define a proper interface if more properties are needed provisioningService?: ProvisioningService, dbService?: DbService, ): Promise<void> {
295-298: Consider usingjose.JWTPayloadreturn type.The return type
Promise<any | null>could leverage thejoselibrary's type for better type safety.async function validateToken( authHeader: string | null, - ): Promise<any | null> { + ): Promise<jose.JWTPayload | null> {.env.example (1)
74-79: LGTM - New environment variables for Loki and eid-wallet token.The additions align with the PR's Loki service integration and token-based authentication. Consider adding a section comment for the Loki variables for consistency with other grouped configurations (e.g.,
# Loki Configuration).LOAD_TEST_USER_COUNT=6 PUBLIC_EID_WALLET_TOKEN=obtained-from-post-registry-service-/platforms/certification +# Loki Configuration LOKI_URL=http://localhost:3100 LOKI_USERNAME=admin LOKI_PASSWORD=adminscripts/get-eid-wallet-token.sh (1)
45-47: Remove trailing blank lines.The file has extra blank lines at the end which can be cleaned up.
echo "$TOKEN" - - -infrastructure/control-panel/src/lib/services/evaultService.ts (1)
98-115: Consider defining a return type interface forgetEVaultDetails.The
Promise<any>return type could be replaced with a specific interface matching the expected response structure for better type safety and IDE support.+interface EVaultDetails { + id: string; + w3id: string; + uri: string; + healthStatus: string; + // Add other fields as needed +} + /** * Get details for a specific eVault by evaultId */ -static async getEVaultDetails(evaultId: string): Promise<any> { +static async getEVaultDetails(evaultId: string): Promise<EVaultDetails> {infrastructure/control-panel/src/lib/services/registry.ts (2)
27-41: Error handling silently returns empty array.Consider whether silently returning an empty array on error is the desired behavior. Callers won't be able to distinguish between "registry has no vaults" and "registry is unreachable."
If distinguishing these cases matters, consider throwing or returning a result type:
} catch (error) { console.error("Error fetching evaults from registry:", error); - return []; + throw error; // Let caller decide how to handle }
71-115: Extract duplicate name mapping logic.The platform name mapping logic at lines 81-92 is duplicated at lines 100-113 in the catch block fallback. Consider extracting to a helper function.
+function mapPlatformName(namePart: string): string { + const nameMap: Record<string, string> = { + pictique: "Pictique", + blabsy: "Blabsy", + charter: "Group Charter", + cerberus: "Cerberus", + evoting: "eVoting", + dreamsync: "DreamSync", + ereputation: "eReputation", + }; + const lower = namePart.toLowerCase(); + return nameMap[lower] || namePart.charAt(0).toUpperCase() + namePart.slice(1); +}Then replace both occurrences with
name = mapPlatformName(namePart);infrastructure/control-panel/src/routes/api/evaults/[evaultId]/logs/+server.ts (1)
6-46: Clean implementation consistent with existing patterns.The route follows the same evault lookup pattern as the details endpoint and provides appropriate error responses. The structured error responses with
logs: []allow clients to handle errors gracefully.Consider adding validation for the
tailparameter to prevent excessive queries:const tail = parseInt(url.searchParams.get('tail') || '100', 10); +const validatedTail = Math.min(Math.max(tail, 1), 1000); // Clamp between 1-1000infrastructure/control-panel/src/routes/evaults/[evaultId]/+page.svelte (1)
69-73: Consider null-safe access forevault.uri.If
evaultis truthy buturiis undefined/null, this will displayundefined. Use optional chaining or a fallback.{#if evault} <p class="text-sm text-gray-500"> - URI: {evault.uri} + URI: {evault.uri || 'N/A'} </p> {/if}infrastructure/control-panel/src/routes/api/evaults/+server.ts (2)
5-13: Redundant fields in EVault interface.The interface has redundancy:
idduplicatesevault, andserviceUrlduplicatesuri. If this is for backward compatibility during migration, consider adding a comment. Otherwise, consolidate to reduce confusion.export interface EVault { - id: string; // evault identifier (evault field from registry) + id: string; // evault identifier (evault field from registry) - kept for backward compatibility name: string; // display name (ename or evault) ename: string; // w3id identifier uri: string; // resolved service URI evault: string; // evault identifier status: string; // derived from health check - serviceUrl?: string; // same as uri for display + serviceUrl?: string; // same as uri - kept for backward compatibility }
29-38: Inconsistent status terminology across endpoints.This endpoint uses
Active/Inactivefor status, while the details endpoint (/evaults/[evaultId]/details) usesHealthy/Unhealthy/UnreachableforhealthStatus. Consider aligning the terminology to avoid confusion in the UI.The list endpoint could either:
- Use
Healthy/Unhealthy/Unreachableto match details, or- Keep
Active/Inactivebut ensure the UI handles both consistentlyIf keeping both, document the distinction clearly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
infrastructure/eid-wallet/src-tauri/gen/android/.idea/codeStyles/Project.xmlis excluded by!**/gen/**infrastructure/eid-wallet/src-tauri/gen/android/.idea/codeStyles/codeStyleConfig.xmlis excluded by!**/gen/**infrastructure/eid-wallet/src-tauri/gen/android/.idea/kotlinc.xmlis excluded by!**/gen/**
📒 Files selected for processing (25)
.env.example(1 hunks)db/init-multiple-databases.sh(1 hunks)dev-docker-compose.yaml(2 hunks)infrastructure/control-panel/src/lib/components/EVaultList.svelte(4 hunks)infrastructure/control-panel/src/lib/services/evaultService.ts(2 hunks)infrastructure/control-panel/src/lib/services/loki.ts(1 hunks)infrastructure/control-panel/src/lib/services/registry.ts(1 hunks)infrastructure/control-panel/src/routes/+page.svelte(8 hunks)infrastructure/control-panel/src/routes/api/evaults/+server.ts(1 hunks)infrastructure/control-panel/src/routes/api/evaults/[evaultId]/details/+server.ts(1 hunks)infrastructure/control-panel/src/routes/api/evaults/[evaultId]/logs/+server.ts(1 hunks)infrastructure/control-panel/src/routes/api/evaults/[namespace]/[pod]/details/+server.ts(0 hunks)infrastructure/control-panel/src/routes/api/evaults/[namespace]/[pod]/logs/+server.ts(0 hunks)infrastructure/control-panel/src/routes/api/evaults/[namespace]/[pod]/metrics/+server.ts(0 hunks)infrastructure/control-panel/src/routes/evaults/[evaultId]/+page.svelte(5 hunks)infrastructure/control-panel/src/routes/monitoring/+page.svelte(3 hunks)infrastructure/control-panel/svelte.config.js(1 hunks)infrastructure/control-panel/vite.config.ts(1 hunks)infrastructure/eid-wallet/src/lib/global/controllers/evault.ts(2 hunks)infrastructure/eid-wallet/src/routes/(auth)/onboarding/+page.svelte(2 hunks)infrastructure/evault-core/src/core/http/server.ts(3 hunks)platforms/registry/src/index.ts(5 hunks)platforms/registry/src/services/HealthCheckService.ts(4 hunks)platforms/registry/src/services/VaultService.ts(1 hunks)scripts/get-eid-wallet-token.sh(1 hunks)
💤 Files with no reviewable changes (3)
- infrastructure/control-panel/src/routes/api/evaults/[namespace]/[pod]/metrics/+server.ts
- infrastructure/control-panel/src/routes/api/evaults/[namespace]/[pod]/details/+server.ts
- infrastructure/control-panel/src/routes/api/evaults/[namespace]/[pod]/logs/+server.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-13T10:34:52.527Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 415
File: infrastructure/eid-wallet/src/env.d.ts:8-8
Timestamp: 2025-11-13T10:34:52.527Z
Learning: In infrastructure/eid-wallet, PUBLIC_PLATFORM_URL should not be added to .env.example or configured as a static environment variable. The platform URL is extracted dynamically through URI parsing according to the protocol specification, and all fallbacks for platform URL are being removed.
Applied to files:
.env.examplescripts/get-eid-wallet-token.sh
📚 Learning: 2025-11-21T15:49:12.904Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 458
File: platforms/eReputation-api/src/services/ReferenceSigningSessionService.ts:60-60
Timestamp: 2025-11-21T15:49:12.904Z
Learning: In platforms/eReputation-api/src/services/ReferenceSigningSessionService.ts, the VITE_EREPUTATION_BASE_URL environment variable intentionally has no fallback value. The service should fail if this environment variable is not set, following a fail-fast design pattern.
Applied to files:
infrastructure/control-panel/src/lib/services/registry.ts
🧬 Code graph analysis (6)
infrastructure/control-panel/src/routes/api/evaults/[evaultId]/details/+server.ts (3)
infrastructure/control-panel/src/routes/api/evaults/+server.ts (1)
GET(15-58)infrastructure/control-panel/src/routes/api/evaults/[evaultId]/logs/+server.ts (1)
GET(6-46)infrastructure/control-panel/src/lib/services/registry.ts (1)
registryService(160-160)
infrastructure/control-panel/vite.config.ts (1)
infrastructure/eid-wallet/vitest.workspace.js (1)
dirname(8-11)
infrastructure/control-panel/src/routes/api/evaults/+server.ts (3)
infrastructure/control-panel/src/routes/api/evaults/[evaultId]/details/+server.ts (1)
GET(5-44)infrastructure/control-panel/src/routes/api/evaults/[evaultId]/logs/+server.ts (1)
GET(6-46)infrastructure/control-panel/src/lib/services/registry.ts (1)
registryService(160-160)
infrastructure/control-panel/src/routes/api/evaults/[evaultId]/logs/+server.ts (4)
infrastructure/control-panel/src/routes/api/evaults/+server.ts (1)
GET(15-58)infrastructure/control-panel/src/routes/api/evaults/[evaultId]/details/+server.ts (1)
GET(5-44)infrastructure/control-panel/src/lib/services/registry.ts (1)
registryService(160-160)infrastructure/control-panel/src/lib/services/loki.ts (1)
lokiService(213-213)
infrastructure/control-panel/src/lib/services/registry.ts (1)
tests/src/factories/index.ts (1)
Platform(1-1)
infrastructure/evault-core/src/core/http/server.ts (1)
infrastructure/evault-core/src/core/http/types.ts (2)
TypedRequest(14-16)TypedReply(18-18)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 79-79: [UnorderedKey] The LOKI_PASSWORD key should go before the LOKI_URL key
(UnorderedKey)
🔇 Additional comments (32)
db/init-multiple-databases.sh (1)
31-33: Verify heredoc syntax and ensure proper error propagation on database creation failure.The heredoc uses
<<-EOSQLcorrectly to allow indentation. However, ensure that if theCREATE DATABASEstatement fails, the error is properly reported. Sinceset -eis enabled at the top, any non-zero exit from psql should cause the script to exit. Verify that this behavior is intentional and desired (i.e., partial failures should fail the entire initialization).Test the error propagation by attempting to create a database with an invalid name or when the postgres user lacks permissions, and confirm the script exits with a failure status.
platforms/registry/src/services/VaultService.ts (1)
1-2: Type-only imports align with usage
RepositoryandVaultare only used in type positions here, so switching toimport typecleanly removes unnecessary runtime dependencies with no behavior change, assuming the repository/entity are wired via the DataSource/DI elsewhere.platforms/registry/src/index.ts (5)
2-4: LGTM: Import additions look good.The CORS and dotenv imports are properly added, and the dotenv configuration is correctly placed to load environment variables for subsequent usage in the server.
Also applies to: 29-29
13-20: LGTM: Formatting improvement.The multiline formatting enhances readability without changing functionality.
47-50: LGTM: Improved structured logging.The multiline object format improves log structure and readability, making it easier to parse errors in log aggregation systems.
133-143: LGTM: Improved array formatting.The multiline format with trailing commas improves readability and reduces diff noise in future changes.
192-209: LGTM: Improved code formatting.The multiline formatting of the
resolveUricall and added whitespace improve readability without changing functionality.platforms/registry/src/services/HealthCheckService.ts (2)
1-1: Axios import style change is fineSwitching to double quotes for the axios import is purely stylistic and consistent; no functional impact.
83-87: Port validation logic looks correct and idiomaticParsing the port as base‑10 and enforcing the
1–65535range is appropriate and safely filters out invalid values.infrastructure/control-panel/svelte.config.js (1)
11-14: LGTM - Environment configuration aligns with monorepo pattern.The
env.dirpointing to the monorepo root (../../) enables consistent environment variable loading across the control-panel, matching the approach invite.config.ts.infrastructure/eid-wallet/src/lib/global/controllers/evault.ts (1)
178-180: LGTM - Minor stylistic improvement.The change from bracket notation (
headers["Authorization"]) to dot notation (headers.Authorization) is equivalent and slightly more idiomatic for valid identifier keys.infrastructure/evault-core/src/core/http/server.ts (1)
365-465: LGTM - Well-structured endpoint with proper validation.The
/public-keyendpoint has comprehensive validation for headers, authentication, and body content with appropriate HTTP status codes for each error case.infrastructure/control-panel/vite.config.ts (2)
7-8: LGTM - Correct ESM __dirname computation.The pattern correctly derives
__dirnamefromimport.meta.urlsince native__dirnameis not available in ESM modules. This is consistent with the monorepo's approach seen in other files.
12-15: LGTM - Environment configuration aligns with monorepo strategy.Setting
envDirto the monorepo root andenvPrefixto"PUBLIC_"enables consistent environment variable loading. Server-side variables (likeLOKI_*) are loaded automatically whilePUBLIC_prefixed vars are exposed to the client.infrastructure/control-panel/src/lib/services/evaultService.ts (1)
77-96: LGTM - Clean refactor to evaultId-based API.The method signature change from
(namespace, podName)to(evaultId, tail?)aligns with the registry-driven architecture. TheencodeURIComponentusage correctly handles special characters in the evaultId.dev-docker-compose.yaml (1)
129-141: Neo4j PID cleanup approach looks reasonable.This is a common workaround for Neo4j container restart issues where stale PID files prevent startup. The redundant cleanup paths ensure coverage across different Neo4j versions.
infrastructure/control-panel/src/lib/services/registry.ts (2)
10-16: Interface definition looks good.The
RegistryVaultinterface captures the essential fields for registry-based vault discovery. The optional fieldsoriginalUriandresolvedprovide flexibility for different resolution states.
117-123: Hardcoded status and uptime values.Status is always set to
"Active"and uptime to"24h"regardless of actual platform health. If real status/uptime data is needed, consider adding health checks similar to the EVaults API.Is this intentional placeholder behavior, or should actual health checks be added here?
infrastructure/control-panel/src/routes/monitoring/+page.svelte (2)
103-104: Flexible label derivation handles varying EVault data shapes.The fallback chain (
name || ename || evault || id || 'eVault') accommodates the different field names used across the registry-based and legacy EVault representations.
509-543: Improved EVault matching logic with comprehensive field checks.The updated matching logic correctly handles the
@prefix in ename fields and checks multiple identifier fields. The debug logging is helpful for troubleshooting.infrastructure/control-panel/src/lib/services/loki.ts (1)
101-142: Well-structured log aggregation with deduplication.The method correctly tries multiple query patterns, aggregates results, deduplicates by timestamp+line, and returns the last N entries. The approach handles the varying label formats across different log sources.
One minor note: The
catchblock at line 126-128 will rarely trigger sincequeryLogsalready catches errors internally and returns an empty array. The try-catch is still valid for unexpected errors, but the log message could be misleading.infrastructure/control-panel/src/routes/api/evaults/[evaultId]/details/+server.ts (1)
1-44: Clean implementation following established patterns.The route handler correctly:
- Fetches eVaults from registry and finds by
evaultorename- Performs health check with appropriate timeout
- Returns structured response with healthStatus
- Handles errors with proper status codes
This aligns well with the sibling logs endpoint pattern.
infrastructure/control-panel/src/lib/components/EVaultList.svelte (3)
140-142: LGTM!The empty state message correctly reflects the shift from Kubernetes to registry-based discovery.
156-169: Column headers appropriately updated for registry model.The table headers now reflect the registry-driven EVault structure with
eName (w3id)andURIreplacing the Kubernetes-specificNamespaceandAgecolumns.
183-207: Robust fallback handling for status and URI display.The status badge logic correctly handles
Active/Inactivestates, and the URI rendering provides appropriate fallbacks with a clear message when unavailable.infrastructure/control-panel/src/routes/+page.svelte (3)
33-41: LGTM!The search filter appropriately includes the new registry-driven fields (
ename,evault,id) with null-safe optional chaining.
71-90: Well-structured table mapping with proper fallbacks.The
mappedEVaultsDatacorrectly maps the new EVault structure with appropriate fallback chains for the eVault ID, eName, and URI fields.
347-355: Correct navigation implementation.The row click handler properly derives the evaultId using the fallback chain and correctly encodes it for URL navigation.
infrastructure/control-panel/src/routes/evaults/[evaultId]/+page.svelte (3)
14-34: Clean validation and data fetching flow.The early validation of
evaultIdand the sequential fetch pattern (get all → find target → fetch details) is well-structured with appropriate error handling.
36-43: Efficient parallel data fetching.Using
Promise.allto fetch logs and details concurrently is the right approach for reducing latency.
155-169: Health status badge styling is intuitive.The color coding (green for Healthy, yellow for Unhealthy, red for others including Unreachable) provides clear visual feedback for the eVault's health state.
infrastructure/control-panel/src/routes/api/evaults/+server.ts (1)
21-50: Good use of concurrent health checks.Using
Promise.allwith individual health checks per vault is efficient. The 2-second timeout prevents slow vaults from blocking the entire response.
There was a problem hiding this comment.
Actionable comments posted: 1
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/monitoring/+page.svelte (1)
156-163: TheisPausedstate is unused — pause button has no effect.The pause/resume button toggles
isPaused(line 640), but the SSE message handler never checks this state. Events will continue to be processed and visualized regardless of the button state.eventSource.onmessage = (event) => { + if (isPaused) return; try { const data = JSON.parse(event.data); handleFlowEvent(data); } catch (error) { console.error('Error parsing SSE data:', error); } };
🧹 Nitpick comments (5)
infrastructure/control-panel/src/routes/monitoring/+page.svelte (5)
165-174: Consider exponential backoff for SSE reconnection.The current reconnection logic uses a fixed 2-second delay. For better resilience against persistent failures (e.g., server down), consider implementing exponential backoff to avoid rapid reconnection attempts that could overwhelm the server when it recovers.
+ let reconnectAttempts = 0; + const MAX_RECONNECT_DELAY = 30000; + function subscribeToEvents() { + reconnectAttempts = 0; eventSource = new EventSource('/api/events'); // ... eventSource.onerror = (error) => { console.error('SSE connection error:', error); + const delay = Math.min(1000 * Math.pow(2, reconnectAttempts), MAX_RECONNECT_DELAY); + reconnectAttempts++; setTimeout(() => { if (eventSource) { eventSource.close(); subscribeToEvents(); } - }, 2000); + }, delay); };
509-553: Verbose debug logging may impact performance and readability in production.The per-iteration logging on non-matches (lines 539-549) will produce significant console output when there are many eVaults, making it harder to find relevant debug information. Consider:
- Moving the
normalizehelper outside thefindIndexcallback to avoid recreating it on each iteration- Reducing the per-iteration logging or gating it behind a debug flag
+ // Helper to normalize IDs by removing @ symbol for comparison + const normalize = (id: string | undefined) => id?.replace('@', '') || ''; + function getEvaultIndex(w3id: string): number { console.log('getEvaultIndex called with w3id:', w3id); - console.log('selectedEVaults:', selectedEVaults); // Strip the @ symbol from w3id if present const cleanW3id = w3id.replace('@', ''); - console.log('Cleaned w3id:', cleanW3id); - - // Match against ename (w3id), evault, or id fields - // Helper to normalize IDs by removing @ symbol for comparison - const normalize = (id: string | undefined) => id?.replace('@', '') || ''; const index = selectedEVaults.findIndex((e, idx) => { // ... matching logic ... - - } else { - // Log why it didn't match for debugging - console.log(`eVault ${idx} didn't match:`, { - eventW3id: w3id, - eventCleanW3id: cleanW3id, - evaultEname: e.ename, - // ... extensive logging ... - }); } return matches; });
722-728: Unused loop index variable.The loop variable
iis declared but never used. Consider removing it to avoid lint warnings.- {#each flowMessages as message, i} + {#each flowMessages as message} <div class="rounded bg-gray-50 p-2 font-mono text-sm"> {message} </div> {/each}
147-151: Pending timeouts are not cancelled on component destruction.Multiple
setTimeoutcalls throughout the component (lines 236, 266, 269, 273, 305, 309, 359, 400, 431) modify component state after delays. If the component unmounts before these timeouts complete, they may attempt to update stale state. Consider tracking and clearing pending timeouts inonDestroy.+ let pendingTimeouts: ReturnType<typeof setTimeout>[] = []; + + function safeTimeout(fn: () => void, delay: number) { + const id = setTimeout(() => { + pendingTimeouts = pendingTimeouts.filter(t => t !== id); + fn(); + }, delay); + pendingTimeouts.push(id); + return id; + } + onDestroy(() => { if (eventSource) { eventSource.close(); } + pendingTimeouts.forEach(clearTimeout); });Then replace
setTimeoutcalls withsafeTimeoutthroughout.
476-499: Inconsistent fallback behavior betweengetPlatformIndexandgetEvaultIndex.
getEvaultIndexcorrectly returns-1when no match is found (and the caller handles this), butgetPlatformIndexfalls back to0(line 498), which could silently visualize data flow to the wrong platform. Consider aligning the behavior.- return index >= 0 ? index : 0; + if (index === -1) { + console.log('No matching platform found for:', platformName); + } + return index; }Then update
handleEvaultSyncEventto also check forplatformIndex === -1before visualization.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.env.example(1 hunks)infrastructure/control-panel/src/routes/monitoring/+page.svelte(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-05-08T09:02:43.311Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/stories/Button.svelte:0-0
Timestamp: 2025-05-08T09:02:43.311Z
Learning: The project uses Svelte 5, which uses the new "runes" API including $props() for handling component props instead of the export let syntax from Svelte 3/4.
Applied to files:
infrastructure/control-panel/src/routes/monitoring/+page.svelte
📚 Learning: 2025-05-08T09:02:43.311Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/stories/Button.svelte:0-0
Timestamp: 2025-05-08T09:02:43.311Z
Learning: The project uses Svelte 5, which introduces the new "runes" API including $props() for handling component props instead of the export let syntax from Svelte 3/4.
Applied to files:
infrastructure/control-panel/src/routes/monitoring/+page.svelte
📚 Learning: 2025-11-13T10:34:52.527Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 415
File: infrastructure/eid-wallet/src/env.d.ts:8-8
Timestamp: 2025-11-13T10:34:52.527Z
Learning: In infrastructure/eid-wallet, PUBLIC_PLATFORM_URL should not be added to .env.example or configured as a static environment variable. The platform URL is extracted dynamically through URI parsing according to the protocol specification, and all fallbacks for platform URL are being removed.
Applied to files:
.env.example
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 79-79: [UnorderedKey] The LOKI_PASSWORD key should go before the LOKI_URL key
(UnorderedKey)
[warning] 81-81: [DuplicatedKey] The LOKI_URL key is duplicated
(DuplicatedKey)
[warning] 82-82: [DuplicatedKey] The LOKI_USERNAME key is duplicated
(DuplicatedKey)
[warning] 83-83: [DuplicatedKey] The LOKI_PASSWORD key is duplicated
(DuplicatedKey)
[warning] 83-83: [UnorderedKey] The LOKI_PASSWORD key should go before the LOKI_URL key
(UnorderedKey)
⏰ 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). (4)
- GitHub Check: test
- GitHub Check: test
- GitHub Check: test-web3-adapter-integration
- GitHub Check: build
🔇 Additional comments (3)
infrastructure/control-panel/src/routes/monitoring/+page.svelte (2)
1-31: LGTM!The imports are well-organized, and the component correctly uses Svelte 5 runes (
$state) for reactive state management. The pattern of statically importingBackgroundandControlswhile dynamically loadingSvelteFlowComponentis appropriate for code splitting.
97-110: LGTM!The label derivation logic correctly handles the new registry-based data model with appropriate fallback chains (
name → ename → evault → id → 'eVault'for labels anduri → serviceUrl → 'Unknown'for sub-labels). This provides good backward compatibility with different data sources..env.example (1)
75-75: Good addition for EID wallet token.The new
PUBLIC_EID_WALLET_TOKENenvironment variable aligns with the registry-driven authentication model and includes a helpful comment about obtaining the value.
| LOKI_URL=http://localhost:3100 | ||
| LOKI_USERNAME=admin | ||
| LOKI_PASSWORD=admin | ||
|
|
||
| LOKI_URL=http://146.190.29.56:3100 | ||
| LOKI_USERNAME=admin | ||
| LOKI_PASSWORD=admin |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the .env.example file and check its content around the specified lines
fd -name ".env.example" -type fRepository: MetaState-Prototype-Project/prototype
Length of output: 311
🏁 Script executed:
#!/bin/bash
# Find the .env.example file
fd ".env.example" -type fRepository: MetaState-Prototype-Project/prototype
Length of output: 251
🏁 Script executed:
#!/bin/bash
# Find the .env.example file
fd ".env.example"Repository: MetaState-Prototype-Project/prototype
Length of output: 154
🏁 Script executed:
#!/bin/bash
# Check the content of both .env.example files, focusing on lines around 77-83
echo "=== infrastructure/control-panel/.env.example ==="
wc -l infrastructure/control-panel/.env.example
echo ""
echo "Lines 70-90:"
sed -n '70,90p' infrastructure/control-panel/.env.example
echo ""
echo "=== platforms/pictique/.env.example ==="
wc -l platforms/pictique/.env.example
echo ""
echo "Lines 70-90:"
sed -n '70,90p' platforms/pictique/.env.exampleRepository: MetaState-Prototype-Project/prototype
Length of output: 277
🏁 Script executed:
#!/bin/bash
# Search for files containing LOKI_URL
echo "=== Files containing LOKI_URL ==="
rg "LOKI_URL" -l
echo ""
echo "=== Context around LOKI_URL matches ==="
rg "LOKI_URL" -B2 -A2Repository: MetaState-Prototype-Project/prototype
Length of output: 1387
🏁 Script executed:
#!/bin/bash
# Check git diff to see what changed in the PR
echo "=== Git diff for infrastructure/control-panel/.env.example ==="
git diff HEAD infrastructure/control-panel/.env.example
echo ""
echo "=== Full content of infrastructure/control-panel/.env.example ==="
cat -n infrastructure/control-panel/.env.exampleRepository: MetaState-Prototype-Project/prototype
Length of output: 443
🏁 Script executed:
#!/bin/bash
# Look for .env files in infrastructure/eid-wallet (mentioned in learnings)
echo "=== Search for .env files in infrastructure/eid-wallet ==="
fd ".env" infrastructure/eid-wallet
echo ""
echo "=== Search for any .env files in infrastructure/ ==="
fd ".env" infrastructure/
echo ""
echo "=== Check git log to see recent changes to .env.example files ==="
git log --oneline infrastructure/control-panel/.env.example | head -10Repository: MetaState-Prototype-Project/prototype
Length of output: 341
🏁 Script executed:
#!/bin/bash
# Check current branch and git status
echo "=== Current branch info ==="
git branch -v
git log --oneline -5
echo ""
echo "=== Check if there are other .env.example files ==="
find . -name ".env*" -type f
echo ""
echo "=== Search for the IP 146.190.29.56 in the entire codebase ==="
rg "146.190.29.56"Repository: MetaState-Prototype-Project/prototype
Length of output: 614
🏁 Script executed:
#!/bin/bash
# Check the root .env.example file
echo "=== Root .env.example file ==="
wc -l .env.example
echo ""
echo "=== Lines 70-90 of root .env.example ==="
sed -n '70,90p' .env.example
echo ""
echo "=== Search for LOKI in root .env.example ==="
grep -n "LOKI" .env.exampleRepository: MetaState-Prototype-Project/prototype
Length of output: 740
🏁 Script executed:
#!/bin/bash
# Check git diff for root .env.example
echo "=== Git diff for .env.example ==="
git diff HEAD .env.example
echo ""
echo "=== Git log for .env.example ==="
git log --oneline .env.example | head -5
echo ""
echo "=== Full content of .env.example to verify context ==="
cat -n .env.example | tail -20Repository: MetaState-Prototype-Project/prototype
Length of output: 1012
🏁 Script executed:
#!/bin/bash
# Search for .env file parsing/loading to understand how duplicates are handled
echo "=== Search for dotenv or env loading patterns ==="
rg "dotenv|process\.env|EnvConfig|env\.load" --type=ts --type=js -B2 -A2 | head -60
echo ""
echo "=== Check if .env.example is referenced in package.json or build config ==="
fd "package.json|vite.config|webpack.config|tsconfig" | xargs grep -l "env.example" 2>/dev/null || echo "No references found"Repository: MetaState-Prototype-Project/prototype
Length of output: 4124
Remove duplicate Loki environment variables from .env.example.
Lines 77–83 contain duplicate LOKI_URL, LOKI_USERNAME, and LOKI_PASSWORD keys. The second set (lines 81–83) will overwrite the first during dotenv parsing, causing the production IP address (146.190.29.56) to be used instead of the localhost development configuration. Additionally, .env.example should contain a single canonical set of example values, not multiple configurations.
Apply this diff to keep only the development configuration:
LOKI_URL=http://localhost:3100
LOKI_USERNAME=admin
LOKI_PASSWORD=admin
-
-LOKI_URL=http://146.190.29.56:3100
-LOKI_USERNAME=admin
-LOKI_PASSWORD=adminAlternatively, if production configuration reference is needed:
LOKI_URL=http://localhost:3100
LOKI_USERNAME=admin
LOKI_PASSWORD=admin
+
+# Production Loki configuration (use via environment secrets, not this file):
+# LOKI_URL=http://146.190.29.56:3100
+# LOKI_USERNAME=admin
+# LOKI_PASSWORD=admin📝 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.
| LOKI_URL=http://localhost:3100 | |
| LOKI_USERNAME=admin | |
| LOKI_PASSWORD=admin | |
| LOKI_URL=http://146.190.29.56:3100 | |
| LOKI_USERNAME=admin | |
| LOKI_PASSWORD=admin | |
| LOKI_URL=http://localhost:3100 | |
| LOKI_USERNAME=admin | |
| LOKI_PASSWORD=admin |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 79-79: [UnorderedKey] The LOKI_PASSWORD key should go before the LOKI_URL key
(UnorderedKey)
[warning] 81-81: [DuplicatedKey] The LOKI_URL key is duplicated
(DuplicatedKey)
[warning] 82-82: [DuplicatedKey] The LOKI_USERNAME key is duplicated
(DuplicatedKey)
[warning] 83-83: [DuplicatedKey] The LOKI_PASSWORD key is duplicated
(DuplicatedKey)
[warning] 83-83: [UnorderedKey] The LOKI_PASSWORD key should go before the LOKI_URL key
(UnorderedKey)
🤖 Prompt for AI Agents
In .env.example around lines 77 to 83, there are duplicate
LOKI_URL/LOKI_USERNAME/LOKI_PASSWORD entries; remove the duplicate production
block so only the development configuration (LOKI_URL=http://localhost:3100,
LOKI_USERNAME=admin, LOKI_PASSWORD=admin) remains, deleting the second set (the
146.190.29.56 lines) to ensure a single canonical example and prevent dotenv
from being overwritten by the production IP.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
infrastructure/control-panel/src/routes/monitoring/+page.svelte (1)
286-327: Potential race condition with rapid events.If multiple
evault_sync_eventmessages arrive in quick succession (within the 1-second or 3-second timeouts), thesetTimeoutcallbacks will interfere with each other. For example:
- Event A triggers at t=0, schedules edge clearing at t=4s
- Event B triggers at t=0.5s, clears edges at t=1.5s (line 293)
- Event A's edge disappears prematurely
Consider using unique edge IDs and only removing edges matching the current event, or cancelling pending timeouts when a new event arrives.
infrastructure/control-panel/src/routes/+page.svelte (1)
490-494: Stale empty state message references kubectl/pods.The message "No eVault pods found. Make sure kubectl is configured and eVault pods are running." references the old pod-based discovery model. Given the refactor to a registry-driven model, this should be updated.
{:else if evaults.length === 0} <div class="py-8 text-center text-gray-500"> - No eVault pods found. Make sure kubectl is configured and eVault pods are - running. + No eVaults found. Make sure the registry service is running and eVaults are registered. </div>
🧹 Nitpick comments (5)
infrastructure/control-panel/src/routes/+layout.svelte (1)
54-62: Consider extracting the ID-to-object resolution logic into a shared utility.The matching logic
(e.evault || e.ename || e.id) === idis duplicated across+layout.svelte,+page.svelte, andmonitoring/+page.svelte. Extracting this into a shared helper (e.g.,getEvaultIdentifier(evault)andfindEvaultById(evaults, id)) would reduce duplication and ensure consistent behavior.infrastructure/control-panel/src/routes/monitoring/+page.svelte (2)
551-630: Consider reducing or gating debug logging for production.The
getEvaultIndexfunction contains extensiveconsole.logstatements (lines 552-554, 557, 566-600, 605-622) that output detailed match information for every eVault on every event. This is useful during development but may clutter production logs and leak potentially sensitive identifiers.Consider either:
- Removing the per-item logging, keeping only the final result
- Gating with a debug flag:
if (import.meta.env.DEV) console.log(...)
215-224: Consider adding exponential backoff for SSE reconnection.The current reconnection uses a fixed 2-second delay. If the server is down for an extended period, this results in continuous reconnection attempts every 2 seconds. Adding exponential backoff and/or a maximum retry count would be more resilient.
+ let reconnectAttempts = 0; + const maxReconnectDelay = 30000; + eventSource.onerror = (error) => { console.error('SSE connection error:', error); + reconnectAttempts++; + const delay = Math.min(2000 * Math.pow(2, reconnectAttempts - 1), maxReconnectDelay); setTimeout(() => { if (eventSource) { eventSource.close(); subscribeToEvents(); } - }, 2000); + }, delay); };infrastructure/control-panel/src/routes/+page.svelte (2)
256-274: Redundant data preparation with+layout.svelte.This function stores both
selectedEVaultsData(objects) andselectedEVaults(IDs) before navigation. However, the "Start Monitoring" button in+layout.sveltealso fetches and storesselectedEVaultsData. If the button in layout is the primary navigation path, this function's object storage may be redundant.Consider consolidating the navigation logic to avoid dual storage paths.
193-238: Consider removing or gating debug console.log statements.The selection handlers contain extensive debug logging with emoji prefixes (🎯, ✅, ❌, 💾). While helpful during development, these should be removed or gated behind a debug flag for production to avoid cluttering the console.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrastructure/control-panel/src/routes/+layout.svelte(2 hunks)infrastructure/control-panel/src/routes/+page.svelte(13 hunks)infrastructure/control-panel/src/routes/monitoring/+page.svelte(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-08T09:02:43.311Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/stories/Button.svelte:0-0
Timestamp: 2025-05-08T09:02:43.311Z
Learning: The project uses Svelte 5, which uses the new "runes" API including $props() for handling component props instead of the export let syntax from Svelte 3/4.
Applied to files:
infrastructure/control-panel/src/routes/monitoring/+page.svelte
📚 Learning: 2025-05-08T09:02:43.311Z
Learnt from: pixel-punk-20
Repo: MetaState-Prototype-Project/prototype PR: 121
File: platforms/metagram/src/stories/Button.svelte:0-0
Timestamp: 2025-05-08T09:02:43.311Z
Learning: The project uses Svelte 5, which introduces the new "runes" API including $props() for handling component props instead of the export let syntax from Svelte 3/4.
Applied to files:
infrastructure/control-panel/src/routes/monitoring/+page.svelte
⏰ 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). (3)
- GitHub Check: test
- GitHub Check: build
- GitHub Check: test-web3-adapter-integration
🔇 Additional comments (5)
infrastructure/control-panel/src/routes/monitoring/+page.svelte (2)
37-91: LGTM with a minor observation.The fallback pattern (
selectedEVaultsData || selectedEVaults) and type detection (typeof parsed[0] === 'string') provide good backwards compatibility. The error handling correctly resets state on failure.One edge case: if
parsedpasses theparsed.length > 0check butparsed[0]isnullorundefined,typeof parsed[0]would be'object'or'undefined', potentially causing the code to treat it as objects when it shouldn't. This is unlikely given the upstream code but worth noting.
148-160: LGTM - Node labeling uses appropriate fallbacks.The label fallback chain (
evault.name || evault.ename || evault.evault || evault.id || 'eVault') and subLabel fallback (evault.uri || evault.serviceUrl || 'Unknown') handle the various possible data shapes from the registry-driven model well.infrastructure/control-panel/src/routes/+page.svelte (3)
33-42: LGTM - Search covers all identifier fields.The search correctly uses optional chaining and covers
name,ename,evault, andidfields, aligning with the new composite identifier approach.
116-130: LGTM - Correctly maps ID-based selections to paginated indices.The derived values properly bridge the ID-based selection storage with the index-based table component interface, handling pagination correctly.
368-453: LGTM - Robust restoration logic with backwards compatibility.The restoration logic correctly:
- Uses a flag to prevent multiple restorations
- Handles both simple ID arrays (new format) and object arrays (backwards compatibility)
- Validates restored selections against current data
Note: The condition
evaults.length >= 0is always true for arrays, which is intentional to allow restoration to proceed even when the fetch returns empty. The actual filtering (storedData.filter(...)) handles the case where evaults is empty by producing an empty selection.
| // Fetch full objects and store them for monitoring page | ||
| try { | ||
| // Fetch evaults if we have IDs | ||
| if (evaultsData) { | ||
| const evaultIds = JSON.parse(evaultsData); | ||
| if (Array.isArray(evaultIds) && evaultIds.length > 0 && typeof evaultIds[0] === 'string') { | ||
| const { EVaultService } = await import('$lib/services/evaultService'); | ||
| const allEVaults = await EVaultService.getEVaults(); | ||
| const evaultObjects = evaultIds | ||
| .map((id: string) => allEVaults.find((e: any) => (e.evault || e.ename || e.id) === id)) | ||
| .filter(Boolean); | ||
| sessionStorage.setItem('selectedEVaultsData', JSON.stringify(evaultObjects)); | ||
| } | ||
| } | ||
|
|
||
| // Fetch platforms if we have URLs | ||
| if (platformsData) { | ||
| const platformUrls = JSON.parse(platformsData); | ||
| if (Array.isArray(platformUrls) && platformUrls.length > 0 && typeof platformUrls[0] === 'string') { | ||
| const { registryService } = await import('$lib/services/registry'); | ||
| const allPlatforms = await registryService.getPlatforms(); | ||
| const platformObjects = platformUrls | ||
| .map((url: string) => allPlatforms.find((p: any) => p.url === url)) | ||
| .filter(Boolean); | ||
| sessionStorage.setItem('selectedPlatformsData', JSON.stringify(platformObjects)); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| console.error('Error fetching data for monitoring:', error); | ||
| } |
There was a problem hiding this comment.
Navigation proceeds even when data fetching fails.
If the try block throws (e.g., network error), the catch logs the error but navigation to /monitoring still occurs at line 82. The monitoring page may then show stale or missing data.
Consider either blocking navigation on error or ensuring the monitoring page gracefully handles missing selectedEVaultsData/selectedPlatformsData.
} catch (error) {
console.error('Error fetching data for monitoring:', error);
+ alert('Failed to fetch data. Please try again.');
+ return;
}📝 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.
| // Fetch full objects and store them for monitoring page | |
| try { | |
| // Fetch evaults if we have IDs | |
| if (evaultsData) { | |
| const evaultIds = JSON.parse(evaultsData); | |
| if (Array.isArray(evaultIds) && evaultIds.length > 0 && typeof evaultIds[0] === 'string') { | |
| const { EVaultService } = await import('$lib/services/evaultService'); | |
| const allEVaults = await EVaultService.getEVaults(); | |
| const evaultObjects = evaultIds | |
| .map((id: string) => allEVaults.find((e: any) => (e.evault || e.ename || e.id) === id)) | |
| .filter(Boolean); | |
| sessionStorage.setItem('selectedEVaultsData', JSON.stringify(evaultObjects)); | |
| } | |
| } | |
| // Fetch platforms if we have URLs | |
| if (platformsData) { | |
| const platformUrls = JSON.parse(platformsData); | |
| if (Array.isArray(platformUrls) && platformUrls.length > 0 && typeof platformUrls[0] === 'string') { | |
| const { registryService } = await import('$lib/services/registry'); | |
| const allPlatforms = await registryService.getPlatforms(); | |
| const platformObjects = platformUrls | |
| .map((url: string) => allPlatforms.find((p: any) => p.url === url)) | |
| .filter(Boolean); | |
| sessionStorage.setItem('selectedPlatformsData', JSON.stringify(platformObjects)); | |
| } | |
| } | |
| } catch (error) { | |
| console.error('Error fetching data for monitoring:', error); | |
| } | |
| // Fetch full objects and store them for monitoring page | |
| try { | |
| // Fetch evaults if we have IDs | |
| if (evaultsData) { | |
| const evaultIds = JSON.parse(evaultsData); | |
| if (Array.isArray(evaultIds) && evaultIds.length > 0 && typeof evaultIds[0] === 'string') { | |
| const { EVaultService } = await import('$lib/services/evaultService'); | |
| const allEVaults = await EVaultService.getEVaults(); | |
| const evaultObjects = evaultIds | |
| .map((id: string) => allEVaults.find((e: any) => (e.evault || e.ename || e.id) === id)) | |
| .filter(Boolean); | |
| sessionStorage.setItem('selectedEVaultsData', JSON.stringify(evaultObjects)); | |
| } | |
| } | |
| // Fetch platforms if we have URLs | |
| if (platformsData) { | |
| const platformUrls = JSON.parse(platformsData); | |
| if (Array.isArray(platformUrls) && platformUrls.length > 0 && typeof platformUrls[0] === 'string') { | |
| const { registryService } = await import('$lib/services/registry'); | |
| const allPlatforms = await registryService.getPlatforms(); | |
| const platformObjects = platformUrls | |
| .map((url: string) => allPlatforms.find((p: any) => p.url === url)) | |
| .filter(Boolean); | |
| sessionStorage.setItem('selectedPlatformsData', JSON.stringify(platformObjects)); | |
| } | |
| } | |
| } catch (error) { | |
| console.error('Error fetching data for monitoring:', error); | |
| alert('Failed to fetch data. Please try again.'); | |
| return; | |
| } |
🤖 Prompt for AI Agents
in infrastructure/control-panel/src/routes/+layout.svelte around lines 50 to 79,
the try/catch around fetching selectedEVaultsData/selectedPlatformsData logs
errors but allows code later (line ~82) to navigate to /monitoring, which can
cause stale/missing data there; change behavior so navigation is blocked on
fetch failure or the monitoring page always receives deterministic values:
either rethrow or return a failure signal from the catch to prevent subsequent
navigation, or in the catch populate sessionStorage with empty arrays
(selectedEVaultsData and selectedPlatformsData as JSON arrays) and set a flag
(e.g., sessionStorage.setItem('monitoringDataError','1')) so the caller can
detect the failure and avoid navigating; implement one of these approaches and
ensure the navigation decision checks the flag or the error result before
routing to /monitoring.
Description of change
makes the control panel usable with the new infrastructure
Issue Number
n/a
Type of change
How the change has been tested
manual
Change checklist
Summary by CodeRabbit
New Features
Infrastructure
Chores
✏️ Tip: You can customize this high-level summary in your review settings.