Skip to content

feat: add restore, prune-backups, sync --dry-run, and desktop cap war… - #5

Closed
GGzili wants to merge 2 commits into
Wangnov:mainfrom
GGzili:main
Closed

GGzili wants to merge 2 commits into
Wangnov:mainfrom
GGzili:main

Conversation

@GGzili

@GGzili GGzili commented May 11, 2026

Copy link
Copy Markdown

…ning

  • Add restore <backup> command to restore SQLite from .bak backup files
  • Add prune-backups --keep N command to remove old backups, keep N most recent
  • Add --dry-run flag to sync command for preview without writing
  • Add 50-session desktop cap warning to status command
  • Full i18n support (EN/ZH_HANS) for all new features
  • Auto-list available backups when no path argument given to restore
  • Backup directory auto-discovery alongside SQLite file

GGzili added 2 commits May 12, 2026 02:27
…ning

- Add `restore <backup>` command to restore SQLite from .bak backup files
- Add `prune-backups --keep N` command to remove old backups, keep N most recent
- Add `--dry-run` flag to `sync` command for preview without writing
- Add 50-session desktop cap warning to `status` command
- Full i18n support (EN/ZH_HANS) for all new features
- Auto-list available backups when no path argument given to restore
- Backup directory auto-discovery alongside SQLite file
@Wangnov

Wangnov commented May 18, 2026

Copy link
Copy Markdown
Owner

Thanks for adding these maintenance features. The direction is useful, but I think this PR needs a few changes before merge, mainly around data-safety semantics.

A few notes based on the current main: #7 has already landed with the Windows hidden VBS startup path and follow-up fixes, so please rebase this branch onto current main and rerun the test suite before final review. This PR only appears to touch src/main.rs, so it should not need to modify the Windows service path.

Blocking items:

  1. restore without an explicit backup path should not restore anything

Right now codex-threadripper restore lists available backups, picks the first backup, and then proceeds to restore it unless --dry-run is set. That is too dangerous for a destructive command. A user running restore with no args is likely expecting a list/help-like action, not an implicit overwrite of state_5.sqlite.

Suggested behavior:

  • restore with no backup arg: list available backups and exit without writing
  • restore <BACKUP_PATH>: perform the restore
  • if we want “restore latest”, make that explicit, e.g. restore --latest, ideally with a confirmation/force flag later
  1. prune-backups should not rely on filesystem creation time for ordering

The backup filenames already include a timestamp. Please sort backups by the timestamp parsed from the filename instead of metadata().created(). Filesystem birth/creation time can be unavailable or misleading after copies, restores, downloads, or archive extraction; using it to decide which backups to delete can remove the wrong files.

Suggested ordering:

  • parse state_5.sqlite.<timestamp>.bak
  • sort by parsed timestamp descending
  • for unparseable .bak files, either skip them from automatic pruning or put them at the end with a warning
  1. sync --dry-run currently only previews SQLite changes

The non-dry-run sync path can also update rollout JSONL metadata, but the dry-run path only reports SQLite distribution/mismatched rows. That makes the dry-run output incomplete and potentially misleading.

Please either:

  • add a rollout dry-run scan that reports would-update / would-prepare / would-skip counts without writing, or
  • clearly label the current output as SQLite-only dry-run
  1. Please update README/help coverage for the new commands

The new restore, prune-backups, and sync --dry-run behavior should be documented, especially because restore/prune are data-affecting operations.

Once these are fixed, I think the feature set itself is worth having.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants