Skip to content

Conversation

@sawka
Copy link
Member

@sawka sawka commented Nov 6, 2025

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 6, 2025

Walkthrough

The FileRestoreBackupCommand function in the wshserver package has been modified to expand both the backup and restore file paths using wavebase.ExpandHomeDir before use. Error handling has been added to capture any failures during path expansion, and the expanded paths are subsequently passed to filebackup.RestoreBackup. The change does not affect any public function signatures.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Localized change to a single function within one file
  • Path expansion logic applied consistently to two paths with error handling
  • No breaking changes to public APIs
  • Consider verifying that error handling for path expansion failures is appropriately propagated and doesn't mask underlying issues

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to evaluate whether a description relates to the changeset. Add a description explaining the path mismatch issue, why path expansion is necessary, and the impact of these changes on backup/restore functionality.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'fix path mismatch issue, always expand path' directly addresses the main change: expanding file paths to fix a mismatch issue in FileRestoreBackupCommand.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sawka/backup-mismatch

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 58392f7 and d22a0c2.

📒 Files selected for processing (1)
  • pkg/wshrpc/wshserver/wshserver.go (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: esimkowitz
Repo: wavetermdev/waveterm PR: 1725
File: pkg/remote/fileshare/wavefs/wavefs.go:441-494
Timestamp: 2025-01-29T04:21:11.649Z
Learning: The `CopyRemote` function in WaveTerm's file operations has proper error handling that prevents partial writes by failing fast and using context cancellation. Each step (path cleaning, file operations, tar reading/writing) is guarded by error checks that prevent proceeding with writes on error.
Learnt from: esimkowitz
Repo: wavetermdev/waveterm PR: 1790
File: cmd/wsh/cmd/wshcmd-readfile.go:34-34
Timestamp: 2025-01-22T01:04:20.614Z
Learning: Path sanitization in Wave Terminal is handled by individual fileshare client implementations rather than at the command level, as different file systems may have different path requirements and formats.
📚 Learning: 2025-01-22T01:28:41.417Z
Learnt from: esimkowitz
Repo: wavetermdev/waveterm PR: 1790
File: pkg/remote/fileshare/wshfs/wshfs.go:122-122
Timestamp: 2025-01-22T01:28:41.417Z
Learning: The RpcClient in pkg/remote/fileshare/wshfs/wshfs.go is initialized and handled downstream by either main-server or wshcmd-connserver, as documented in the package comment.

Applied to files:

  • pkg/wshrpc/wshserver/wshserver.go
🧬 Code graph analysis (1)
pkg/wshrpc/wshserver/wshserver.go (2)
pkg/wavebase/wavebase.go (1)
  • ExpandHomeDir (141-155)
pkg/filebackup/filebackup.go (1)
  • RestoreBackup (90-124)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (go)
  • GitHub Check: Build for TestDriver.ai
🔇 Additional comments (1)
pkg/wshrpc/wshserver/wshserver.go (1)

445-453: LGTM! Path expansion correctly fixes the metadata mismatch issue.

The changes ensure both backup and restore paths are expanded before being passed to filebackup.RestoreBackup. This is necessary because the backup metadata stores the full expanded path (the FullPath field), and the restore function validates that metadata.FullPath == restoreToFileName. Without expansion, a user-provided path like ~/file.txt would fail to match /home/user/file.txt from the metadata, even though they refer to the same location.

The implementation is correct:

  • Both paths are expanded consistently using wavebase.ExpandHomeDir
  • Path traversal protection is already built into ExpandHomeDir
  • Error handling is clear and descriptive
  • The expanded paths are used in the RestoreBackup call

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.5.0)

Error: unknown linters: 'unusedfunc,unusedparams', run 'golangci-lint help linters' to see the list of supported linters
The command is terminated due to an error: unknown linters: 'unusedfunc,unusedparams', run 'golangci-lint help linters' to see the list of supported linters


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sawka sawka merged commit 4621882 into main Nov 7, 2025
8 checks passed
@sawka sawka deleted the sawka/backup-mismatch branch November 7, 2025 21:09
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