Support restores from backups on /intern - #2843
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds an admin restore page for backup ZIP uploads. Adds asynchronous restore jobs with persisted status, progress, cancellation, cleanup, and completion links. Adds restore and backup routes, rate limiting, migration support, and shared superuser email configuration. ChangesAdmin app restore
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant RestorePage
participant DashboardRoutes
participant AppRestoreJobs
participant Restore
Admin->>RestorePage: Select ZIP and submit restore
RestorePage->>DashboardRoutes: POST /dash/restores/zip
DashboardRoutes->>AppRestoreJobs: Start waiting restore job
AppRestoreJobs->>Restore: Run restore with progress callbacks
Restore-->>AppRestoreJobs: Report totals and progress
RestorePage->>DashboardRoutes: Poll restore job
DashboardRoutes-->>RestorePage: Return status and progress
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@server/src/instant/db/app_restore_jobs.clj`:
- Line 212: Update the restore dispatch around run-restore! so restore jobs
cannot execute concurrently process-wide. Replace the per-upload unbounded
background launch with a single bounded worker or database-backed lease/queue,
ensuring new jobs are queued or rejected while one restore is active and
preserving the one-restore-at-a-time contract.
- Around line 163-185: Update the restore worker around mark-processing! and
mark-completed! to honor failed status transitions: verify mark-processing!
successfully claims the waiting job before starting restore/restore-from-zip,
and exit without launching work when it updates zero rows. Add a cancellation
checkpoint during active processing before ownership transfer, and if
mark-completed! updates zero rows because cancellation won, remove the restored
app before returning while preserving the cancelled status.
- Around line 198-213: Implement startup reconciliation for persisted
nonterminal restore jobs, and initialize it from the application startup path in
core. Ensure waiting jobs are resumed safely, while processing jobs are marked
failed or otherwise finalized without rerunning restore work; clean up each
job’s staged ZIP, including orphaned staged files where applicable. Anchor the
changes around start-restore!, run-restore!, and the core initialization flow.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fb407cc3-88d6-457f-bd5e-1f36f2b96070
📒 Files selected for processing (9)
client/www/app/intern/content.tsxclient/www/app/intern/restore/content.tsxclient/www/app/intern/restore/page.tsxserver/resources/migrations/123_app_restore_jobs.down.sqlserver/resources/migrations/123_app_restore_jobs.up.sqlserver/src/instant/core.cljserver/src/instant/dash/routes.cljserver/src/instant/db/app_restore_jobs.cljserver/src/instant/restore.clj
Contributor
|
View Vercel preview at instant-www-js-restores-from-admin-jsv.vercel.app. |
dwwoelfel
marked this pull request as ready for review
August 6, 2026 23:50
dwwoelfel
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/src/instant/dash/routes.clj (1)
449-449: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTreat an empty title as absent.
get-inreturns""when the optional query parameter is submitted empty. In Clojure,""is truthy, so the restore can save a blank override instead of using the title from the backup.Use
ex/get-optional-param!withstring-util/coerce-non-blank-str.Proposed fix
- title (get-in req [:params :title]) + title (ex/get-optional-param! req + [:params :title] + string-util/coerce-non-blank-str)🤖 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 `@server/src/instant/dash/routes.clj` at line 449, Update the title binding in the restore request flow to use ex/get-optional-param! with string-util/coerce-non-blank-str, so an empty submitted title becomes absent and the backup title is preserved.
🤖 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 `@server/src/instant/config.clj`:
- Around line 59-65: Update superuser-email to match bootstrap validation for
INSTANT_SUPERUSER_EMAIL: trim surrounding whitespace, treat blank values as
unset, and reject invalid emails consistently instead of silently converting
them to nil. Reuse the existing shared normalization/validation helper if
available, while preserving nil for an unset variable.
---
Outside diff comments:
In `@server/src/instant/dash/routes.clj`:
- Line 449: Update the title binding in the restore request flow to use
ex/get-optional-param! with string-util/coerce-non-blank-str, so an empty
submitted title becomes absent and the backup title is preserved.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 71acae2a-6874-451a-b54b-3a532761dcac
📒 Files selected for processing (2)
server/src/instant/config.cljserver/src/instant/dash/routes.clj
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/src/instant/dash/routes.clj (1)
2049-2064: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftMake multi-bucket rate-limit consumption atomic.
Lines 2053-2056 peek all buckets, but Lines 2062-2064 consume them one at a time. Under contention, the app token can be consumed successfully, then the IP bucket can reject the request. The request fails, but the app quota is lost. Repeated contention can create false app-level rate limits.
Use an atomic reservation for both buckets, or refund every token consumed when a later bucket rejects the request.
🤖 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 `@server/src/instant/dash/routes.clj` around lines 2049 - 2064, Make the multi-bucket flow around retry-at and the consume-user-rate-limit-retry-at calls atomic: reserve capacity for all bucket-params as one operation, or track successful consumptions and refund every earlier bucket if a later consumption rejects. Ensure a failed request never leaves any bucket token consumed, while preserving the existing retry-at reporting behavior.
🧹 Nitpick comments (1)
server/src/instant/dash/routes.clj (1)
456-461: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winEnforce a restore ZIP size limit.
server/src/instant/core.cljmerges:max-entity-size -1, and the restore route streams that body directly intoserver/src/instant/db/app_restore_jobs.clj::store-zip!, which copies the stream toinstant-restore-uploads/<job-id>.zipwithout a bound. Add an endpoint/request-size cap for restore uploads; delete the partial staged zip if the limit is exceeded.🤖 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 `@server/src/instant/dash/routes.clj` around lines 456 - 461, Enforce a maximum request size for the restore upload before or while the body reaches the restore route’s app-restore-jobs/start-restore! and store-zip! flow, rather than inheriting the unlimited :max-entity-size setting. Ensure oversized uploads are rejected and any partially written instant-restore-uploads/<job-id>.zip is deleted when the limit is exceeded, while preserving normal restore behavior for uploads within the cap.
🤖 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.
Outside diff comments:
In `@server/src/instant/dash/routes.clj`:
- Around line 2049-2064: Make the multi-bucket flow around retry-at and the
consume-user-rate-limit-retry-at calls atomic: reserve capacity for all
bucket-params as one operation, or track successful consumptions and refund
every earlier bucket if a later consumption rejects. Ensure a failed request
never leaves any bucket token consumed, while preserving the existing retry-at
reporting behavior.
---
Nitpick comments:
In `@server/src/instant/dash/routes.clj`:
- Around line 456-461: Enforce a maximum request size for the restore upload
before or while the body reaches the restore route’s
app-restore-jobs/start-restore! and store-zip! flow, rather than inheriting the
unlimited :max-entity-size setting. Ensure oversized uploads are rejected and
any partially written instant-restore-uploads/<job-id>.zip is deleted when the
limit is exceeded, while preserving normal restore behavior for uploads within
the cap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7de575a1-16b6-4291-a35e-bf6ef516f5ed
📒 Files selected for processing (3)
server/src/instant/config.cljserver/src/instant/dash/routes.cljserver/src/tasks.clj
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
2 participants
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.
This adds the UI that lets server admins restore an app from a backup.
This is a follow-up to #2836.
It can be used by Instant admins (i.e. us) and admins of self-hosted instances to import apps from Instant. If someone wants to switch to self-hosted, they can turn off writes on Instant, generate a backup, then import it into their self-hosted instance from
/intern/restore.Since it's only meant to be used by administrators, I just put the zip file on disk in a temp directory and restore directly from the zip. This makes restoring a lot easier because we can just seek to different positions on the disk when we're putting files into the files queue. If it fails, you'll just have to retry from the beginning.
Screen.Recording.2026-08-06.at.4.43.27.PM.mov
If you want to restore to the same app_id, but something is already there, we'll show you an error message that tells you to delete the app from the db directly: