Skip to content

fix(drive): sheet-already-restored guard misfires on a different deleted sheet and advises deleting a good one #1740

Description

@newhoggy

Problem

detect_sheet_restore (restore.rs:825-863) checks previously_restored_sheet_id before the structural diff (:838-849 vs :850-862). Once a backup has been restored from, any later restore for a different deleted sheet is refused SheetAlreadyRestored, with advice that destroys a good sheet:

Refused: this backup's deleted sheet was already restored … as '{title}' (id {sheet_id}), which is still there — restoring again would only add a second copy. Delete that sheet first if you do want another one. (cli/drive/lease.rs:551-558)

Trace: backup has {A, B, C}; C was deleted and restored as C' (row records restored_sheet_id = C'). The user then deletes B and runs restore T. C' is live → AlreadyRestored{C'}, B is never mentioned. If they follow the advice and delete C', live = {A}, the diff finds {B, C} missing → ambiguous → NoTypedRestorePath. Net effect: the tool talked the user into deleting a good sheet for nothing.

Pre-#1716 (git show 9c19fd15^:src/drive/lease/restore.rs) the same input went straight to the diff → two missing → NoTypedRestorePath, which was honest. ADR-0080 §10's new paragraph (l.616-617) reasons only about re-deleting the same sheet. No test has a second, different sheet deleted after a restore (restoring_the_same_sheet_backup_twice_refuses_instead_of_duplicating_it and a_restored_sheet_deleted_again_is_restored_again both use a two-sheet backup with one missing).

No capability is lost (B was unreachable either way); the regression is in the instruction given.

Proposal

Run the diff first. Return AlreadyRestored only when the single missing-live sheet is the one the row already restored; otherwise fall through to the existing NoTypedRestorePath. Add the multiple-deletion test and a sentence to ADR-0080 §10.

Scope

src/drive/lease/restore.rs, docs/adrs/adr-0080.md.


Found in a code review of the lease fix wave fe550423..8a7b8400 (PRs #1703-#1726, merged 2026-09-17/18), reviewed 2026-09-18. Line numbers are at 8a7b8400.

Related: #1689 (the PR that added the guard), #1742 (test gaps)

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions