feat: add volume export functionality and UI integration#38
Conversation
- Introduced a new **Exports** tab in the Volume Detail screen, allowing users to manage volume exports. - Implemented backend API endpoints for volume exports, including creation and download functionalities. - Added a **Quick export** flow for exporting volumes to local files, images, or registries. - Enhanced the UI with components for displaying export history and initiating exports. - Updated the changelog to reflect these new features and improvements.
|
Warning Review limit reached
Next review available in: 11 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (20)
📝 WalkthroughWalkthroughThis PR adds volume export and scheduled export capabilities to Calf. Backend changes introduce a runtime export engine, JSON-backed export/schedule stores, HTTP handlers, and a scheduler daemon. UI changes add quick-export and schedule-export screens, an Exports tab, client models/methods, and file-picker plugin wiring across platforms. ChangesVolume Export & Scheduling
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Ticker
participant exportScheduler
participant ScheduleStore
participant Server
participant Runtime
Ticker->>exportScheduler: tick
exportScheduler->>ScheduleStore: ListAll()
ScheduleStore-->>exportScheduler: schedules
exportScheduler->>exportScheduler: filter due schedules
exportScheduler->>Server: executeVolumeExport(request)
Server->>Runtime: ExportVolume(ctx, opts)
Runtime-->>Server: result / error
Server-->>exportScheduler: export status
exportScheduler->>ScheduleStore: Save(updated schedule)
sequenceDiagram
participant User
participant VolumeDetailView
participant ApiClient
participant FileSystem
User->>VolumeDetailView: tap download
VolumeDetailView->>ApiClient: downloadVolumeExport(volumeName, exportId)
ApiClient-->>VolumeDetailView: bytes
VolumeDetailView->>User: getSaveLocation prompt
User-->>VolumeDetailView: chosen path
VolumeDetailView->>FileSystem: write bytes
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Resolve conflicts keeping volume export scheduler alongside build history sync and build detail API methods. Co-authored-by: Cursor <[email protected]>
![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)
ui/lib/api/client.dart (1)
641-653: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCatch only parse failures when formatting
nextRunAt.
DateTime.parseshould only fall back for invalid date strings; catching every exception can hide unrelated defects in this display helper. Based on learnings, “Do not write generic catch-alls; handle each specific error case individually so logs, UI, and API responses clearly identify what failed and why.”Proposed fix
- } catch (_) { + } on FormatException { return nextRunAt; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/lib/api/client.dart` around lines 641 - 653, The `nextRunAt` display helper in `client.dart` is using a blanket catch that can hide non-parse bugs. Update the formatting logic around `DateTime.parse` to handle only parsing failures explicitly, and let unexpected exceptions surface instead of falling back silently. Keep the existing `nextRunAt` formatting path intact for valid dates, and only return the raw `nextRunAt` value when the parse itself fails.Source: Learnings
ui/lib/screens/volume_quick_export_screen.dart (1)
55-79: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGeneric catch-alls surface raw error strings to the UI.
_loadImages,_browseFolder, and_saveall fall back to a barecatch (error)that displayserror.toString()directly to the user. This repeats across the file and doesn't distinguish network failures, validation errors, or backend error codes, making it hard for users/logs to pinpoint the actual cause.Based on learnings, "Do not write generic catch-alls; handle each specific error case individually so logs, UI, and API responses clearly identify what failed and why."
Also applies to: 97-122, 124-160
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/lib/screens/volume_quick_export_screen.dart` around lines 55 - 79, The error handling in _loadImages, _browseFolder, and _save is using a generic catch and surfacing error.toString() directly to the UI. Update the exception handling in VolumeQuickExportScreen to catch and map specific failure cases separately (for example network, validation, and backend/API errors), and set a user-facing message that reflects the actual failure type instead of the raw exception string. Keep the mounted checks and apply the same pattern consistently across the relevant methods so the UI and logs clearly identify what failed and why.ui/lib/screens/volume_schedule_export_screen.dart (1)
103-127: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSame generic catch-all pattern repeated here.
Same issue flagged in
volume_quick_export_screen.dart: every async operation (_loadImages,_browseFolder,_applyChanges,_confirmDelete) falls back to a barecatch (error)and showserror.toString()to the user, rather than handling specific failure modes.Based on learnings, "Do not write generic catch-alls; handle each specific error case individually so logs, UI, and API responses clearly identify what failed and why."
Also applies to: 258-283, 341-395, 397-445
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/lib/screens/volume_schedule_export_screen.dart` around lines 103 - 127, The async flow in _loadImages still uses a generic catch-all and surfaces error.toString() directly, which should be replaced with handling for the specific failure modes used elsewhere in VolumeScheduleExportScreen. Update _loadImages (and the related _browseFolder, _applyChanges, and _confirmDelete paths mentioned in the comment) to catch and branch on concrete exception types, then set clearer UI error states and logging for each case instead of a single broad catch (error).backend/internal/runtime/volume_export.go (2)
71-79: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCleanup defer reuses the parent, possibly-canceled
ctx.If
ctxis canceled/times out mid-export, thenerdctl rm -fcleanup in the deferred func will also fail immediately (same canceled context), leaking the staging container. Using a short-lived detached context for teardown would make cleanup reliable regardless of the caller's cancellation state.♻️ Suggested approach
defer func() { - _, _ = run(ctx, "nerdctl", "rm", "-f", containerName) + cleanupCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + _, _ = run(cleanupCtx, "nerdctl", "rm", "-f", containerName) }()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/runtime/volume_export.go` around lines 71 - 79, The deferred cleanup in volume export reuses the same possibly-canceled context, so the `nerdctl rm -f` teardown can fail after caller timeout/cancellation. Update the `defer` inside `volumeExport` to use a fresh short-lived background context for cleanup instead of the parent `ctx`, and keep the existing `run`/`containerName` flow unchanged so the staging container is always removed reliably.
42-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated
"alpine:3.20"literal.The base image tag is hard-coded twice. Extracting it to a package-level constant avoids the two copies drifting apart on future upgrades.
♻️ Suggested fix
+const exportBaseImage = "alpine:3.20" + func exportVolumeArchive(ctx context.Context, run commandRunner, volumeName, archivePath string) error { ... - "alpine:3.20", + exportBaseImage,Also applies to: 72-72
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/runtime/volume_export.go` at line 42, The alpine base image tag is duplicated in the runtime volume export setup, so extract the hard-coded "alpine:3.20" into a single package-level constant and reuse it in the relevant places. Update the values referenced from the volume export code path (including the image list and any other use in volume_export.go) to point to that constant so future version bumps stay in sync.backend/internal/runtime/mock.go (1)
205-229: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuses
ContainerErrfor a volume-export failure.
ExportVolumereturnsm.ContainerErrfor a generic failure trigger, but that field is otherwise scoped to container operations elsewhere inMock. Reusing it here couples unrelated failure domains and prevents a test from simulating a container error and an export error independently, obscuring what a given test failure is actually exercising.♻️ Suggested fix: add a dedicated error field
type Mock struct { ... ContainerErr error ImageErr error + ExportErr error ... } func (m *Mock) ExportVolume(_ context.Context, opts VolumeExportOptions) (string, error) { m.mu.Lock() defer m.mu.Unlock() - if m.ContainerErr != nil { - return "", m.ContainerErr + if m.ExportErr != nil { + return "", m.ExportErr }Based on learnings, "Do not use generic catch-alls; handle each specific error case individually so logs, UI, and API responses identify exactly what failed and why."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/runtime/mock.go` around lines 205 - 229, ExportVolume currently reuses Mock.ContainerErr as a generic failure trigger, which couples volume export failures to container errors. Add a dedicated error field on Mock for export-volume failures and have Mock.ExportVolume check that field instead of ContainerErr, so container and export tests can fail independently. Update the ExportVolume method and any related mock setup helpers to use the new field while keeping the existing validation branches for opts.Type, opts.Folder, and opts.ImageRef unchanged.Source: Learnings
backend/internal/api/volumes.go (1)
78-114: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant/unreachable OPTIONS checks.
handleVolumeActionalready returns 204 for any OPTIONS request at function entry (pre-existing check, not shown in this diff range). The four newif r.Method == http.MethodOptions {...}blocks added here (forexports,export-schedules, theexport-schedules/{id}branch, and theexports/{id}/downloadbranch) can never execute, since any OPTIONS request already returned earlier. This is dead code contributing to the flagged complexity.♻️ Proposed cleanup
case "exports": - if r.Method == http.MethodOptions { - w.WriteHeader(http.StatusNoContent) - return - } - s.handleVolumeExports(w, r, name) return case "export-schedules": - if r.Method == http.MethodOptions { - w.WriteHeader(http.StatusNoContent) - return - } - s.handleVolumeExportSchedules(w, r, name) return } } if len(parts) == 3 && parts[1] == "export-schedules" { - if r.Method == http.MethodOptions { - w.WriteHeader(http.StatusNoContent) - return - } - s.handleVolumeExportScheduleItem(w, r, name, parts[2]) return } if len(parts) == 4 && parts[1] == "exports" && parts[3] == "download" { - if r.Method == http.MethodOptions { - w.WriteHeader(http.StatusNoContent) - return - } - s.handleVolumeExportDownload(w, r, name, parts[2]) return }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/api/volumes.go` around lines 78 - 114, The new OPTIONS handling inside handleVolumeAction is unreachable because the method already returns 204 for OPTIONS at the top of the function. Remove the redundant http.MethodOptions checks from the exports, export-schedules, export-schedules/{id}, and exports/{id}/download branches so the routing logic stays simple and avoids dead code.backend/internal/volumeexport/name_pattern_test.go (1)
1-61: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMissing coverage for image-ref expansion / uppercase volume names.
All tests exercise the file-name expansion path only. Adding a test for
ExpandExportImageRefPattern/DefaultImageRefPatternwith a mixed-case volume name would have caught the lowercase-ref bug flagged inname_pattern.go.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/volumeexport/name_pattern_test.go` around lines 1 - 61, The current tests in TestExpandExportNamePattern and TestResolveScheduledExportNames only cover file-name expansion, so add coverage for image-ref generation as well. Introduce a test that exercises ExpandExportImageRefPattern with DefaultImageRefPattern using a mixed-case volume name and assert the result is normalized/lowercased correctly. Use the existing volumeexport helpers and Schedule-related paths so the new test would catch the lowercase-ref bug in name_pattern.go.backend/internal/api/volume_export_runner.go (1)
19-91: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo explicit timeout for API-triggered exports.
executeVolumeExportis called fromhandleVolumeExportCreatewith the barer.Context()(no additional deadline), while the scheduler path wraps the same call with a 30-minutecontext.WithTimeout. If the runtime export hangs (e.g. a stuckdocker/nerdctlcommand) and the client keeps the connection open, the request goroutine blocks indefinitely with no bound. For consistency with the scheduler path, consider applying a similar explicit timeout in the HTTP handler as well.As per coding guidelines, "
backend/**/*.go: In Go backend code, thread acontext.Contextwith an explicit timeout or cancellation through every handler-to-runtime path".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/api/volume_export_runner.go` around lines 19 - 91, `executeVolumeExport` is invoked from `handleVolumeExportCreate` without a bounded deadline, so API-triggered exports can block indefinitely if the runtime hangs. Update the HTTP handler path to create a timeout context before calling `executeVolumeExport`, matching the scheduler flow that already uses `context.WithTimeout`, and ensure the timeout is propagated through `Server.executeVolumeExport` into `runtime.ExportVolume`.Source: Coding guidelines
backend/internal/api/server.go (1)
40-52: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConstructor starts an unstoppable-by-default background goroutine with hardcoded storage paths.
New()unconditionally spins upexportScheduler(which immediately ticks and reads/writes under~/.config/calf/...viavolumeexport.NewStore/NewScheduleStore, both of which resolveos.UserHomeDir()with no override hook). Every caller ofNew()— including tests — now performs real filesystem I/O in the user's home directory unless it later callsShutdown(). See thebackend/test/api/api_test.goreview for the concrete downstream impact (goroutine leak + real fs writes innewTestServer).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/internal/api/server.go` around lines 40 - 52, New() is eagerly starting exportScheduler, which creates an always-on background goroutine and immediately touches hardcoded home-directory storage through volumeexport/NewStore and NewScheduleStore. Make Server construction side-effect free by deferring scheduler creation/startup out of New() (or gating it behind an explicit opt-in/init method), and use the existing Server fields like cfg/runtime to add a test-friendly way to override or disable the storage path. Ensure callers such as newTestServer can construct Server without triggering filesystem I/O or needing Shutdown() just to avoid leaks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.cursor/rules/calf.mdc:
- Around line 125-127: Update the project tree branch markers in the rules file
so the sibling entries under the volume screens are rendered correctly. In the
tree snippet containing volume_detail_screen.dart,
volume_quick_export_screen.dart, and volume_schedule_export_screen.dart, change
the first two child markers to use the non-final branch form and keep only the
last entry as the final branch. This is just a formatting fix in the tree
listing, so adjust the markers around those three filenames accordingly.
In `@backend/internal/api/volume_export_scheduler.go`:
- Around line 39-44: `Stop()` currently waits for `runSchedule()` to finish but
the schedule context is only based on `context.Background()` with a fixed
timeout, so shutdown cannot cancel an in-flight export. Update `exportScheduler`
so `Stop()` cancels the scheduler context immediately (for example by
introducing a cancelable context tied to `s.stop` or a `context.WithCancel`
field), and have `runSchedule` use that context instead of an isolated
background timeout. Make sure the goroutine in `run()`/`runSchedule()` observes
cancellation and exits promptly so `<-s.done` returns during `Server.Shutdown`
rather than blocking until the export completes or times out.
In `@backend/internal/api/volume_export_schedules.go`:
- Around line 42-56: The HTTP handlers are returning raw internal error strings
directly via writeError, which can leak filesystem/path details to clients.
Update handleVolumeExportSchedulesList and the other affected handlers in
backend/internal/api to stop passing err.Error() straight to the response;
instead map failures through writeRuntimeError (or writeJSON where appropriate)
with the correct HTTP status code. Use the existing handler/store functions like
volumeScheduleStore and store.List as the places where errors should be
converted before writing the response.
In `@backend/internal/runtime/volume_export.go`:
- Around line 52-72: The export container name in exportVolumeToImage is not
unique because it is built from filepath.Base(archivePath), which is always the
same archive filename and can collide across concurrent or repeated exports.
Update the containerName generation in exportVolumeToImage to derive from a
unique part of the export path, such as the parent directory or another
export-specific identifier, so each nerdctl run uses a distinct name. Make the
same change in the other export path handling referenced by the related block so
local_image, new_image, and registry exports all avoid name collisions.
In `@backend/internal/volumeexport/name_pattern.go`:
- Around line 82-85: The image-ref path built from sanitizeVolumeToken is still
preserving uppercase characters, which can produce invalid Docker/nerdctl
repository names. Update the TypeNewImage and TypeRegistry flow in
name_pattern.go to apply strings.ToLower only when constructing image refs,
while leaving the file export path unchanged. Keep sanitizeVolumeToken focused
on trimming and replacing path separators, and use the lowercased token only in
the image-related naming logic.
In `@backend/internal/volumeexport/schedule.go`:
- Around line 95-130: ListAll in ScheduleStore is silently skipping unreadable
or malformed schedule files, which can hide whole volumes from
exportScheduler.tick(). Update ScheduleStore.ListAll so it does not discard
os.ReadFile/json.Unmarshal failures without notice; either return/report
skipped-file errors or surface them to the caller. If ScheduleStore stays
logger-free, adjust the caller in exportScheduler/volume_export_scheduler.go to
provide logging or handle a returned skipped-file count so operators can see
which files were ignored.
In `@backend/internal/volumeexport/store.go`:
- Around line 56-86: The Store.List method is swallowing readMeta failures and
silently skipping corrupted export entries. Update the List flow to surface or
at least log readMeta errors when iterating entries, using the existing
Store/readMeta path so operators can see which export metadata failed. Keep
valid exports included, but avoid a bare continue on error; instead add context
with volumeName and entry.Name() and return or report the failure in a visible
way.
- Around line 155-171: Reject path-traversal segments before building volume
export paths: the current sanitization in Store.volumeDir, Store.exportDir, and
sanitizeName only replaces a few characters, so a raw ".." segment from
backend/internal/api/volumes.go can still escape s.root via filepath.Join.
Harden the API boundary in the volume handlers that forward r.URL.Path into
name/exportID, or update sanitizeName to explicitly map "." and ".." to a safe
fallback like "unknown" before volumeDir/exportDir joins.
In `@CLAUDE.md`:
- Around line 123-125: The project tree branch markers under the volume screens
are incorrect because volume_detail_screen.dart is no longer the only child.
Update the entries in the tree so the first two children use the proper
branching symbol with volume_detail_screen.dart and
volume_quick_export_screen.dart, leaving the last child as the terminating
branch, and keep the existing labels aligned with those symbols.
In `@ui/lib/api/client.dart`:
- Around line 1069-1082: The create export schedule request in client.dart is
ignoring the public daysOfWeek and times parameters when dayTimes is empty, so
the payload can be missing timing data. Update the volume export schedule method
to serialize either _scheduleTimingBody(dayTimes) or, when dayTimes is not
provided, the daysOfWeek/times fields directly in the same request body. Keep
the fix inside the schedule creation method that builds the jsonEncode payload
so callers using those arguments are correctly represented.
In `@ui/lib/screens/volume_detail_screen.dart`:
- Around line 291-302: Prompt for the save location before starting the export
download in the volume detail flow, since the current VolumeDetailScreen logic
downloads first and only then calls getSaveLocation. Update the download/export
handling around _downloadingExportId so it resolves the suggested file name,
asks for the destination, and returns early if the user cancels before calling
widget.apiClient.downloadVolumeExport or File.writeAsBytes. Keep the existing
mounted/setState cleanup behavior in the cancel path.
---
Nitpick comments:
In `@backend/internal/api/server.go`:
- Around line 40-52: New() is eagerly starting exportScheduler, which creates an
always-on background goroutine and immediately touches hardcoded home-directory
storage through volumeexport/NewStore and NewScheduleStore. Make Server
construction side-effect free by deferring scheduler creation/startup out of
New() (or gating it behind an explicit opt-in/init method), and use the existing
Server fields like cfg/runtime to add a test-friendly way to override or disable
the storage path. Ensure callers such as newTestServer can construct Server
without triggering filesystem I/O or needing Shutdown() just to avoid leaks.
In `@backend/internal/api/volume_export_runner.go`:
- Around line 19-91: `executeVolumeExport` is invoked from
`handleVolumeExportCreate` without a bounded deadline, so API-triggered exports
can block indefinitely if the runtime hangs. Update the HTTP handler path to
create a timeout context before calling `executeVolumeExport`, matching the
scheduler flow that already uses `context.WithTimeout`, and ensure the timeout
is propagated through `Server.executeVolumeExport` into `runtime.ExportVolume`.
In `@backend/internal/api/volumes.go`:
- Around line 78-114: The new OPTIONS handling inside handleVolumeAction is
unreachable because the method already returns 204 for OPTIONS at the top of the
function. Remove the redundant http.MethodOptions checks from the exports,
export-schedules, export-schedules/{id}, and exports/{id}/download branches so
the routing logic stays simple and avoids dead code.
In `@backend/internal/runtime/mock.go`:
- Around line 205-229: ExportVolume currently reuses Mock.ContainerErr as a
generic failure trigger, which couples volume export failures to container
errors. Add a dedicated error field on Mock for export-volume failures and have
Mock.ExportVolume check that field instead of ContainerErr, so container and
export tests can fail independently. Update the ExportVolume method and any
related mock setup helpers to use the new field while keeping the existing
validation branches for opts.Type, opts.Folder, and opts.ImageRef unchanged.
In `@backend/internal/runtime/volume_export.go`:
- Around line 71-79: The deferred cleanup in volume export reuses the same
possibly-canceled context, so the `nerdctl rm -f` teardown can fail after caller
timeout/cancellation. Update the `defer` inside `volumeExport` to use a fresh
short-lived background context for cleanup instead of the parent `ctx`, and keep
the existing `run`/`containerName` flow unchanged so the staging container is
always removed reliably.
- Line 42: The alpine base image tag is duplicated in the runtime volume export
setup, so extract the hard-coded "alpine:3.20" into a single package-level
constant and reuse it in the relevant places. Update the values referenced from
the volume export code path (including the image list and any other use in
volume_export.go) to point to that constant so future version bumps stay in
sync.
In `@backend/internal/volumeexport/name_pattern_test.go`:
- Around line 1-61: The current tests in TestExpandExportNamePattern and
TestResolveScheduledExportNames only cover file-name expansion, so add coverage
for image-ref generation as well. Introduce a test that exercises
ExpandExportImageRefPattern with DefaultImageRefPattern using a mixed-case
volume name and assert the result is normalized/lowercased correctly. Use the
existing volumeexport helpers and Schedule-related paths so the new test would
catch the lowercase-ref bug in name_pattern.go.
In `@ui/lib/api/client.dart`:
- Around line 641-653: The `nextRunAt` display helper in `client.dart` is using
a blanket catch that can hide non-parse bugs. Update the formatting logic around
`DateTime.parse` to handle only parsing failures explicitly, and let unexpected
exceptions surface instead of falling back silently. Keep the existing
`nextRunAt` formatting path intact for valid dates, and only return the raw
`nextRunAt` value when the parse itself fails.
In `@ui/lib/screens/volume_quick_export_screen.dart`:
- Around line 55-79: The error handling in _loadImages, _browseFolder, and _save
is using a generic catch and surfacing error.toString() directly to the UI.
Update the exception handling in VolumeQuickExportScreen to catch and map
specific failure cases separately (for example network, validation, and
backend/API errors), and set a user-facing message that reflects the actual
failure type instead of the raw exception string. Keep the mounted checks and
apply the same pattern consistently across the relevant methods so the UI and
logs clearly identify what failed and why.
In `@ui/lib/screens/volume_schedule_export_screen.dart`:
- Around line 103-127: The async flow in _loadImages still uses a generic
catch-all and surfaces error.toString() directly, which should be replaced with
handling for the specific failure modes used elsewhere in
VolumeScheduleExportScreen. Update _loadImages (and the related _browseFolder,
_applyChanges, and _confirmDelete paths mentioned in the comment) to catch and
branch on concrete exception types, then set clearer UI error states and logging
for each case instead of a single broad catch (error).
🪄 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 Plus
Run ID: 5bc2356a-5cb0-4939-9fd8-96026a8c1f5b
⛔ Files ignored due to path filters (1)
ui/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (37)
.cursor/rules/calf.mdcCHANGELOG.mdCLAUDE.mdROADMAP.mdbackend/internal/api/server.gobackend/internal/api/volume_export_runner.gobackend/internal/api/volume_export_scheduler.gobackend/internal/api/volume_export_schedules.gobackend/internal/api/volume_exports.gobackend/internal/api/volumes.gobackend/internal/runtime/lima.gobackend/internal/runtime/mock.gobackend/internal/runtime/native.gobackend/internal/runtime/runtime.gobackend/internal/runtime/volume_export.gobackend/internal/volumeexport/name_pattern.gobackend/internal/volumeexport/name_pattern_test.gobackend/internal/volumeexport/schedule.gobackend/internal/volumeexport/schedule_timing.gobackend/internal/volumeexport/schedule_timing_test.gobackend/internal/volumeexport/store.gobackend/test/api/api_test.gobackend/version/version.goui/lib/api/client.dartui/lib/export_name_pattern.dartui/lib/screens/volume_detail_screen.dartui/lib/screens/volume_quick_export_screen.dartui/lib/screens/volume_schedule_export_screen.dartui/linux/flutter/generated_plugin_registrant.ccui/linux/flutter/generated_plugins.cmakeui/macos/Flutter/GeneratedPluginRegistrant.swiftui/macos/Runner/DebugProfile.entitlementsui/macos/Runner/Release.entitlementsui/pubspec.yamlui/test/widget_test.dartui/windows/flutter/generated_plugin_registrant.ccui/windows/flutter/generated_plugins.cmake
| func sanitizeVolumeToken(value string) string { | ||
| replacer := strings.NewReplacer("/", "_", "\\", "_") | ||
| return replacer.Replace(strings.TrimSpace(value)) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby code
git ls-files backend/internal/volumeexport && echo '---'
ast-grep outline backend/internal/volumeexport/name_pattern.go --view expanded || true
echo '--- FILE ---'
cat -n backend/internal/volumeexport/name_pattern.go | sed -n '1,220p'
echo '--- SEARCH FOR IMAGE REF SANITIZATION ---'
rg -n "sanitizeVolumeToken|ExpandExportImageRefPattern|DefaultImageRefPattern|forFileExport|ToLower|lowercase|image ref|repository" backend/internal/volumeexport backend/internal -g '!**/*_test.go'
echo '--- DOCKER IMAGE NAME RULES IN REPO ---'
rg -n "lowercase|repository name|image name|Docker" . -g '!**/node_modules/**' -g '!**/vendor/**' | sed -n '1,120p'Repository: enegalan/calf
Length of output: 22002
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- SCHEDULE / VALIDATION ---'
cat -n backend/internal/volumeexport/schedule.go | sed -n '1,240p'
echo '--- API USAGE ---'
cat -n backend/internal/api/volume_export_schedules.go | sed -n '180,280p'
echo '--- RUNTIME IMAGE REF HANDLING ---'
cat -n backend/internal/runtime/volume_export.go | sed -n '1,220p'
echo '--- IMAGE REPOSITORY HELPERS ---'
rg -n "imageRepositoryName|reference|repo" backend/internal/runtime -g '!**/*_test.go'
if rg -n "func imageRepositoryName|func .*Reference|func .*Repository" backend/internal/runtime -g '!**/*_test.go'; then
:
fiRepository: enegalan/calf
Length of output: 19064
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- NAME PATTERN TESTS ---'
cat -n backend/internal/volumeexport/name_pattern_test.go | sed -n '1,260p'
echo '--- SEARCH FOR UPPERCASE / SANITIZE TESTS ---'
rg -n "MyVolume|uppercase|sanitizeVolumeToken|DefaultImageRefPattern|ExpandExportImageRefPattern|TypeNewImage|TypeRegistry" backend -g '*_test.go'
echo '--- SEARCH FOR SCHEDULE VALIDATION ---'
rg -n "ValidateScheduleInput|NormalizeSchedule|sanitizeName|volume name|image reference" backend/internal/volumeexport backend/internal/api -g '!**/*_test.go'Repository: enegalan/calf
Length of output: 2253
🌐 Web query:
Docker image reference repository names lowercase uppercase invalid repository name must be lowercase documentation
💡 Result:
In Docker, repository names must be lowercase [1][2]. If you attempt to use uppercase letters in a repository name, you will receive an error such as: "invalid reference format: repository name must be lowercase" [3][4][5]. This requirement is enforced by the Docker engine and container registries to maintain consistency and compatibility across different platforms [2]. While tags (the identifiers following the colon, e.g.,:latest) are permitted to contain both uppercase and lowercase letters [1][6][2], the repository component itself—which includes the namespace and image name—must strictly adhere to lowercase alphanumeric characters [1][7][2]. Key points regarding naming conventions: - Repository names: Must be lowercase [1][2]. They may contain digits and separators (periods, one or two underscores, or one or more dashes) but cannot start or end with a separator [1][8][2]. - Tags: Unlike repository names, tags are case-sensitive and allow uppercase letters [1][6][2]. - Enforcement: This restriction is validated at the client level (e.g., when running docker build or docker tag) and is often enforced by registry servers as well [9][5][2]. If you encounter this error, you must rename your image to use only lowercase letters to proceed [2].
Citations:
- 1: https://github.com/moby/docker-image-spec/blob/main/spec.md
- 2: https://quickutil.dev/blog/docker-image-parser-guide/
- 3: https://github.com/containers/image/blob/main/docker/reference/normalize.go
- 4: https://stackoverflow.com/questions/48522615/docker-error-invalid-reference-format-repository-name-must-be-lowercase
- 5: Error message (caused by uppercase letter in name argument to "docker tag") does not mention casing docker/cli#112
- 6: Bug/proposal: Docker registry: Repository names: allows uppercase distribution/distribution#2771
- 7: https://forums.docker.com/t/docker-registry-v2-spec-and-repository-naming-rule/5466
- 8: https://github.com/containers/image/blob/93fa49b0f1fb78470512e0484012ca7ad3c5c804/docker/reference/regexp.go
- 9: docker push allows uppercase letters in repository names, which conflicts with docker-index moby/moby#1895
Lowercase the volume token for image refs
sanitizeVolumeToken only strips path separators, so TypeNewImage/TypeRegistry can still build refs like MyVolume-backup:.... Docker/nerdctl require the repository name to be lowercase, so uppercase volume names will make export fail. Keep file exports unchanged and apply strings.ToLower only on the image-ref path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/internal/volumeexport/name_pattern.go` around lines 82 - 85, The
image-ref path built from sanitizeVolumeToken is still preserving uppercase
characters, which can produce invalid Docker/nerdctl repository names. Update
the TypeNewImage and TypeRegistry flow in name_pattern.go to apply
strings.ToLower only when constructing image refs, while leaving the file export
path unchanged. Keep sanitizeVolumeToken focused on trimming and replacing path
separators, and use the lowercased token only in the image-related naming logic.
…e export - Enhanced error handling in volume export API methods to provide clearer feedback on failures. - Introduced context management for volume export scheduling and execution, ensuring better control over timeouts. - Updated volume export naming patterns to ensure consistent formatting and sanitization. - Refactored UI components to streamline volume export interactions and improve user experience. - Added tests for new error handling and naming pattern functionalities.
1 participant
Summary by CodeRabbit