fix(upgrade): prune old backups + add --no-backup flag#398
Open
ezequielmm wants to merge 2 commits intoGentleman-Programming:mainfrom
Open
fix(upgrade): prune old backups + add --no-backup flag#398ezequielmm wants to merge 2 commits intoGentleman-Programming:mainfrom
ezequielmm wants to merge 2 commits intoGentleman-Programming:mainfrom
Conversation
The upgrade executor created a pre-upgrade backup snapshot but never invoked backup.Prune to enforce DefaultRetentionCount. Backups accumulated indefinitely under ~/.gentle-ai/backups/, eventually filling the disk and breaking subsequent upgrades with "no space left on device" errors during snapshot creation. Real-world impact: a user accumulated 24 upgrade backups totalling ~98 GB before discovering it. The most recent backups were ~15 GB each because they include agent config roots that are not fully covered by backupExcludeSubdirs (e.g. arbitrary files under ~/.gemini/antigravity/). The retention.Prune function already exists and is invoked correctly for install/sync via internal/cli/run.go (prepareBackupStep.Run). This patch mirrors that behavior in the upgrade path: after creating the snapshot, prune unpinned backups beyond DefaultRetentionCount=5. Pinned backups are never deleted. Pruning runs even when the snapshot itself fails — that is exactly the recovery path when the failure was caused by accumulated backups exhausting disk space. Refs Gentleman-Programming#368 (upgrade backup failures), relates to Gentleman-Programming#185.
Add an opt-in escape hatch for users who do not want a pre-upgrade backup snapshot for a given run. Default behavior is unchanged (backup remains safe-by-default). gentle-ai upgrade --no-backup [tool...] When --no-backup is set, ExecuteWithOptions skips both the snapshot creation and retention pruning of the backup directory. The backups directory is left entirely untouched. This satisfies the use cases discussed in Gentleman-Programming#185: * Users on tight disk space who want to upgrade without the backup walk allocating GBs of duplicate config data. * Users who manage their config under version control / dotfile managers and treat the upgrade backup as redundant. * Recovery flows where ~/.gentle-ai/backups/ is full or unreadable and the user explicitly wants to bypass it for a single run. Implementation notes: * SkipBackup is a new field on ExecuteOptions; zero value preserves the original behavior, so all existing callers (TUI included) are unaffected. * The CLI parser in app.runUpgrade now recognizes --no-backup and routes through upgrade.ExecuteWithOptions directly. The TUI dispatcher still uses the upgradeExecute test seam unchanged. * No new dependencies, no new exported APIs beyond the field. Closes Gentleman-Programming#185.
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
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.
Summary
The
gentle-ai upgradecommand silently accumulates pre-upgrade backup snapshots under~/.gentle-ai/backups/because the upgrade executor never invokesbackup.Prune. Over many upgrades this fills the disk and breaks subsequent upgrades withno space left on deviceerrors during snapshot creation.This PR fixes that, and adds a
--no-backupopt-out for users who want to bypass the backup subsystem for a single run.Real-world impact (the report that motivated this PR): a user accumulated 24 upgrade backups totalling ~98 GB before discovering it. Each recent backup was ~15 GB because agent config roots like
~/.gemini/antigravity/walk into runtime data not fully covered bybackupExcludeSubdirs.Closes #185. Refs #368.
Changes
Two semantic commits, intentionally split for clean bisect/revert:
1.
fix(upgrade): prune old backups after pre-upgrade snapshotinternal/update/upgrade/executor.go::ExecuteWithOptionsnow invokesbackup.Prune(backupRoot, backup.DefaultRetentionCount)after the snapshot block, mirroring exactly the pattern already used byinternal/cli/run.go::prepareBackupStep.Runfor install/sync.Prunesemantics).2.
feat(upgrade): add --no-backup flagSkipBackup boolfield onExecuteOptions(zero-value preserves original behavior — no breaking change for existing callers, including the TUI dispatcher).app.runUpgraderecognizes--no-backupand routes throughupgrade.ExecuteWithOptionsdirectly. When set, both snapshot creation and retention pruning are skipped (~/.gentle-ai/backups/is left entirely untouched).upgradeExecutetest seam used bytuiUpgradeis unchanged.Test plan
go build ./...— cleango vet ./...— cleango test ./...— full suite passes (all 39 packages green)--dry-runpath unchanged--no-backupparses correctly alongside--dry-runand tool filters~/.gentle-ai/backups/to see retention enforcementBackward compatibility
ExecuteOptions.SkipBackup. No dependency changes.Out of scope (deliberately)
backupExcludeSubdirsto fully exclude non-config payloads under agent roots (root cause of why each backup is GB-sized — relates to issues Backup captures user files from ~/.gemini/antigravity/tmp/ — propose exclude #345 / bug(backup): pre-upgrade backup hangs indefinitely on Windows when Cursor extensions directory contains node_modules #203). That is a separate discussion about the inclusion strategy and deserves its own PR with reviewer input.