Improve terminal backup#2007
Conversation
Greptile SummaryThis PR improves the terminal backup flow by introducing a new dedicated Key changes and findings:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Settings as terminalSettings.js
participant BackupPage as TerminalBackup page
participant TerminalJS as Terminal.backup
participant Shell as Executor shell
participant FS as system.copyToUri
User->>Settings: tap Backup
Settings->>BackupPage: TerminalBackup(onclose)
BackupPage->>User: show settings page (alpineBase, home)
User->>BackupPage: configure options and tap backup button
BackupPage->>BackupPage: FileBrowser folder picker returns url
BackupPage->>TerminalJS: backup with alpineBase, packages, home options
TerminalJS->>Shell: check isInstalled
TerminalJS->>Shell: tar -cf $PREFIX/aterm_backup.tar with excludes and includes
Shell-->>TerminalJS: ok
TerminalJS-->>BackupPage: resolve dataDirectory + usr/aterm_backup.tar
BackupPage->>FS: copy backup to user selected folder
FS-->>BackupPage: done
BackupPage->>User: alert Backup successful
Reviews (1): Last reviewed commit: "format" | Re-trigger Greptile |
| if (opts.packages) { | ||
| includeFiles.push("alpine/data"); | ||
| excludePaths.splice(excludePaths.indexOf("alpine/data"), 1); | ||
| } |
There was a problem hiding this comment.
alpine/data missing from excludePaths, causing wrong element to be spliced
"alpine/data" is not present in the excludePaths array, so excludePaths.indexOf("alpine/data") returns -1. Calling excludePaths.splice(-1, 1) then removes the last element — "alpine/tmp" — instead of "alpine/data". This results in two unintended behaviors:
- When
packagesis false,alpine/datais never excluded from the tar archive (user-installed packages are always included). - When
packagesis true,alpine/tmpis not excluded, so temporary files end up in the backup.
The fix is to add "alpine/data" to excludePaths so the splice call removes the correct entry:
| if (opts.packages) { | |
| includeFiles.push("alpine/data"); | |
| excludePaths.splice(excludePaths.indexOf("alpine/data"), 1); | |
| } | |
| if (opts.packages) { | |
| includeFiles.push("alpine/data"); | |
| excludePaths.splice(excludePaths.indexOf("alpine/data"), 1); |
And add "alpine/data" to the excludePaths array:
const excludePaths = [
"alpine/data", // <-- add this
"alpine/system",
...
"alpine/tmp",
];| import alert from "dialogs/alert"; | ||
| import loader from "dialogs/loader"; | ||
| import FileBrowser from "pages/fileBrowser"; | ||
| import helpers from "utils/helpers"; |
| ARGS="$ARGS -b $PREFIX/public:/home" | ||
| ARGS="$ARGS -b $PREFIX/public:/root" |
There was a problem hiding this comment.
/home and /root bound to the same directory
Both /home and /root inside the Alpine container now point to $PREFIX/public. While this is consistent with the backup's handling of the home option (which only archives public), it means any process writing to /home/<user> or /root shares the same underlying directory. This can cause unexpected collisions — for example, if a package creates /root/.bashrc and a user's home directory is also /home/root, they'll overwrite each other's dotfiles.
Consider whether the intent is truly to unify these paths, and add a comment explaining the reasoning if so.
| function setPackagesTooltip() { | ||
| const packagesRow = document.querySelector('[data-key="packages"]'); | ||
| if (!packagesRow) return; | ||
| const input = packagesRow.querySelector(".input-checkbox input"); | ||
| if (input) { | ||
| input.disabled = !!backupSettings.alpineBase; | ||
| } | ||
| } |
There was a problem hiding this comment.
Functions reference a commented-out UI element
The packages settings item is disabled in the items array (wrapped in a block comment), so these helper functions will always have a null reference for the row. setPackagesTooltip() returns early immediately, enforcePackageRule() never updates the checkbox UI, and the matching case in callback is never triggered.
Also, backupSettings.public is declared in the initial state object but never consumed by any item or callback — it can be removed.
No description provided.