feat: mirror repositories #95
Conversation
feat: mirror backup repositories
WalkthroughAdds mirror repository support for backup schedules: new DB junction table and migrations, server APIs and services for managing and copying mirrors (including compatibility checks), client UI and React Query wrappers, SSE mirror events, restic cross-repo copy, and updated snapshot DTOs/types. Changes
Sequence DiagramssequenceDiagram
participant U as User
participant UI as ScheduleMirrorsConfig (Client)
participant Q as React Query
participant API as Server API
participant Svc as Backups Service
participant DB as Database
participant Restic as Restic Worker
participant SSE as Server Events
U->>UI: Open mirrors config / add/toggle/remove
UI->>Q: fetch getScheduleMirrors & getMirrorCompatibility
Q->>API: GET /backups/{id}/mirrors
API->>Svc: request mirrors
Svc->>DB: read backup_schedule_mirrors_table & repositories
DB-->>Svc: mirrors + repo configs
Svc-->>API: mirrors data
API-->>Q: return mirrors
Q-->>UI: render assignments & compatibility
U->>UI: Save changes
UI->>Q: mutate updateScheduleMirrors
Q->>API: PUT /backups/{id}/mirrors
API->>Svc: update mirrors
Svc->>DB: upsert backup_schedule_mirrors_table
DB-->>Svc: success
Svc-->>API: updated mirrors
API-->>Q: mutation response
Q-->>UI: show success
sequenceDiagram
participant Scheduler as Backup Runner
participant Svc as Backups Service
participant Restic as Restic Worker
participant DB as Database
participant SSE as Server Events
Scheduler->>Svc: backup finished for schedule
Svc->>SSE: emit "mirror:started" (for each enabled mirror)
Svc->>Restic: call restic.copy(source, mirror, { snapshotId/tag })
Restic-->>Svc: copy result (success/error)
Svc->>DB: update last_copy_at, last_copy_status, last_copy_error
Svc->>SSE: emit "mirror:completed" with status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 8
🧹 Nitpick comments (7)
app/server/db/schema.ts (1)
163-176: Consider adding indexes for foreign key lookups.Query performance could benefit from indexes on
scheduleIdandrepositoryId, especially when listing mirrors for a schedule or finding all schedules using a repository.export const backupScheduleMirrorsTable = sqliteTable("backup_schedule_mirrors_table", { id: int().primaryKey({ autoIncrement: true }), scheduleId: int("schedule_id") .notNull() .references(() => backupSchedulesTable.id, { onDelete: "cascade" }), repositoryId: text("repository_id") .notNull() .references(() => repositoriesTable.id, { onDelete: "cascade" }), enabled: int("enabled", { mode: "boolean" }).notNull().default(true), lastCopyAt: int("last_copy_at", { mode: "number" }), lastCopyStatus: text("last_copy_status").$type<"success" | "error">(), lastCopyError: text("last_copy_error"), createdAt: int("created_at", { mode: "number" }).notNull().default(sql`(unixepoch() * 1000)`), }, (table) => ({ uniqueScheduleMirror: unique().on(table.scheduleId, table.repositoryId), + scheduleIdIdx: index("idx_schedule_mirrors_schedule_id").on(table.scheduleId), + repositoryIdIdx: index("idx_schedule_mirrors_repository_id").on(table.repositoryId), }));app/client/modules/backups/components/schedule-mirrors-config.tsx (3)
141-156: Consider extracting duplicate logic into a helper function.The
handleResetfunction duplicates the mapping logic from theuseEffecton lines 75-90. This could be extracted into a shared helper to reduce duplication and ensure consistency.+const buildAssignmentsMap = (mirrors: typeof currentMirrors) => { + const map = new Map<string, MirrorAssignment>(); + if (mirrors) { + for (const mirror of mirrors) { + map.set(mirror.repositoryId, { + repositoryId: mirror.repositoryId, + enabled: mirror.enabled, + lastCopyAt: mirror.lastCopyAt, + lastCopyStatus: mirror.lastCopyStatus, + lastCopyError: mirror.lastCopyError, + }); + } + } + return map; +}; + const handleReset = () => { - if (currentMirrors) { - const map = new Map<string, MirrorAssignment>(); - for (const mirror of currentMirrors) { - map.set(mirror.repositoryId, { - repositoryId: mirror.repositoryId, - enabled: mirror.enabled, - lastCopyAt: mirror.lastCopyAt, - lastCopyStatus: mirror.lastCopyStatus, - lastCopyError: mirror.lastCopyError, - }); - } - setAssignments(map); - setHasChanges(false); - } + setAssignments(buildAssignmentsMap(currentMirrors)); + setHasChanges(false); };
169-172: Clarify the double-negative logic for readability.The expression
compat?.compatible !== falseuses a double negative that can be confusing. It means "compatible is true or undefined". Consider making this more explicit:const hasAvailableRepositories = selectableRepositories.some((r) => { const compat = compatibilityMap.get(r.id); - return compat?.compatible !== false; + return compat?.compatible === true; });Or if you want to include repositories without compatibility info:
- return compat?.compatible !== false; + return !compat || compat.compatible;
283-286: Filter out undefined values before mapping to avoid returning null.While React handles
nullreturns in map callbacks, it's cleaner to filter first. TheassignedRepositoriesalready filters undefined values on line 176, but theassignmentcheck on line 285 could still returnnullif the Map gets out of sync.-{assignedRepositories.map((repository) => { - const assignment = assignments.get(repository.id); - if (!assignment) return null; +{assignedRepositories + .filter((repository) => assignments.has(repository.id)) + .map((repository) => { + const assignment = assignments.get(repository.id)!;app/client/components/snapshots-table.tsx (1)
103-116: Simplify the conditional rendering for the Schedule column.The current implementation has redundant logic: both the
Linkand fallbackspanrender-when no backup exists, and thehiddenattribute pattern is unconventional for React.Consider a cleaner conditional approach:
<TableCell> - <div className="flex items-center gap-2"> - <Link - hidden={!backup} - to={backup ? `/backups/${backup.id}` : "#"} - onClick={(e) => e.stopPropagation()} - className="hover:underline" - > - <span className="text-sm">{backup ? backup.id : "-"}</span> - </Link> - <span hidden={!!backup} className="text-sm text-muted-foreground"> - - - </span> - </div> + <div className="flex items-center gap-2"> + {backup ? ( + <Link + to={`/backups/${backup.id}`} + onClick={(e) => e.stopPropagation()} + className="hover:underline text-sm" + > + {backup.id} + </Link> + ) : ( + <span className="text-sm text-muted-foreground">-</span> + )} + </div> </TableCell>app/server/utils/backend-compatibility.ts (2)
38-92: Consider error handling for decryption failures.If
cryptoUtils.decryptthrows an error (e.g., corrupted data, missing keys), the exception will propagate uncaught. Consider wrapping decryption calls in try-catch to return a safe default or provide a meaningful error message to callers.
109-123: Minor redundancy withhasCompatibleCredentials.The null/different-group checks duplicate logic already in
hasCompatibleCredentials(lines 34-36). While this provides a minor optimization by avoiding the async call, it introduces maintenance overhead if the compatibility rules change.Consider whether the optimization is necessary, or if the function could simply delegate directly to
hasCompatibleCredentials.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
app/client/api-client/@tanstack/react-query.gen.ts(2 hunks)app/client/api-client/sdk.gen.ts(2 hunks)app/client/api-client/types.gen.ts(3 hunks)app/client/components/snapshots-table.tsx(4 hunks)app/client/hooks/use-server-events.ts(3 hunks)app/client/modules/backups/components/schedule-mirrors-config.tsx(1 hunks)app/client/modules/backups/routes/backup-details.tsx(3 hunks)app/client/modules/repositories/tabs/snapshots.tsx(3 hunks)app/drizzle/0018_bizarre_zzzax.sql(1 hunks)app/drizzle/meta/0018_snapshot.json(1 hunks)app/drizzle/meta/_journal.json(1 hunks)app/server/core/events.ts(1 hunks)app/server/db/schema.ts(3 hunks)app/server/modules/backups/backups.controller.ts(3 hunks)app/server/modules/backups/backups.dto.ts(2 hunks)app/server/modules/backups/backups.service.ts(5 hunks)app/server/modules/events/events.controller.ts(2 hunks)app/server/modules/repositories/repositories.controller.ts(2 hunks)app/server/modules/repositories/repositories.dto.ts(1 hunks)app/server/utils/backend-compatibility.ts(1 hunks)app/server/utils/restic.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (9)
app/server/utils/restic.ts (2)
app/schemas/restic.ts (1)
RepositoryConfig(91-91)app/server/utils/errors.ts (1)
ResticError(31-41)
app/server/modules/events/events.controller.ts (1)
app/server/core/events.ts (1)
serverEvents(45-45)
app/client/modules/backups/components/schedule-mirrors-config.tsx (4)
app/server/db/schema.ts (1)
Repository(63-63)app/client/api-client/@tanstack/react-query.gen.ts (3)
getScheduleMirrorsOptions(763-774)getMirrorCompatibilityOptions(798-809)updateScheduleMirrorsMutation(779-791)app/client/components/repository-icon.tsx (1)
RepositoryIcon(9-23)app/client/components/status-dot.tsx (1)
StatusDot(12-61)
app/server/utils/backend-compatibility.ts (1)
app/schemas/restic.ts (1)
RepositoryConfig(91-91)
app/server/modules/backups/backups.dto.ts (1)
app/server/modules/repositories/repositories.dto.ts (1)
repositorySchema(11-23)
app/client/modules/repositories/tabs/snapshots.tsx (2)
app/client/api-client/@tanstack/react-query.gen.ts (1)
listBackupSchedulesOptions(572-583)app/client/components/snapshots-table.tsx (1)
SnapshotsTable(31-199)
app/client/components/snapshots-table.tsx (3)
app/client/lib/types.ts (1)
Snapshot(20-20)app/server/db/schema.ts (1)
BackupSchedule(109-109)app/client/api-client/sdk.gen.ts (1)
deleteSnapshot(306-311)
app/client/api-client/@tanstack/react-query.gen.ts (3)
app/client/api-client/sdk.gen.ts (4)
Options(7-19)getScheduleMirrors(482-487)updateScheduleMirrors(492-501)getMirrorCompatibility(506-511)app/client/api-client/client/index.ts (1)
Options(18-18)app/client/api-client/client/types.gen.ts (1)
Options(232-241)
app/client/api-client/sdk.gen.ts (1)
app/client/api-client/types.gen.ts (6)
GetScheduleMirrorsData(2117-2124)GetScheduleMirrorsResponses(2126-2212)UpdateScheduleMirrorsData(2216-2228)UpdateScheduleMirrorsResponses(2230-2316)GetMirrorCompatibilityData(2320-2327)GetMirrorCompatibilityResponses(2329-2338)
🔇 Additional comments (31)
app/drizzle/meta/0018_snapshot.json (1)
1-740: Schema structure is well-designed overall.The database schema demonstrates good normalization, consistent naming conventions, proper foreign key relationships with cascade delete semantics, and appropriate use of unique indexes on business identifiers. The addition of
backup_schedule_mirrors_tableintegrates cleanly into the existing structure.app/drizzle/meta/_journal.json (1)
131-137: Journal entry is correctly formatted.The migration journal entry for
0018_bizarre_zzzaxis properly structured with correct version, timestamp sequencing, and metadata. No issues detected.app/server/utils/restic.ts (1)
43-43: LGTM!Adding the
tagsfield to the snapshot schema appropriately extends the metadata model.app/server/modules/events/events.controller.ts (1)
73-114: LGTM!The mirror event handlers follow the established SSE pattern and include proper cleanup on client disconnect.
app/server/modules/repositories/repositories.dto.ts (1)
177-177: LGTM!The
tagsfield addition aligns with the updated snapshot metadata model.app/client/modules/repositories/tabs/snapshots.tsx (2)
24-26: LGTM!Fetching backup schedules enables the UI to display schedule associations for snapshots.
139-139: LGTM!Passing the schedules data with a safe fallback to empty array is correct.
app/server/modules/repositories/repositories.controller.ts (1)
93-93: LGTM!The nullish coalescing operator ensures the response always includes a
tagsarray, preventing potential client-side errors.Also applies to: 117-117
app/client/modules/backups/routes/backup-details.tsx (1)
57-57: LGTM!The loader fetches repositories and the UI conditionally renders mirror configuration when multiple repositories exist. The logic correctly requires at least 2 repositories (primary + mirror).
Also applies to: 231-237
app/server/core/events.ts (1)
27-34: LGTM!The mirror event type definitions follow the established pattern and include appropriate payload structures.
app/client/hooks/use-server-events.ts (2)
40-46: LGTM! Well-structured MirrorEvent interface.The interface appropriately includes an
errorfield for capturing copy failures, which aligns with thelastCopyErrorfield in the database schema and provides useful context for debugging.
138-157: Mirror event handlers follow the established pattern correctly.The implementation is consistent with existing backup and volume event handlers. Cache invalidation on
mirror:completedensures the UI reflects the latest mirror status.app/client/api-client/sdk.gen.ts (1)
479-511: Auto-generated mirror API functions are correctly structured.The new
getScheduleMirrors,updateScheduleMirrors, andgetMirrorCompatibilityfunctions follow the established patterns in this auto-generated file. The PUT endpoint correctly includes theContent-Type: application/jsonheader.app/server/modules/backups/backups.controller.ts (1)
124-142: Mirror endpoints are well-structured and follow established patterns.The implementation correctly mirrors the existing notification endpoints pattern. The use of
Number.parseInt(param, 10)is consistent throughout the controller, and the PUT endpoint properly validates the request body.app/client/modules/backups/components/schedule-mirrors-config.tsx (1)
194-355: Well-implemented UI component with good UX patterns.The component handles multiple states gracefully: empty state with helpful messaging, loading states via the mutation's
isPending, and clear visual feedback for compatibility issues. The use of tooltips for incompatible repositories and the status indicators provide good user feedback.app/drizzle/0018_bizarre_zzzax.sql (1)
1-13: No action required—FK constraints are safe in this migration.The
backup_schedule_mirrors_tableis created for the first time in this migration (no prior references exist), so it will be empty when the referenced tables (backup_schedules_tableandrepositories_table) are dropped and recreated. FK constraint violations only occur if foreign key rows exist in the child table; since this table is newly created and contains no data, FK enforcement poses no risk during the subsequent table rebuilds.app/client/api-client/@tanstack/react-query.gen.ts (1)
758-809: LGTM - Auto-generated code follows established patterns.The new mirror-related React Query wrappers (
getScheduleMirrorsQueryKey,getScheduleMirrorsOptions,updateScheduleMirrorsMutation,getMirrorCompatibilityQueryKey,getMirrorCompatibilityOptions) are consistent with existing query/mutation patterns in the file.app/client/components/snapshots-table.tsx (1)
87-89: LGTM - Tag parsing logic is sound.The defensive parsing of tags (
map(Number).filter(!isNaN)) correctly handles the string-to-number conversion, and using.find()to locate the associated backup is appropriate.app/client/api-client/types.gen.ts (1)
2117-2341: LGTM - Auto-generated mirror types are consistent.The new type definitions for schedule mirrors (
GetScheduleMirrorsData,UpdateScheduleMirrorsData,GetMirrorCompatibilityDataand their responses) follow the established patterns in this auto-generated file.app/server/modules/backups/backups.service.ts (4)
270-273: Background mirror copy decoupled from backup success - verify this is intentional.The fire-and-forget pattern means the backup will report success even if mirror copies fail. Mirror failures are logged and stored in the database, and SSE events are emitted, but the backup status won't reflect mirror issues.
This design is reasonable for background operations, but ensure the UI adequately surfaces mirror failures to users (via SSE events or polling the mirrors endpoint).
431-446: LGTM - getMirrors implementation.The function correctly validates schedule existence and retrieves mirror assignments with repository details.
448-493: LGTM - updateMirrors validation and persistence logic.Good defensive patterns:
- Validates schedule exists
- Prevents adding primary repository as mirror
- Checks each mirror repository exists
- Validates compatibility before making changes
- Delete-then-insert is acceptable for this use case
567-585: LGTM - getMirrorCompatibility implementation.Correctly filters out the primary repository and checks compatibility for all other repositories.
app/server/modules/backups/backups.dto.ts (3)
40-52: LGTM - scheduleMirrorSchema is well-defined.The schema captures all necessary fields for mirror status tracking including
lastCopyAt,lastCopyStatus,lastCopyError, and embeds the full repository details.
312-338: LGTM - Mirror update DTOs and route descriptors.The
updateScheduleMirrorsBodyschema is clear and minimal, requiring justrepositoryIdandenabledfor each mirror. The route descriptor follows established patterns.
340-363: LGTM - Mirror compatibility schema and endpoint.The
mirrorCompatibilitySchemacleanly represents the compatibility check result withrepositoryId,compatible, andreasonfields.app/server/utils/backend-compatibility.ts (5)
1-4: LGTM!Imports and type definition are appropriate. The
BackendConflictGroupunion type clearly defines the supported backend groups withnullfor non-conflicting backends.
6-25: LGTM!The backend-to-conflict-group mapping is well-structured. Grouping
r2withs3is appropriate given R2's S3-compatibility, and defaulting unknown backends tonullis a safe fallback.
87-89: Clarify intent: SFTP always returns incompatible.The SFTP case unconditionally returns
false, meaning SFTP repositories can never be considered compatible for mirroring with other SFTP repositories. If this is intentional (e.g., due to SSH key complexity), consider adding a comment explaining why.
95-99: LGTM!The interface is clear and well-suited for returning compatibility check results.
142-148: LGTM!Clear, actionable error message that explains the Restic limitation and provides helpful guidance for users.
app/client/modules/backups/components/schedule-mirrors-config.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
app/server/utils/restic.ts (1)
717-726: Validate that source and destination repositories differ.Copying snapshots from a repository to itself wastes resources and could cause unexpected behavior. Add validation after computing the repo URLs.
const copy = async ( sourceConfig: RepositoryConfig, destConfig: RepositoryConfig, options: { tag?: string; snapshotId?: string; }, ) => { const sourceRepoUrl = buildRepoUrl(sourceConfig); const destRepoUrl = buildRepoUrl(destConfig); + + if (sourceRepoUrl === destRepoUrl) { + throw new Error("Source and destination repositories must be different"); + }app/client/modules/backups/components/schedule-mirrors-config.tsx (1)
75-90: Nice guard to avoid overwriting unsaved mirror edits.The
useEffectnow checks!hasChangesbefore rehydratingassignmentsfromcurrentMirrors, which addresses the earlier concern about refetches clobbering local, unsaved changes. This is a good balance between keeping in sync with the server and preserving in‑progress edits.
🧹 Nitpick comments (4)
app/server/modules/backups/backups.service.ts (1)
480-490: Consider wrapping delete-and-insert in a transaction.The delete-then-insert pattern at Lines 480-490 could leave the schedule without mirrors if the insert fails. While unlikely, wrapping in a transaction ensures atomicity.
+ await db.transaction(async (tx) => { - await db.delete(backupScheduleMirrorsTable).where(eq(backupScheduleMirrorsTable.scheduleId, scheduleId)); + await tx.delete(backupScheduleMirrorsTable).where(eq(backupScheduleMirrorsTable.scheduleId, scheduleId)); - if (data.mirrors.length > 0) { - await db.insert(backupScheduleMirrorsTable).values( - data.mirrors.map((mirror) => ({ - scheduleId, - repositoryId: mirror.repositoryId, - enabled: mirror.enabled, - })), - ); - } + if (data.mirrors.length > 0) { + await tx.insert(backupScheduleMirrorsTable).values( + data.mirrors.map((mirror) => ({ + scheduleId, + repositoryId: mirror.repositoryId, + enabled: mirror.enabled, + })), + ); + } + });app/client/modules/backups/components/schedule-mirrors-config.tsx (3)
92-126: Prefer functionalsetAssignmentsupdates to avoid stale state.All three mutators (
addRepository,removeRepository,toggleEnabled) cloneassignmentsfrom the render closure. Under concurrent updates, this can drop changes if two handlers run before React flushes state.Using functional updates keeps it robust without changing behavior:
-const addRepository = (repositoryId: string) => { - const newAssignments = new Map(assignments); - newAssignments.set(repositoryId, { - repositoryId, - enabled: true, - lastCopyAt: null, - lastCopyStatus: null, - lastCopyError: null, - }); - - setAssignments(newAssignments); - setHasChanges(true); - setIsAddingNew(false); -}; +const addRepository = (repositoryId: string) => { + setAssignments((prev) => { + const next = new Map(prev); + next.set(repositoryId, { + repositoryId, + enabled: true, + lastCopyAt: null, + lastCopyStatus: null, + lastCopyError: null, + }); + return next; + }); + setHasChanges(true); + setIsAddingNew(false); +}; -const removeRepository = (repositoryId: string) => { - const newAssignments = new Map(assignments); - newAssignments.delete(repositoryId); - setAssignments(newAssignments); - setHasChanges(true); -}; +const removeRepository = (repositoryId: string) => { + setAssignments((prev) => { + const next = new Map(prev); + next.delete(repositoryId); + return next; + }); + setHasChanges(true); +}; -const toggleEnabled = (repositoryId: string) => { - const assignment = assignments.get(repositoryId); - if (!assignment) return; - - const newAssignments = new Map(assignments); - newAssignments.set(repositoryId, { - ...assignment, - enabled: !assignment.enabled, - }); - - setAssignments(newAssignments); - setHasChanges(true); -}; +const toggleEnabled = (repositoryId: string) => { + setAssignments((prev) => { + const assignment = prev.get(repositoryId); + if (!assignment) return prev; + const next = new Map(prev); + next.set(repositoryId, { ...assignment, enabled: !assignment.enabled }); + return next; + }); + setHasChanges(true); +};
165-168: Clarify compatibility behavior while data is loading to avoid misleading “incompatible” UX.When
compatibilityis still loading,compatibilityMapis empty, so for each selectable repo:
compatisundefineddisabled={!compat?.compatible}→true- Tooltip content uses the generic “not compatible” message and is not hidden (
hidden: compat?.compatible→false)This transiently makes every option look disabled and “not compatible” even though you’re just waiting on the compatibility query.
It’d be clearer to distinguish “loading” from “incompatible”, for example by:
- Only showing the “not compatible” tooltip when
compat?.compatible === false, and- Optionally using a “Checking compatibility…” message when
compatibilityhasn’t resolved yet, or hiding the tooltip entirely in that state.Conceptually:
const compat = compatibilityMap.get(repository.id); const isIncompatible = compat?.compatible === false; const isLoadingCompatibility = !compat && !compatibility; <SelectItem value={repository.id} disabled={isLoadingCompatibility ? true : isIncompatible} > … </SelectItem> <TooltipContent side="right" className={cn("max-w-xs", { hidden: !isIncompatible && !isLoadingCompatibility })} > <p> {isLoadingCompatibility ? "Checking compatibility…" : compat?.reason || "This repository is not compatible for mirroring."} </p> </TooltipContent>This avoids briefly labeling everything as incompatible while the compatibility query is just in flight.
Also applies to: 219-244, 235-240
261-267: Consider guarding the empty state on query loading to avoid a brief “no mirrors” flicker.Right now, when the component mounts,
assignedRepositoriesis empty untilcurrentMirrorsloads, so users can briefly see “No mirror repositories configured for this schedule” even if there are existing mirrors.If you want to avoid that flicker, you could gate the empty state on a loading flag or on having received initial data, e.g.:
const { data: currentMirrors, isLoading: isMirrorsLoading } = useQuery({ … }); … {assignedRepositories.length === 0 && !isMirrorsLoading ? ( // empty state ) : …}This would keep the UI neutral while data is still being fetched.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/client/modules/backups/components/schedule-mirrors-config.tsx(1 hunks)app/drizzle/0018_bizarre_zzzax.sql(1 hunks)app/drizzle/0019_heavy_shen.sql(1 hunks)app/drizzle/meta/0019_snapshot.json(1 hunks)app/drizzle/meta/_journal.json(1 hunks)app/server/db/schema.ts(4 hunks)app/server/modules/backups/backups.service.ts(5 hunks)app/server/utils/restic.ts(14 hunks)
✅ Files skipped from review due to trivial changes (1)
- app/drizzle/meta/0019_snapshot.json
🚧 Files skipped from review as they are similar to previous changes (1)
- app/drizzle/meta/_journal.json
🧰 Additional context used
🧬 Code graph analysis (2)
app/server/utils/restic.ts (2)
app/schemas/restic.ts (1)
RepositoryConfig(91-91)app/server/utils/errors.ts (1)
ResticError(31-41)
app/client/modules/backups/components/schedule-mirrors-config.tsx (4)
app/server/db/schema.ts (1)
Repository(63-63)app/client/api-client/@tanstack/react-query.gen.ts (3)
getScheduleMirrorsOptions(763-774)getMirrorCompatibilityOptions(798-809)updateScheduleMirrorsMutation(779-791)app/client/components/repository-icon.tsx (1)
RepositoryIcon(9-23)app/client/components/status-dot.tsx (1)
StatusDot(12-61)
🔇 Additional comments (7)
app/server/db/schema.ts (1)
159-192: Well-structured junction table for mirror repositories.The schema correctly implements:
- Auto-increment primary key for row-level updates
- Foreign keys with cascade deletes for referential integrity
- Unique constraint on
(scheduleId, repositoryId)preventing duplicate assignments (addressing prior feedback)- Status tracking fields for last copy operations
- Proper relations to both schedule and repository
app/drizzle/0019_heavy_shen.sql (1)
1-1: LGTM!The unique index correctly enforces the composite uniqueness constraint on
(schedule_id, repository_id), aligning with the schema definition.app/server/utils/restic.ts (2)
788-794: Good refactor centralizing common argument handling.The simplified
addCommonArgs(args, env)signature cleanly handles SFTP arguments via the environment, reducing duplication across command builders.
43-43: LGTM!The optional
tagsfield correctly extends the snapshot schema to support tag-based filtering in mirror operations.app/server/modules/backups/backups.service.ts (2)
270-273: Mirror copy failures are logged but not retried.The fire-and-forget pattern is appropriate for secondary mirrors, but consider implementing a retry mechanism or queue for failed mirror copies in a future iteration.
431-446: LGTM!The
getMirrorsfunction correctly validates schedule existence and retrieves associated mirrors with repository details.app/drizzle/0018_bizarre_zzzax.sql (1)
1-139: LGTM!The migration correctly:
- Creates the new
backup_schedule_mirrors_tablewith proper foreign key relationships- Wraps all table rebuild operations within
PRAGMA foreign_keys=OFF/ON(Lines 15-139), addressing the prior feedback about constraint violations during intermediate states- Preserves data during table rebuilds via insert-from-old-table pattern
- Recreates necessary unique indexes
* feat: mirror repositories feat: mirror backup repositories * chore: pr feedbacks
* feat: mirror repositories feat: mirror backup repositories * chore: pr feedbacks
Automatically copy snapshots to mirror repositories
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.