Skip to content

fix(shell-sync): preserve synced guest changes on TUI interruption#5002

Open
unsuman wants to merge 1 commit into
lima-vm:masterfrom
unsuman:fix/shell-sync-crash
Open

fix(shell-sync): preserve synced guest changes on TUI interruption#5002
unsuman wants to merge 1 commit into
lima-vm:masterfrom
unsuman:fix/shell-sync-crash

Conversation

@unsuman
Copy link
Copy Markdown
Member

@unsuman unsuman commented May 18, 2026

Fixes #4997

When SIGINT is sent during the exit dialogue of limactl shell --sync, changes made during the synced session are lost. This PR fixes that.

@unsuman unsuman added this to the v2.1.2 milestone May 18, 2026
@unsuman unsuman added area/cli limactl CLI user experience easy-to-review labels May 18, 2026
@unsuman unsuman changed the title fix(shell-sync): preserve synced guest changes on crash fix(shell-sync): preserve synced guest changes on TUI interruption May 18, 2026
@unsuman unsuman force-pushed the fix/shell-sync-crash branch from 13f4bfe to ec82721 Compare May 18, 2026 15:48
Copy link
Copy Markdown
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic needs to be inverted to cover other failure cases too.

Comment thread cmd/limactl/shell.go Outdated
remoteSource := fmt.Sprintf("%s:%s", inst.Name, destRsyncDir)
clean := filepath.Clean(hostCurrentDir)
dirForCleanup := shellescape.Quote(filepath.Join(*inst.Config.User.Home, clean))
cleanupGuestWorkdir := true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the logic should be reverted: preserve the data, unless it has been confirmed it has been successfully copied or the user decided to discard. That automatically covers other failure scenarios that are not being considered in this PR.

Suggested change
cleanupGuestWorkdir := true
cleanupGuestWorkdir := false

Possible other failures detected by Opus 4.7:

The defer still runs rm -rf on every other early-return path inside askUserForRsyncBack, several of which can fire after a successful guest session and would also silently destroy the user's work:

  • cmd/limactl/shell.go:504 — os.MkdirTemp failure on the host
  • cmd/limactl/shell.go:514 — os.MkdirAll failure on the host
  • cmd/limactl/shell.go:527 — non-Interrupt error from uiutil.Select ("failed to open TUI")
  • cmd/limactl/shell.go:549 — rsyncDirectory to the host temp dir fails (when user picked "View")
  • cmd/limactl/shell.go:559/569/589/600/607 — pager/diff pipe failures
  • cmd/limactl/shell.go:460 — even the happy "Yes" path: if the final rsyncBack rsync itself fails (disk full, network blip), the defer still wipes the guest dir, losing the only remaining copy.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, should we prompt the user during --sync if the target directory already exists?

If a user runs limactl shell --sync and Lima finds that the directory already exists in the guest, it should check for diff against the host. If there are any changes, Lima will prompt the user with this dialogue before starting the new session.

WARN[...] This directory already exists in the guest.
? ⚠️ Sync back changes to host? (added: 0, deleted: 0, modified: 1, metadata: 1)  [Use arrows to move, type to filter]
> Yes
  No (start fresh)
  No (keep guest changes)
  View the changed contents

Comment thread cmd/limactl/shell.go Outdated
Fixes lima-vm#4997

Signed-off-by: Ansuman Sahoo <anshumansahoo500@gmail.com>
@unsuman unsuman force-pushed the fix/shell-sync-crash branch from ec82721 to 755ec52 Compare May 19, 2026 10:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/cli limactl CLI user experience easy-to-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

limactl shell: synced directory is deleted when sync dialog does not exit gracefully

2 participants