Skip to content

Conversation

sosweetham
Copy link
Member

@sosweetham sosweetham commented Oct 13, 2025

Description of change

Issue Number

Type of change

  • Breaking (any change that would cause existing functionality to not work as expected)
  • New (a change which implements a new feature)
  • Update (a change which updates existing functionality)
  • Fix (a change which fixes an issue)
  • Docs (changes to the documentation)
  • Chore (refactoring, build scripts or anything else that isn't user-facing)

How the change has been tested

Change checklist

  • I have ensured that the CI Checks pass locally
  • I have removed any unnecessary logic
  • My code is well documented
  • I have signed my commits
  • My code follows the pattern of the application
  • I have self reviewed my code

Summary by CodeRabbit

  • New Features

    • Registry exposes a lightweight MOTD endpoint with status and message.
    • Site-wide maintenance banner now appears across platforms when MOTD indicates maintenance.
    • MOTD changes take effect immediately without service restarts.
  • Documentation

    • Added environment example entries to configure the registry URL for frontend apps.
  • Chores

    • Registry status set to "maintenance", causing banners to display the provided message.

@sosweetham sosweetham requested a review from coodos as a code owner October 13, 2025 09:22
Copy link
Contributor

coderabbitai bot commented Oct 13, 2025

Walkthrough

Adds a dynamic MOTD: registry now reads and watches motd.json and exposes GET /motd; multiple frontend platforms fetch that endpoint and mount a MaintenanceBanner component to display a maintenance message when status is maintenance.

Changes

Cohort / File(s) Summary
MOTD data file
platforms/registry/motd.json
Updated MOTD payload: status changed to "maintenance" and message to "Registry service is down".
Registry server MOTD feature
platforms/registry/src/index.ts
Adds loading/parsing of motd.json, watches the file for changes, maintains an in-memory MOTD, and exposes GET /motd returning the current MOTD.
Blabsy: component + mount
platforms/blabsy/src/components/common/maintenance-banner.tsx, platforms/blabsy/src/pages/_app.tsx, platforms/blabsy/.env.example
New React MaintenanceBanner that GETs /motd and displays a yellow banner when status is maintenance; mounted in global App; env example added for NEXT_PUBLIC_REGISTRY_URL.
eVoting: component + layout
platforms/eVoting/src/components/MaintenanceBanner.tsx, platforms/eVoting/src/app/layout.tsx
New client React component that fetches /motd and conditionally renders a maintenance banner; inserted into RootLayout.
Group Charter Manager: component + layout
platforms/group-charter-manager/src/components/MaintenanceBanner.tsx, platforms/group-charter-manager/src/app/layout.tsx
New client React MaintenanceBanner fetching /motd and mounted in the root layout.
Pictique: Svelte component + layout
platforms/pictique/src/lib/fragments/MaintenanceBanner/.../MaintenanceBanner.svelte, platforms/pictique/src/routes/+layout.svelte, platforms/pictique/.env.example
New Svelte MaintenanceBanner that fetches /motd on mount and conditionally displays a banner; layout updated to include the banner; env example added for PUBLIC_REGISTRY_URL.
Env docs
.env.example
Added NEXT_PUBLIC_REGISTRY_URL to top-level .env.example and reorganized section headers for Next.js / Svelte configs.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Frontend as Frontends (Blabsy / eVoting / Pictique / ...)
  participant Registry as Registry Server
  participant FS as Filesystem (motd.json)
  participant Cache as MOTD Cache

  rect rgb(250,250,255)
    note over Registry,FS: Registry startup & file watch
    Registry->>FS: Read motd.json
    FS-->>Registry: JSON {status,message}
    Registry->>Cache: Initialize cache
    Registry->>FS: Watch motd.json for changes
  end

  rect rgb(245,255,245)
    note over Frontend,Registry: Frontend requests MOTD
    Frontend->>Registry: GET /motd
    Registry->>Cache: Read current MOTD
    Cache-->>Registry: {status,message}
    Registry-->>Frontend: 200 OK + JSON
    Frontend->>Frontend: Render MaintenanceBanner if status == "maintenance"
  end

  rect rgb(255,245,240)
    note over FS,Registry: motd.json update flow
    FS-->>Registry: file change event
    Registry->>FS: Re-read motd.json
    FS-->>Registry: Updated JSON
    Registry->>Cache: Update MOTD cache
    Registry-->>Frontends: Subsequent GET /motd returns updated MOTD
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Suggested reviewers

  • coodos

Poem

I nibble lines and watch the logs,
A yellow banner flutters on the blogs.
When motd says "maintenance" clear and loud,
I thump my paws and call it proud.
Hop, deploy — the users see it proud! 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request description consists solely of the unpopulated template headings and contains no actual content for the change description, issue number, type of change, testing details, or completed checklist items, leaving all required sections empty. Please fill in each template section with the actual change summary, reference the related issue number if applicable, select the appropriate type of change, provide details of how the change has been tested, and complete the checklist to confirm that all tasks have been addressed.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly reflects the introduction of the MOTD feature and uses a conventional commit prefix to denote a new feature; it directly highlights the main change without extraneous details.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/motd

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4644f3d and 1fc4cf7.

📒 Files selected for processing (2)
  • platforms/registry/motd.json (1 hunks)
  • platforms/registry/src/index.ts (2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
platforms/registry/src/index.ts (1)
infrastructure/web3-adapter/src/index.js (1)
  • fs (42-42)
🔇 Additional comments (2)
platforms/registry/motd.json (1)

1-4: LGTM!

The MOTD JSON structure is clean and minimal, with appropriate fields for status and message.

platforms/registry/src/index.ts (1)

74-76: Verify authentication on /motd endpoint

The /motd route in platforms/registry/src/index.ts (lines 74-76) currently has no preHandler middleware—confirm whether it’s meant to be public or if it should enforce authentication.

import { KubernetesService } from "./services/KubernetesService";
import cors from "@fastify/cors";

import fs from "node:fs";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Synchronous I/O blocks the event loop and lacks error handling.

The implementation has several serious issues:

  1. Blocking I/O: fs.readFileSync blocks the event loop during startup and file reloads, degrading server responsiveness.
  2. No error handling: Missing try-catch will crash the server if the file is missing or contains invalid JSON.
  3. Path resolution risk: __dirname may not resolve correctly after TypeScript compilation, depending on build configuration.

Apply this diff to use async I/O with proper error handling:

-import fs from "node:fs";
+import fs from "node:fs/promises";

-function loadMotdJSON() {
-    const motdJSON = fs.readFileSync(path.resolve(__dirname, "../motd.json"), "utf8");
-    return JSON.parse(motdJSON) as {
+async function loadMotdJSON() {
+    try {
+        const motdPath = path.resolve(__dirname, "../motd.json");
+        const motdJSON = await fs.readFile(motdPath, "utf8");
+        return JSON.parse(motdJSON) as {
+            status: "up" | "maintenance"
+            message: string
+        };
+    } catch (error) {
+        server.log.error({ message: "Failed to load MOTD", error });
+        return {
+            status: "up" as const,
+            message: "Registry service is running"
+        };
+    }
+}
+
+let motd = {
-        status: "up" | "maintenance"
-        message: string
-    };
+    status: "up" as const,
+    message: "Registry service is running"
-}
+};

-let motd = loadMotdJSON();
+(async () => {
+    motd = await loadMotdJSON();
+})();

Note: You'll also need to update the file watcher (see next comment).

Also applies to: 13-19, 21-21

Comment on lines +23 to +25
fs.watchFile(path.resolve(__dirname, "../motd.json"), (_curr, _prev) => {
motd = loadMotdJSON();
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Major: Use fs.watch instead of fs.watchFile and add cleanup.

fs.watchFile uses inefficient stat polling and creates a resource leak without cleanup.

Apply this diff to use event-based watching with proper cleanup:

-fs.watchFile(path.resolve(__dirname, "../motd.json"), (_curr, _prev) => {
-    motd = loadMotdJSON();
-});
+const motdWatcher = fs.watch(
+    path.resolve(__dirname, "../motd.json"),
+    async (eventType) => {
+        if (eventType === "change") {
+            motd = await loadMotdJSON();
+            server.log.info("MOTD reloaded");
+        }
+    }
+);
+
+// Cleanup on server close
+server.addHook("onClose", async () => {
+    motdWatcher.close();
+});

Note: This assumes you've converted loadMotdJSON() to async as suggested in the previous comment.

📝 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.

Suggested change
fs.watchFile(path.resolve(__dirname, "../motd.json"), (_curr, _prev) => {
motd = loadMotdJSON();
});
const motdWatcher = fs.watch(
path.resolve(__dirname, "../motd.json"),
async (eventType) => {
if (eventType === "change") {
motd = await loadMotdJSON();
server.log.info("MOTD reloaded");
}
}
);
// Cleanup on server close
server.addHook("onClose", async () => {
motdWatcher.close();
});

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (2)
platforms/eVoting/src/components/MaintenanceBanner.tsx (1)

11-37: Same issues as blabsy implementation.

This implementation has the same issues identified in platforms/blabsy/src/components/common/maintenance-banner.tsx:

  1. Missing axios timeout configuration
  2. No periodic refetching mechanism
  3. Code duplication across platforms

Please refer to the review comments on platforms/blabsy/src/components/common/maintenance-banner.tsx for detailed suggestions on addressing these issues through shared packages or hooks.

platforms/group-charter-manager/src/components/MaintenanceBanner.tsx (1)

11-37: Same issues as other MaintenanceBanner implementations.

This is the third duplicate implementation with identical issues:

  1. Missing axios timeout configuration
  2. No periodic refetching mechanism
  3. Missing void operator on fetchMotd() call
  4. Code duplication across platforms

Please refer to the comprehensive review comments on platforms/blabsy/src/components/common/maintenance-banner.tsx for detailed suggestions on extracting shared logic and addressing these issues.

🧹 Nitpick comments (2)
platforms/pictique/src/lib/fragments/MaintenanceBanner/MaintenanceBanner.svelte (1)

10-11: Validate response data before assignment to prevent runtime errors.

The response data is assigned directly to motd without validation. If the API returns data in an unexpected format, this could lead to runtime errors when the template tries to access motd.status or motd.message.

Apply this diff to add basic response validation:

 			const response = await axios.get(`${registryUrl}/motd`, {
 				timeout: 5000,
 			});
-			motd = response.data;
+			const data = response.data;
+			if (data && typeof data === 'object' && 
+			    ('status' in data) && ('message' in data)) {
+				motd = data;
+			} else {
+				console.warn('Invalid MOTD response format:', data);
+			}
 		} catch (error) {
platforms/eVoting/src/components/MaintenanceBanner.tsx (1)

25-25: Add void operator for consistency.

Unlike the blabsy implementation which uses void fetchMotd(), this version calls fetchMotd() without the void operator. While both work, the void operator explicitly indicates the promise is intentionally not awaited, improving code clarity.

Apply this diff:

-        fetchMotd();
+        void fetchMotd();

Note: This issue will be resolved if the component is extracted to a shared package as suggested in the review of platforms/blabsy/src/components/common/maintenance-banner.tsx.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1fc4cf7 and 7620d5d.

📒 Files selected for processing (9)
  • platforms/blabsy/src/components/common/maintenance-banner.tsx (1 hunks)
  • platforms/blabsy/src/pages/_app.tsx (2 hunks)
  • platforms/eVoting/src/app/layout.tsx (2 hunks)
  • platforms/eVoting/src/components/MaintenanceBanner.tsx (1 hunks)
  • platforms/group-charter-manager/src/app/layout.tsx (2 hunks)
  • platforms/group-charter-manager/src/components/MaintenanceBanner.tsx (1 hunks)
  • platforms/pictique/src/lib/fragments/MaintenanceBanner/MaintenanceBanner.svelte (1 hunks)
  • platforms/pictique/src/routes/+layout.svelte (2 hunks)
  • platforms/registry/motd.json (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
platforms/eVoting/src/components/MaintenanceBanner.tsx (2)
platforms/blabsy/src/components/common/maintenance-banner.tsx (1)
  • MaintenanceBanner (9-35)
platforms/group-charter-manager/src/components/MaintenanceBanner.tsx (1)
  • MaintenanceBanner (11-37)
platforms/group-charter-manager/src/app/layout.tsx (1)
platforms/group-charter-manager/src/components/MaintenanceBanner.tsx (1)
  • MaintenanceBanner (11-37)
platforms/group-charter-manager/src/components/MaintenanceBanner.tsx (2)
platforms/blabsy/src/components/common/maintenance-banner.tsx (1)
  • MaintenanceBanner (9-35)
platforms/eVoting/src/components/MaintenanceBanner.tsx (1)
  • MaintenanceBanner (11-37)
platforms/eVoting/src/app/layout.tsx (1)
platforms/eVoting/src/components/MaintenanceBanner.tsx (1)
  • MaintenanceBanner (11-37)
platforms/blabsy/src/pages/_app.tsx (1)
platforms/blabsy/src/components/common/maintenance-banner.tsx (1)
  • MaintenanceBanner (9-35)
platforms/blabsy/src/components/common/maintenance-banner.tsx (2)
platforms/eVoting/src/components/MaintenanceBanner.tsx (1)
  • MaintenanceBanner (11-37)
platforms/group-charter-manager/src/components/MaintenanceBanner.tsx (1)
  • MaintenanceBanner (11-37)
🔇 Additional comments (5)
platforms/pictique/src/lib/fragments/MaintenanceBanner/MaintenanceBanner.svelte (1)

18-22: LGTM! Conditional rendering logic is correct.

The maintenance banner only renders when motd?.status === 'maintenance', which correctly handles both the null state (during fetch) and the 'up' status case.

platforms/blabsy/src/pages/_app.tsx (1)

6-6: LGTM!

The MaintenanceBanner integration is well-placed. Rendering it before the context providers ensures it displays immediately without dependencies on authentication or theme context.

Also applies to: 28-28

platforms/eVoting/src/app/layout.tsx (1)

4-4: LGTM!

The MaintenanceBanner is correctly integrated in the root layout. Placing it before the AuthProvider ensures it displays on all pages regardless of authentication state.

Also applies to: 22-22

platforms/group-charter-manager/src/app/layout.tsx (1)

7-7: LGTM!

The MaintenanceBanner integration follows the same pattern as other platforms and is correctly placed before the AuthProvider.

Also applies to: 27-27

platforms/registry/motd.json (1)

1-4: Ensure motd.json status is intentional
The service reads platforms/registry/motd.json and returns it at GET /motd; with "maintenance" clients will display a maintenance banner. Confirm this is intended for production, otherwise set status to "up" and adjust the message.

Comment on lines +1 to +35
import { useEffect, useState } from 'react';
import axios from 'axios';

interface Motd {
status: 'up' | 'maintenance';
message: string;
}

export function MaintenanceBanner(): JSX.Element | null {
const [motd, setMotd] = useState<Motd | null>(null);

useEffect(() => {
const fetchMotd = async () => {
try {
const registryUrl = process.env.NEXT_PUBLIC_REGISTRY_URL || 'http://localhost:4321';
const response = await axios.get<Motd>(`${registryUrl}/motd`);
setMotd(response.data);
} catch (error) {
console.error('Failed to fetch motd:', error);
}
};

void fetchMotd();
}, []);

if (motd?.status !== 'maintenance') {
return null;
}

return (
<div className='bg-yellow-500 px-4 py-3 text-center text-sm font-medium text-black'>
⚠️ {motd.message}
</div>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Eliminate code duplication across platforms.

The MaintenanceBanner component is nearly identical across three platforms (blabsy, eVoting, group-charter-manager), with only minor styling and directive differences. This creates a significant maintenance burden.

Consider one of these approaches:

Option 1 (Recommended): Extract to a shared package:

// packages/ui/src/maintenance-banner.tsx
'use client';

export interface MaintenanceBannerProps {
    className?: string;
}

export function MaintenanceBanner({ className = 'bg-yellow-500 px-4 py-3 text-center text-sm font-medium text-black' }: MaintenanceBannerProps) {
    // ... shared logic
}

Then import in each platform with platform-specific styling if needed.

Option 2: If styling must differ significantly, extract just the hook logic:

// packages/hooks/src/use-motd.ts
export function useMotd() {
    const [motd, setMotd] = useState<Motd | null>(null);
    // ... fetch logic
    return motd;
}

Comment on lines +4 to +7
interface Motd {
status: 'up' | 'maintenance';
message: string;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Extract shared type definition to prevent drift.

The Motd interface is duplicated across three platforms (blabsy, eVoting, group-charter-manager). This creates a maintenance burden and risk of type inconsistencies.

Consider creating a shared types package (e.g., packages/types/src/motd.ts) and importing from there:

// packages/types/src/motd.ts
export interface Motd {
    status: 'up' | 'maintenance';
    message: string;
}

Then import in each platform:

-interface Motd {
-    status: 'up' | 'maintenance';
-    message: string;
-}
+import type { Motd } from '@repo/types';
🤖 Prompt for AI Agents
In platforms/blabsy/src/components/common/maintenance-banner.tsx around lines 4
to 7, the Motd interface is duplicated across multiple platforms; extract it to
a shared types package (e.g., packages/types/src/motd.ts) that exports the Motd
interface, update this file to import Motd from that package instead of
declaring it locally, and update the other platforms (eVoting,
group-charter-manager) to import the same shared type to prevent drift and
ensure a single source of truth.

Comment on lines +12 to +24
useEffect(() => {
const fetchMotd = async () => {
try {
const registryUrl = process.env.NEXT_PUBLIC_REGISTRY_URL || 'http://localhost:4321';
const response = await axios.get<Motd>(`${registryUrl}/motd`);
setMotd(response.data);
} catch (error) {
console.error('Failed to fetch motd:', error);
}
};

void fetchMotd();
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add timeout and consider periodic refetching.

The axios request lacks a timeout configuration, which could cause the request to hang indefinitely if the registry is unresponsive. Additionally, the MOTD is fetched only once on mount, so users won't see status updates without a page refresh.

Apply these improvements:

 useEffect(() => {
     const fetchMotd = async () => {
         try {
             const registryUrl = process.env.NEXT_PUBLIC_REGISTRY_URL || 'http://localhost:4321';
-            const response = await axios.get<Motd>(`${registryUrl}/motd`);
+            const response = await axios.get<Motd>(`${registryUrl}/motd`, {
+                timeout: 5000, // 5 second timeout
+            });
             setMotd(response.data);
         } catch (error) {
             console.error('Failed to fetch motd:', error);
         }
     };
 
     void fetchMotd();
+    
+    // Optionally refetch every 5 minutes to catch status changes
+    const interval = setInterval(() => void fetchMotd(), 5 * 60 * 1000);
+    return () => clearInterval(interval);
 }, []);
📝 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.

Suggested change
useEffect(() => {
const fetchMotd = async () => {
try {
const registryUrl = process.env.NEXT_PUBLIC_REGISTRY_URL || 'http://localhost:4321';
const response = await axios.get<Motd>(`${registryUrl}/motd`);
setMotd(response.data);
} catch (error) {
console.error('Failed to fetch motd:', error);
}
};
void fetchMotd();
}, []);
useEffect(() => {
const fetchMotd = async () => {
try {
const registryUrl = process.env.NEXT_PUBLIC_REGISTRY_URL || 'http://localhost:4321';
const response = await axios.get<Motd>(`${registryUrl}/motd`, {
timeout: 5000, // 5 second timeout
});
setMotd(response.data);
} catch (error) {
console.error('Failed to fetch motd:', error);
}
};
void fetchMotd();
// Optionally refetch every 5 minutes to catch status changes
const interval = setInterval(() => void fetchMotd(), 5 * 60 * 1000);
return () => clearInterval(interval);
}, []);
🤖 Prompt for AI Agents
In platforms/blabsy/src/components/common/maintenance-banner.tsx around lines 12
to 24, the axios GET has no timeout and the MOTD is fetched only once; update
the fetch to include a request timeout (e.g., axios timeout or AbortController)
and implement periodic refetching using setInterval (choose a sensible interval
like 30–60s), ensure you handle request cancellation on unmount/interval reset
to avoid leaks, and keep error handling intact while updating state only when
the component is still mounted.

Comment on lines 7 to 15
onMount(async () => {
try {
const registryUrl = import.meta.env.VITE_REGISTRY_URL || 'http://localhost:4321';
const response = await axios.get(`${registryUrl}/motd`);
motd = response.data;
} catch (error) {
console.error('Failed to fetch motd:', error);
}
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add timeout and improve error handling for the MOTD fetch.

The axios request lacks a timeout configuration, which could cause the fetch to hang indefinitely if the registry is unresponsive. Additionally, the error handling only logs to console without providing any user feedback or fallback behavior.

Apply this diff to add timeout and improve reliability:

 	onMount(async () => {
 		try {
 			const registryUrl = import.meta.env.VITE_REGISTRY_URL || 'http://localhost:4321';
-			const response = await axios.get(`${registryUrl}/motd`);
+			const response = await axios.get(`${registryUrl}/motd`, {
+				timeout: 5000,
+			});
 			motd = response.data;
 		} catch (error) {
-			console.error('Failed to fetch motd:', error);
+			console.error('Failed to fetch MOTD:', error);
+			// Silently fail - no banner shown if fetch fails
 		}
 	});
📝 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.

Suggested change
onMount(async () => {
try {
const registryUrl = import.meta.env.VITE_REGISTRY_URL || 'http://localhost:4321';
const response = await axios.get(`${registryUrl}/motd`);
motd = response.data;
} catch (error) {
console.error('Failed to fetch motd:', error);
}
});
onMount(async () => {
try {
const registryUrl = import.meta.env.VITE_REGISTRY_URL || 'http://localhost:4321';
const response = await axios.get(`${registryUrl}/motd`, {
timeout: 5000,
});
motd = response.data;
} catch (error) {
console.error('Failed to fetch MOTD:', error);
// Silently fail - no banner shown if fetch fails
}
});
🤖 Prompt for AI Agents
In
platforms/pictique/src/lib/fragments/MaintenanceBanner/MaintenanceBanner.svelte
around lines 7 to 15, the MOTD fetch has no timeout and only logs errors to
console; update the axios.get call to include a timeout (e.g. 2–5s) and check
the HTTP status, and in the catch branch set a safe fallback for motd (like a
default message or null) and a reactive error flag so the UI can show
user-visible feedback; also differentiate timeout/network errors vs non-2xx
responses in the error handling and ensure the error is logged with details for
debugging.

onMount(async () => {
try {
const registryUrl = import.meta.env.VITE_REGISTRY_URL || 'http://localhost:4321';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

❓ Verification inconclusive

Hardcoded localhost fallback may not work in all environments.

The fallback http://localhost:4321 will only work in local development. In production or staging environments where the registry runs on a different host, this fallback could cause silent failures.

Consider:

  • Documenting that VITE_REGISTRY_URL must be set for non-local environments
  • Or providing a more robust fallback strategy (e.g., relative URL or throwing an error if env var is missing in production)

Run the following script to check if VITE_REGISTRY_URL is documented:


🏁 Script executed:

#!/bin/bash
# Check for documentation of VITE_REGISTRY_URL
rg -n "VITE_REGISTRY_URL" --type=md -C3

Length of output: 0


Missing documentation and unsafe localhost fallback
No references to VITE_REGISTRY_URL were found in the docs; the hardcoded fallback (http://localhost:4321) will break in staging/production. Add documentation requiring this env var in non-local environments or enforce it at runtime (e.g., throw an error or use a relative URL when import.meta.env.VITE_REGISTRY_URL is undefined).

🤖 Prompt for AI Agents
In
platforms/pictique/src/lib/fragments/MaintenanceBanner/MaintenanceBanner.svelte
around line 9, the code falls back to a hardcoded 'http://localhost:4321' when
import.meta.env.VITE_REGISTRY_URL is missing, which is unsafe for
staging/production and undocumented; update the file to enforce presence of
VITE_REGISTRY_URL in non-local environments by: 1) removing or avoiding a
localhost fallback, 2) throwing a clear runtime error when the env var is
undefined in NODE_ENV !== 'development' (or use a relative URL if intended for
production), and 3) add/update project docs to require VITE_REGISTRY_URL for
non-local deployments with example values and where to set it (CI/env config).

Comment on lines +50 to +59
<div class="flex h-[100dvh] flex-col overflow-hidden">
<MaintenanceBanner />
<main
class="flex-1 overflow-hidden {page.url.pathname.includes('/profile')
? 'px-0'
: 'px-4'} md:px-0"
>
{@render children()}
</main>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Review overflow handling to ensure content remains scrollable.

The wrapper div has overflow-hidden while the inner main also has overflow-hidden and flex-1. This combination may prevent the main content from scrolling when it exceeds the available viewport height (especially when the maintenance banner is displayed).

Consider:

  • Removing overflow-hidden from the wrapper div, or
  • Changing the inner main to use overflow-y-auto instead of overflow-hidden

Apply this diff to allow main content to scroll:

-	<div class="flex h-[100dvh] flex-col overflow-hidden">
+	<div class="flex h-[100dvh] flex-col">
 		<MaintenanceBanner />
 		<main
-			class="flex-1 overflow-hidden {page.url.pathname.includes('/profile')
+			class="flex-1 overflow-y-auto {page.url.pathname.includes('/profile')
 				? 'px-0'
 				: 'px-4'}  md:px-0"
 		>
📝 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.

Suggested change
<div class="flex h-[100dvh] flex-col overflow-hidden">
<MaintenanceBanner />
<main
class="flex-1 overflow-hidden {page.url.pathname.includes('/profile')
? 'px-0'
: 'px-4'} md:px-0"
>
{@render children()}
</main>
</div>
<div class="flex h-[100dvh] flex-col">
<MaintenanceBanner />
<main
class="flex-1 overflow-y-auto {page.url.pathname.includes('/profile')
? 'px-0'
: 'px-4'} md:px-0"
>
{@render children()}
</main>
</div>
🤖 Prompt for AI Agents
In platforms/pictique/src/routes/+layout.svelte around lines 50 to 59, the outer
wrapper and inner main both use overflow-hidden which can prevent scrolling of
main content; remove overflow-hidden from the wrapper div (or at minimum change
the inner main's overflow to overflow-y-auto) so the main can scroll when its
content exceeds the viewport; update the class on the wrapper to drop
overflow-hidden and/or change main's class to use overflow-y-auto (and keep
flex-1) so the page remains scrollable when the maintenance banner or other
content reduces available height.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
.env.example (1)

8-9: Keep .env keys sorted to satisfy dotenv-linter.

dotenv-linter flags the new ordering; swap the two keys so PUBLIC_PROVISIONER_URL precedes PUBLIC_REGISTRY_URL.

-PUBLIC_REGISTRY_URL=your_public_registry_url_here
-PUBLIC_PROVISIONER_URL=your_public_provisioner_url_here
+PUBLIC_PROVISIONER_URL=your_public_provisioner_url_here
+PUBLIC_REGISTRY_URL=your_public_registry_url_here

Based on static analysis hints.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7620d5d and 0f641cb.

📒 Files selected for processing (4)
  • .env.example (1 hunks)
  • platforms/blabsy/.env.example (1 hunks)
  • platforms/pictique/.env.example (1 hunks)
  • platforms/pictique/src/lib/fragments/MaintenanceBanner/MaintenanceBanner.svelte (1 hunks)
✅ Files skipped from review due to trivial changes (2)
  • platforms/pictique/.env.example
  • platforms/blabsy/.env.example
🚧 Files skipped from review as they are similar to previous changes (1)
  • platforms/pictique/src/lib/fragments/MaintenanceBanner/MaintenanceBanner.svelte
🧰 Additional context used
🪛 dotenv-linter (3.3.0)
.env.example

[warning] 9-9: [UnorderedKey] The PUBLIC_PROVISIONER_URL key should go before the PUBLIC_REGISTRY_URL key

(UnorderedKey)

@coodos coodos merged commit a435021 into main Oct 13, 2025
1 of 4 checks passed
@coodos coodos deleted the feat/motd branch October 13, 2025 10:38
@coderabbitai coderabbitai bot mentioned this pull request Oct 14, 2025
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants