feat: qol esigner and file manager improvements #658
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds deeplink-based authentication pages for both apps, storage quota tracking and GET /api/storage, client/server upload quota enforcement (5MB client limit and 413 responses), storage dashboard and menu link, rename UIs and upload progress, notification messages updated with dynamic file/folder/eSigner links, mobile-aware signing helpers and responsive UI changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant DeeplinkPage as Frontend Deeplink Page
participant AuthAPI as Auth API (/api/auth)
participant LocalStore as Local Store
participant FilesPage as /files
Browser->>DeeplinkPage: Open deeplink URL (ename, session, signature[, appVersion])
DeeplinkPage->>DeeplinkPage: Parse params (search/hash/full URL) with retry
alt Params valid
DeeplinkPage->>DeeplinkPage: Clean URL (remove query/hash)
DeeplinkPage->>AuthAPI: POST /api/auth {ename, session, signature, appVersion}
alt Success (token, user)
AuthAPI-->>DeeplinkPage: {token, user}
DeeplinkPage->>LocalStore: Store token & user
DeeplinkPage->>FilesPage: Navigate to /files
else Failure
AuthAPI-->>DeeplinkPage: Error
DeeplinkPage->>Browser: Show error UI + "Go to Login"
end
else Missing params
DeeplinkPage->>Browser: Retry then show missing-params error
end
sequenceDiagram
participant User
participant Browser
participant StorageUI as Storage Page
participant FileAPI as File Manager API (/api/storage)
participant DB as Database
User->>Browser: Navigate to /storage
Browser->>StorageUI: Mount storage page
StorageUI->>StorageUI: Verify authentication
alt Authenticated
StorageUI->>FileAPI: GET /api/storage
FileAPI->>DB: Sum file sizes & counts for user
DB-->>FileAPI: {used, fileCount, folderCount}
FileAPI-->>StorageUI: {used, limit, fileCount, folderCount}
StorageUI->>Browser: Render usage bar, cards, alerts
else Unauthenticated
StorageUI->>Browser: Redirect to /auth
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks❌ Failed checks (2 warnings, 1 inconclusive)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (13)
📒 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: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
platforms/file-manager/src/routes/(protected)/files/+page.svelte (1)
1341-1345: Add auth token to download link in preview modal.The download endpoint requires authentication, but the download link at line 1342 doesn't include the auth token. When users click the download button in the preview modal, the request will fail with a 401 error. The preview endpoint (line 628) already handles this correctly by appending
?token=${token || ''}to the URL. Apply the same pattern to the download link.Affected code
<a href={`${API_BASE_URL}/api/files/${previewFile.id}/download`} download={previewFile.name} class="px-4 py-2 bg-blue-600 text-white rounded-lg hover:bg-blue-700 transition-colors" >Should include the token query parameter like the preview URL does at line 628:
const token = localStorage.getItem('file_manager_auth_token'); previewUrl = `${API_BASE_URL}/api/files/${file.id}/preview?token=${token || ''}`;
🤖 Fix all issues with AI Agents
In @platforms/esigner-api/src/services/NotificationService.ts:
- Around line 288-289: PUBLIC_ESIGNER_BASE_URL must not default to localhost and
file IDs must be URL-encoded: remove the '|| "http://localhost:3004"' fallback
when building esignerUrl in NotificationService (where fileLink is constructed)
and instead validate that process.env.PUBLIC_ESIGNER_BASE_URL is present (throw
or surface a clear error if missing); when building fileLink use an encoded ID
(e.g., encodeURIComponent(file.id)) so special characters are safe. Apply the
same change where the same pattern appears in NotificationService (other
occurrences around lines 319 and 347) and in SignatureService at the usage on
line 75.
In @platforms/esigner/src/routes/(auth)/deeplink-login/+page.svelte:
- Line 3: The import of goto is unused while the code uses window.location.href
for navigation, bypassing SvelteKit client-side routing; replace the direct
assignments to window.location.href (around the handlers at the spots referenced
by lines ~78 and ~116) with calls to the imported goto(path) so client-side
navigation is used, or if you prefer not to use SvelteKit navigation, remove the
unused import of goto to eliminate the unused-import warning; update usages
accordingly to either goto('/target') or remove the import.
- Around line 43-57: The 500ms delay retry around reading ename, session, and
signature is unreliable; replace it by reading URL params from SvelteKit's page
store instead of window.location (use the page store or the load function),
locate the block that references ename, session, signature and the
URLSearchParams fallback, and change it to derive these values from
$page.url.searchParams or from the load context so you get deterministic params
on navigation; remove the setTimeout/new Promise retry and ensure
isLoading/error flow uses the store-derived values.
- Around line 76-77: The code currently calls
localStorage.setItem("esigner_auth_token", data.token) and
localStorage.setItem("esigner_user", JSON.stringify(data.user)), which exposes
auth tokens to XSS; instead remove these localStorage writes and switch to
server-set httpOnly cookies: send the token to a backend login/session endpoint
(or return Set-Cookie from your existing auth endpoint) so the server sets an
httpOnly, Secure, SameSite cookie for authentication, and only store
non-sensitive UI state (if needed) in memory or in a non-persistent store;
update the +page.svelte deeplink-login flow to POST the received token (or
exchange the code) to the server session endpoint and rely on the cookie rather
than calling localStorage.setItem("esigner_auth_token", ...) or storing the full
user object in localStorage.
In @platforms/file-manager-api/src/controllers/FileController.ts:
- Around line 39-49: The current concurrent upload quota check in
FileController.ts (the block calling this.fileService.getUserStorageUsage and
comparing used + req.file.size to limit) can race under parallel requests;
change to an atomic reserve-and-commit flow in the FileService: add a new method
like fileService.reserveStorage(userId, size) that obtains a row-level lock or
uses an optimistic version check to atomically verify and increment a
reserved/used counter (or uses a DB transaction) and returns success/failure,
call reserveStorage before starting the upload, then call
fileService.commitStorage(userId, size) after the file is persisted (or
fileService.rollbackReservation on failure); update FileController.ts to use
reserveStorage instead of the naive getUserStorageUsage check and handle
reservation failures by returning the 413 response.
- Around line 29-37: The Multer in-memory upload is currently configured to
allow up to 100MB while the controller enforces 5MB with a manual MAX_FILE_SIZE
check (the block that checks req.file.size and returns 413); update the Multer
configuration used for uploads (the multer instance/options used by your upload
middleware) to set the fileSize limit to 5 * 1024 * 1024 (5MB) so oversized
files are rejected before being buffered, then remove the redundant
MAX_FILE_SIZE constant and the req.file.size if-check from FileController.ts; if
you need custom responses for limit errors, add a Multer error-handling
middleware to translate Multer's limit error into your desired 413 JSON
response.
In @platforms/file-manager-api/src/services/NotificationService.ts:
- Line 238: The URL construction for sharing links interpolates raw IDs and
should URL-encode them: replace `${fileManagerUrl}/files/${file.id}?view=shared`
with using encodeURIComponent(file.id) (and similarly
encodeURIComponent(folder.id) where a folder link is built around the other
occurrence at line 267); update the variable construction (e.g., fileLink and
the folder link) to call encodeURIComponent on the ID query parameters so
special characters don’t break the URL.
In @platforms/file-manager/src/routes/(auth)/deeplink-login/+page.svelte:
- Around line 43-57: Replace the delay-and-retry workaround in +page.svelte:
stop parsing window.location and removing the setTimeout block that rechecks
ename/session/signature; instead read URL params at the proper lifecycle using
SvelteKit's page store (import { page } from '$app/stores' and/or use onMount)
and get ename, session, signature via $page.url.searchParams.get(...), failing
fast if any are missing; update the code paths that set isLoading and error to
use these values and remove the delayed Promise and manual hash parsing.
- Line 3: The import of goto is unused while navigation uses
window.location.href which forces full reloads; either remove the unused import
of goto from the top of the file, or replace direct assignments to
window.location.href with SvelteKit client navigation calls using goto (e.g.,
use goto(targetUrl) for successful redirects and use goto(errorUrl) for the
error button instead of window.location.href) so routing stays client-side and
avoids full page reloads.
- Around line 76-77: Right now the code calls
localStorage.setItem("file_manager_auth_token", ...) and
localStorage.setItem("file_manager_user", ...) which exposes tokens to XSS;
change authentication state handling to use SvelteKit server-side session
cookies (set an httpOnly, Secure cookie in the server endpoint instead of
writing to localStorage) and remove client-side token persistence, or if you
must keep client storage document CSP, input sanitization, and short token
lifetimes; also replace the hard-coded default appVersion '0.4.0' with a
configurable value (env/config) so appVersion is not baked into +page.svelte.
In @platforms/file-manager/src/routes/(protected)/files/[id]/+page.svelte:
- Line 19: The rename modal state (showRenameModal) is never set to true because
there’s no UI trigger; add a Rename button into the Details/metadata card header
(the card that displays file details) that calls an event to open the modal
(e.g., on:click={() => showRenameModal = true}) and ensure it has an accessible
label and appropriate visibility (e.g., shown only when the file is editable).
Also wire this same trigger to any existing rename handler or form used by the
modal so opening the modal initializes any required state (e.g., current
filename) before display.
- Around line 292-312: In handleRenameFile, after a successful patch and reload
(inside the try block where toast.success, showRenameModal is set false and
await loadFile(file.id) is called), reset newDisplayName (e.g., newDisplayName =
'') so the modal won't show the previous value; alternatively or additionally,
ensure the code that opens the rename modal assigns newDisplayName =
file.displayName so the input is initialized to the current name when the modal
opens.
- Around line 42-49: The code reads the URL "view" param but never uses it; set
a component state flag (e.g., declare let cameFromSharedView = false) and assign
cameFromSharedView = true when viewParam === 'shared', then update the
back/navigation logic referenced around lines 120-128 to conditionally navigate
back to the shared view when cameFromSharedView is true (instead of always
returning to "my files"); alternatively if you prefer to drop the feature,
remove the entire viewParam extraction block to avoid dead code.
In @platforms/file-manager/src/routes/(protected)/files/+page.svelte:
- Around line 194-196: The code currently checks `currentFolderId !== undefined`
(redundant because `currentFolderId` is initialized to null) and appends
`'null'` which relies on backend parsing; change the conditional to only append
when a real id exists (e.g., `currentFolderId !== null && currentFolderId !==
undefined` or `currentFolderId != null`) and call `formData.append('folderId',
currentFolderId)` without falling back to the string `'null'` so you omit the
field when there is no folder id; update the logic around `currentFolderId` and
the `formData.append` call accordingly.
- Around line 221-227: The code handling 413 responses accesses
errorData.available without guarding it; update the block that reads
error.response?.status and constructs the toast message to defensively check
that errorData and errorData.available are present (e.g., using optional
chaining and a fallback value or "unknown"), and only call .toFixed(2) when
available is a numeric value; if unavailable, show a safe fallback message like
"Storage quota exceeded. Available: unknown" or compute from
Number(errorData.available || 0) to avoid runtime errors when rendering the
toast.error.
- Around line 337-339: The rename handler calls updateFile but it wasn't
imported; update the import that currently brings in files, fetchFiles,
uploadFile, deleteFile, moveFile to also import updateFile from
$lib/stores/files (so functions like updateFile, fetchFiles, uploadFile,
deleteFile, moveFile and the files store are all imported together) to avoid the
runtime ReferenceError when renaming a file.
In @platforms/file-manager/src/routes/(protected)/storage/+page.svelte:
- Around line 13-21: The onMount block is subscribing to the isAuthenticated
store without cleaning it up; modify the onMount handler to capture the
subscription returned by isAuthenticated.subscribe and call unsubscribe in a
cleanup using Svelte's onDestroy (or return an unsubscribe function from
onMount), or replace the manual subscription with Svelte's auto-subscription
($isAuthenticated) in the component so you no longer call
isAuthenticated.subscribe; ensure the redirect logic that calls goto('/auth')
and the await fetchStorageUsage() remain functionally equivalent.
🧹 Nitpick comments (9)
platforms/file-manager/src/routes/(auth)/deeplink-login/+page.svelte (3)
21-27: Simplify redundant URL parsing.Lines 22-24 create a URL object from
window.location.hrefand extract.search, butwindow.location.searchis already available at line 13 and would be the same value.🔎 Proposed simplification
} else { - try { - const fullUrl = new URL(window.location.href); - searchString = fullUrl.search; - } catch (e) { - // Ignore parsing errors - } + // Already checked window.location.search at line 13 + // No additional parsing needed }
65-65: Hard-coded default appVersion may mask compatibility issues.The fallback to
'0.4.0'could hide version mismatches between client and server. Consider:
- Making appVersion required if it's critical for compatibility
- Reading from package.json or build-time environment variable
- Logging when the default is used
🔎 Proposed improvement
-const requestBody = { ename, session, signature, appVersion: appVersion || '0.4.0' }; +const defaultAppVersion = import.meta.env.VITE_APP_VERSION || '0.4.0'; +const requestBody = { + ename, + session, + signature, + appVersion: appVersion || defaultAppVersion +}; +if (!appVersion) { + console.warn(`appVersion not provided in deeplink, using default: ${defaultAppVersion}`); +}
1-123: Consider extracting shared deeplink login logic to avoid duplication.This file is nearly identical to
platforms/esigner/src/routes/(auth)/deeplink-login/+page.svelte, differing only in:
- API base URL environment variable
- localStorage key prefixes
This duplication creates maintenance burden and increases the risk of inconsistent fixes.
Consider creating a shared utility function:
🔎 Proposed shared utility
Create
lib/auth/deeplink-login.ts:export async function handleDeeplinkLogin(config: { apiBaseUrl: string; tokenKey: string; userKey: string; redirectPath?: string; }) { // Move the core login logic here // Return { success: boolean, error?: string, data?: any } }Then use it in both pages:
import { handleDeeplinkLogin } from '$lib/auth/deeplink-login'; onMount(async () => { const result = await handleDeeplinkLogin({ apiBaseUrl: import.meta.env.VITE_FILE_MANAGER_BASE_URL, tokenKey: 'file_manager_auth_token', userKey: 'file_manager_user', redirectPath: '/files' }); if (!result.success) { error = result.error; isLoading = false; } });platforms/esigner/src/routes/(auth)/deeplink-login/+page.svelte (1)
65-65: Hard-coded default appVersion may mask compatibility issues.Same as file-manager: the
'0.4.0'fallback could hide version mismatches.See the file-manager review for the proposed improvement using environment variables.
platforms/file-manager/src/routes/(protected)/files/+page.svelte (2)
129-142: Consider using SvelteKit's$pagestore for URL parameters.The
$pagestore is already imported but not used here. Using$page.url.searchParamsis more idiomatic in SvelteKit and reactive to URL changes.Additionally, consider validating
folderIdParambefore assignment to prevent setting arbitrary strings as the current folder ID.🔎 Suggested improvement
- const searchParams = new URLSearchParams(window.location.search); - const viewParam = searchParams.get('view'); - const folderIdParam = searchParams.get('folderId'); + const searchParams = get(page).url.searchParams; + const viewParam = searchParams.get('view'); + const folderIdParam = searchParams.get('folderId');
1286-1296: Consider adding autofocus to the rename input.For better UX, the input field should receive focus automatically when the modal opens, allowing users to start typing immediately.
🔎 Suggested improvement
<input type="text" bind:value={newName} placeholder="Enter new name" - class="w-full px-4 py-2 border border-gray-300 rounded-lg mb-4" + class="w-full px-4 py-2 border border-gray-300 rounded-lg mb-4" + autofocus onkeydown={(e) => { if (e.key === 'Enter' && newName.trim()) { handleRename(); } }} />platforms/file-manager/src/routes/(protected)/files/[id]/+page.svelte (1)
692-725: Make modal backdrop behavior consistent with other modals.The Rename Modal lacks backdrop click-to-close functionality that's present in the Access Modal (line 560) and Tag Modal (line 628). Additionally, the inner content div (line 695) is missing
onclick={(e) => e.stopPropagation()}to prevent event bubbling.🔎 Proposed consistency improvements
-<div class="fixed inset-0 bg-black/50 flex items-center justify-center z-50"> - <div class="bg-white rounded-lg p-6 max-w-md w-full mx-4"> +<div class="fixed inset-0 bg-black/50 flex items-center justify-center z-50" onclick={() => { showRenameModal = false; newDisplayName = ''; }}> + <div class="bg-white rounded-lg p-6 max-w-md w-full mx-4" onclick={(e) => e.stopPropagation()}> <h3 class="text-xl font-bold mb-4">Rename File</h3>platforms/file-manager-api/src/services/FileService.ts (1)
321-332: Consider making the storage limit configurable.The storage limit is hardcoded to 1GB (1073741824 bytes). As the application scales, different users may require different quotas. Consider making this configurable via an environment variable or database field.
🔎 Proposed refactor using environment variable
async getUserStorageUsage(userId: string): Promise<{ used: number; limit: number }> { const result = await this.fileRepository .createQueryBuilder('file') .select('SUM(file.size)', 'total') .where('file.ownerId = :userId', { userId }) .getRawOne(); const used = parseInt(result?.total || '0', 10); - const limit = 1073741824; // 1GB in bytes + const limit = parseInt(process.env.DEFAULT_STORAGE_LIMIT || '1073741824', 10); // Default: 1GB in bytes return { used, limit }; }platforms/file-manager/src/routes/(protected)/storage/+page.svelte (1)
23-36: Add defensive check for zero limit to prevent division by zero.While unlikely given the default 1GB limit, if the backend returns
limit: 0, line 29 will result inNaNorInfinity, which could cause UI rendering issues.🔎 Proposed fix: Add a safe calculation
used = response.data.used; limit = response.data.limit; - percentage = (used / limit) * 100; + percentage = limit > 0 ? (used / limit) * 100 : 0; } catch (error) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
platforms/esigner/.svelte-kit/generated/client/app.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/esigner/.svelte-kit/generated/client/nodes/5.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/esigner/.svelte-kit/generated/client/nodes/6.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/esigner/.svelte-kit/generated/client/nodes/7.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/esigner/.svelte-kit/generated/client/nodes/8.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/esigner/.svelte-kit/non-ambient.d.tsis excluded by!**/.svelte-kit/**platforms/esigner/.svelte-kit/types/route_meta_data.jsonis excluded by!**/.svelte-kit/**platforms/esigner/.svelte-kit/types/src/routes/$types.d.tsis excluded by!**/.svelte-kit/**platforms/esigner/.svelte-kit/types/src/routes/(auth)/deeplink-login/$types.d.tsis excluded by!**/.svelte-kit/**
📒 Files selected for processing (11)
platforms/esigner-api/src/services/NotificationService.tsplatforms/esigner/src/routes/(auth)/deeplink-login/+page.svelteplatforms/file-manager-api/src/controllers/FileController.tsplatforms/file-manager-api/src/index.tsplatforms/file-manager-api/src/services/FileService.tsplatforms/file-manager-api/src/services/NotificationService.tsplatforms/file-manager/src/lib/components/UserMenuDropdown.svelteplatforms/file-manager/src/routes/(auth)/deeplink-login/+page.svelteplatforms/file-manager/src/routes/(protected)/files/+page.svelteplatforms/file-manager/src/routes/(protected)/files/[id]/+page.svelteplatforms/file-manager/src/routes/(protected)/storage/+page.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
platforms/file-manager/src/routes/(protected)/storage/+page.svelteplatforms/file-manager/src/routes/(auth)/deeplink-login/+page.svelteplatforms/esigner/src/routes/(auth)/deeplink-login/+page.svelteplatforms/file-manager/src/routes/(protected)/files/[id]/+page.svelteplatforms/file-manager/src/routes/(protected)/files/+page.svelteplatforms/file-manager/src/lib/components/UserMenuDropdown.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). (1)
- GitHub Check: build
🔇 Additional comments (12)
platforms/file-manager/src/routes/(protected)/files/+page.svelte (5)
82-86: LGTM!State declarations for the rename modal and upload progress are correctly implemented using Svelte 5's
$state()rune.
887-899: LGTM!The rename button implementation follows the established pattern for action buttons, correctly closes the dropdown, and properly restricts the action to owned items.
1019-1032: LGTM!The upload progress UI provides clear visual feedback with a progress bar and percentage display. The transition animation enhances UX.
1034-1048: LGTM!Good UX improvements: controls are properly disabled during upload, progress is shown in the button text, and all state is reset on cancel.
181-186: LGTM!Client-side file size validation provides immediate feedback to users and prevents unnecessary upload attempts for oversized files.
platforms/file-manager/src/lib/components/UserMenuDropdown.svelte (1)
62-104: LGTM! Clean implementation of the Storage menu item.The new Storage menu option is well-integrated with proper navigation, event handling, and consistent styling. The SVG icons are appropriately structured, and the dropdown behavior is preserved.
platforms/file-manager-api/src/index.ts (1)
98-100: LGTM! Storage route properly configured.The new storage usage endpoint is correctly placed, uses appropriate authentication middleware, and follows the existing API patterns.
platforms/esigner-api/src/services/NotificationService.ts (1)
301-301: HTML rendering is already supported and properly implemented.The notification messages with HTML anchor tags are intentionally designed this way. The
ChatMessage.sveltecomponent explicitly renders HTML using Svelte's{@html}syntax (lines 45-49 and 104-107) and includes regex-based styling for anchor tags, transforming them to include Tailwind classes (text-blue-600 hover:text-blue-800 underline). The system correctly displays functional, styled links without stripping or escaping HTML tags. No changes needed.platforms/file-manager-api/src/controllers/FileController.ts (1)
364-376: LGTM!The storage usage endpoint is well-implemented with proper authentication and error handling.
platforms/file-manager/src/routes/(protected)/storage/+page.svelte (3)
38-56: LGTM!The helper functions for formatting bytes and determining color classes based on usage thresholds are well-implemented and correct.
59-88: LGTM!The storage display UI is well-structured with proper loading states, responsive design, and dynamic progress bar visualization.
90-187: LGTM!The storage dashboard UI components (summary cards, warning alerts, action buttons, and info panel) are well-implemented with appropriate visual feedback and accurate limit information.
| const esignerUrl = process.env.PUBLIC_ESIGNER_BASE_URL || 'http://localhost:3004'; | ||
| const fileLink = `${esignerUrl}/files/${file.id}`; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for PUBLIC_ESIGNER_BASE_URL configuration across the codebase
rg -i "PUBLIC_ESIGNER_BASE_URL" --type-list | head -20Repository: MetaState-Prototype-Project/prototype
Length of output: 470
🏁 Script executed:
# Look for environment configuration files for esigner-api
fd -type f -name ".env*" platforms/esigner-api/ 2>/dev/null | head -20Repository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
# Search for how file.id is defined and used
rg "file\.id" platforms/esigner-api/src/services/NotificationService.ts -B 3 -A 3Repository: MetaState-Prototype-Project/prototype
Length of output: 1137
🏁 Script executed:
# Check if there are URL encoding utilities being used elsewhere
rg "encodeURI|encodeURIComponent" platforms/esigner-api/src/ -lRepository: MetaState-Prototype-Project/prototype
Length of output: 135
🏁 Script executed:
# Verify the complete context of the generateInvitationMessage function
sed -n '276,302p' platforms/esigner-api/src/services/NotificationService.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 1104
🏁 Script executed:
# Search for PUBLIC_ESIGNER_BASE_URL across entire repo
rg "PUBLIC_ESIGNER_BASE_URL" -nRepository: MetaState-Prototype-Project/prototype
Length of output: 1889
🏁 Script executed:
# Check where environment variables are configured for esigner-api
rg -type yaml -type json "PUBLIC_ESIGNER_BASE_URL" platforms/esigner-api/ 2>/dev/null || rg "PUBLIC_ESIGNER_BASE_URL" platforms/ infrastructure/Repository: MetaState-Prototype-Project/prototype
Length of output: 1846
🏁 Script executed:
# Find File type definition to understand file.id
rg -A 5 "interface File\|class File\|type File" platforms/esigner-api/src/ | head -40Repository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
# Search for environment configuration patterns in the project
fd -type f \( -name "*.example" -o -name "*.config.*" -o -name ".env.template" \) | grep -E "(esigner|env)"Repository: MetaState-Prototype-Project/prototype
Length of output: 251
🏁 Script executed:
# Check if there's a pattern of removing fallbacks similar to the learning
rg "process\.env\." platforms/esigner-api/src/services/ | grep -c "\|\|"Repository: MetaState-Prototype-Project/prototype
Length of output: 82
🏁 Script executed:
# Find File type definition
rg "interface File|type File" platforms/esigner-api/src/ -A 10 | head -50Repository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
# Check if there's a broader refactoring or issue about removing fallbacks
rg "PUBLIC_" platforms/esigner-api/src/ | grep -v "||" | head -20Repository: MetaState-Prototype-Project/prototype
Length of output: 920
🏁 Script executed:
# Look for .env configuration to see if PUBLIC_ESIGNER_BASE_URL is documented
find . -name ".env*" -o -name "*.example" | head -20Repository: MetaState-Prototype-Project/prototype
Length of output: 205
🏁 Script executed:
# Check the registry to see how PUBLIC_ESIGNER_BASE_URL is passed
sed -n '170,180p' platforms/registry/src/index.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 380
🏁 Script executed:
# Check the root .env.example
cat .env.example | grep -i "esigner\|platform" | head -20Repository: MetaState-Prototype-Project/prototype
Length of output: 213
🏁 Script executed:
# Check if there are any comments or documentation about PUBLIC_ESIGNER_BASE_URL
rg -B 2 -A 2 "PUBLIC_ESIGNER_BASE_URL" platforms/registry/src/index.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 270
🏁 Script executed:
# Verify if file.id could contain special characters by checking File usage
rg "file\.id" platforms/esigner-api/src/ -B 2 -A 2 | head -40Repository: MetaState-Prototype-Project/prototype
Length of output: 3673
🏁 Script executed:
# Check SignatureService to see how it handles missing PUBLIC_ESIGNER_BASE_URL
sed -n '70,85p' platforms/esigner-api/src/services/SignatureService.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 764
Remove localhost fallback from PUBLIC_ESIGNER_BASE_URL—environment variable must be explicitly configured in production.
The localhost default will not work in production and conflicts with the architectural pattern of removing static fallbacks from PUBLIC_* environment variables. Additionally, file.id should be URL-encoded to safely handle special characters.
This pattern appears in:
NotificationService.tslines 288, 319, 347SignatureService.tsline 75
Recommended fix
- const esignerUrl = process.env.PUBLIC_ESIGNER_BASE_URL || 'http://localhost:3004';
- const fileLink = `${esignerUrl}/files/${file.id}`;
+ const esignerUrl = process.env.PUBLIC_ESIGNER_BASE_URL;
+ if (!esignerUrl) {
+ throw new Error("PUBLIC_ESIGNER_BASE_URL is not configured");
+ }
+ const fileLink = `${esignerUrl}/files/${encodeURIComponent(file.id)}`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const esignerUrl = process.env.PUBLIC_ESIGNER_BASE_URL || 'http://localhost:3004'; | |
| const fileLink = `${esignerUrl}/files/${file.id}`; | |
| const esignerUrl = process.env.PUBLIC_ESIGNER_BASE_URL; | |
| if (!esignerUrl) { | |
| throw new Error("PUBLIC_ESIGNER_BASE_URL is not configured"); | |
| } | |
| const fileLink = `${esignerUrl}/files/${encodeURIComponent(file.id)}`; |
🤖 Prompt for AI Agents
In @platforms/esigner-api/src/services/NotificationService.ts around lines 288 -
289, PUBLIC_ESIGNER_BASE_URL must not default to localhost and file IDs must be
URL-encoded: remove the '|| "http://localhost:3004"' fallback when building
esignerUrl in NotificationService (where fileLink is constructed) and instead
validate that process.env.PUBLIC_ESIGNER_BASE_URL is present (throw or surface a
clear error if missing); when building fileLink use an encoded ID (e.g.,
encodeURIComponent(file.id)) so special characters are safe. Apply the same
change where the same pattern appears in NotificationService (other occurrences
around lines 319 and 347) and in SignatureService at the usage on line 75.
| if (!ename || !session || !signature) { | ||
| // Add a small delay to allow URL to fully parse before showing error | ||
| await new Promise(resolve => setTimeout(resolve, 500)); | ||
| // Re-check one more time after delay | ||
| const finalParams = new URLSearchParams(window.location.search || (window.location.hash.includes('?') ? window.location.hash.substring(window.location.hash.indexOf('?') + 1) : '') || ''); | ||
| ename = finalParams.get('ename') || ename; | ||
| session = finalParams.get('session') || session; | ||
| signature = finalParams.get('signature') || signature; | ||
| if (!ename || !session || !signature) { | ||
| error = "Missing required authentication parameters"; | ||
| isLoading = false; | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Investigate the root cause instead of using delay-based retry.
Same timing/retry issue as in file-manager. The 500ms delay workaround is unreliable and should be replaced with proper URL parameter handling using SvelteKit's page store.
See the file-manager review for the detailed alternative approach.
🤖 Prompt for AI Agents
In @platforms/esigner/src/routes/(auth)/deeplink-login/+page.svelte around lines
43 - 57, The 500ms delay retry around reading ename, session, and signature is
unreliable; replace it by reading URL params from SvelteKit's page store instead
of window.location (use the page store or the load function), locate the block
that references ename, session, signature and the URLSearchParams fallback, and
change it to derive these values from $page.url.searchParams or from the load
context so you get deterministic params on navigation; remove the setTimeout/new
Promise retry and ensure isLoading/error flow uses the store-derived values.
| localStorage.setItem("esigner_auth_token", data.token); | ||
| localStorage.setItem("esigner_user", JSON.stringify(data.user)); |
There was a problem hiding this comment.
Security risk: Authentication tokens in localStorage are vulnerable to XSS.
Same security concern as in file-manager. Storing tokens in localStorage exposes them to XSS attacks.
Consider using httpOnly cookies via server-side session management instead. See the file-manager review for verification script and details.
🤖 Prompt for AI Agents
In @platforms/esigner/src/routes/(auth)/deeplink-login/+page.svelte around lines
76 - 77, The code currently calls localStorage.setItem("esigner_auth_token",
data.token) and localStorage.setItem("esigner_user", JSON.stringify(data.user)),
which exposes auth tokens to XSS; instead remove these localStorage writes and
switch to server-set httpOnly cookies: send the token to a backend login/session
endpoint (or return Set-Cookie from your existing auth endpoint) so the server
sets an httpOnly, Secure, SameSite cookie for authentication, and only store
non-sensitive UI state (if needed) in memory or in a non-persistent store;
update the +page.svelte deeplink-login flow to POST the received token (or
exchange the code) to the server session endpoint and rely on the cookie rather
than calling localStorage.setItem("esigner_auth_token", ...) or storing the full
user object in localStorage.
| // Check file size limit (5MB) | ||
| const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB in bytes | ||
| if (req.file.size > MAX_FILE_SIZE) { | ||
| return res.status(413).json({ | ||
| error: "File size exceeds 5MB limit", | ||
| maxSize: MAX_FILE_SIZE, | ||
| fileSize: req.file.size | ||
| }); | ||
| } |
There was a problem hiding this comment.
Configure Multer's limit to match the 5MB validation.
The file size validation at 5MB happens after Multer has already loaded the file into memory (up to 100MB as configured on line 6). This means rejected files still consume memory and processing resources.
🔎 Proposed fix: Update Multer configuration
Update the Multer configuration to enforce the 5MB limit directly:
const upload = multer({
- limits: { fileSize: 100 * 1024 * 1024 }, // 100MB limit
+ limits: { fileSize: 5 * 1024 * 1024 }, // 5MB limit
storage: multer.memoryStorage(),
});Then remove the redundant manual check:
- // Check file size limit (5MB)
- const MAX_FILE_SIZE = 5 * 1024 * 1024; // 5MB in bytes
- if (req.file.size > MAX_FILE_SIZE) {
- return res.status(413).json({
- error: "File size exceeds 5MB limit",
- maxSize: MAX_FILE_SIZE,
- fileSize: req.file.size
- });
- }
-Note: Multer will automatically return a 413 error when the limit is exceeded. If you need custom error handling, add Multer error middleware.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @platforms/file-manager-api/src/controllers/FileController.ts around lines 29
- 37, The Multer in-memory upload is currently configured to allow up to 100MB
while the controller enforces 5MB with a manual MAX_FILE_SIZE check (the block
that checks req.file.size and returns 413); update the Multer configuration used
for uploads (the multer instance/options used by your upload middleware) to set
the fileSize limit to 5 * 1024 * 1024 (5MB) so oversized files are rejected
before being buffered, then remove the redundant MAX_FILE_SIZE constant and the
req.file.size if-check from FileController.ts; if you need custom responses for
limit errors, add a Multer error-handling middleware to translate Multer's limit
error into your desired 413 JSON response.
| let previewUrl = $state<string | null>(null); | ||
| let showAccessModal = $state(false); | ||
| let showTagModal = $state(false); | ||
| let showRenameModal = $state(false); |
There was a problem hiding this comment.
Rename modal state added but no UI trigger found.
The state variables for the rename modal are declared correctly, but after reviewing the entire component (lines 334-556), there's no button or UI element that sets showRenameModal = true. This means users cannot access the rename functionality.
🔎 Add a rename button to the UI
Consider adding a rename button in the metadata card section. For example, add it near line 408 in the Details card header:
<div class="bg-white rounded-lg shadow-sm border border-gray-200 p-4 sm:p-6">
- <h2 class="text-lg font-semibold text-gray-900 mb-4">Details</h2>
+ <div class="flex items-center justify-between mb-4">
+ <h2 class="text-lg font-semibold text-gray-900">Details</h2>
+ {#if file.ownerId === $currentUser?.id}
+ <button
+ onclick={() => { showRenameModal = true; newDisplayName = file.displayName || file.name; }}
+ class="text-blue-600 hover:text-blue-700 text-sm font-medium"
+ >
+ Rename
+ </button>
+ {/if}
+ </div>
<dl class="space-y-3 text-sm">Also applies to: 27-27
🤖 Prompt for AI Agents
In @platforms/file-manager/src/routes/(protected)/files/[id]/+page.svelte at
line 19, The rename modal state (showRenameModal) is never set to true because
there’s no UI trigger; add a Rename button into the Details/metadata card header
(the card that displays file details) that calls an event to open the modal
(e.g., on:click={() => showRenameModal = true}) and ensure it has an accessible
label and appropriate visibility (e.g., shown only when the file is editable).
Also wire this same trigger to any existing rename handler or form used by the
modal so opening the modal initializes any required state (e.g., current
filename) before display.
| // Check for view param from notification link | ||
| const searchParams = new URLSearchParams(window.location.search); | ||
| const viewParam = searchParams.get('view'); | ||
| if (viewParam === 'shared') { | ||
| // Store flag for potential back navigation | ||
| // Could be used to return to shared view instead of my files | ||
| } |
There was a problem hiding this comment.
Incomplete view parameter handling - dead code.
The view query parameter is extracted from the URL but never used. The code reads the parameter and checks if it equals 'shared', but no state is stored and no logic branches on this value. The comments suggest this is placeholder functionality for handling navigation from notification links.
Either complete this feature by storing the flag and implementing the navigation behavior, or remove this unused code to avoid confusion:
🔎 Option 1: Remove the dead code
-
- // Check for view param from notification link
- const searchParams = new URLSearchParams(window.location.search);
- const viewParam = searchParams.get('view');
- if (viewParam === 'shared') {
- // Store flag for potential back navigation
- // Could be used to return to shared view instead of my files
- }
-
await loadFile(fileId);🔎 Option 2: Complete the feature with state management
+ let cameFromSharedView = $state(false);
+
// Check for view param from notification link
const searchParams = new URLSearchParams(window.location.search);
const viewParam = searchParams.get('view');
if (viewParam === 'shared') {
- // Store flag for potential back navigation
- // Could be used to return to shared view instead of my files
+ cameFromSharedView = true;
}Then use cameFromSharedView in your navigation logic at lines 120-128.
📝 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.
| // Check for view param from notification link | |
| const searchParams = new URLSearchParams(window.location.search); | |
| const viewParam = searchParams.get('view'); | |
| if (viewParam === 'shared') { | |
| // Store flag for potential back navigation | |
| // Could be used to return to shared view instead of my files | |
| } | |
| await loadFile(fileId); |
| // Check for view param from notification link | |
| const searchParams = new URLSearchParams(window.location.search); | |
| const viewParam = searchParams.get('view'); | |
| if (viewParam === 'shared') { | |
| // Store flag for potential back navigation | |
| // Could be used to return to shared view instead of my files | |
| } | |
| let cameFromSharedView = $state(false); | |
| // Check for view param from notification link | |
| const searchParams = new URLSearchParams(window.location.search); | |
| const viewParam = searchParams.get('view'); | |
| if (viewParam === 'shared') { | |
| cameFromSharedView = true; | |
| } |
🤖 Prompt for AI Agents
In @platforms/file-manager/src/routes/(protected)/files/[id]/+page.svelte around
lines 42 - 49, The code reads the URL "view" param but never uses it; set a
component state flag (e.g., declare let cameFromSharedView = false) and assign
cameFromSharedView = true when viewParam === 'shared', then update the
back/navigation logic referenced around lines 120-128 to conditionally navigate
back to the shared view when cameFromSharedView is true (instead of always
returning to "my files"); alternatively if you prefer to drop the feature,
remove the entire viewParam extraction block to avoid dead code.
| async function handleRenameFile() { | ||
| if (!newDisplayName.trim()) { | ||
| toast.error('Please enter a name'); | ||
| return; | ||
| } | ||
| try { | ||
| isLoading = true; | ||
| await apiClient.patch(`/api/files/${file.id}`, { | ||
| displayName: newDisplayName.trim() | ||
| }); | ||
| toast.success('File renamed successfully'); | ||
| showRenameModal = false; | ||
| await loadFile(file.id); | ||
| } catch (error) { | ||
| console.error('Failed to rename file:', error); | ||
| toast.error('Failed to rename file'); | ||
| } finally { | ||
| isLoading = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Reset newDisplayName after successful rename.
The function logic is correct, but newDisplayName is not reset after a successful rename (line 304). This means if the user opens the rename modal again, they'll see the previously entered value instead of the current file name.
🔎 Proposed fix
toast.success('File renamed successfully');
showRenameModal = false;
+ newDisplayName = '';
await loadFile(file.id);
} catch (error) {Alternatively, initialize newDisplayName with the current file name when opening the modal (see the suggested fix in the earlier comment about adding the rename button).
🤖 Prompt for AI Agents
In @platforms/file-manager/src/routes/(protected)/files/[id]/+page.svelte around
lines 292 - 312, In handleRenameFile, after a successful patch and reload
(inside the try block where toast.success, showRenameModal is set false and
await loadFile(file.id) is called), reset newDisplayName (e.g., newDisplayName =
'') so the modal won't show the previous value; alternatively or additionally,
ensure the code that opens the rename modal assigns newDisplayName =
file.displayName so the input is initialized to the current name when the modal
opens.
| onMount(async () => { | ||
| isAuthenticated.subscribe((auth) => { | ||
| if (!auth) { | ||
| goto('/auth'); | ||
| } | ||
| }); | ||
| await fetchStorageUsage(); | ||
| }); |
There was a problem hiding this comment.
Subscription leak: unsubscribe from the store on component destroy.
The isAuthenticated.subscribe() call creates a subscription that is never cleaned up, which will cause a memory leak if the component is mounted and unmounted multiple times.
🔎 Proposed fix: Properly manage the subscription lifecycle
onMount(async () => {
- isAuthenticated.subscribe((auth) => {
+ const unsubscribe = isAuthenticated.subscribe((auth) => {
if (!auth) {
goto('/auth');
}
});
await fetchStorageUsage();
+
+ return () => {
+ unsubscribe();
+ };
});Alternatively, use Svelte's auto-subscription feature with the $ prefix if you're referencing the store in the template:
- onMount(async () => {
- isAuthenticated.subscribe((auth) => {
- if (!auth) {
- goto('/auth');
- }
- });
-
+ onMount(async () => {
+ if (!$isAuthenticated) {
+ goto('/auth');
+ }
+
await fetchStorageUsage();
});📝 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.
| onMount(async () => { | |
| isAuthenticated.subscribe((auth) => { | |
| if (!auth) { | |
| goto('/auth'); | |
| } | |
| }); | |
| await fetchStorageUsage(); | |
| }); | |
| onMount(async () => { | |
| const unsubscribe = isAuthenticated.subscribe((auth) => { | |
| if (!auth) { | |
| goto('/auth'); | |
| } | |
| }); | |
| await fetchStorageUsage(); | |
| onDestroy(() => { | |
| unsubscribe(); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In @platforms/file-manager/src/routes/(protected)/storage/+page.svelte around
lines 13 - 21, The onMount block is subscribing to the isAuthenticated store
without cleaning it up; modify the onMount handler to capture the subscription
returned by isAuthenticated.subscribe and call unsubscribe in a cleanup using
Svelte's onDestroy (or return an unsubscribe function from onMount), or replace
the manual subscription with Svelte's auto-subscription ($isAuthenticated) in
the component so you no longer call isAuthenticated.subscribe; ensure the
redirect logic that calls goto('/auth') and the await fetchStorageUsage() remain
functionally equivalent.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
platforms/esigner/src/routes/(auth)/deeplink-login/+page.svelte (2)
105-105: Use consistent navigation withgoto()instead of direct window assignment.Line 71 correctly uses
goto('/files')for client-side navigation, but this line useswindow.location.href = "/", which bypasses SvelteKit's routing and forces a full page reload.🔎 Proposed fix
- onclick={() => window.location.href = "/"} + onclick={() => goto("/")}
13-42: Replace manual URL parsing with SvelteKit's page store.The multi-step fallback logic (search → hash → full URL) is fragile and timing-dependent. SvelteKit provides
$page.url.searchParamswhich offers deterministic access to URL parameters.🔎 Recommended approach
Import the page store:
import { page } from '$app/stores';Then replace the manual parsing:
// Instead of lines 13-38, use: const ename = $page.url.searchParams.get('ename'); const session = $page.url.searchParams.get('session'); const signature = $page.url.searchParams.get('signature'); const appVersion = $page.url.searchParams.get('appVersion');Alternatively, use a
loadfunction in+page.tsto extract parameters server-side before rendering.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
platforms/esigner/src/routes/(auth)/deeplink-login/+page.svelteplatforms/file-manager/src/routes/(auth)/deeplink-login/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- platforms/file-manager/src/routes/(auth)/deeplink-login/+page.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 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:
platforms/esigner/src/routes/(auth)/deeplink-login/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
platforms/esigner/src/routes/(auth)/deeplink-login/+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). (1)
- GitHub Check: build
🔇 Additional comments (1)
platforms/esigner/src/routes/(auth)/deeplink-login/+page.svelte (1)
45-59: Delay-based retry remains unreliable; use SvelteKit's page store.The 500ms retry workaround is still present. This arbitrary delay does not guarantee URL parameters are available and can fail unpredictably. As noted in previous reviews, replace this with SvelteKit's
$page.url.searchParamsfor deterministic parameter access.Likely an incorrect or invalid review comment.
| const response = await apiClient.post('/api/auth', requestBody); | ||
| if (response.data.token && response.data.user) { | ||
| login(response.data.token, response.data.user); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find platforms/esigner/src/lib/stores -name "*.ts" -o -name "*.js" -o -name "*.svelte" | head -20Repository: MetaState-Prototype-Project/prototype
Length of output: 300
🏁 Script executed:
fd "auth" platforms/esigner/src/lib/stores --type fRepository: MetaState-Prototype-Project/prototype
Length of output: 121
🏁 Script executed:
cat platforms/esigner/src/lib/stores/auth.ts 2>/dev/null || cat platforms/esigner/src/lib/stores/auth.js 2>/dev/null || echo "auth store not found in expected locations"Repository: MetaState-Prototype-Project/prototype
Length of output: 2223
🏁 Script executed:
cat platforms/esigner/src/lib/utils/axios.ts 2>/dev/null || cat platforms/esigner/src/lib/utils/axios.js 2>/dev/nullRepository: MetaState-Prototype-Project/prototype
Length of output: 781
Authentication tokens are stored in localStorage, creating an XSS vulnerability.
The login() function calls setAuthToken() (in $lib/utils/axios.ts:18) which directly stores the authentication token in localStorage via localStorage.setItem('esigner_auth_token', token). Tokens stored in localStorage are accessible to any JavaScript executing on the page, making them vulnerable to XSS attacks.
Migrate to httpOnly cookies set by the server instead. This prevents JavaScript from accessing the token, significantly improving security posture.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI Agents
In @platforms/esigner/src/lib/utils/mobile-detection.ts:
- Around line 1-6: The isMobileDevice function mixes user-agent and viewport
checks causing inconsistent results; split into two clear helpers (e.g.,
isMobileByUserAgent and isMobileByViewport) and update callers to choose the
appropriate strategy or add an optional parameter to isMobileDevice(mode:
'ua'|'viewport'|'hybrid') that selects the detection method; ensure
isMobileByUserAgent uses the existing regex against navigator.userAgent and
isMobileByViewport uses a configurable breakpoint (default 768) against
window.innerWidth, and remove the current unconditional OR combination in
isMobileDevice so behavior is predictable.
- Around line 8-10: The getDeepLinkUrl passthrough function should be removed:
delete the exported function getDeepLinkUrl(qrData: string) from
mobile-detection.ts, remove its import wherever used (e.g., the component that
imports getDeepLinkUrl), and replace calls to getDeepLinkUrl(qrData) with the
direct qrData value (or the original variable) since no
transformation/validation occurs; update imports/exports accordingly to avoid
unused-import errors.
In @platforms/file-manager/src/routes/(protected)/files/+page.svelte:
- Around line 221-227: The error handler in the if (error.response?.status ===
413) block reads errorData.available without checking it; update the conditional
that handles Storage quota to verify errorData?.available is present and numeric
before using (e.g. Number.isFinite(errorData.available)) and only compute
(errorData.available / 1024 / 1024).toFixed(2) when valid; otherwise fall back
to a safe message like "available space unknown" or omit the numeric value in
the toast.error call, keeping the existing toast.error branches and the
errorData.error?.includes('Storage quota') check.
- Around line 329-356: The rename handler calls updateFile but it isn't
imported, causing a runtime ReferenceError; update the imports at the top of the
file to include updateFile from the files store so handleRename can call
updateFile(itemToRename.id, newName.trim()); while here the folder rename uses
apiClient.patch directly, consider adding and importing an updateFolder function
in the folders store and replace the
apiClient.patch(`/api/folders/${itemToRename.id}`, { name: ... }) call with
updateFolder(itemToRename.id, newName.trim()) for consistency with
deleteFile/moveFile usage and to keep API logic centralized.
- Around line 194-196: The code currently checks `currentFolderId !== undefined`
(redundant since `currentFolderId` is initialized to null) and appends the
literal string 'null', which is fragile; change the logic around
`formData.append('folderId', ...)` to only append when `currentFolderId` is not
null or undefined (e.g., `if (currentFolderId != null)
formData.append('folderId', String(currentFolderId));`) so you omit the field
when there is no folder instead of sending the string 'null'.
🧹 Nitpick comments (1)
platforms/file-manager/src/routes/(protected)/files/+page.svelte (1)
129-143: Deep link handling looks good, but consider using SvelteKit's built-in API.The URL parameter handling for navigation from notifications is implemented correctly. However,
window.location.searchcould be replaced with$page.url.searchParamsfor better SvelteKit integration.🔎 Optional refactor to use $page.url.searchParams
- const searchParams = new URLSearchParams(window.location.search); - const viewParam = searchParams.get('view'); - const folderIdParam = searchParams.get('folderId'); + const viewParam = $page.url.searchParams.get('view'); + const folderIdParam = $page.url.searchParams.get('folderId');
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
platforms/esigner/src/lib/utils/mobile-detection.tsplatforms/esigner/src/routes/(protected)/files/[id]/+page.svelteplatforms/file-manager/src/routes/(protected)/files/+page.svelte
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-04-16T18:04:28.983Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 99
File: infrastructure/w3id/tests/w3id.test.ts:48-57
Timestamp: 2025-04-16T18:04:28.983Z
Learning: In test files, suggestions to add defensive coding patterns (like extra null checks) are often unnecessary, as tests are designed to fail explicitly when expectations aren't met. The test in `infrastructure/w3id/tests/w3id.test.ts` validates that `id.logs` is an instance of `IDLogManager` before accessing its repository property.
Applied to files:
platforms/file-manager/src/routes/(protected)/files/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
platforms/file-manager/src/routes/(protected)/files/+page.svelteplatforms/esigner/src/routes/(protected)/files/[id]/+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). (1)
- GitHub Check: build
🔇 Additional comments (10)
platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte (1)
286-596: Excellent responsive design implementation!The mobile-adaptive layout changes throughout this component are well-executed:
- Proper responsive container sizing (
min-hinstead of fixedh)- Mobile-first padding and typography scales (
px-4 sm:px-6,text-xl sm:text-2xl)- Appropriate layout shifts (
flex-col lg:flex-row,hidden lg:flex)- Text overflow handling with
break-words,truncate, andbreak-all- Mobile-specific preview button for accessing content on smaller screens
- Conditional sign flow UI (deep link button on mobile, QR code on desktop)
The responsive patterns align with Tailwind best practices and provide a smooth experience across device sizes.
platforms/file-manager/src/routes/(protected)/files/+page.svelte (9)
82-86: LGTM! State variables properly declared for rename functionality.The state variables for the rename feature are well-defined and follow the existing pattern in the component.
102-103: LGTM! Upload progress tracking state added.The
uploadProgressandisUploadingstate variables enable real-time upload progress feedback in the UI.
180-186: LGTM! Client-side file size validation prevents unnecessary uploads.The 5MB size check with user-friendly error messaging improves UX by catching oversized files before upload.
202-207: LGTM! Upload progress tracking implemented correctly.The
onUploadProgresshandler with percentage calculation provides real-time feedback to users.
317-327: LGTM! Rename modal initialization is correct.The
openRenameModalfunction properly captures item metadata and initializes the modal state.
725-763: LGTM! Responsive design improvements enhance mobile UX.The responsive layout changes with conditional text and spacing ensure the interface works well on different screen sizes.
888-900: LGTM! Rename action properly integrated into dropdown menu.The rename button follows the established pattern for dropdown actions and is correctly scoped to owned items.
1020-1049: LGTM! Upload progress UI provides excellent user feedback.The progress bar, percentage display, and disabled button states during upload create a polished user experience.
1282-1319: LGTM! Rename modal implementation is complete and accessible.The rename modal includes proper keyboard support (Enter to submit), validation (disabled when empty), and loading states. The implementation follows the pattern of other modals in the component.
| export function isMobileDevice(): boolean { | ||
| if (typeof window === 'undefined') return false; | ||
|
|
||
| return /Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent) || | ||
| (window.innerWidth <= 768); | ||
| } |
There was a problem hiding this comment.
Mixed detection methods may cause inconsistent mobile identification.
Combining user agent regex with viewport width (<= 768px) means a resized desktop browser will be treated as mobile, while a tablet in landscape mode might not be. This can lead to unexpected UX inconsistencies.
Consider separating these concerns or choosing one primary detection method for more predictable behavior.
🤖 Prompt for AI Agents
In @platforms/esigner/src/lib/utils/mobile-detection.ts around lines 1 - 6, The
isMobileDevice function mixes user-agent and viewport checks causing
inconsistent results; split into two clear helpers (e.g., isMobileByUserAgent
and isMobileByViewport) and update callers to choose the appropriate strategy or
add an optional parameter to isMobileDevice(mode: 'ua'|'viewport'|'hybrid') that
selects the detection method; ensure isMobileByUserAgent uses the existing regex
against navigator.userAgent and isMobileByViewport uses a configurable
breakpoint (default 768) against window.innerWidth, and remove the current
unconditional OR combination in isMobileDevice so behavior is predictable.
| export function getDeepLinkUrl(qrData: string): string { | ||
| return qrData; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove unnecessary passthrough function.
The getDeepLinkUrl function simply returns its input without any transformation, validation, or logic. This adds no value and creates unnecessary indirection.
🔎 Proposed fix to remove the passthrough function
Remove the function from this file:
-export function getDeepLinkUrl(qrData: string): string {
- return qrData;
-}Then update the import and usage in platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte:
-import { isMobileDevice, getDeepLinkUrl } from '$lib/utils/mobile-detection';
+import { isMobileDevice } from '$lib/utils/mobile-detection';And replace the function call with direct usage:
<a
- href={getDeepLinkUrl(signingSession.qrData)}
+ href={signingSession.qrData}
class="w-full px-6 py-3 bg-blue-600 text-white rounded-lg hover:bg-blue-700 transition-colors text-center font-medium"
>📝 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.
| export function getDeepLinkUrl(qrData: string): string { | |
| return qrData; | |
| } |
🤖 Prompt for AI Agents
In @platforms/esigner/src/lib/utils/mobile-detection.ts around lines 8 - 10, The
getDeepLinkUrl passthrough function should be removed: delete the exported
function getDeepLinkUrl(qrData: string) from mobile-detection.ts, remove its
import wherever used (e.g., the component that imports getDeepLinkUrl), and
replace calls to getDeepLinkUrl(qrData) with the direct qrData value (or the
original variable) since no transformation/validation occurs; update
imports/exports accordingly to avoid unused-import errors.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI Agents
In @platforms/file-manager/src/routes/(protected)/files/+page.svelte:
- Around line 221-227: The code assumes errorData.available exists when handling
the 413 Storage quota branch; add a defensive check on errorData and
errorData.available (e.g., typeof errorData?.available === 'number' &&
Number.isFinite(errorData.available')) before computing bytes->MB and calling
toast.error, and if the value is missing/invalid show a fallback message like
"Storage quota exceeded (available space unknown)" or use a safe default value;
update the block around errorData and the Storage quota branch in the file
handling upload errors to perform this validation before formatting the MB
string.
- Around line 194-196: The condition checking currentFolderId !== undefined is
redundant because currentFolderId is initialized to null; update the logic
around formData.append('folderId', ...) in the submit routine to handle null
explicitly: if currentFolderId is not null append the actual ID, otherwise
either omit the folderId field (preferred) or append a backend-agreed sentinel;
adjust the check referencing currentFolderId and the formData.append call so it
uses currentFolderId !== null (or skips appending) rather than !== undefined.
- Around line 754-755: The Tailwind classes use a non-existent xs: breakpoint so
both texts can appear; update the responsive utilities to a valid breakpoint
(e.g., replace xs:inline / xs:hidden with sm:inline / sm:hidden) and ensure the
first span uses hidden sm:inline and the second uses sm:hidden so "Folder" shows
on small screens and "New Folder" shows at sm and up (refer to the spans
containing "New Folder" and "Folder" and their class attributes).
In @platforms/file-manager/src/routes/(protected)/storage/+page.svelte:
- Around line 98-100: The decorative SVG icons (e.g., the <svg class="w-4 h-4
sm:w-6 sm:h-6 text-blue-600"> instances shown) are missing accessibility
markers; add aria-hidden="true" to each purely decorative <svg> element in this
component (and the other listed instances) so screen readers ignore them, and
leave accessible icons unchanged.
🧹 Nitpick comments (6)
platforms/file-manager-api/src/services/FileService.ts (1)
345-345: Consider making the storage limit configurable.The 1GB storage limit is hardcoded. For better flexibility, consider moving this to a configuration file or environment variable, especially if different users or tiers might have different quotas in the future.
🔎 Proposed refactor to use environment variable
In your environment configuration:
// config.ts or similar export const STORAGE_LIMIT = process.env.STORAGE_LIMIT_BYTES ? parseInt(process.env.STORAGE_LIMIT_BYTES) : 1073741824; // 1GB defaultThen update the method:
+import { STORAGE_LIMIT } from '../config'; + async getUserStorageUsage(userId: string): Promise<{ used: number; limit: number; fileCount: number; folderCount: number }> { const FOLDER_SIZE = 4 * 1024; // 4KB per folder // ... existing query code ... const used = fileSize + folderSize; - const limit = 1073741824; // 1GB in bytes + const limit = STORAGE_LIMIT; return { used, limit, fileCount, folderCount }; }platforms/file-manager/src/routes/(protected)/storage/+page.svelte (1)
33-33: Add defensive check for division by zero.While the backend currently guarantees a non-zero limit (1GB), the percentage calculation doesn't guard against division by zero. If the limit is ever 0, this will produce
InfinityorNaN, breaking the UI.🔎 Proposed defensive fix
-percentage = (used / limit) * 100; +percentage = limit > 0 ? (used / limit) * 100 : 0;platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte (1)
137-202: Consider removing verbose console logs before production.The SSE connection improvements are solid:
- Properly closes existing connections before creating new ones (prevents connection leaks)
- Handles all terminal states (signed, expired, security_violation) with cleanup
- Wraps JSON parsing in try-catch for error resilience
However, the extensive console.log statements (lines 147, 150, 155, 158, 161, 174, 182, 191, 196) will clutter production logs. Consider:
- Removing these logs before merging to production
- Or using a debug flag:
if (DEBUG) console.log(...)- Or using a proper logging library with configurable levels
Example: Replace console.log with conditional debug logging
+const DEBUG_SSE = import.meta.env.DEV; + function watchSigningSession(sessionId: string) { // Close existing connection if any if (sseConnection) { sseConnection.close(); sseConnection = null; } const baseUrl = PUBLIC_ESIGNER_BASE_URL || 'http://localhost:3004'; const sseUrl = new URL(`/api/signatures/session/${sessionId}`, baseUrl).toString(); - console.log('Starting SSE connection to:', sseUrl); + if (DEBUG_SSE) console.log('[SSE] Starting connection to:', sseUrl); sseConnection = new EventSource(sseUrl); sseConnection.onopen = () => { - console.log('SSE connection opened'); + if (DEBUG_SSE) console.log('[SSE] Connection opened'); }; sseConnection.onmessage = (e) => { - console.log('SSE message received:', e.data); + if (DEBUG_SSE) console.log('[SSE] Message received:', e.data); // ... rest of the handler }; }platforms/file-manager/src/routes/(protected)/files/+page.svelte (3)
129-142: Consider cleaning URL params after reading to prevent stale navigation state.If the user refreshes the page after navigating from a notification link, the same view/folder will be reapplied. This may or may not be desired behavior. Additionally, consider using SvelteKit's
$page.url.searchParamsfor more idiomatic param access.🔎 Optional: Clean URL after reading params
// Then navigate to folder if specified if (folderIdParam) { currentFolderId = folderIdParam; } + + // Clean URL params to prevent stale state on refresh + if (viewParam || folderIdParam) { + const url = new URL(window.location.href); + url.searchParams.delete('view'); + url.searchParams.delete('folderId'); + window.history.replaceState({}, '', url.toString()); + }
317-356: Good implementation of rename functionality.The rename workflow correctly handles both files and folders with proper validation, error handling, and state cleanup.
Minor consistency note: Line 342 uses a direct
apiClient.patchcall for folders while files use the store'supdateFile. Consider addingupdateFolderto the folders store for consistency with other operations (deleteFolder, moveFolder).
1288-1325: LGTM!The Rename modal follows established patterns with proper validation, Enter-key submission, and loading state handling.
Optional enhancement: Consider adding
autofocusto the input for immediate keyboard interaction when the modal opens.🔎 Optional: Add autofocus to rename input
<input type="text" bind:value={newName} placeholder="Enter new name" + autofocus class="w-full px-4 py-2 border border-gray-300 rounded-lg mb-4"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
platforms/esigner/src/routes/(protected)/files/+page.svelteplatforms/esigner/src/routes/(protected)/files/[id]/+page.svelteplatforms/file-manager-api/src/services/FileService.tsplatforms/file-manager/src/routes/(protected)/files/+page.svelteplatforms/file-manager/src/routes/(protected)/storage/+page.svelte
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-04-16T18:04:28.983Z
Learnt from: coodos
Repo: MetaState-Prototype-Project/prototype PR: 99
File: infrastructure/w3id/tests/w3id.test.ts:48-57
Timestamp: 2025-04-16T18:04:28.983Z
Learning: In test files, suggestions to add defensive coding patterns (like extra null checks) are often unnecessary, as tests are designed to fail explicitly when expectations aren't met. The test in `infrastructure/w3id/tests/w3id.test.ts` validates that `id.logs` is an instance of `IDLogManager` before accessing its repository property.
Applied to files:
platforms/file-manager/src/routes/(protected)/files/+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 uses the new "runes" API including $props() for handling component props instead of the export let syntax from Svelte 3/4.
Applied to files:
platforms/file-manager/src/routes/(protected)/files/+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:
platforms/file-manager/src/routes/(protected)/files/+page.svelte
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
platforms/file-manager/src/routes/(protected)/files/+page.svelteplatforms/esigner/src/routes/(protected)/files/[id]/+page.svelteplatforms/file-manager/src/routes/(protected)/storage/+page.svelteplatforms/esigner/src/routes/(protected)/files/+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). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (14)
platforms/file-manager/src/routes/(protected)/storage/+page.svelte (1)
15-23: Subscription leak: Clean up the store subscription.The
isAuthenticated.subscribe()creates a subscription that is never cleaned up, causing a memory leak. This issue was previously flagged and should be addressed.Likely an incorrect or invalid review comment.
platforms/esigner/src/routes/(protected)/files/+page.svelte (2)
1-167: Inconsistency between AI summary and actual code changes.The AI summary claims this file adds:
- Imports for
updateFile,isMobileDevice, andgetDeepLinkUrl- Rename functionality with
openRenameModal()andhandleRename()- Client-side upload enhancements with
isUploading,uploadProgress, and 5MB limit enforcement- URL query parameter handling for view switching and folder navigation
- Mobile-specific deep-link behavior
However, the annotated code shows only responsive UI styling changes (Tailwind breakpoint classes like
sm:,lg:,break-words,flex-col/flex-rowswitches, etc.). None of the functional changes described in the summary are present in the provided code.
43-165: LGTM! Responsive improvements enhance mobile UX.The responsive styling changes improve the mobile experience by:
- Using mobile-first breakpoints (
sm:,lg:) for adaptive layouts- Switching flex direction appropriately (
flex-colon mobile,sm:flex-rowon larger screens)- Handling text overflow with
break-wordsandtruncate- Making buttons full-width on mobile (
w-full sm:w-auto)- Adjusting padding and text sizes responsively
These changes align with the PR's stated goal of "qol esigner and file manager improvements" and follow Tailwind best practices.
platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte (3)
23-63: Cleanup pattern improved, but verify auth check timing.The refactor from async onMount to a non-async version with explicit cleanup is a good improvement. The cleanup function properly unsubscribes from stores and closes the SSE connection.
However, verify that the auth check at line 24 doesn't cause issues: if authentication fails during the async data loading (lines 38-41), the component will navigate away but the async IIFE will continue executing in the background. Consider adding an early return check or using an abort signal if this becomes a problem.
336-420: LGTM! Mobile-responsive layout improvements.The layout refactoring provides a good mobile experience:
- Changed
h-[calc(100vh-5rem)]tomin-h-[calc(100vh-5rem)]allows content to expand naturally on mobile- Preview panel hidden on mobile (
hidden lg:flex), accessible via button that opens in new tab- Sidebar becomes full-width on mobile (
flex-1 lg:flex-[0.3])- Responsive padding, text sizes, and spacing throughout
The flex-based layout with breakpoints follows Tailwind best practices.
604-647: Mobile-aware signing flow enhances UX.The conditional rendering based on mobile detection provides an optimal experience for each platform:
- Mobile: Direct deep link button opens the eID Wallet app natively
- Desktop: QR code to scan with a mobile device
- Instructions adapt to the detected platform
This pattern is appropriate for mobile authentication flows. Ensure the mobile-detection accuracy has been tested across various devices (see earlier verification request for the mobile-detection utility).
platforms/file-manager/src/routes/(protected)/files/+page.svelte (8)
7-7: LGTM!The
updateFileimport has been added, resolving the previously flagged runtime error for the rename functionality.
82-103: LGTM!New state variables for rename modal and upload progress tracking follow the existing patterns and use Svelte 5 runes correctly.
180-186: LGTM!Good addition of client-side file size validation with a clear error message showing the actual file size. This prevents unnecessary upload attempts for oversized files.
649-652: LGTM!Simple and effective truncation utility. The combination with
titleattributes in the UI ensures users can still see full names on hover.
773-789: LGTM!Good responsive improvements:
flex-wrapallows breadcrumbs to wrap on narrow screens, and thetitleattribute ensures full folder names are accessible via hover.
817-872: LGTM!Well-implemented responsive table design: hiding non-essential columns (Size, Modified) on mobile while maintaining the core file browsing experience. The
hidden sm:table-cellpattern is the correct approach.
894-906: LGTM!The Rename option follows the established pattern for dropdown actions with proper authorization checks (owner-only), consistent styling, and appropriate event handling.
1026-1054: LGTM!Excellent upload progress UX implementation: real-time percentage display, smooth progress bar animation, and proper button state management to prevent double-submissions during upload.
| import { qrcode } from 'svelte-qrcode-action'; | ||
| import { PUBLIC_ESIGNER_BASE_URL } from '$env/static/public'; | ||
| import { toast } from '$lib/stores/toast'; | ||
| import { isMobileDevice, getDeepLinkUrl } from '$lib/utils/mobile-detection'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate the mobile-detection module
fd -e ts -e js "mobile-detection"Repository: MetaState-Prototype-Project/prototype
Length of output: 539
🏁 Script executed:
# Find all TypeScript/JavaScript files in the utils directory
fd -e ts -e js . "src/lib/utils" 2>/dev/null | head -20Repository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
# Search for the file path mentioned in the import
find . -path "*/lib/utils/mobile-detection*" -type fRepository: MetaState-Prototype-Project/prototype
Length of output: 555
🏁 Script executed:
# Examine the mobile-detection implementation for esigner
cat platforms/esigner/src/lib/utils/mobile-detection.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 386
🏁 Script executed:
# Check the +page.svelte file to understand how these functions are used
grep -n -B2 -A2 "isMobileDevice\|getDeepLinkUrl" platforms/esigner/src/routes/\(protected\)/files/\[id\]/+page.svelteRepository: MetaState-Prototype-Project/prototype
Length of output: 1176
🏁 Script executed:
# Search for where signingSession.qrData is set or initialized
rg "signingSession.*qrData|qrData.*=" platforms/esigner/src/routes/\(protected\)/files/\[id\]/+page.svelte -B2 -A2Repository: MetaState-Prototype-Project/prototype
Length of output: 497
🏁 Script executed:
# Search for signingSession definition
rg "signingSession\s*[=:]" platforms/esigner/src/routes/\(protected\)/files/\[id\]/+page.svelte -B5 -A5Repository: MetaState-Prototype-Project/prototype
Length of output: 801
🏁 Script executed:
# Check for deep link scheme configuration or manifest files
find platforms/esigner -name "svelte.config.js" -o -name "vite.config.ts" -o -name "android.json" -o -name "Info.plist" -o -name "AndroidManifest.xml" | head -10Repository: MetaState-Prototype-Project/prototype
Length of output: 148
🏁 Script executed:
# Search for createSigningSession implementation
rg "function createSigningSession|const createSigningSession|export.*createSigningSession" platforms/esigner -A10Repository: MetaState-Prototype-Project/prototype
Length of output: 992
🏁 Script executed:
# Search for API calls that might return qrData
rg "qrData" platforms/esigner -B3 -A3Repository: MetaState-Prototype-Project/prototype
Length of output: 5564
Implement proper deep link formatting in getDeepLinkUrl().
The isMobileDevice() function is solid, but getDeepLinkUrl() is currently a passthrough stub that returns qrData unchanged. Since the backend appears to return a URI string, either:
- If the backend returns a deep link scheme (e.g.,
eid-wallet://...), the function should validate and potentially transform it as needed - If the backend returns an HTTP URL, the function should convert it to the proper deep link scheme for the eID Wallet app
The function name suggests it performs formatting, but the current implementation adds no value. Clarify the contract with the backend API and implement actual deep link formatting if needed. Also verify the deep link scheme is properly registered in the native eID Wallet app manifest.
| if (currentFolderId !== undefined) { | ||
| formData.append('folderId', currentFolderId || 'null'); | ||
| } |
There was a problem hiding this comment.
Redundant condition: currentFolderId !== undefined is always true.
Since currentFolderId is initialized to null (line 75), the check !== undefined always passes. Additionally, appending the string 'null' depends on backend-specific parsing.
🔎 Suggested fix
- if (currentFolderId !== undefined) {
- formData.append('folderId', currentFolderId || 'null');
+ if (currentFolderId) {
+ formData.append('folderId', currentFolderId);
}📝 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.
| if (currentFolderId !== undefined) { | |
| formData.append('folderId', currentFolderId || 'null'); | |
| } | |
| if (currentFolderId) { | |
| formData.append('folderId', currentFolderId); | |
| } |
🤖 Prompt for AI Agents
In @platforms/file-manager/src/routes/(protected)/files/+page.svelte around
lines 194 - 196, The condition checking currentFolderId !== undefined is
redundant because currentFolderId is initialized to null; update the logic
around formData.append('folderId', ...) in the submit routine to handle null
explicitly: if currentFolderId is not null append the actual ID, otherwise
either omit the folderId field (preferred) or append a backend-agreed sentinel;
adjust the check referencing currentFolderId and the formData.append call so it
uses currentFolderId !== null (or skips appending) rather than !== undefined.
| if (error.response?.status === 413) { | ||
| const errorData = error.response.data; | ||
| if (errorData.error?.includes('Storage quota')) { | ||
| toast.error(`Storage quota exceeded. You have ${(errorData.available / 1024 / 1024).toFixed(2)}MB available.`); | ||
| } else { | ||
| toast.error('File size exceeds 5MB limit'); | ||
| } |
There was a problem hiding this comment.
Add defensive check for errorData.available.
Accessing errorData.available without verifying its existence could cause issues if the API response structure differs from expected.
🔎 Suggested fix
if (error.response?.status === 413) {
const errorData = error.response.data;
- if (errorData.error?.includes('Storage quota')) {
+ if (errorData.error?.includes('Storage quota') && typeof errorData.available === 'number') {
toast.error(`Storage quota exceeded. You have ${(errorData.available / 1024 / 1024).toFixed(2)}MB available.`);
} else {
toast.error('File size exceeds 5MB limit');
}📝 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.
| if (error.response?.status === 413) { | |
| const errorData = error.response.data; | |
| if (errorData.error?.includes('Storage quota')) { | |
| toast.error(`Storage quota exceeded. You have ${(errorData.available / 1024 / 1024).toFixed(2)}MB available.`); | |
| } else { | |
| toast.error('File size exceeds 5MB limit'); | |
| } | |
| if (error.response?.status === 413) { | |
| const errorData = error.response.data; | |
| if (errorData.error?.includes('Storage quota') && typeof errorData.available === 'number') { | |
| toast.error(`Storage quota exceeded. You have ${(errorData.available / 1024 / 1024).toFixed(2)}MB available.`); | |
| } else { | |
| toast.error('File size exceeds 5MB limit'); | |
| } |
🤖 Prompt for AI Agents
In @platforms/file-manager/src/routes/(protected)/files/+page.svelte around
lines 221 - 227, The code assumes errorData.available exists when handling the
413 Storage quota branch; add a defensive check on errorData and
errorData.available (e.g., typeof errorData?.available === 'number' &&
Number.isFinite(errorData.available')) before computing bytes->MB and calling
toast.error, and if the value is missing/invalid show a fallback message like
"Storage quota exceeded (available space unknown)" or use a safe default value;
update the block around errorData and the Storage quota branch in the file
handling upload errors to perform this validation before formatting the MB
string.
| <span class="hidden xs:inline">New Folder</span> | ||
| <span class="xs:hidden">Folder</span> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Finding Tailwind configuration files ==="
fd -t f \( -name "tailwind.config.*" -o -name "tailwind.js" \) 2>/dev/null
echo ""
echo "=== Searching for tailwind config in common locations ==="
for f in tailwind.config.js tailwind.config.ts tailwind.config.cjs tailwind.config.mjs postcss.config.js postcss.config.ts; do
if [ -f "$f" ]; then
echo "Found: $f"
fi
done
echo ""
echo "=== Checking package.json for tailwind/tailwindcss info ==="
if [ -f "package.json" ]; then
jq '.devDependencies, .dependencies' package.json 2>/dev/null | grep -i tailwind || echo "No explicit tailwind dependency info"
fi
echo ""
echo "=== Searching for xs breakpoint definition ==="
rg -n "xs\s*:" --type js --type ts --type json 2>/dev/null | grep -i "breakpoint\|theme\|screen" | head -10
echo ""
echo "=== Looking for theme configuration with breakpoints ==="
rg -B2 -A2 "breakpoint" --type js --type ts --type json 2>/dev/null | head -30Repository: MetaState-Prototype-Project/prototype
Length of output: 388
🏁 Script executed:
#!/bin/bash
echo "=== Finding SvelteKit config ==="
fd -t f \( -name "svelte.config.*" -o -name "postcss.config.*" \) 2>/dev/null
echo ""
echo "=== Checking svelte.config.js in common locations ==="
for f in svelte.config.js svelte.config.ts; do
if [ -f "$f" ]; then
echo "=== Contents of $f ==="
cat -n "$f"
fi
done
echo ""
echo "=== Looking at the reviewed file to understand context ==="
if [ -f "platforms/file-manager/src/routes/(protected)/files/+page.svelte" ]; then
echo "=== File exists, checking line count ==="
wc -l "platforms/file-manager/src/routes/(protected)/files/+page.svelte"
echo ""
echo "=== Checking imports and style sections (first 50 lines) ==="
head -n 50 "platforms/file-manager/src/routes/(protected)/files/+page.svelte"
fi
echo ""
echo "=== Searching for xs breakpoint in entire codebase ==="
rg -n "xs:" 2>/dev/null | head -20
echo ""
echo "=== Searching for Tailwind class usage to confirm Tailwind is used ==="
rg -n "class=\".*(?:sm:|md:|lg:)" 2>/dev/null | head -10Repository: MetaState-Prototype-Project/prototype
Length of output: 7221
🏁 Script executed:
#!/bin/bash
echo "=== Looking for tailwind config in file-manager platform ==="
fd -t f -e js -e ts -e json "tailwind\|postcss" platforms/file-manager/ 2>/dev/null
echo ""
echo "=== Checking platforms/file-manager directory structure ==="
ls -la platforms/file-manager/ 2>/dev/null | head -20
echo ""
echo "=== Searching for xs: usage in file-manager platform ==="
rg -n "xs:" platforms/file-manager/ 2>/dev/null | head -20
echo ""
echo "=== Checking for app.css or global styles in file-manager ==="
fd -t f -e css platforms/file-manager/ 2>/dev/null | head -10
echo ""
echo "=== Looking at svelte.config.js or vite config in file-manager ==="
fd -t f \( -name "svelte.config.*" -o -name "vite.config.*" \) platforms/file-manager/ 2>/dev/null
</parameter>Repository: MetaState-Prototype-Project/prototype
Length of output: 1240
🏁 Script executed:
#!/bin/bash
echo "=== Content of platforms/file-manager/svelte.config.js ==="
cat -n platforms/file-manager/svelte.config.js
echo ""
echo "=== Content of platforms/file-manager/vite.config.ts ==="
cat -n platforms/file-manager/vite.config.ts
echo ""
echo "=== Checking platforms/file-manager/package.json for tailwind ==="
if [ -f "platforms/file-manager/package.json" ]; then
jq '.devDependencies, .dependencies' platforms/file-manager/package.json
fi
echo ""
echo "=== Checking root-level tailwind.config.js ==="
if [ -f "tailwind.config.js" ]; then
head -50 tailwind.config.js
else
echo "No root tailwind.config.js"
fi
echo ""
echo "=== Checking for app.css in file-manager src ==="
fd -t f -e css platforms/file-manager/src/
echo ""
echo "=== Checking app.html for style tags ==="
if [ -f "platforms/file-manager/src/app.html" ]; then
cat -n platforms/file-manager/src/app.html
fiRepository: MetaState-Prototype-Project/prototype
Length of output: 2839
🏁 Script executed:
#!/bin/bash
echo "=== Checking for any CSS that might define xs breakpoint ==="
fd -t f -e css -e scss -e postcss platforms/file-manager/src/ 2>/dev/null
echo ""
echo "=== Checking for global styles in layout files ==="
fd -t f "+layout.svelte" platforms/file-manager/src/ 2>/dev/null | xargs grep -l "style\|css" 2>/dev/null
echo ""
echo "=== Searching for theme or config in +layout files ==="
fd -t f "+layout.svelte" platforms/file-manager/src/ 2>/dev/null | head -3 | xargs cat -n 2>/dev/null | head -100
echo ""
echo "=== Checking if xs: appears elsewhere in file-manager (confirming it's only in lines 754-755) ==="
rg -c "xs:" platforms/file-manager/src/routes/\(protected\)/files/+page.svelte
</parameter>Repository: MetaState-Prototype-Project/prototype
Length of output: 475
🏁 Script executed:
#!/bin/bash
echo "=== Confirming xs: appears only at lines 754-755 ==="
rg -n "xs:" platforms/file-manager/src/routes/\(protected\)/files/+page.svelte 2>/dev/null
echo ""
echo "=== Checking the exact context around those lines ==="
sed -n '750,760p' platforms/file-manager/src/routes/\(protected\)/files/+page.svelte
echo ""
echo "=== Verifying no custom tailwind config exists at all ==="
find platforms/file-manager -name "*.config.*" -o -name "*tailwind*" 2>/dev/null | grep -v node_modules | grep -v .svelte-kit
</parameter>Repository: MetaState-Prototype-Project/prototype
Length of output: 1093
🌐 Web query:
Tailwind CSS v4 default breakpoints configuration
💡 Result:
Tailwind CSS v4 default breakpoints (min-width):
- sm: 40rem — 640px. [1]
- md: 48rem — 768px. [1]
- lg: 64rem — 1024px. [1]
- xl: 80rem — 1280px. [1]
- 2xl: 96rem — 1536px. [1]
(You can customize these via the v4 @theme --breakpoint-* CSS variables.) [1][2]
Sources:
[1] Tailwind CSS — Responsive design / Breakpoints.
[2] Tailwind v4 breakpoints overview / examples.
xs: is not a default Tailwind breakpoint and is not configured in this project — these classes will not work.
Tailwind CSS v4's default breakpoints are sm, md, lg, xl, and 2xl. The project uses Tailwind v4 with no custom configuration. Without an xs: breakpoint defined, the xs:hidden and xs:inline classes will have no effect, and both "New Folder" and "Folder" text will render simultaneously.
🤖 Prompt for AI Agents
In @platforms/file-manager/src/routes/(protected)/files/+page.svelte around
lines 754 - 755, The Tailwind classes use a non-existent xs: breakpoint so both
texts can appear; update the responsive utilities to a valid breakpoint (e.g.,
replace xs:inline / xs:hidden with sm:inline / sm:hidden) and ensure the first
span uses hidden sm:inline and the second uses sm:hidden so "Folder" shows on
small screens and "New Folder" shows at sm and up (refer to the spans containing
"New Folder" and "Folder" and their class attributes).
| <svg class="w-4 h-4 sm:w-6 sm:h-6 text-blue-600" fill="none" stroke="currentColor" viewBox="0 0 24 24"> | ||
| <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M7 16a4 4 0 01-.88-7.903A5 5 0 1115.9 6L16 6a5 5 0 011 9.9M15 13l-3-3m0 0l-3 3m3-3v12" /> | ||
| </svg> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add aria-hidden to decorative SVG icons.
The SVG icons throughout the component are decorative (paired with text labels) but lack aria-hidden="true" attributes. Screen readers will attempt to announce them, which may create a confusing experience. Mark purely decorative icons as hidden from assistive technology.
🔎 Example fix for decorative icons
-<svg class="w-4 h-4 sm:w-6 sm:h-6 text-blue-600" fill="none" stroke="currentColor" viewBox="0 0 24 24">
+<svg class="w-4 h-4 sm:w-6 sm:h-6 text-blue-600" fill="none" stroke="currentColor" viewBox="0 0 24 24" aria-hidden="true">
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M7 16a4 4 0 01-.88-7.903A5 5 0 1115.9 6L16 6a5 5 0 011 9.9M15 13l-3-3m0 0l-3 3m3-3v12" />
</svg>Apply this pattern to all decorative SVG icons in the component.
Also applies to: 112-114, 126-128, 140-142, 155-157, 169-171, 187-189, 196-198, 206-208
🤖 Prompt for AI Agents
In @platforms/file-manager/src/routes/(protected)/storage/+page.svelte around
lines 98 - 100, The decorative SVG icons (e.g., the <svg class="w-4 h-4 sm:w-6
sm:h-6 text-blue-600"> instances shown) are missing accessibility markers; add
aria-hidden="true" to each purely decorative <svg> element in this component
(and the other listed instances) so screen readers ignore them, and leave
accessible icons unchanged.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte (1)
137-202: Solid SSE lifecycle management with room for refinement.The enhanced SSE connection handling properly prevents leaks, handles multiple status types, and ensures cleanup. The try-catch around message parsing and consistent connection closure across all paths are excellent additions.
Consider two optional refinements:
- Extract message handler: The
onmessagehandler spans ~40 lines and manages multiple concerns. Consider extracting it:function handleSigningStatusUpdate(data: any) { if (data.type === 'signed' && data.status === 'completed') { return { action: 'complete', message: 'Signature container signed successfully!' }; } else if (data.type === 'expired' || data.status === 'expired') { return { action: 'expired', message: 'Signing session expired. Please try again.' }; } else if (data.type === 'security_violation') { return { action: 'violation', message: 'Security violation detected. Signing failed.' }; } return null; }
- Logging utility: Replace multiple
console.logcalls (lines 147, 151, 155, 158, 161, 174, 182, 191, 196) with a structured logging utility for better production observability and easier log level management.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
platforms/esigner/.svelte-kit/generated/server/internal.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/file-manager/.svelte-kit/generated/client/app.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/file-manager/.svelte-kit/generated/client/nodes/5.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/file-manager/.svelte-kit/generated/client/nodes/6.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/file-manager/.svelte-kit/generated/client/nodes/7.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/file-manager/.svelte-kit/generated/client/nodes/8.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/file-manager/.svelte-kit/generated/server/internal.jsis excluded by!**/.svelte-kit/**,!**/generated/**platforms/file-manager/.svelte-kit/non-ambient.d.tsis excluded by!**/.svelte-kit/**platforms/file-manager/.svelte-kit/types/route_meta_data.jsonis excluded by!**/.svelte-kit/**platforms/file-manager/.svelte-kit/types/src/routes/$types.d.tsis excluded by!**/.svelte-kit/**platforms/file-manager/.svelte-kit/types/src/routes/(auth)/deeplink-login/$types.d.tsis excluded by!**/.svelte-kit/**platforms/file-manager/.svelte-kit/types/src/routes/(protected)/$types.d.tsis excluded by!**/.svelte-kit/**platforms/file-manager/.svelte-kit/types/src/routes/(protected)/storage/$types.d.tsis excluded by!**/.svelte-kit/**
📒 Files selected for processing (1)
platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-05T11:46:43.214Z
Learnt from: Bekiboo
Repo: MetaState-Prototype-Project/prototype PR: 653
File: platforms/pictique/src/routes/(auth)/auth/+page.svelte:173-173
Timestamp: 2026-01-05T11:46:43.214Z
Learning: In Tailwind CSS v4, gradient utilities were renamed from bg-gradient-to-{direction} to bg-linear-to-{direction}. For all Svelte components (e.g., any .svelte files in the project), replace usages like bg-gradient-to-r, bg-gradient-to-l, etc. with bg-linear-to-r, bg-linear-to-l, etc. Ensure consistency across the codebase, update any documentation or design tokens, and run visual/test checks to confirm gradients render as intended.
Applied to files:
platforms/esigner/src/routes/(protected)/files/[id]/+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). (1)
- GitHub Check: build
🔇 Additional comments (5)
platforms/esigner/src/routes/(protected)/files/[id]/+page.svelte (5)
23-63: Well-structured lifecycle management.The refactor to use a synchronous
onMountwith an async IIFE for data loading is correct. Cleanup properly unsubscribes from all stores and closes the SSE connection, following Svelte best practices.
336-420: Well-executed responsive design.The mobile-first responsive approach effectively adapts the UI across devices. Key improvements include:
- Proper viewport calculations and flexible layouts
- Preview hidden on mobile with an accessible alternative (new tab button)
- Appropriate scaling of padding, typography, and spacing
- Clean 70/30 desktop split using Tailwind flex utilities
442-456: LGTM!The file information layout uses appropriate responsive utilities and overflow handling. The
titleattribute on the truncated filename andbreak-allon long MIME types ensure content remains accessible without breaking the layout.
522-528: LGTM!Adding
flex-shrink-0to status badges ensures they maintain their size and readability even when signee names are long and cause text wrapping.
604-646: Mobile-adaptive signing flow implemented correctly.The conditional rendering based on
isMobileDevice()provides appropriate UX for each platform:
- Mobile: Direct deep link button for seamless app handoff
- Desktop: QR code scanning workflow
The implementation structure is sound, with consistent messaging and clear instructions for each path. Ensure
getDeepLinkUrl()properly formats the deep link as previously flagged.
coodos
force-pushed
the
feat/file-manager-ux-improvments
branch
from
d2a6bf8 to
aae3fe0
Compare
Description of change
Issue Number
Type of Change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Improvements
UI
✏️ Tip: You can customize this high-level summary in your review settings.