Skip to content

Conversation

sawka
Copy link
Member

@sawka sawka commented Oct 8, 2025

thinking about installing "wave" to the path. so instead of users running wsh [command] they can run wave [command] which would be more intuitive.

for now we'd install both, but the idea would be to deprecate wsh, give warning messages about using wsh, and then in the future remove the wsh name completely.

Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

Walkthrough

Adds support for copying the Wave binary alongside the existing wsh binary during installation and initialization. Introduces a new exported constant wavebase.RemoteFullWaveBinPath set to "~/.waveterm/bin/wave". Updates remote install templates to include a cp step and adds a wavePath template variable. In shell initialization, defines a wave destination path and performs an additional AtomicRenameCopy to that path, updating logs accordingly. Similar cp steps are added for WSL installation templates with error propagation. No changes to exported APIs other than the new constant.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title “wsh => wave” concisely captures the primary change of introducing the “wave” command in place of “wsh” by using a familiar arrow notation to indicate the mapping. It directly relates to the central purpose of the pull request without introducing unrelated details.
Description Check ✅ Passed The description clearly explains the intent to install “wave” alongside “wsh,” outlines the plan to deprecate “wsh,” and matches the actual code changes for adding a new executable and warnings. It is directly related to the changeset and provides relevant context for reviewers.
✨ 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/wsh-to-wave

📜 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 7681214 and 270d712.

📒 Files selected for processing (4)
  • pkg/remote/connutil.go (2 hunks)
  • pkg/util/shellutil/shellutil.go (1 hunks)
  • pkg/wavebase/wavebase.go (1 hunks)
  • pkg/wslconn/wsl-util.go (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
pkg/wslconn/wsl-util.go (1)
pkg/wavebase/wavebase.go (1)
  • RemoteFullWaveBinPath (65-65)
pkg/util/shellutil/shellutil.go (1)
pkg/util/utilfn/utilfn.go (1)
  • AtomicRenameCopy (819-849)
pkg/remote/connutil.go (1)
pkg/wavebase/wavebase.go (1)
  • RemoteFullWaveBinPath (65-65)
⏰ 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: Build for TestDriver.ai
  • GitHub Check: Analyze (go)
  • GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (9)
pkg/wavebase/wavebase.go (1)

65-65: LGTM! Consistent constant definition.

The new RemoteFullWaveBinPath constant follows the established naming pattern and path structure, paralleling the existing RemoteFullWshBinPath.

pkg/wslconn/wsl-util.go (3)

108-108: LGTM! Consistent template additions.

The cp commands are properly added to both the bash and default install template sets, following the same pattern as the other installation commands.

Also applies to: 116-116


159-159: LGTM! Proper template variable binding.

The wavePath is correctly added to installWords and bound to the new wavebase.RemoteFullWaveBinPath constant.


222-225: LGTM! Proper error handling and execution order.

The cp command is executed after chmod completes, ensuring the wave binary inherits the correct executable permissions. Error handling properly propagates failures.

pkg/remote/connutil.go (2)

98-98: LGTM! Consistent template modification.

The cp command is properly integrated into the install template with appropriate error handling (|| exit 1), following the same pattern as the other installation steps.


120-120: LGTM! Consistent implementation across installation methods.

The wavePath template variable is properly added, mirroring the WSL implementation and ensuring consistency across different remote installation paths.

pkg/util/shellutil/shellutil.go (3)

412-412: LGTM! Consistent path handling.

The waveDstPath initialization properly mirrors the wshDstPath logic, including platform-specific .exe extension handling for Windows.

Also applies to: 415-415


421-424: LGTM! Proper atomic copy with clear error handling.

The second AtomicRenameCopy correctly creates the wave binary with the same source and permissions as the wsh binary. The error message clearly identifies this as the wave binary copy operation.


426-426: LGTM! Clear and accurate logging.

The log message properly documents both copy destinations, providing useful information for troubleshooting and verification.


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.

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.

1 participant