Open
Conversation
added 4 commits
April 1, 2026 17:48
- server/routes/backups.ts: API for backup list/download/delete - src/features/backups/: BackupsPanel component - src/components/TopBar.tsx: Backups button + panel integration - package.json: renamed to CerAImic-Nerve-Center - upstream remote: daggerhashimoto/openclaw-nerve
BackupsPanel (full-page view, Chat | Backups | Tasks): - 4 backup sections: Local, Memory, Archive, NAS - Schedule bar showing active crontab entries - Status indicators (green/red) per section - File list with download/delete actions - Refresh on demand BackupsSettings (Settings drawer): - Shows all backup env vars with current values - Documents 3 backup types: Local, NAS, GitHub - Links to docs/BACKUPS_FEATURE.md Backups API (server/routes/backups.ts): - GET /api/backups — full status + backups list - GET /api/backups/:name — download archive - DELETE /api/backups/:name — delete archive Backup config API (server/routes/backup-settings.ts): - GET /api/backups/config — read env vars All backup paths configurable via .env — no hardcoding. See docs/BACKUPS_FEATURE.md for full architecture.
…healthy status indicator - Redesigned BackupsPanel with per-backup 14-day calendar - NAS backups show Full (green) / Incr (blue) type badges - Header badge shows All Healthy / Issues Detected across all backups - Backup type parsing added to backups API (backupType, dayOfWeek) - Added example.env with backup configuration docs - Various bug fixes and UI improvements
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds Backups (backend API, env-driven config, CLI/NAS workflows, UI panel and settings) and Org Chart features; mounts new backup routes; adds frontend panels/settings and command/view-mode integrations; updates package metadata and example env; adds documentation for Backups and Org Chart. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client (UI)
participant API as Server API
participant FS as Filesystem
participant SSH as Remote SSH
UI->>API: GET /api/backups
API->>FS: scan local backup dir(s) & nas-sync cache
FS-->>API: list of local and cached NAS archives
alt BACKUP_NAS_ENABLED
API->>SSH: ls remote backup dir
SSH-->>API: remote listing or error
end
API->>API: compute 14-day calendar & statuses
API-->>UI: JSON {local, nas, calendar, totals}
UI->>UI: render dashboard & calendar
UI->>API: GET /api/backups/:name (download)
API->>FS: locate & stream file
FS-->>UI: archive stream
UI->>API: DELETE /api/backups/:name
API->>FS: remove file
FS-->>API: deletion result
API-->>UI: delete confirmation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (10)
src/components/TopBar.tsx (1)
20-21: Remove unusedDatabaseimport.
Databaseis imported but not used anywhere in the component. OnlyFolderOpenis used for the Backups button.🧹 Remove unused import
- Database, FolderOpen,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/TopBar.tsx` around lines 20 - 21, The import list in TopBar.tsx includes an unused symbol `Database`; remove `Database` from the import statement so only `FolderOpen` (and any other actually used icons) are imported, e.g., update the import that currently lists `Database, FolderOpen` to drop `Database` to eliminate the unused import warning and keep imports minimal.src/App.tsx (1)
310-316: The 'backups' view mode won't persist across page reloads.The view mode initialization only recognizes
'kanban'from localStorage (line 313). If the user is on the backups view and refreshes, they'll return to'chat'. This differs from kanban which persists.If this is intentional (backups is a transient admin view), consider adding a comment. Otherwise, update the persistence logic:
💡 Persist backups view mode
const [viewMode, setViewModeRaw] = useState<ViewMode>(() => { try { const saved = localStorage.getItem('nerve:viewMode'); - if (saved === 'kanban') return 'kanban'; + if (saved === 'kanban' || saved === 'backups') return saved; } catch { /* ignore */ } return 'chat'; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/App.tsx` around lines 310 - 316, The current viewMode initializer only reads 'kanban' from localStorage so 'backups' won't persist; update the initialization in the viewMode useState (and any place that writes the key via setViewModeRaw or its wrapper) to also recognize and persist 'backups' for the 'nerve:viewMode' key, or add a clear comment if treating 'backups' as intentionally transient; locate the viewMode state and its setter (viewMode, setViewModeRaw) and ensure reads accept 'backups' and writes to localStorage save 'backups' when that mode is selected.src/features/command-palette/commands.ts (1)
6-6: Consider adding a command palette entry for the Backups view.The
kanbanview has dedicated commands (open-kanban,open-chat) in lines 193-215, but there's no correspondingopen-backupscommand. For consistency, users should be able to switch to the Backups view via the command palette.💡 Suggested command palette entry
Add to the
onSetViewModeconditional block around line 207:{ id: 'open-chat', label: 'Open Chat View', action: () => actions.onSetViewMode!('chat'), category: 'kanban' as const, keywords: ['chat', 'conversation', 'view'], }, + { + id: 'open-backups', + label: 'Open Backups View', + action: () => actions.onSetViewMode!('backups'), + category: 'navigation' as const, + keywords: ['backups', 'backup', 'archive', 'view'], + }, { id: 'create-kanban-task',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/command-palette/commands.ts` at line 6, Add a command-palette entry and handler for the Backups view to mirror the existing open-kanban/open-chat commands: update the ViewMode usages and the onSetViewMode conditional to handle 'backups' by adding a command id (e.g., "open-backups") that calls the same logic path as open-kanban/open-chat and invokes the view switch (setViewMode('backups') or equivalent). Ensure the new command id is registered where other commands are declared and that onSetViewMode recognizes the 'backups' branch so selecting the command opens the Backups view.server/routes/backup-settings.ts (1)
9-28: Consider adding input validation or sanitization for parsed values.While
parseIntwith a fallback default handles NaN cases, consider adding bounds checking for retention days to prevent unreasonable values (e.g., negative numbers or extremely large values).💡 Add bounds checking
const parseRetentionDays = (value: string | undefined, defaultVal: number): number => { const parsed = parseInt(value || String(defaultVal), 10); if (Number.isNaN(parsed) || parsed < 1 || parsed > 365) return defaultVal; return parsed; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/backup-settings.ts` around lines 9 - 28, The GET handler for '/api/backups/config' uses parseInt for BACKUP_RETENTION_DAYS and BACKUP_NAS_RETENTION_DAYS but lacks bounds checks; add a small helper (e.g., parseRetentionDays) and use it in the route to validate and sanitize parsed values (reject NaN, clamp to a safe range like 1–365, and fall back to the existing defaults) so BACKUP_RETENTION_DAYS and BACKUP_NAS_RETENTION_DAYS cannot be negative or unreasonably large.src/features/settings/BackupsSettings.tsx (1)
70-85: Silent error handling may hide configuration issues.The catch block at line 82 silently swallows all errors. Consider at least logging the error or updating an error state so users know when config loading fails.
💡 Optional: Add error state
+ const [error, setError] = useState<string | null>(null); const fetchConfig = async () => { try { + setError(null); const resp = await fetch('/api/backups/config'); if (resp.ok) { const data = await resp.json(); const { BACKUP_NAS_PASSWORD, ...rest } = data; setConfig({ ...DEFAULT_CONFIG, ...rest }); } - } catch { /* ignore */ } + } catch (err) { + setError('Failed to load configuration'); + } setLoading(false); setLastRefresh(new Date()); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/settings/BackupsSettings.tsx` around lines 70 - 85, The fetchConfig function currently swallows all errors in its catch block; update it to surface failures by logging the caught error (e.g., with console.error or an application logger) and/or setting an error state (add a local error state and call setError on failure) so the UI can show a failure; keep the existing behavior of finally setting setLoading(false) and setLastRefresh(new Date()), and reference fetchConfig, useEffect, setConfig, setLoading, setLastRefresh, and DEFAULT_CONFIG when making the change.src/features/backups/BackupsPanel.tsx (2)
159-160: Potential performance concern: repeated Date calculations.Both
localHistoryandnasHistorycreate anew Date(Date.now() - 14 * 24 * 60 * 60 * 1000)on each computation. Consider extracting this threshold date to a constant or memoizing it.💡 Suggested improvement
+ const fourteenDaysAgo = useMemo(() => Date.now() - 14 * 24 * 60 * 60 * 1000, []); - const localHistory = useMemo(() => localSection ? localSection.backups.filter(b => new Date(b.modified) >= new Date(Date.now() - 14 * 24 * 60 * 60 * 1000)) : [], [localSection]); - const nasHistory = useMemo(() => nasSection ? nasSection.backups.filter(b => new Date(b.modified) >= new Date(Date.now() - 14 * 24 * 60 * 60 * 1000)) : [], [nasSection]); + const localHistory = useMemo(() => localSection ? localSection.backups.filter(b => new Date(b.modified).getTime() >= fourteenDaysAgo) : [], [localSection, fourteenDaysAgo]); + const nasHistory = useMemo(() => nasSection ? nasSection.backups.filter(b => new Date(b.modified).getTime() >= fourteenDaysAgo) : [], [nasSection, fourteenDaysAgo]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/backups/BackupsPanel.tsx` around lines 159 - 160, The filter threshold Date is re-created for both localHistory and nasHistory, causing redundant work; extract or memoize the cutoff as a single value (e.g., const cutoff = useMemo(() => new Date(Date.now() - 14 * 24 * 60 * 60 * 1000), [])) and use that cutoff in the localSection.backups.filter(...) and nasSection.backups.filter(...) calls so both localHistory and nasHistory reference the same precomputed Date instead of computing it twice.
143-155: Consider replacing nativeconfirm()andalert()with custom modal components.Using
confirm()(line 144) andalert()(line 151) blocks the main thread and provides inconsistent UX across browsers. Consider using the app's existing modal/dialog patterns if available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/backups/BackupsPanel.tsx` around lines 143 - 155, The handleDelete callback currently uses the blocking browser dialogs confirm() and alert(), so replace those with the app's modal/dialog and toast patterns: instead of confirm(...) call your app's confirm dialog (e.g., openConfirmModal/openDialog) and await the user's choice before proceeding, and replace alert(...) with the app's non-blocking notification/toast API (e.g., showToast/notify) to surface errors; keep the rest of the flow intact (setDeleting(entry.name), call fetch(`/api/backups/${encodeURIComponent(entry.name)}`, { method: 'DELETE' }), handle resp.ok, call fetchData(), setDeleting(null)) and ensure error details from the caught err are passed into the toast/notification.server/routes/backups.ts (2)
50-52: Unused wrapper functionformatBytes.
formatBytesis just an alias forformatSize. Consider removing the wrapper and usingformatSizedirectly.♻️ Remove unused wrapper
-function formatBytes(bytes: number): string { - return formatSize(bytes); -}Then replace calls to
formatByteswithformatSize.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/backups.ts` around lines 50 - 52, Remove the redundant wrapper function formatBytes and replace all its call sites with formatSize; specifically delete the function declaration function formatBytes(bytes: number): string { return formatSize(bytes); } and update any references to formatBytes(...) to call formatSize(...) instead, ensuring import/exports remain consistent and no other code depends on formatBytes.
316-328: DELETE endpoint lacks authorization beyond session auth.While the route is protected by session authentication (per auth middleware), there's no additional authorization check. Any authenticated user can delete any backup. Consider whether role-based access control is needed for destructive operations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/routes/backups.ts` around lines 316 - 328, The DELETE route handler (app.delete('/:name', async (c) => { ... })) currently deletes backups for any authenticated session; add an authorization check early in that handler using the session/user info available on the request (e.g., from c.req or c.get session) to enforce RBAC—allow deletion only for users with the required role (e.g., "admin" or "backup:delete") or the resource owner, and return a 403 when unauthorized; keep the existing logic that enumerates getBackupDirs(), uses scanDir() to find the entry and rmSync(entry.path) to remove it, but gate that work behind the new role check and/or ownership verification.docs/BACKUPS_FEATURE.md (1)
17-35: Add language specifiers to fenced code blocks.Several code blocks are missing language identifiers, which affects syntax highlighting and linting compliance. Use
bashorshellfor command examples andtextorplaintextfor file naming conventions.📝 Example fixes for a few blocks
-``` +```bash openclaw backup create --output "$BACKUP_LOCAL_DIR" --verify-
+text
full-YYYY-MM-DD.tar.gz ← Full backup (Sunday or catch-up)
incr-YYYY-MM-DD.tar.gz ← Incremental backup (Mon-Sat)-``` +```bash BACKUP_LOCAL_ENABLED=true BACKUP_LOCAL_DIR=~/Backups ...</details> Also applies to: 53-79, 119-126 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/BACKUPS_FEATURE.mdaround lines 17 - 35, The fenced code blocks in the
document lack language specifiers, breaking syntax highlighting and lint rules;
update each block shown (the one containing the command starting with "openclaw
backup create --output "$BACKUP_LOCAL_DIR" --verify", the file/name example
block containing "full-YYYY-MM-DD.tar.gz" / "incr-YYYY-MM-DD.tar.gz", and the
env block containing "BACKUP_LOCAL_ENABLED=true" / "BACKUP_LOCAL_DIR=~/Backups")
to include appropriate languages (usebash orshell for commands and env
vars, andtext orplaintext for filename examples); apply the same change
to the other mentioned blocks (lines referenced in the review: the blocks around
the examples at 53-79 and 119-126) so all fenced blocks have a language tag.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@docs/BACKUPS_FEATURE.md:
- Around line 130-136: Update the "Retention Rules" table to reflect that NAS
retention is configurable by replacing the "hardcoded in script" entry with the
env var nameBACKUP_NAS_RETENTION_DAYS(which is documented earlier as
BACKUP_NAS_RETENTION_DAYS=30), and ensure both rows consistently show the
correct config var names (BACKUP_RETENTION_DAYSfor Local and
BACKUP_NAS_RETENTION_DAYSfor NAS) so the table matches the documented
configuration.- Around line 97-99: The docs claim BackupsSettings.tsx "reads/writes backup env
vars" but the component only reads; either update the docs to say it only reads
(change "reads/writes" to "reads") or implement write capability in the
BackupsSettings.tsx component: add editable inputs and a save action (e.g., add
onSaveBackupSettings handler that calls an updateBackupEnvVars API/utility),
wire validation and error/success feedback, and ensure the component's
integration in SettingsDrawer exposes the save workflow; adjust the
documentation accordingly to reflect the implemented behavior.In
@example.env:
- Line 16: The BACKUP_NAS_BACKUP_PATH value contains spaces ("Files
Neon/backups/openclaw-full"); either quote it in the .env (e.g.,
BACKUP_NAS_BACKUP_PATH="Files Neon/backups/openclaw-full") or ensure the code
that consumes BACKUP_NAS_BACKUP_PATH (any function that builds shell commands
for scp/rsync) properly escapes/quotes the variable before invoking the shell
(e.g., pass as an argv element or wrap in quotes) so space-containing paths are
handled safely.In
@server/app.ts:
- Around line 93-96: There is a routing conflict between backupSettingsRoutes
(which exposes GET /api/backups/config when mounted via app.route('/', route))
and backupsRoutes (mounted at app.route('/api/backups', backupsRoutes) which
defines GET /:name); change the routing to avoid the catch‑all collision by
either (A) updating backupsRoutes to use an explicit subpath for the resource
route (e.g., replace GET /:name with GET /files/:name or GET /by-name/:name) so
it no longer matches "config", or (B) consolidate backupSettingsRoutes and
backupsRoutes into a single backup router and register the handlers in a
deterministic order (ensure the explicit /config handler is registered before
any parameterized routes), or explicitly mount backupSettingsRoutes at
'/api/backups/config' instead of relying on the routes loop; update references
to backupsRoutes, backupSettingsRoutes, app.route('/', route), and
app.route('/api/backups', backupsRoutes) accordingly.In
@server/routes/backup-settings.ts:
- Line 19: The response currently exposes the secret BACKUP_NAS_PASSWORD; remove
the BACKUP_NAS_PASSWORD property from the exported config/response in
server/routes/backup-settings.ts and instead add a non-sensitive boolean (e.g.,
BACKUP_NAS_PASSWORD_SET) derived from the environment (true if
env.BACKUP_NAS_PASSWORD is present) so the UI can know whether a password exists
without ever returning the secret; update any consumers to read
BACKUP_NAS_PASSWORD_SET and ensure BACKUP_NAS_PASSWORD is never serialized or
sent in API responses.In
@server/routes/backups.ts:
- Around line 196-209: The current missed-backup check (inside the
env('BACKUP_LOCAL_ENABLED') branch) only parses schedule into
scheduleHour/scheduleMin and compares to now, which can produce false positives
because it ignores day-of-month/day-of-week and timezone; replace this ad-hoc
parsing with a proper cron evaluation: use a cron library (e.g., cron-parser) to
parse the full schedule string from env('BACKUP_LOCAL_SCHEDULE') and compute the
previous scheduled run time (and next) in the server's intended timezone, then
determine if now is more than 5 minutes past that previous scheduled run and no
backup exists before flagging missed; update the condition that builds
missed.push messages (the blocks referencing scheduleHour/scheduleMin, now,
localStatus, nasStatus) to use the library-derived previousRun timestamp and
formatted time for the message so day constraints and timezones are respected.- Around line 16-17: The HOME fallback currently hardcodes '/home/najef' which
is incorrect; replace the fallback in the HOME constant to use os.homedir()
(importing os) or default to an empty string and add validation so STATUS_FILE =
join(HOME, 'Backups', 'backup-status.json') uses a real home directory; update
code paths that reference HOME/STATUS_FILE (the HOME constant and STATUS_FILE
constant) to handle missing/empty HOME by throwing/logging a clear error or
skipping backup operations.- Around line 131-134: The SSH command built in the backups route uses
StrictHostKeyChecking=no and interpolates remotePath into a shell command,
creating MITM risk and possible command injection; change the SSH invocation to
use StrictHostKeyChecking=accept-new (or omit to rely on known_hosts) and stop
using a shell-interpolated string with execSync — instead call the SSH binary
with child_process.spawnSync/execFile and pass args as an array (e.g., the place
where sshCmd is constructed and execSync is called), and additionally
validate/sanitize remotePath, host and user (e.g., whitelist or reject
characters like ;|&$`<> ) before passing it as an argument to the spawned
process.- Around line 300-314: The route handler app.get('/:name') is passing a Node
fs.ReadStream (createReadStream(entry.path)) directly to c.body and hardcoding
Content-Type to application/gzip; change it to convert the Node stream to a
WHATWG ReadableStream (e.g., use
stream.Readable.toWeb(createReadStream(entry.path))) before calling c.body, and
set the Content-Type dynamically based on the file extension of entry.path
(handle .zip -> application/zip, .gz and .tar.gz -> application/gzip, fallback
to application/octet-stream); keep the existing Content-Disposition logic and
return the converted ReadableStream from c.body.In
@src/features/backups/BackupsPanel.tsx:
- Around line 283-284: In BackupsPanel.tsx the info bar renders the "Last:" line
twice using the same JSX (both referencing
formatRelative(currentSection.lastBackup)); remove the duplicated span so only a
single containing "Last: <span className="text-foreground
font-medium">{formatRelative(currentSection.lastBackup)}" remains
(locate the duplicate around the currentSection.lastBackup usage in the
BackupsPanel component).In
@src/features/settings/BackupsSettings.tsx:
- Around line 10-24: The interface BackupEnvConfig defines
BACKUP_NAS_BACKUP_PATH but the code reads config.BACKUP_NAS_PATH causing a
TypeScript error and wrong "not configured" behavior; update either the
interface or the usages so the property name matches: rename the interface key
to BACKUP_NAS_PATH or change all references (e.g., config.BACKUP_NAS_PATH) to
config.BACKUP_NAS_BACKUP_PATH (also check the same mismatch around the other
references mentioned) so the symbol names BackupEnvConfig,
BACKUP_NAS_BACKUP_PATH and any occurrences of BACKUP_NAS_PATH are consistent.
Nitpick comments:
In@docs/BACKUPS_FEATURE.md:
- Around line 17-35: The fenced code blocks in the document lack language
specifiers, breaking syntax highlighting and lint rules; update each block shown
(the one containing the command starting with "openclaw backup create --output
"$BACKUP_LOCAL_DIR" --verify", the file/name example block containing
"full-YYYY-MM-DD.tar.gz" / "incr-YYYY-MM-DD.tar.gz", and the env block
containing "BACKUP_LOCAL_ENABLED=true" / "BACKUP_LOCAL_DIR=~/Backups") to
include appropriate languages (usebash orshell for commands and env
vars, andtext orplaintext for filename examples); apply the same change
to the other mentioned blocks (lines referenced in the review: the blocks around
the examples at 53-79 and 119-126) so all fenced blocks have a language tag.In
@server/routes/backup-settings.ts:
- Around line 9-28: The GET handler for '/api/backups/config' uses parseInt for
BACKUP_RETENTION_DAYS and BACKUP_NAS_RETENTION_DAYS but lacks bounds checks; add
a small helper (e.g., parseRetentionDays) and use it in the route to validate
and sanitize parsed values (reject NaN, clamp to a safe range like 1–365, and
fall back to the existing defaults) so BACKUP_RETENTION_DAYS and
BACKUP_NAS_RETENTION_DAYS cannot be negative or unreasonably large.In
@server/routes/backups.ts:
- Around line 50-52: Remove the redundant wrapper function formatBytes and
replace all its call sites with formatSize; specifically delete the function
declaration function formatBytes(bytes: number): string { return
formatSize(bytes); } and update any references to formatBytes(...) to call
formatSize(...) instead, ensuring import/exports remain consistent and no other
code depends on formatBytes.- Around line 316-328: The DELETE route handler (app.delete('/:name', async (c)
=> { ... })) currently deletes backups for any authenticated session; add an
authorization check early in that handler using the session/user info available
on the request (e.g., from c.req or c.get session) to enforce RBAC—allow
deletion only for users with the required role (e.g., "admin" or
"backup:delete") or the resource owner, and return a 403 when unauthorized; keep
the existing logic that enumerates getBackupDirs(), uses scanDir() to find the
entry and rmSync(entry.path) to remove it, but gate that work behind the new
role check and/or ownership verification.In
@src/App.tsx:
- Around line 310-316: The current viewMode initializer only reads 'kanban' from
localStorage so 'backups' won't persist; update the initialization in the
viewMode useState (and any place that writes the key via setViewModeRaw or its
wrapper) to also recognize and persist 'backups' for the 'nerve:viewMode' key,
or add a clear comment if treating 'backups' as intentionally transient; locate
the viewMode state and its setter (viewMode, setViewModeRaw) and ensure reads
accept 'backups' and writes to localStorage save 'backups' when that mode is
selected.In
@src/components/TopBar.tsx:
- Around line 20-21: The import list in TopBar.tsx includes an unused symbol
Database; removeDatabasefrom the import statement so onlyFolderOpen
(and any other actually used icons) are imported, e.g., update the import that
currently listsDatabase, FolderOpento dropDatabaseto eliminate the
unused import warning and keep imports minimal.In
@src/features/backups/BackupsPanel.tsx:
- Around line 159-160: The filter threshold Date is re-created for both
localHistory and nasHistory, causing redundant work; extract or memoize the
cutoff as a single value (e.g., const cutoff = useMemo(() => new Date(Date.now()- 14 * 24 * 60 * 60 * 1000), [])) and use that cutoff in the
localSection.backups.filter(...) and nasSection.backups.filter(...) calls so
both localHistory and nasHistory reference the same precomputed Date instead of
computing it twice.- Around line 143-155: The handleDelete callback currently uses the blocking
browser dialogs confirm() and alert(), so replace those with the app's
modal/dialog and toast patterns: instead of confirm(...) call your app's confirm
dialog (e.g., openConfirmModal/openDialog) and await the user's choice before
proceeding, and replace alert(...) with the app's non-blocking
notification/toast API (e.g., showToast/notify) to surface errors; keep the rest
of the flow intact (setDeleting(entry.name), call
fetch(/api/backups/${encodeURIComponent(entry.name)}, { method: 'DELETE' }),
handle resp.ok, call fetchData(), setDeleting(null)) and ensure error details
from the caught err are passed into the toast/notification.In
@src/features/command-palette/commands.ts:
- Line 6: Add a command-palette entry and handler for the Backups view to mirror
the existing open-kanban/open-chat commands: update the ViewMode usages and the
onSetViewMode conditional to handle 'backups' by adding a command id (e.g.,
"open-backups") that calls the same logic path as open-kanban/open-chat and
invokes the view switch (setViewMode('backups') or equivalent). Ensure the new
command id is registered where other commands are declared and that
onSetViewMode recognizes the 'backups' branch so selecting the command opens the
Backups view.In
@src/features/settings/BackupsSettings.tsx:
- Around line 70-85: The fetchConfig function currently swallows all errors in
its catch block; update it to surface failures by logging the caught error
(e.g., with console.error or an application logger) and/or setting an error
state (add a local error state and call setError on failure) so the UI can show
a failure; keep the existing behavior of finally setting setLoading(false) and
setLastRefresh(new Date()), and reference fetchConfig, useEffect, setConfig,
setLoading, setLastRefresh, and DEFAULT_CONFIG when making the change.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `25c552fc-15f2-4307-b396-f1ef7082b683` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 6260878394c6facc6402dc903bbc23d2e0d669e9 and 8562162a1590edadb0a37477e75d9443c9993df1. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `package-lock.json` is excluded by `!**/package-lock.json` </details> <details> <summary>📒 Files selected for processing (12)</summary> * `docs/BACKUPS_FEATURE.md` * `example.env` * `package.json` * `server/app.ts` * `server/routes/backup-settings.ts` * `server/routes/backups.ts` * `src/App.tsx` * `src/components/TopBar.tsx` * `src/features/backups/BackupsPanel.tsx` * `src/features/backups/index.ts` * `src/features/command-palette/commands.ts` * `src/features/settings/BackupsSettings.tsx` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+97
to
+99
| ### Settings UI | ||
| - `src/features/settings/BackupsSettings.tsx` — reads/writes backup env vars | ||
| - Integrated into `SettingsDrawer` |
There was a problem hiding this comment.
Documentation states "reads/writes" but component only reads.
Line 98 says BackupsSettings.tsx "reads/writes backup env vars", but the component only fetches and displays config—it doesn't provide any write/edit functionality.
📝 Suggested fix
### Settings UI
-- `src/features/settings/BackupsSettings.tsx` — reads/writes backup env vars
+- `src/features/settings/BackupsSettings.tsx` — displays backup env vars (read-only)
- Integrated into `SettingsDrawer`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### Settings UI | |
| - `src/features/settings/BackupsSettings.tsx` — reads/writes backup env vars | |
| - Integrated into `SettingsDrawer` | |
| ### Settings UI | |
| - `src/features/settings/BackupsSettings.tsx` — displays backup env vars (read-only) | |
| - Integrated into `SettingsDrawer` |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/BACKUPS_FEATURE.md` around lines 97 - 99, The docs claim
BackupsSettings.tsx "reads/writes backup env vars" but the component only reads;
either update the docs to say it only reads (change "reads/writes" to "reads")
or implement write capability in the BackupsSettings.tsx component: add editable
inputs and a save action (e.g., add onSaveBackupSettings handler that calls an
updateBackupEnvVars API/utility), wire validation and error/success feedback,
and ensure the component's integration in SettingsDrawer exposes the save
workflow; adjust the documentation accordingly to reflect the implemented
behavior.
There was a problem hiding this comment.
@najef1979-code, the Comment image shows the live BackupsPanel UI running in the NERVE / OpenClaw Cockpit interface. It displays:
- A BACKUPS header with an "All Healthy" green badge and schedule times (01:00 CET · 01:30 CET)
- Two tabs: NAS Offsite (3 files · 491.7MB) and Local CLI (2 files · 2.97GB)
- A 14-day calendar row with "Success" and "Today" markers
- A file list showing
full-2026-04-01.tar.gz(Full · Wednesday),incr-2026-04-01.tar.gz(Incr · Wednesday), andincr-2026-03-31.tar.gz(Incr · Tuesday) - A footer: "5 backups · 3.45GB total"
- Status bar: CONNECTED · 01:16:52 · 22 active sessions · Uptime 00:31:22
The screenshot confirms the panel is a display-only dashboard — there are no input fields, edit controls, or save actions anywhere. This supports the original concern: docs/BACKUPS_FEATURE.md line 98 should be corrected from "reads/writes backup env vars" to "displays backup env vars (read-only)":
### Settings UI
-- `src/features/settings/BackupsSettings.tsx` — reads/writes backup env vars
+- `src/features/settings/BackupsSettings.tsx` — displays backup env vars (read-only)
- Integrated into `SettingsDrawer`This is a one-line doc fix. Would you like me to plan that change?
| BACKUP_NAS_ENABLED=false | ||
| BACKUP_NAS_HOST=192.168.2.3 | ||
| BACKUP_NAS_USER=CoWorker.Neon | ||
| BACKUP_NAS_BACKUP_PATH=Files Neon/backups/openclaw-full |
There was a problem hiding this comment.
Path with spaces may need quoting depending on usage.
BACKUP_NAS_BACKUP_PATH=Files Neon/backups/openclaw-full contains spaces. While this is valid in .env files (values don't require quotes), ensure the consuming code handles spaces correctly when passing to shell commands like scp or rsync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@example.env` at line 16, The BACKUP_NAS_BACKUP_PATH value contains spaces
("Files Neon/backups/openclaw-full"); either quote it in the .env (e.g.,
BACKUP_NAS_BACKUP_PATH="Files Neon/backups/openclaw-full") or ensure the code
that consumes BACKUP_NAS_BACKUP_PATH (any function that builds shell commands
for scp/rsync) properly escapes/quotes the variable before invoking the shell
(e.g., pass as an argv element or wrap in quotes) so space-containing paths are
handled safely.
Comment on lines
+93
to
+96
| voicePhrasesRoutes, fileBrowserRoutes, channelsRoutes, kanbanRoutes, backupSettingsRoutes, | ||
| ]; | ||
| for (const route of routes) app.route('/', route); | ||
| app.route('/api/backups', backupsRoutes); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what routes are defined in backupsRoutes to verify potential conflicts
cat server/routes/backups.ts 2>/dev/null || echo "File not found"Repository: daggerhashimoto/openclaw-nerve
Length of output: 11919
🏁 Script executed:
#!/bin/bash
# Find backupSettings route file
fd -t f 'backupSettings' server/routes/Repository: daggerhashimoto/openclaw-nerve
Length of output: 56
🏁 Script executed:
#!/bin/bash
# Check imports in server/app.ts around lines 49-50
sed -n '45,55p' server/app.tsRepository: daggerhashimoto/openclaw-nerve
Length of output: 558
🏁 Script executed:
#!/bin/bash
# Find backup-settings file
fd -t f 'backup-settings' server/routes/Repository: daggerhashimoto/openclaw-nerve
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Read backup-settings.ts
cat server/routes/backup-settings.tsRepository: daggerhashimoto/openclaw-nerve
Length of output: 1262
Route conflict risk between /api/backups/config and /api/backups/:name.
backupSettingsRoutes defines GET /api/backups/config and is mounted at / (line 95 via the routes loop), while backupsRoutes is mounted at /api/backups (line 96) with a GET /:name catch-all route. Both routes match requests to /api/backups/config, but the config endpoint currently works because backupSettingsRoutes is registered first. This registration-order dependency is fragile and could break if mount order changes.
Consider one of:
- Define explicit paths in
backupsRoutes(e.g.,GET /files/:name) instead of catch-all parameters - Consolidate backup routes into a single module with explicit route ordering
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/app.ts` around lines 93 - 96, There is a routing conflict between
backupSettingsRoutes (which exposes GET /api/backups/config when mounted via
app.route('/', route)) and backupsRoutes (mounted at app.route('/api/backups',
backupsRoutes) which defines GET /:name); change the routing to avoid the
catch‑all collision by either (A) updating backupsRoutes to use an explicit
subpath for the resource route (e.g., replace GET /:name with GET /files/:name
or GET /by-name/:name) so it no longer matches "config", or (B) consolidate
backupSettingsRoutes and backupsRoutes into a single backup router and register
the handlers in a deterministic order (ensure the explicit /config handler is
registered before any parameterized routes), or explicitly mount
backupSettingsRoutes at '/api/backups/config' instead of relying on the routes
loop; update references to backupsRoutes, backupSettingsRoutes, app.route('/',
route), and app.route('/api/backups', backupsRoutes) accordingly.
| BACKUP_NAS_ENABLED: env.BACKUP_NAS_ENABLED === 'true', | ||
| BACKUP_NAS_HOST: env.BACKUP_NAS_HOST || '', | ||
| BACKUP_NAS_USER: env.BACKUP_NAS_USER || '', | ||
| BACKUP_NAS_PASSWORD: env.BACKUP_NAS_PASSWORD || '', |
There was a problem hiding this comment.
Critical: Do not expose BACKUP_NAS_PASSWORD in the API response.
Returning the NAS password in the config endpoint exposes credentials to any authenticated user viewing the settings UI or inspecting network traffic. Passwords should never be returned from API endpoints.
🔒 Remove password from response
BACKUP_NAS_HOST: env.BACKUP_NAS_HOST || '',
BACKUP_NAS_USER: env.BACKUP_NAS_USER || '',
- BACKUP_NAS_PASSWORD: env.BACKUP_NAS_PASSWORD || '',
+ BACKUP_NAS_HAS_PASSWORD: Boolean(env.BACKUP_NAS_PASSWORD),
BACKUP_NAS_BACKUP_PATH: env.BACKUP_NAS_BACKUP_PATH || '',If the UI needs to indicate whether a password is configured, return a boolean flag instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| BACKUP_NAS_PASSWORD: env.BACKUP_NAS_PASSWORD || '', | |
| BACKUP_NAS_HOST: env.BACKUP_NAS_HOST || '', | |
| BACKUP_NAS_USER: env.BACKUP_NAS_USER || '', | |
| BACKUP_NAS_HAS_PASSWORD: Boolean(env.BACKUP_NAS_PASSWORD), | |
| BACKUP_NAS_BACKUP_PATH: env.BACKUP_NAS_BACKUP_PATH || '', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/backup-settings.ts` at line 19, The response currently exposes
the secret BACKUP_NAS_PASSWORD; remove the BACKUP_NAS_PASSWORD property from the
exported config/response in server/routes/backup-settings.ts and instead add a
non-sensitive boolean (e.g., BACKUP_NAS_PASSWORD_SET) derived from the
environment (true if env.BACKUP_NAS_PASSWORD is present) so the UI can know
whether a password exists without ever returning the secret; update any
consumers to read BACKUP_NAS_PASSWORD_SET and ensure BACKUP_NAS_PASSWORD is
never serialized or sent in API responses.
Comment on lines
+131
to
+134
| if (host && user && remotePath) { | ||
| try { | ||
| const sshCmd = `ssh -o BatchMode=yes -o StrictHostKeyChecking=no ${user}@${host} "ls -la --time-style=long-iso '${remotePath}'" 2>/dev/null`; | ||
| const output = execSync(sshCmd, { encoding: 'utf8', timeout: 15000 }); |
There was a problem hiding this comment.
SSH command construction has security concerns.
StrictHostKeyChecking=nodisables host key verification, making the connection vulnerable to MITM attacks.- Command injection risk: While env vars are generally trusted,
remotePathis interpolated directly into the shell command. If it contains shell metacharacters, unexpected behavior could occur.
🛡️ Suggested improvements
- const sshCmd = `ssh -o BatchMode=yes -o StrictHostKeyChecking=no ${user}@${host} "ls -la --time-style=long-iso '${remotePath}'" 2>/dev/null`;
+ // Escape single quotes in remotePath to prevent shell injection
+ const escapedPath = remotePath.replace(/'/g, "'\\''");
+ const sshCmd = `ssh -o BatchMode=yes -o ConnectTimeout=10 ${user}@${host} "ls -la --time-style=long-iso '${escapedPath}'" 2>/dev/null`;Consider removing StrictHostKeyChecking=no and letting the system use known_hosts, or at minimum use StrictHostKeyChecking=accept-new to only accept new keys without prompting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/backups.ts` around lines 131 - 134, The SSH command built in
the backups route uses StrictHostKeyChecking=no and interpolates remotePath into
a shell command, creating MITM risk and possible command injection; change the
SSH invocation to use StrictHostKeyChecking=accept-new (or omit to rely on
known_hosts) and stop using a shell-interpolated string with execSync — instead
call the SSH binary with child_process.spawnSync/execFile and pass args as an
array (e.g., the place where sshCmd is constructed and execSync is called), and
additionally validate/sanitize remotePath, host and user (e.g., whitelist or
reject characters like ;|&$`<> ) before passing it as an argument to the spawned
process.
Comment on lines
+196
to
+209
| if (i === 0 && env('BACKUP_LOCAL_ENABLED')) { | ||
| const schedule = env('BACKUP_LOCAL_SCHEDULE', '0 1 * * *'); | ||
| const parts = schedule.split(' '); | ||
| const scheduleHour = parseInt(parts[1]); | ||
| const scheduleMin = parseInt(parts[0]); | ||
| const now = new Date(); | ||
| if (now.getHours() > scheduleHour || (now.getHours() === scheduleHour && now.getMinutes() > scheduleMin + 5)) { | ||
| if (localStatus === 'none' && nasStatus === 'none') { | ||
| missed.push(`Local backup missed — scheduled ${String(scheduleHour).padStart(2,'0')}:${String(scheduleMin).padStart(2,'0')} but no backup found`); | ||
| } else if (localStatus === 'none') { | ||
| missed.push(`Local backup missed — NAS backup ran but local backup not found`); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Missed backup detection logic may produce false positives.
The logic checks if current time is past the scheduled time + 5 minutes (line 202), but doesn't account for:
- Days when backups are intentionally not scheduled (if schedule has day-of-week constraints)
- Timezone differences between server and cron execution environment
The cron schedule parsing only extracts hour and minute, ignoring day-of-week/month fields.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/backups.ts` around lines 196 - 209, The current missed-backup
check (inside the env('BACKUP_LOCAL_ENABLED') branch) only parses schedule into
scheduleHour/scheduleMin and compares to now, which can produce false positives
because it ignores day-of-month/day-of-week and timezone; replace this ad-hoc
parsing with a proper cron evaluation: use a cron library (e.g., cron-parser) to
parse the full schedule string from env('BACKUP_LOCAL_SCHEDULE') and compute the
previous scheduled run time (and next) in the server's intended timezone, then
determine if now is more than 5 minutes past that previous scheduled run and no
backup exists before flagging missed; update the condition that builds
missed.push messages (the blocks referencing scheduleHour/scheduleMin, now,
localStatus, nasStatus) to use the library-derived previousRun timestamp and
formatted time for the message so day constraints and timezones are respected.
Comment on lines
+300
to
+314
| app.get('/:name', async (c) => { | ||
| const name = c.req.param('name'); | ||
| const dirs = getBackupDirs(); | ||
| for (const dir of dirs) { | ||
| const entries = scanDir(dir.name, dir.path); | ||
| const entry = entries.find(e => e.name === name); | ||
| if (entry) { | ||
| const fileName = basename(entry.path); | ||
| c.header('Content-Disposition', `attachment; filename="${fileName}"`); | ||
| c.header('Content-Type', 'application/gzip'); | ||
| return c.body(createReadStream(entry.path)); | ||
| } | ||
| } | ||
| return c.json({ error: 'Backup not found' }, 404); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Hono v4 support passing Node.js fs.ReadStream directly to c.body() for streaming file downloads?
💡 Result:
No, Hono v4 does not support passing Node.js fs.ReadStream directly to c.body. The c.body method accepts Data types: string | ArrayBuffer | ReadableStream (web standard) or potentially BodyInit/Buffer in recent updates, but fs.ReadStream is a Node.js-specific Readable (old stream2 API), not a web ReadableStream. Users must convert it to a web ReadableStream, as shown in community examples like wrapping with new ReadableStream({start(controller){ stream.on('data', chunk => controller.enqueue(chunk)); ... }}).
Citations:
- 1: [Feature Request] Support
sendFilehonojs/node-server#205 - 2: Missing
Buffer<ArrayBufferLike>type in the parameter ofc.bodyhonojs/hono#3729 - 3: https://hono.dev/docs/helpers/streaming
- 4: https://www.honojs.com/docs/api/context
- 5: https://github.com/honojs/hono/blob/8217d9ec/src/request.ts
- 6: https://github.com/honojs/hono/blob/8217d9ec/src/context.ts
🏁 Script executed:
# Check Hono version
fd -e "json" -type f | xargs grep -l "\"hono\"" | head -5Repository: daggerhashimoto/openclaw-nerve
Length of output: 244
🏁 Script executed:
# Find the scanDir function to understand what file types it handles
rg "scanDir" -A 10 -B 2Repository: daggerhashimoto/openclaw-nerve
Length of output: 5022
🏁 Script executed:
# Check what getBackupDirs returns
rg "getBackupDirs" -A 15 -B 2Repository: daggerhashimoto/openclaw-nerve
Length of output: 3877
🏁 Script executed:
# Check Hono version in package.json
cat package.json | grep -A 2 -B 2 "hono"Repository: daggerhashimoto/openclaw-nerve
Length of output: 441
🏁 Script executed:
# Search for other c.body() patterns in the codebase
rg "c\.body\(" -B 3 -A 3Repository: daggerhashimoto/openclaw-nerve
Length of output: 536
Fix stream handling and Content-Type for file downloads in Hono v4.
The code has two critical issues:
- Broken stream handling: Passing Node.js
fs.ReadStreamdirectly toc.body()will fail in Hono v4, which only accepts web standardReadableStream. This must be converted. - Wrong Content-Type: Hardcoded to
application/gzip(line 309), but the endpoint serves.tar.gz,.gz, and.zipfiles, each requiring different MIME types.
Fix for stream conversion and dynamic Content-Type
const fileName = basename(entry.path);
c.header('Content-Disposition', `attachment; filename="${fileName}"`);
- c.header('Content-Type', 'application/gzip');
- return c.body(createReadStream(entry.path));
+
+ let contentType = 'application/octet-stream';
+ if (fileName.endsWith('.zip')) {
+ contentType = 'application/zip';
+ } else if (fileName.endsWith('.tar.gz')) {
+ contentType = 'application/gzip';
+ } else if (fileName.endsWith('.gz')) {
+ contentType = 'application/gzip';
+ }
+ c.header('Content-Type', contentType);
+
+ const stream = createReadStream(entry.path);
+ return c.body(
+ new ReadableStream({
+ start(controller) {
+ stream.on('data', chunk => controller.enqueue(chunk));
+ stream.on('end', () => controller.close());
+ stream.on('error', err => controller.error(err));
+ }
+ })
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| app.get('/:name', async (c) => { | |
| const name = c.req.param('name'); | |
| const dirs = getBackupDirs(); | |
| for (const dir of dirs) { | |
| const entries = scanDir(dir.name, dir.path); | |
| const entry = entries.find(e => e.name === name); | |
| if (entry) { | |
| const fileName = basename(entry.path); | |
| c.header('Content-Disposition', `attachment; filename="${fileName}"`); | |
| c.header('Content-Type', 'application/gzip'); | |
| return c.body(createReadStream(entry.path)); | |
| } | |
| } | |
| return c.json({ error: 'Backup not found' }, 404); | |
| }); | |
| app.get('/:name', async (c) => { | |
| const name = c.req.param('name'); | |
| const dirs = getBackupDirs(); | |
| for (const dir of dirs) { | |
| const entries = scanDir(dir.name, dir.path); | |
| const entry = entries.find(e => e.name === name); | |
| if (entry) { | |
| const fileName = basename(entry.path); | |
| c.header('Content-Disposition', `attachment; filename="${fileName}"`); | |
| let contentType = 'application/octet-stream'; | |
| if (fileName.endsWith('.zip')) { | |
| contentType = 'application/zip'; | |
| } else if (fileName.endsWith('.tar.gz')) { | |
| contentType = 'application/gzip'; | |
| } else if (fileName.endsWith('.gz')) { | |
| contentType = 'application/gzip'; | |
| } | |
| c.header('Content-Type', contentType); | |
| const stream = createReadStream(entry.path); | |
| return c.body( | |
| new ReadableStream({ | |
| start(controller) { | |
| stream.on('data', chunk => controller.enqueue(chunk)); | |
| stream.on('end', () => controller.close()); | |
| stream.on('error', err => controller.error(err)); | |
| } | |
| }) | |
| ); | |
| } | |
| } | |
| return c.json({ error: 'Backup not found' }, 404); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/routes/backups.ts` around lines 300 - 314, The route handler
app.get('/:name') is passing a Node fs.ReadStream (createReadStream(entry.path))
directly to c.body and hardcoding Content-Type to application/gzip; change it to
convert the Node stream to a WHATWG ReadableStream (e.g., use
stream.Readable.toWeb(createReadStream(entry.path))) before calling c.body, and
set the Content-Type dynamically based on the file extension of entry.path
(handle .zip -> application/zip, .gz and .tar.gz -> application/gzip, fallback
to application/octet-stream); keep the existing Content-Disposition logic and
return the converted ReadableStream from c.body.
Comment on lines
+283
to
+284
| <span>Last: <span className="text-foreground font-medium">{formatRelative(currentSection.lastBackup)}</span></span> | ||
| <span>Last: <span className="text-foreground font-medium">{formatRelative(currentSection.lastBackup)}</span></span> |
There was a problem hiding this comment.
Duplicate line: "Last:" is displayed twice.
Lines 283 and 284 are identical, showing the last backup time twice in the info bar.
🐛 Remove duplicate line
<span>Last: <span className="text-foreground font-medium">{formatRelative(currentSection.lastBackup)}</span></span>
- <span>Last: <span className="text-foreground font-medium">{formatRelative(currentSection.lastBackup)}</span></span>
<span>·</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <span>Last: <span className="text-foreground font-medium">{formatRelative(currentSection.lastBackup)}</span></span> | |
| <span>Last: <span className="text-foreground font-medium">{formatRelative(currentSection.lastBackup)}</span></span> | |
| <span>Last: <span className="text-foreground font-medium">{formatRelative(currentSection.lastBackup)}</span></span> | |
| <span>·</span> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/backups/BackupsPanel.tsx` around lines 283 - 284, In
BackupsPanel.tsx the info bar renders the "Last:" line twice using the same JSX
(both referencing formatRelative(currentSection.lastBackup)); remove the
duplicated span so only a single <span> containing "Last: <span
className=\"text-foreground
font-medium\">{formatRelative(currentSection.lastBackup)}</span>" remains
(locate the duplicate around the currentSection.lastBackup usage in the
BackupsPanel component).
Comment on lines
+10
to
+24
| interface BackupEnvConfig { | ||
| BACKUP_LOCAL_ENABLED: boolean; | ||
| BACKUP_LOCAL_DIR: string; | ||
| BACKUP_RETENTION_DAYS: number; | ||
| BACKUP_LOCAL_SCHEDULE: string; | ||
| BACKUP_NAS_ENABLED: boolean; | ||
| BACKUP_NAS_HOST: string; | ||
| BACKUP_NAS_USER: string; | ||
| BACKUP_NAS_BACKUP_PATH: string; | ||
| BACKUP_NAS_RETENTION_DAYS: number; | ||
| BACKUP_GITHUB_ENABLED: boolean; | ||
| BACKUP_GITHUB_REPO: string; | ||
| BACKUP_GITHUB_BRANCH: string; | ||
| BACKUP_GITHUB_PATH: string; | ||
| } |
There was a problem hiding this comment.
TypeScript error: Property BACKUP_NAS_PATH does not exist on BackupEnvConfig.
The interface defines BACKUP_NAS_BACKUP_PATH (line 18), but line 190 references config.BACKUP_NAS_PATH. This will cause a TypeScript compilation error and would always display "not configured" even when the value is set.
🐛 Proposed fix
<SettingRow
label="NAS Path"
- value={config.BACKUP_NAS_PATH || 'not configured'}
+ value={config.BACKUP_NAS_BACKUP_PATH || 'not configured'}
isPath
description="Absolute path on the NAS where backups are stored"
/>Also applies to: 188-193
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/settings/BackupsSettings.tsx` around lines 10 - 24, The
interface BackupEnvConfig defines BACKUP_NAS_BACKUP_PATH but the code reads
config.BACKUP_NAS_PATH causing a TypeScript error and wrong "not configured"
behavior; update either the interface or the usages so the property name
matches: rename the interface key to BACKUP_NAS_PATH or change all references
(e.g., config.BACKUP_NAS_PATH) to config.BACKUP_NAS_BACKUP_PATH (also check the
same mismatch around the other references mentioned) so the symbol names
BackupEnvConfig, BACKUP_NAS_BACKUP_PATH and any occurrences of BACKUP_NAS_PATH
are consistent.
- Add OrgChart component with ElkJS-powered tree layout - Display agent photos, names, roles with activity status indicators - Green/Yellow/Gray dots based on last activity (5/15 min thresholds) - Theme-aware colors (dark/light mode support) - Horizontal tree flow with connecting lines - Add agent photos to public/org/agents/ - Add OrgChart to view mode via TopBar - Include ORG-CHART-SETUP.md documentation
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ORG-CHART-SETUP.md`:
- Around line 186-190: Replace the incorrect "/org" route instruction with steps
that reflect the actual flow: tell users to open the app (npm run dev) then use
the TopBar view mode control to switch to the Org Chart (reference the viewMode
state wiring in src/App.tsx and the selector in src/components/TopBar.tsx);
alternatively, if you prefer a direct entry point, document adding a URL query
or fragment that your App reads to initialize viewMode (e.g., implement/read a
?view=org param in the viewMode logic in src/App.tsx) and describe that option
in the doc.
- Around line 24-26: The fenced code blocks that currently have plain ``` fences
(the one containing "Nerve-CerAImic/public/org/agents/" and the directory tree
block starting with "Nerve-CerAImic/") must include a language specifier (e.g.,
"text") to satisfy markdownlint MD040; update those fenced blocks to use ```text
instead of ``` and apply the same change to the duplicate block later in the
file (the block spanning the tree under "Nerve-CerAImic/" referenced at the
second occurrence).
In `@src/App.tsx`:
- Around line 926-939: The BackupsPanel and OrgChart must be treated as
exclusive full view modes: update all view-mode checks that currently only
special-case 'kanban' (the render branches around the workspace/chat UI — e.g.,
wherever workspace, chat, or Kanban components are conditionally hidden/shown)
to also hide the normal workspace/chat UI when viewMode === 'backups' or
viewMode === 'org' (referencing the viewMode variable and the BackupsPanel and
OrgChart components), and extend the localStorage restore logic (the function
that reads/restores viewMode) to recognize and restore 'backups' and 'org' in
addition to 'kanban' so those modes persist on reload.
In `@src/components/TopBar.tsx`:
- Around line 277-286: The ViewMode union type is missing the "org" variant
which causes type errors where TopBar.tsx uses viewMode === "org" and where
App.tsx references "org"; update the ViewMode type (the union defined in the
symbol ViewMode in src/features/command-palette/commands.ts) to include "org"
(i.e., add the "org" string literal to the union) so the comparisons and
onViewModeChange("org") calls in TopBar.tsx and App.tsx type-check correctly.
In `@src/features/org/OrgChart.tsx`:
- Around line 107-117: AgentNode's status prop type is too narrow (declared as
'active' | 'offline') while getAgentStatus() returns the broader
AgentActivityStatus union ('recent', 'idle', etc.), causing a type mismatch and
unreachable branches; update the AgentNode component prop type signature to use
AgentActivityStatus (the union type used by getAgentStatus) and adjust any
related type annotations or imports so calls that pass getAgentStatus() (e.g.,
the caller that renders <AgentNode ... status={getAgentStatus(...)} />) compile
correctly and the 'recent'/'idle' branches become reachable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 67284449-e726-443a-9489-33cafd5187e4
⛔ Files ignored due to path filters (18)
public/org/agents/alex.pngis excluded by!**/*.pngpublic/org/agents/arthur.pngis excluded by!**/*.pngpublic/org/agents/david.pngis excluded by!**/*.pngpublic/org/agents/edgar.pngis excluded by!**/*.pngpublic/org/agents/emma.pngis excluded by!**/*.pngpublic/org/agents/gemini.pngis excluded by!**/*.pngpublic/org/agents/james.pngis excluded by!**/*.pngpublic/org/agents/jordan.pngis excluded by!**/*.pngpublic/org/agents/main.pngis excluded by!**/*.pngpublic/org/agents/marcus.pngis excluded by!**/*.pngpublic/org/agents/najef.pngis excluded by!**/*.pngpublic/org/agents/neon.pngis excluded by!**/*.pngpublic/org/agents/orion.pngis excluded by!**/*.pngpublic/org/agents/oscar.pngis excluded by!**/*.pngpublic/org/agents/riley.pngis excluded by!**/*.pngpublic/org/agents/sarah.pngis excluded by!**/*.pngpublic/org/agents/susan.pngis excluded by!**/*.pngpublic/org/agents/virginia.pngis excluded by!**/*.png
📒 Files selected for processing (6)
ORG-CHART-SETUP.mdpublic/org/agents/.gitkeepsrc/App.tsxsrc/components/TopBar.tsxsrc/features/org/OrgChart.tsxsrc/features/org/teams.ts
- Add OrgChart component with ElkJS-powered tree layout - Display agent photos, names, roles with activity status indicators - Activity dots: green (0-5min), yellow (5-15min), gray (15+min) - Theme-aware colors (dark/light mode support via CSS variables) - Horizontal tree flow with connecting lines - Add agent photos to public/org/agents/ - Add OrgChart to view mode via TopBar - Include ORG-CHART-SETUP.md documentation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Backups feature provides visibility and management of all OpenClaw backup types. Configuration lives entirely in
Nerve/.env, no values are hardcoded. Two backup targets are configured: Local CLI and NAS Offsite.Summary by CodeRabbit
New Features
Documentation
Chores