working on more terminal context #2444
Conversation
sawka
commented
Oct 17, 2025
sawka
commented
- add automatic OSC 7 support to bash and zsh
- add new wave OSC 16162 (planck length) to get up-to-date shell information into blockrtinfo. currently implemented only for zsh. bash will not support as rich of data as zsh, but we'll be able to do some.
- new rtinfo will be used to provide better context for AI in the future, and to make sure AI is running safe commands.
- added a small local machine description to tab context (so AI knows we're running on MacOS, Linux, or Windows)
…es for easier editing. added _waveterm_si funcs for sending new OSC code 16162 (like OSC 133). also send OSC 7. POC parsing in termwrap.
…C code. also working on zle integration
…d to tab context prompt.
|
Warning Rate limit exceeded@sawka has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughAdds embedded startup scripts and multiple shell integration scripts (bash, zsh, fish, PowerShell) that emit OSC 7 and new Wave OSC 16162 sequences and install prompt/preexec/input-empty hooks. Introduces shell detection/version functions, OSC formatting helpers, and exposes startup script contents via embed variables. Frontend terminal parsing now handles OSC 16162 and OSC 7, mapping shell state into runtime info and TypeScript types. Backend records local shell type/version in telemetry, adds a cached GetSystemSummary(), adjusts telemetry latency logging, and emits additional terminal reset sequences. Build Taskfile now includes .sh sources; minor AI-chat tool and toolschema text updates added. Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
pkg/aiusechat/tools.go (1)
173-173: Remove the commented-out debug code.This commented-out log statement should be removed before merging to keep the codebase clean.
Apply this diff to remove the debug code:
prompt.WriteString("</current_tab_state>") rtn := prompt.String() - // log.Printf("%s\n", rtn) return rtnaiprompts/wave-osc-16162.md (1)
9-11: Add language specifiers to code blocks.The fenced code blocks on lines 9-11 and 172-174 should specify a language for proper syntax highlighting.
Apply this diff:
-``` +```text ESC ] 16162 ; command [;<json-data>] BELAnd: ```diff -``` +```text 1. Shell starts → M;<json> (metadata - shell info) ...Also applies to: 172-174 </blockquote></details> <details> <summary>frontend/app/view/term/termwrap.ts (2)</summary><blockquote> `171-175`: **Reduce log noise for OSC 16162.** This logs on every prompt/command transition. Gate behind the debug logger to avoid console spam. ```diff - console.log("OSC 16162 received:", data, "blockId:", blockId); + dlog("OSC 16162 received:", data, "blockId:", blockId);
243-254: Asynchronous SetRTInfo dispatch is good; consider batching if rate increases.Current setTimeout/fire-and-forget is fine. If events become chatty, consider coalescing multiple rtInfo updates within a short window.
Can you confirm typical event rates (precmd/preexec/inputempty) under rapid typing so we can decide if batching is warranted?
pkg/util/shellutil/shellutil.go (1)
380-416: Optional: add Windows PowerShell version support.If you want a version for powershell.exe, safely query PSVersionTable with a timeout.
Example addition inside getShellVersion:
case ShellType_pwsh: // For powershell.exe, "--version" may not be supported. Try a fallback. if strings.Contains(strings.ToLower(filepath.Base(shellPath)), "powershell") && !strings.Contains(strings.ToLower(filepath.Base(shellPath)), "pwsh") { cmd = exec.CommandContext(ctx, shellPath, "-NoLogo", "-NoProfile", "-Command", "$v=$PSVersionTable.PSVersion; Write-Output ($v.Major.ToString()+\".\"+$v.Minor.ToString())") versionRegex = regexp.MustCompile(`^(\d+\.\d+)\s*$`) } else { cmd = exec.CommandContext(ctx, shellPath, "--version") versionRegex = pwshVersionRegexp }pkg/util/shellutil/shellintegration/zsh_zshrc.sh (1)
52-65: Minor: precmd looks correct; consider JSON-escaping if inputs ever contain quotes.uname/ZSH_VERSION are benign today; if that changes, escape before embedding.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
Taskfile.yml(3 hunks)aiprompts/wave-osc-16162.md(1 hunks)cmd/server/main-server.go(3 hunks)frontend/app/view/term/termwrap.ts(6 hunks)frontend/types/gotypes.d.ts(3 hunks)pkg/aiusechat/tools.go(3 hunks)pkg/blockcontroller/shellcontroller.go(1 hunks)pkg/telemetry/telemetrydata/telemetrydata.go(1 hunks)pkg/util/shellutil/shellintegration/bash_bashrc.sh(1 hunks)pkg/util/shellutil/shellintegration/fish_wavefish.sh(1 hunks)pkg/util/shellutil/shellintegration/pwsh_wavepwsh.sh(1 hunks)pkg/util/shellutil/shellintegration/zsh_zlogin.sh(1 hunks)pkg/util/shellutil/shellintegration/zsh_zprofile.sh(1 hunks)pkg/util/shellutil/shellintegration/zsh_zshenv.sh(1 hunks)pkg/util/shellutil/shellintegration/zsh_zshrc.sh(1 hunks)pkg/util/shellutil/shellutil.go(3 hunks)pkg/wavebase/wavebase.go(1 hunks)pkg/waveobj/blockrtinfo.go(1 hunks)pkg/wcloud/wcloud.go(1 hunks)pkg/wstore/blockrtinfo.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
pkg/blockcontroller/shellcontroller.go (1)
pkg/util/shellutil/shellutil.go (1)
FormatOSC(418-423)
pkg/aiusechat/tools.go (1)
pkg/wavebase/wavebase.go (1)
GetSystemSummary(317-324)
cmd/server/main-server.go (2)
pkg/util/shellutil/shellutil.go (1)
DetectShellTypeAndVersion(356-359)pkg/wavebase/wavebase.go (1)
GetSystemSummary(317-324)
pkg/util/shellutil/shellintegration/bash_bashrc.sh (1)
pkg/util/shellutil/shellintegration/zsh_zshrc.sh (3)
_waveterm_si_blocked(24-26)_waveterm_si_urlencode(28-44)_waveterm_si_osc7(46-50)
pkg/util/shellutil/shellintegration/zsh_zshrc.sh (1)
pkg/util/shellutil/shellintegration/bash_bashrc.sh (3)
_waveterm_si_blocked(34-36)_waveterm_si_urlencode(38-50)_waveterm_si_osc7(52-56)
frontend/app/view/term/termwrap.ts (6)
frontend/util/util.ts (1)
fireAndForget(416-416)frontend/app/store/global.ts (1)
WOS(861-861)pkg/wshrpc/wshrpctypes.go (1)
CommandSetRTInfoData(862-865)pkg/waveobj/blockrtinfo.go (1)
ObjRTInfo(6-20)frontend/app/store/wshclientapi.ts (1)
RpcApi(542-542)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
🪛 LanguageTool
aiprompts/wave-osc-16162.md
[grammar] ~13-~13: There might be a mistake here.
Context: ...; command [;] BEL ``` Where: - ESC = `\033` (escape character) - `BEL` = `...
(QB_NEW_EN)
[grammar] ~152-~152: There might be a mistake here.
Context: ...` When: Can be sent at any time to ensure terminal is not stuck in alternate buff...
(QB_NEW_EN)
[grammar] ~156-~156: There might be a mistake here.
Context: ...perly restoring the screen. Behavior: - Checks if terminal is in alternate buffe...
(QB_NEW_EN)
[grammar] ~210-~210: There might be a mistake here.
Context: ...<encoded_path>` This is sent: - During first precmd (after metadata) - I...
(QB_NEW_EN)
[grammar] ~211-~211: There might be a mistake here.
Context: ...: - During first precmd (after metadata) - In the chpwd hook (whenever directory ...
(QB_NEW_EN)
[grammar] ~212-~212: There might be a mistake here.
Context: ...(after metadata) - In the chpwd hook (whenever directory changes) The path is URL-enc...
(QB_NEW_EN)
🪛 markdownlint-cli2 (0.18.1)
aiprompts/wave-osc-16162.md
9-9: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
172-172: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
pkg/util/shellutil/shellintegration/pwsh_wavepwsh.sh
[warning] 2-2: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[error] 6-6: Couldn't parse this if expression. Fix to allow more checks.
(SC1073)
[error] 6-6: Expected 'then'.
(SC1050)
[error] 6-6: Unexpected . Fix any mentioned problems and try again.
(SC1072)
pkg/util/shellutil/shellintegration/zsh_zshenv.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 5-5: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
pkg/util/shellutil/shellintegration/fish_wavefish.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 3-3: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 3-3: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 3-3: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 3-3: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 6-6: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 10-10: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
pkg/util/shellutil/shellintegration/zsh_zlogin.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 2-2: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
pkg/util/shellutil/shellintegration/zsh_zprofile.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 2-2: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
pkg/util/shellutil/shellintegration/bash_bashrc.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 7-7: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 7-7: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 7-7: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 7-7: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 18-18: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 20-20: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 22-22: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 30-30: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 54-54: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 68-68: Variable was used as an array but is now assigned a string.
(SC2178)
[warning] 68-68: Expanding an array without an index only gives the first element.
(SC2128)
pkg/util/shellutil/shellintegration/zsh_zshrc.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 2-2: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 4-4: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 9-9: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 18-18: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 29-29: functions is referenced but not assigned.
(SC2154)
[warning] 29-29: omz_urlencode is referenced but not assigned.
(SC2154)
[warning] 48-48: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 59-59: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 98-98: add is referenced but not assigned.
(SC2154)
[warning] 98-98: zle is referenced but not assigned.
(SC2154)
[warning] 98-98: hook is referenced but not assigned.
(SC2154)
[warning] 98-98: widget is referenced but not assigned.
(SC2154)
⏰ 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). (5)
- GitHub Check: Build Docsite
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: merge-gatekeeper
🔇 Additional comments (32)
pkg/wcloud/wcloud.go (1)
180-183: LGTM! Clean observability improvement.The latency measurement is correctly positioned around the HTTP request, capturing the full round-trip time including response reading. The logging format is appropriate and the implementation follows Go idioms.
pkg/util/shellutil/shellintegration/fish_wavefish.sh (1)
1-10: LGTM! Shellcheck warnings are false positives.The Fish shell integration script is well-structured. The static analysis warnings are false positives:
- SC2148 (missing shebang): Intentional, as the comment notes this is sourced with
-C, not executed- SC1083 (literal braces): These are Go template placeholders (
{{.WSHBINDIR}}) that will be replaced at runtime- SC1090 (non-constant source): Expected for dynamic script loading via
wsh tokenThe error suppression on line 6 (
2>/dev/null) is appropriate for gracefully handling empty output.pkg/blockcontroller/shellcontroller.go (1)
206-207: LGTM! Clean terminal state reset enhancement.The addition of alternate scroll mode disable (
ESC [?1007l) and OSC 16162 "R" sequence properly extends the terminal reset logic. These changes align with the PR's objective to introduce OSC 16162 support for shell state management.Taskfile.yml (3)
160-160: LGTM! Proper build dependency tracking.Adding
pkg/**/*.shto build sources ensures that shell integration scripts are properly tracked as dependencies, triggering rebuilds when they change. This is consistent with the pattern used for other source files.
192-192: LGTM! Consistent dependency tracking.Correctly includes shell scripts in the server build sources.
220-220: LGTM! Complete coverage across build variants.Shell scripts are properly tracked across all build variants (quickdev, full build).
pkg/util/shellutil/shellintegration/pwsh_wavepwsh.sh (1)
1-13: LGTM! Shellcheck warnings are false positives.The PowerShell integration script is well-structured with proper error handling. The static analysis warnings are false positives:
- Template braces (
{{.WSHBINDIR_PWSH}},{{.PATHSEP}}): Go template placeholders- PowerShell syntax errors: Shellcheck is designed for POSIX shells, not PowerShell
The defensive check on lines 6-8 (verifying non-empty output before
Invoke-Expression) is good practice to avoid executing empty or error output.pkg/util/shellutil/shellintegration/zsh_zprofile.sh (1)
1-2: LGTM! Shellcheck warnings are false positives.Simple and correct implementation for sourcing the user's original zprofile. The static analysis warnings are false positives:
- SC2148: Intentional, this is a sourced file, not an executable script
- SC1090: Expected, sources user's profile file at a fixed path
pkg/util/shellutil/shellintegration/zsh_zlogin.sh (1)
1-7: LGTM! Proper ZDOTDIR handling.The guard condition on lines 5-7 correctly prevents unsetting
ZDOTDIRif the user has modified it after initialization. This complements theWAVETERM_ZDOTDIRpreservation logic inzsh_zshenv.sh.Shellcheck warnings (SC2148, SC1090) are false positives for the same reasons as in other shell integration files.
frontend/types/gotypes.d.ts (3)
713-719: LGTM! Frontend types align with backend schema.The new shell-related fields in
ObjRTInfoproperly mirror the backend changes for tracking shell state, type, version, and command history. All fields are optional, maintaining backward compatibility.
941-942: LGTM! Telemetry types updated for local shell metadata.The addition of
localshell:typeandlocalshell:versiontoTEventPropsenables tracking of the local shell environment in telemetry events.
1011-1012: LGTM! User properties match event properties.Consistent addition of localshell fields to
TEventUserProps.pkg/util/shellutil/shellintegration/zsh_zshenv.sh (1)
1-11: LGTM! Robust ZDOTDIR change detection.The logic correctly:
- Preserves the original
ZDOTDIRbefore sourcing user's.zshenv- Detects if the user modified
ZDOTDIRin their.zshenv- Falls back to sourcing Wave's custom
.zshrcwhenZDOTDIRchangesThis ensures Wave's shell integration remains functional even when users customize their zsh configuration. The logic complements the guard in
zsh_zlogin.sh.Shellcheck warnings (SC2148, SC1090) are false positives as explained in other shell integration files.
pkg/aiusechat/tools.go (2)
160-160: System summary integration looks good.The addition of system summary to the tab state prompt provides valuable context for AI interactions, aligning well with the PR objectives.
201-255: Internal testing tool implementation is sound.The
GetAdderToolDefinitionfunction is well-structured with proper schema validation and error handling for the test adder tool.pkg/wstore/blockrtinfo.go (1)
70-79: Integer field handling looks correct.The added type conversion logic properly handles
int,int64, andfloat64values when setting integer fields, supporting the new shell-related metadata fields introduced in this PR.pkg/telemetry/telemetrydata/telemetrydata.go (1)
65-67: Telemetry field additions are well-structured.The new
LocalShellTypeandLocalShellVersionfields follow the established naming conventions and integrate cleanly with the existing telemetry data model.pkg/wavebase/wavebase.go (2)
317-324: Lazy initialization pattern is correctly implemented.The use of
sync.Onceensures thread-safe, single initialization of the system summary with an appropriate 2-second timeout.
333-371: OS-specific system detection is comprehensive.The implementation handles macOS, Linux, and Windows appropriately:
- macOS: Uses
sw_versfor version- Linux: Reads
/etc/os-release(standard location since 2012)- Windows: Uses PowerShell to query
Win32_OperatingSystemSilent error fallbacks are acceptable for this user-facing summary context.
cmd/server/main-server.go (3)
221-225: Shell detection with appropriate error handling.The error handling defaults to
shellType = "error"andshellVersion = ""when detection fails, ensuring telemetry can still be sent without crashing the startup process.
235-236: Telemetry integration is clean.The shell type and version are properly propagated to the telemetry user properties, providing valuable context for understanding the user environment.
413-413: Background system summary initialization is appropriate.Calling
GetSystemSummary()as a background goroutine after core initialization ensures the summary is cached for later AI context without blocking startup.aiprompts/wave-osc-16162.md (1)
1-204: Comprehensive OSC 16162 documentation.The documentation thoroughly explains the custom OSC escape sequences for shell integration, including format, commands, data types, timing, and examples. This will be valuable for developers working with shell integration.
pkg/waveobj/blockrtinfo.go (1)
13-19: Shell metadata fields are well-defined.The new shell-related fields provide comprehensive runtime information with appropriate types and consistent JSON tag naming (
shell:*). These integrate properly with the OSC 16162 shell integration and telemetry systems.pkg/util/shellutil/shellintegration/bash_bashrc.sh (4)
34-36: Shell integration blocking logic is consistent.The
_waveterm_si_blockedfunction correctly detects tmux/screen environments where shell integration should be disabled, matching the documented behavior and the zsh implementation.
38-50: URL encoding implementation is correct.The
_waveterm_si_urlencodefunction properly escapes reserved characters for URL encoding, handling%first to avoid double-encoding. This is essential for the OSC 7 file URL format.
64-70: PROMPT_COMMAND handling is robust.The
_waveterm_si_append_pcfunction correctly handles both Bash 3.x (string form) and newer versions (array form) ofPROMPT_COMMAND, ensuring compatibility across Bash versions.
13-13: Verify controlled eval usage.The use of
evalwith the output ofwsh tokenis acceptable sincewshis a controlled binary from the Wave Terminal installation. However, ensure that the swap token is properly validated withinwshto prevent any potential injection if the token mechanism were compromised.frontend/app/view/term/termwrap.ts (2)
110-119: Owning OSC 7 is fine; early returns prevent misrouting.Returning true on invalid/empty data is consistent with “we own OSC 7” behavior. LGTM.
236-239: Alt-buffer exit sequence on "R" looks correct.Using ESC [?1049l only when in the alternate buffer is appropriate. LGTM.
pkg/util/shellutil/shellutil.go (1)
418-423: FormatOSC helper looks good.API is minimal and correct for ST terminator sequences.
pkg/util/shellutil/shellintegration/zsh_zshrc.sh (1)
46-50: OSC 7 emission is good; consistent with bash integration.Using $HOST is fine in zsh. LGTM.
| if (data.length > 1024) { | ||
| console.log("Invalid OSC 7, data length too long", data.length); | ||
| return true; | ||
| } | ||
|
|
||
| let pathPart: string; | ||
| try { | ||
| const url = new URL(data); | ||
| if (url.protocol !== "file:") { | ||
| console.log("Invalid OSC 7 command received (non-file protocol)", data); | ||
| return true; | ||
| } | ||
| data = data.substring(nextSlashIdx); | ||
| pathPart = url.pathname; | ||
| } catch (e) { | ||
| console.log("Invalid OSC 7 command received (parse error)", data, e); | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Decode the pathname and normalize Windows paths for OSC 7.
URL.pathname retains percent-encoding and starts with a leading slash on Windows (e.g., /C:/...). Decode and normalize so "cmd:cwd" is a valid OS path.
Apply:
- let pathPart: string;
+ let pathPart: string;
try {
const url = new URL(data);
if (url.protocol !== "file:") {
console.log("Invalid OSC 7 command received (non-file protocol)", data);
return true;
}
- pathPart = url.pathname;
+ // Decode percent-encoding
+ let p = decodeURIComponent(url.pathname);
+ // Normalize Windows drive letter paths from /C:/... -> C:/...
+ if (/^\/[A-Za-z]:\//.test(p)) {
+ p = p.slice(1);
+ }
+ pathPart = p;
} catch (e) {
console.log("Invalid OSC 7 command received (parse error)", data, e);
return true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (data.length > 1024) { | |
| console.log("Invalid OSC 7, data length too long", data.length); | |
| return true; | |
| } | |
| let pathPart: string; | |
| try { | |
| const url = new URL(data); | |
| if (url.protocol !== "file:") { | |
| console.log("Invalid OSC 7 command received (non-file protocol)", data); | |
| return true; | |
| } | |
| data = data.substring(nextSlashIdx); | |
| pathPart = url.pathname; | |
| } catch (e) { | |
| console.log("Invalid OSC 7 command received (parse error)", data, e); | |
| return true; | |
| } | |
| if (data.length > 1024) { | |
| console.log("Invalid OSC 7, data length too long", data.length); | |
| return true; | |
| } | |
| let pathPart: string; | |
| try { | |
| const url = new URL(data); | |
| if (url.protocol !== "file:") { | |
| console.log("Invalid OSC 7 command received (non-file protocol)", data); | |
| return true; | |
| } | |
| // Decode percent-encoding | |
| let p = decodeURIComponent(url.pathname); | |
| // Normalize Windows drive letter paths from /C:/... -> C:/... | |
| if (/^\/[A-Za-z]:\//.test(p)) { | |
| p = p.slice(1); | |
| } | |
| pathPart = p; | |
| } catch (e) { | |
| console.log("Invalid OSC 7 command received (parse error)", data, e); | |
| return true; | |
| } |
🤖 Prompt for AI Agents
In frontend/app/view/term/termwrap.ts around lines 120-136: after parsing the
URL and grabbing url.pathname, decode the percent-encoded pathname (use
decodeURIComponent) and then normalize Windows drive-style paths by stripping a
leading slash when the pathname matches /^\/[A-Za-z]:\// (so "/C:/..." becomes
"C:/..."); finally normalize separators using the platform-appropriate path
normalizer (import path and use path.win32.normalize for Windows or
path.normalize depending on runtime) and assign the resulting decoded+normalized
string to pathPart before continuing.
| shellBase := filepath.Base(shellPath) | ||
| if shellType == ShellType_pwsh && strings.Contains(shellBase, "powershell") && !strings.Contains(shellBase, "pwsh") { | ||
| return "powershell", "", nil | ||
| } | ||
|
|
There was a problem hiding this comment.
Inconsistent shell type: returns "powershell" but no constant exists.
Callers expect ShellType_pwsh. Returning a bare "powershell" string will cause mismatches downstream.
Option A (simplest): normalize to pwsh type and keep empty version when path is powershell.exe:
- if shellType == ShellType_pwsh && strings.Contains(shellBase, "powershell") && !strings.Contains(shellBase, "pwsh") {
- return "powershell", "", nil
- }
+ if shellType == ShellType_pwsh && strings.Contains(shellBase, "powershell") && !strings.Contains(shellBase, "pwsh") {
+ return ShellType_pwsh, "", nil
+ }Option B (if you want to distinguish): add a ShellType_powershell constant and ensure all consumers handle it.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| shellBase := filepath.Base(shellPath) | |
| if shellType == ShellType_pwsh && strings.Contains(shellBase, "powershell") && !strings.Contains(shellBase, "pwsh") { | |
| return "powershell", "", nil | |
| } | |
| shellBase := filepath.Base(shellPath) | |
| if shellType == ShellType_pwsh && strings.Contains(shellBase, "powershell") && !strings.Contains(shellBase, "pwsh") { | |
| return ShellType_pwsh, "", nil | |
| } |
🤖 Prompt for AI Agents
In pkg/util/shellutil/shellutil.go around lines 367-371, the function currently
returns the literal string "powershell" which is inconsistent with callers that
expect the ShellType_pwsh constant; update this branch to return the existing
ShellType_pwsh constant with an empty version and nil error to normalize legacy
powershell.exe paths to the pwsh type (alternative: if you prefer preserving a
distinct type, add a new ShellType_powershell constant and update all consumers
to handle it, but the simplest fix is to return ShellType_pwsh, empty version,
nil).
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pkg/aiusechat/tools.go (1)
207-261: Consider using int64 for sum to prevent overflow.The adder function accumulates into an
int, which could overflow on 32-bit systems or with large arrays. Additionally, while the schema specifies integers, there's no validation that the float64 values (from JSON unmarshaling) are actually whole numbers.Apply this diff to use int64 and add basic validation:
- sum := 0 + sum := int64(0) for i, val := range valuesSlice { floatVal, ok := val.(float64) if !ok { return nil, fmt.Errorf("value at index %d is not a number", i) } - sum += int(floatVal) + intVal := int64(floatVal) + if float64(intVal) != floatVal { + return nil, fmt.Errorf("value at index %d is not an integer", i) + } + sum += intVal }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pkg/aiusechat/tools.go(3 hunks)pkg/aiusechat/tools_readdir.go(1 hunks)pkg/aiusechat/tools_readfile.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/aiusechat/tools.go (1)
pkg/wavebase/wavebase.go (1)
GetSystemSummary(317-324)
⏰ 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). (5)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build Docsite
- GitHub Check: Build for TestDriver.ai
- GitHub Check: merge-gatekeeper
🔇 Additional comments (4)
pkg/aiusechat/tools_readdir.go (1)
195-195: LGTM! Documentation improvement accurately reflects existing functionality.The description update clarifies that '~' is supported for home directory expansion, which is already implemented via
wavebase.ExpandHomeDir()at line 74.pkg/aiusechat/tools_readfile.go (1)
205-205: LGTM! Documentation improvement accurately reflects existing functionality.The description update clarifies that '~' is supported for home directory expansion, which is already implemented via
wavebase.ExpandHomeDir()at line 113. This change is consistent with the parallel documentation update intools_readdir.go.pkg/aiusechat/tools.go (2)
9-9: LGTM! Imports support new functionality.The new imports are necessary for retrieving system information and user context that will be included in AI prompts.
Also applies to: 15-15
161-180: LGTM! System and user context enrichment looks good.The addition of local machine and user information provides valuable context for AI interactions. The error handling gracefully falls back to excluding user information if lookup fails, which is appropriate for this non-critical context enrichment.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
frontend/app/view/term/termwrap.ts (2)
125-136: Windows OSC 7: strip leading slash after decode (duplicate of earlier feedback).You decoded percent-encoding, but Windows paths still arrive as “/C:/…”. Strip the leading slash to yield a valid OS path.
Apply:
- pathPart = decodeURIComponent(url.pathname); + // Decode and normalize Windows drive-letter paths: "/C:/..." -> "C:/..." + let p = decodeURIComponent(url.pathname); + if (/^\/[A-Za-z]:\//.test(p)) { + p = p.slice(1); + } + pathPart = p;
199-211: Add size cap before atob for cmd64 (duplicate of earlier feedback).Prevent UI jank/memory spikes from oversized base64 payloads.
Apply:
- if (cmd.data.cmd64) { + if (cmd.data.cmd64) { + // 8 KiB base64 cap + const MAX_B64 = 8 * 1024; + if (cmd.data.cmd64.length > MAX_B64) { + console.warn("OSC 16162 cmd64 too large, ignoring"); + break; + } try { const decodedCmd = atob(cmd.data.cmd64); rtInfo["shell:lastcmd"] = decodedCmd; } catch (e) { console.error("Error decoding cmd64:", e); - rtInfo["shell:lastcmd"] = null; } - } else { - rtInfo["shell:lastcmd"] = null; - } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/app/view/term/termwrap.ts(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/app/view/term/termwrap.ts (6)
frontend/util/util.ts (1)
fireAndForget(416-416)frontend/app/store/global.ts (1)
WOS(861-861)pkg/wshrpc/wshrpctypes.go (1)
CommandSetRTInfoData(862-865)pkg/waveobj/blockrtinfo.go (1)
ObjRTInfo(6-20)frontend/app/store/wshclientapi.ts (1)
RpcApi(542-542)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
⏰ 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). (5)
- GitHub Check: Build Docsite
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: merge-gatekeeper
🔇 Additional comments (2)
frontend/app/view/term/termwrap.ts (2)
48-108: Wave OSC setmeta handler: LGTMEarly returns and ResolveIds flow look good. No blockers.
338-340: OSC 16162 handler registration: LGTMHooking the handler into xterm’s OSC parser is correct.
frontend/app/view/term/termwrap.ts
Outdated
| console.log("OSC 16162 received:", data, "blockId:", blockId); | ||
|
|
||
| if (!data || data.length === 0) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Redact OSC 16162 payloads; use debug logger.
Raw data may include commands and system info. Log minimally with dlog instead.
Apply:
- console.log("OSC 16162 received:", data, "blockId:", blockId);
+ dlog("OSC 16162 received", { blockId, len: data?.length ?? 0 });Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/app/view/term/termwrap.ts around lines 171-175, replace the
console.log that prints the raw OSC 16162 payload with a call to the debug
logger (`dlog`) and do not emit the full `data`; instead log minimal,
non-sensitive info such as the event marker ("OSC 16162"), the blockId, and a
safe summary (e.g. data length or a fixed-size truncated/hex-preview) so the raw
payload is not exposed; ensure the early-return behavior for empty data remains
unchanged.
frontend/app/view/term/termwrap.ts
Outdated
| console.error("Error decoding cmd64:", e); | ||
| rtInfo["shell:lastcmd"] = null; | ||
| } | ||
| } else { | ||
| rtInfo["shell:lastcmd"] = null; | ||
| } |
There was a problem hiding this comment.
Don’t send nulls in ObjRTInfo; omit keys instead (type/runtime breakage).
"shell:lastcmd" is a string and "shell:lastcmdexitcode" is a number in ObjRTInfo. Assigning null can break TS types and server unmarshalling. Omit the key when value is unknown.
Apply:
- } catch (e) {
- console.error("Error decoding cmd64:", e);
- rtInfo["shell:lastcmd"] = null;
- }
- } else {
- rtInfo["shell:lastcmd"] = null;
- }
+ } catch (e) {
+ console.error("Error decoding cmd64:", e);
+ // omit "shell:lastcmd" when decode fails
+ }
+ }- if (cmd.data.exitcode != null) {
- rtInfo["shell:lastcmdexitcode"] = cmd.data.exitcode;
- } else {
- rtInfo["shell:lastcmdexitcode"] = null;
- }
+ if (cmd.data.exitcode != null) {
+ rtInfo["shell:lastcmdexitcode"] = cmd.data.exitcode;
+ }Also applies to: 224-228
There was a problem hiding this comment.
this is correct. first we're using non-strict for typescript so those DO accept null. ALSO "null" is how we CLEAR a key value in blockrtinfo.
There was a problem hiding this comment.
@sawka, thank you for the clarification! I understand now—setting null explicitly signals that the key should be cleared in blockrtinfo, which is different from simply omitting the key. Given the non-strict TypeScript mode, the null assignment is both type-safe and functionally correct for this use case.
I appreciate the explanation of this design pattern!
✏️ Learnings added
Learnt from: sawka
PR: wavetermdev/waveterm#2444
File: frontend/app/view/term/termwrap.ts:0-0
Timestamp: 2025-10-17T18:58:09.719Z
Learning: In blockrtinfo (ObjRTInfo), setting a field to `null` is the explicit mechanism to CLEAR that key's value, as opposed to omitting the key. This is used in frontend code when handling OSC 16162 commands in termwrap.ts. TypeScript runs in non-strict mode, so null is acceptable for string/number fields.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
frontend/app/view/term/termwrap.ts (4)
129-129: Redact OSC 7 payloads in logs (PII).These console.log statements still output the full
datapayload, which may contain usernames and sensitive directory paths. This was flagged in a previous review but not fully addressed.Apply this diff to redact sensitive information:
- console.log("Invalid OSC 7 command received (non-file protocol)", data); + console.log("Invalid OSC 7 command received (non-file protocol)");- console.log("Invalid OSC 7 command received (parse error)", data, e); + console.log("Invalid OSC 7 command received (parse error)", e);Also applies to: 140-140
177-177: Redact OSC 16162 payloads; use debug logger.The raw
datapayload may include commands and system information. Logging the full payload exposes potentially sensitive information. This was flagged in a previous review.Apply this diff:
- console.log("OSC 16162 received:", data, "blockId:", blockId); + dlog("OSC 16162 received", { blockId, len: data?.length ?? 0 });
206-216: Guard against overly large cmd64 payloads.There's no size limit before calling
atob()oncmd64. A malicious or malformed payload could cause memory spikes and UI jank. This was flagged in a previous review.Apply this diff:
if (cmd.data.cmd64) { + // 8 KiB base64 cap + const MAX_B64 = 8 * 1024; + if (cmd.data.cmd64.length > MAX_B64) { + console.warn("OSC 16162 cmd64 too large, ignoring"); + break; + } try { const decodedCmd = atob(cmd.data.cmd64); rtInfo["shell:lastcmd"] = decodedCmd; } catch (e) { console.error("Error decoding cmd64:", e); - rtInfo["shell:lastcmd"] = null; } - } else { - rtInfo["shell:lastcmd"] = null; }
212-212: Don't send nulls in ObjRTInfo; omit keys instead.According to the
ObjRTInfotype definition inpkg/waveobj/blockrtinfo.go, fields like"shell:lastcmd"are strings and"shell:lastcmdexitcode"are numbers. Assigningnullviolates the TypeScript type contract and can break server-side unmarshalling. This was flagged as critical in a previous review.Apply this diff:
} catch (e) { console.error("Error decoding cmd64:", e); - rtInfo["shell:lastcmd"] = null; + // omit "shell:lastcmd" when decode fails } - } else { - rtInfo["shell:lastcmd"] = null; }if (cmd.data.exitcode != null) { rtInfo["shell:lastcmdexitcode"] = cmd.data.exitcode; - } else { - rtInfo["shell:lastcmdexitcode"] = null; }Also applies to: 215-215, 233-233
🧹 Nitpick comments (1)
frontend/util/platformutil.ts (1)
2-2: Good addition, but refactor line 23 for consistency.The new
PlatformWindowsconstant andisWindows()function follow the existing pattern nicely. However, line 23 still uses the hard-coded string literal"win32"instead of the newly defined constant.Apply this diff to use the constant consistently:
- } else if (PLATFORM == "win32") { + } else if (PLATFORM == PlatformWindows) {Also applies to: 13-15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/app/view/term/termwrap.ts(6 hunks)frontend/util/platformutil.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/app/view/term/termwrap.ts (6)
frontend/util/util.ts (1)
fireAndForget(416-416)frontend/app/store/global.ts (1)
WOS(861-861)pkg/wshrpc/wshrpctypes.go (1)
CommandSetRTInfoData(862-865)pkg/waveobj/blockrtinfo.go (1)
ObjRTInfo(6-20)frontend/app/store/wshclientapi.ts (1)
RpcApi(542-542)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
⏰ 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). (5)
- GitHub Check: Build Docsite
- GitHub Check: Build for TestDriver.ai
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: merge-gatekeeper
🔇 Additional comments (2)
frontend/app/view/term/termwrap.ts (2)
344-346: LGTM!The OSC 16162 handler registration is correctly implemented and follows the same pattern as the existing OSC 7 and OSC 9283 handlers.
242-244: No issues found—the alternate buffer escape sequence handling is safe and correctly guarded.The "R" command handler includes a guard condition that only writes
\x1b[?1049lwhenterminal.buffer.active.type === "alternate". This prevents sending the exit-alternate-buffer sequence inappropriately. The implementation is correct: OSC 16162 appears to be a custom shell integration protocol where "R" signals that an interactive application (vim, less, etc.) has exited; the code then cleanly exits the alternate buffer if needed. The conditional check ensures this escape sequence is sent only at the right time.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
pkg/util/shellutil/shellintegration/zsh_zshrc.sh (2)
40-41: Semicolon encoding fix is correct.Using
%3B(without backslash) yields valid percent-escape.
68-75: Make cmd64 single-line; optionally cap size.Unescaped newlines break JSON; trim them. Consider skipping too-large payloads to match frontend cap.
Apply:
- cmd64=$(printf '%s' "$1" | base64 2>/dev/null) + # Produce single-line base64 (portable across BSD/GNU) + cmd64=$(printf '%s' "$1" | base64 2>/dev/null | tr -d '\n\r') + # Optional: skip overly large payloads (approx <= 8 KiB decoded) + MAX_B64_CHARS=12288 + if [ ${#cmd64} -gt $MAX_B64_CHARS ]; then + cmd64="" + fifrontend/app/view/term/termwrap.ts (3)
129-131: Redact OSC 7 payloads in logs (PII).Avoid logging raw file:// payloads; paths can include usernames.
Apply:
- console.log("Invalid OSC 7 command received (non-file protocol)", data); + console.log("Invalid OSC 7 command received (non-file protocol)"); // redacted ... - console.log("Invalid OSC 7 command received (parse error)", data, e); + console.log("Invalid OSC 7 command received (parse error)", e); // redactedAlso applies to: 140-141
177-177: Redact OSC 16162 payload; use debug logger.Do not print the raw 16162 data; log minimal metadata.
Apply:
- console.log("OSC 16162 received:", data, "blockId:", blockId); + dlog("OSC 16162 received", { blockId, len: data?.length ?? 0 });
212-221: Don’t send nulls in ObjRTInfo; omit keys instead.Nulls can break TS types and server unmarshalling; ObjRTInfo is “omitempty” on the wire.
Apply:
- } catch (e) { - console.error("Error decoding cmd64:", e); - rtInfo["shell:lastcmd"] = null; - } - } - } else { - rtInfo["shell:lastcmd"] = null; - } + } catch (e) { + console.error("Error decoding cmd64:", e); + // omit "shell:lastcmd" on decode failure + } + } + }- if (cmd.data.exitcode != null) { - rtInfo["shell:lastcmdexitcode"] = cmd.data.exitcode; - } else { - rtInfo["shell:lastcmdexitcode"] = null; - } + if (cmd.data.exitcode != null) { + rtInfo["shell:lastcmdexitcode"] = cmd.data.exitcode; + }Based on relevant code snippet showing ObjRTInfo fields with
omitempty.Also applies to: 235-239
🧹 Nitpick comments (6)
frontend/app/view/term/termwrap.ts (2)
125-139: Good: decode and Windows-drive normalization for OSC 7.Decoding percent-encoding and stripping the leading “/” for drive paths fixes common Windows issues.
Optionally also collapse redundant separators and dot-segments before persisting.
206-218: Trim whitespace in cmd64 before length check/atob.Some base64 encoders insert newlines; make receiver tolerant.
Apply:
- if (cmd.data.cmd64) { - const decodedLen = Math.ceil(cmd.data.cmd64.length * 0.75); + if (cmd.data.cmd64) { + const b64 = cmd.data.cmd64.replace(/\s+/g, ""); + const decodedLen = Math.ceil(b64.length * 0.75); if (decodedLen > 8192) { rtInfo["shell:lastcmd"] = `# command too large to store (${decodedLen} bytes)`; } else { try { - const decodedCmd = atob(cmd.data.cmd64); + const decodedCmd = atob(b64); rtInfo["shell:lastcmd"] = decodedCmd; } catch (e) { console.error("Error decoding cmd64:", e); - rtInfo["shell:lastcmd"] = null; + // omit "shell:lastcmd" on decode failure } } - } else { - rtInfo["shell:lastcmd"] = null; }pkg/util/shellutil/shellintegration/zsh_zshrc.sh (4)
98-102: Load zle hook helper correctly before checking.Autoload first, then gate on availability.
Apply:
-if (( $+functions[add-zle-hook-widget] )); then - autoload -Uz add-zle-hook-widget - add-zle-hook-widget zle-line-init _waveterm_si_inputempty - add-zle-hook-widget zle-line-pre-redraw _waveterm_si_inputempty -fi +autoload -Uz add-zle-hook-widget 2>/dev/null +if (( $+functions[add-zle-hook-widget] )); then + add-zle-hook-widget zle-line-init _waveterm_si_inputempty + add-zle-hook-widget zle-line-pre-redraw _waveterm_si_inputempty +fi
46-50: Prefer robust host when emitting OSC 7.$HOST is usually set, but add a fallback to HOSTNAME/hostname.
Apply:
- printf '\033]7;file://%s%s\007' "$HOST" "$encoded_pwd" # OSC 7 - current directory + local _host="${HOST:-${HOSTNAME:-$(hostname -s 2>/dev/null || echo localhost)}}" + printf '\033]7;file://%s%s\007' "$_host" "$encoded_pwd" # OSC 7 - current directory
59-63: Escape JSON strings in “M” message.Version/uname can, in theory, contain quotes/backslashes. Escape to keep JSON valid.
Apply:
+_waveterm_si_json_escape() { + # Minimal JSON string escaper: \, " and newlines + local s="$1" + s=${s//\\/\\\\}; s=${s//\"/\\\"}; s=${s//$'\n'/ } + printf '%s' "$s" +} ... - local uname_info=$(uname -smr 2>/dev/null) - printf '\033]16162;M;{"shell":"zsh","shellversion":"%s","uname":"%s"}\007' "$ZSH_VERSION" "$uname_info" + local uname_info; uname_info=$(uname -smr 2>/dev/null) + printf '\033]16162;M;{"shell":"zsh","shellversion":"%s","uname":"%s"}\007' \ + "$(_waveterm_si_json_escape "$ZSH_VERSION")" "$(_waveterm_si_json_escape "$uname_info")"
28-44: URL-encode helper looks fine; OMZ fallback respected.If non-ASCII appears in paths, consider encoding bytes via iconv/xxd for full RFC 3986 coverage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/app/view/term/termwrap.ts(6 hunks)pkg/util/shellutil/shellintegration/zsh_zshrc.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
pkg/util/shellutil/shellintegration/zsh_zshrc.sh (1)
pkg/util/shellutil/shellintegration/bash_bashrc.sh (3)
_waveterm_si_blocked(34-36)_waveterm_si_urlencode(38-50)_waveterm_si_osc7(52-56)
frontend/app/view/term/termwrap.ts (6)
frontend/util/util.ts (1)
fireAndForget(416-416)frontend/app/store/global.ts (1)
WOS(861-861)pkg/wshrpc/wshrpctypes.go (1)
CommandSetRTInfoData(862-865)pkg/waveobj/blockrtinfo.go (1)
ObjRTInfo(6-20)frontend/app/store/wshclientapi.ts (1)
RpcApi(542-542)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
🪛 Shellcheck (0.11.0)
pkg/util/shellutil/shellintegration/zsh_zshrc.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 2-2: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 4-4: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 9-9: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 18-18: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 29-29: functions is referenced but not assigned.
(SC2154)
[warning] 29-29: omz_urlencode is referenced but not assigned.
(SC2154)
[warning] 48-48: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 59-59: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 98-98: add is referenced but not assigned.
(SC2154)
[warning] 98-98: zle is referenced but not assigned.
(SC2154)
[warning] 98-98: hook is referenced but not assigned.
(SC2154)
[warning] 98-98: widget is referenced but not assigned.
(SC2154)
⏰ 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). (5)
- GitHub Check: Build Docsite
- GitHub Check: merge-gatekeeper
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (3)
frontend/app/view/term/termwrap.ts (3)
50-55: OK to swallow invalid Wave OSC.Returning true prevents propagation to other handlers; matches intent to “own” this OSC.
Also applies to: 62-64, 73-75, 81-83
247-250: Alt-buffer exit guard is correct.Only sending 1049l when in alternate buffer avoids flicker.
349-351: Registering OSC 16162 handler looks good.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
frontend/app/view/term/termwrap.ts (2)
129-130: Redact file paths from logs to prevent PII leakage.The console logs at lines 129 and 140 still output the full
datapayload, which may contain usernames and sensitive directory paths.Apply this diff:
- console.log("Invalid OSC 7 command received (non-file protocol)", data); + console.log("Invalid OSC 7 command received (non-file protocol)");- console.log("Invalid OSC 7 command received (parse error)", data, e); + console.log("Invalid OSC 7 command received (parse error)", e);Also applies to: 140-140
211-211: Omit keys instead of setting null to maintain type safety.Setting
nullforrtInfo["shell:lastcmd"](string) andrtInfo["shell:lastcmdexitcode"](number) violates the TypeScript type contract forObjRTInfoand may cause runtime issues during serialization.Apply this diff:
} catch (e) { console.error("Error decoding cmd64:", e); - rtInfo["shell:lastcmd"] = null; + // omit shell:lastcmd when decode fails } } } else { - rtInfo["shell:lastcmd"] = null; + // omit shell:lastcmd when not provided }if (cmd.data.exitcode != null) { rtInfo["shell:lastcmdexitcode"] = cmd.data.exitcode; - } else { - rtInfo["shell:lastcmdexitcode"] = null; }Also applies to: 215-215, 233-233
🧹 Nitpick comments (1)
pkg/util/shellutil/shellintegration/zsh_zshrc.sh (1)
2-3: Simplify redundant PATH setup.Lines 2–3 prepend
$WAVETERM_WSHBINDIRto$PATH, and lines 12–14 check if it's already in$PATHand add it again if needed. The second check appears redundant unless the environment between these lines could modify$PATH. Consider consolidating to a single, defensive append.WAVETERM_WSHBINDIR={{.WSHBINDIR}} export PATH="$WAVETERM_WSHBINDIR:$PATH" source <(wsh token "$WAVETERM_SWAPTOKEN" zsh 2>/dev/null) unset WAVETERM_SWAPTOKEN # Source the original zshrc only if ZDOTDIR has not been changed if [ "$ZDOTDIR" = "$WAVETERM_ZDOTDIR" ]; then [ -f ~/.zshrc ] && source ~/.zshrc fi - - if [[ ":$PATH:" != *":$WAVETERM_WSHBINDIR:"* ]]; then - export PATH="$WAVETERM_WSHBINDIR:$PATH" - fi - unset WAVETERM_WSHBINDIRAlso applies to: 12-15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/app/view/term/termwrap.ts(6 hunks)pkg/util/shellutil/shellintegration/zsh_zshrc.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/app/view/term/termwrap.ts (6)
frontend/util/util.ts (1)
fireAndForget(416-416)frontend/app/store/global.ts (1)
WOS(861-861)pkg/wshrpc/wshrpctypes.go (1)
CommandSetRTInfoData(862-865)pkg/waveobj/blockrtinfo.go (1)
ObjRTInfo(6-20)frontend/app/store/wshclientapi.ts (1)
RpcApi(542-542)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
pkg/util/shellutil/shellintegration/zsh_zshrc.sh (1)
pkg/util/shellutil/shellintegration/bash_bashrc.sh (3)
_waveterm_si_blocked(34-36)_waveterm_si_urlencode(38-50)_waveterm_si_osc7(52-56)
🪛 Shellcheck (0.11.0)
pkg/util/shellutil/shellintegration/zsh_zshrc.sh
[error] 1-1: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
(SC2148)
[warning] 2-2: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This { is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 2-2: This } is literal. Check expression (missing ;/\n?) or quote it.
(SC1083)
[warning] 4-4: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 9-9: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 18-18: ShellCheck can't follow non-constant source. Use a directive to specify location.
(SC1090)
[warning] 29-29: functions is referenced but not assigned.
(SC2154)
[warning] 29-29: omz_urlencode is referenced but not assigned.
(SC2154)
[warning] 48-48: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 59-59: Declare and assign separately to avoid masking return values.
(SC2155)
[warning] 98-98: add is referenced but not assigned.
(SC2154)
[warning] 98-98: zle is referenced but not assigned.
(SC2154)
[warning] 98-98: hook is referenced but not assigned.
(SC2154)
[warning] 98-98: widget is referenced but not assigned.
(SC2154)
⏰ 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). (4)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Build for TestDriver.ai
- GitHub Check: merge-gatekeeper
🔇 Additional comments (12)
pkg/util/shellutil/shellintegration/zsh_zshrc.sh (7)
8-8: VerifyWAVETERM_ZDOTDIRis properly initialized.Line 8 checks if
$ZDOTDIRequals$WAVETERM_ZDOTDIR, but this variable is never set or exported in this file. Confirm that it's properly initialized in the parent zsh startup or shell environment before this script is sourced, otherwise the conditional will always be false.
28-44: Proper fallback to custom urlencode implementation.The function correctly attempts to use
omz_urlencode(oh-my-zsh) if available, with a solid fallback implementation that matches the bash version. The character escaping order (% first) is correct to avoid double-encoding.
46-50: OSC 7 implementation aligns with bash version.Uses
$HOST(instead of$HOSTNAMEin bash) which is the correct zsh variable. Encoding via_waveterm_si_urlencodeand proper OSC 7 format are correct.
52-65: Proper first-prompt detection and OSC 16162 sequencing.The logic correctly emits system metadata (shell version, uname) on the first prompt, and exit codes on subsequent prompts. The
_WAVETERM_SI_FIRSTPRECMDflag and OSC 16162 format ("A" start sequence) align with intended integration flow.
67-76: Base64 encoding properly handles newlines and empty commands.Line 70 now correctly pipes base64 through
tr -d '\n\r'to produce single-line JSON-safe output. Empty command handling at line 73–74 sends signal without payload. Previous feedback on newline stripping has been addressed.
80-96: Input-empty tracking uses correct zsh variable and state logic.The implementation correctly checks
$BUFFER(zsh's input buffer variable), tracks state changes to avoid redundant emissions, and emits boolean signals via OSC 16162. The guard check against terminal multiplexers is consistent.
98-106: Hook registration pattern is idiomatic for zsh.Conditionally loads
add-zle-hook-widgetwhen available (for interactive editing), then registers standardprecmd,preexec, andchpwdhooks. Proper use ofautoload -Uandadd-zsh-hookcommand.frontend/app/view/term/termwrap.ts (5)
48-108: LGTM! Return value changes prevent OSC propagation.The changes from
return falsetoreturn truecorrectly indicate that Wave OSC 9283 commands are handled and should not propagate to other handlers.
202-213: Good: Prevented oversized cmd64 payloads.The size check correctly prevents memory issues by calculating the decoded size and capping at 8192 bytes. The fallback message is also helpful.
344-346: LGTM! OSC 16162 handler registered correctly.The handler registration correctly passes the terminal instance, enabling the "R" command to manipulate the terminal buffer state.
241-245: Terminal escape sequence for alternate buffer exit is correct.The sequence
\x1b[?1049lis the standard DEC private mode reset for mode 1049 that exits the alternate buffer and restores the normal screen on xterm and compatible terminals. The code correctly uses this sequence within the appropriate conditional check, with no concerns for unintended side effects.
125-142: Verification confirmed—path normalization logic is sound.The test results verify all stated concerns:
- The regex correctly matches Windows paths with both forward and backward slashes.
- The
substring(1)andreplace(/\\/g, "/")operations properly normalize Windows paths.- Unix paths remain unmodified, and URL-encoded characters are correctly decoded.
All test cases pass as expected. Code is ready to ship.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/util/platformutil.ts (1)
16-18: Standardize equality operators across platform comparisons.The file now mixes
==(lines 13, 17, 26) and===(line 24) for platform comparisons. For consistency and TypeScript/JavaScript best practices, standardize on===throughout.Apply this diff to use strict equality consistently:
export function isMacOS(): boolean { - return PLATFORM == PlatformMacOS; + return PLATFORM === PlatformMacOS; } export function isWindows(): boolean { - return PLATFORM == PlatformWindows; + return PLATFORM === PlatformWindows; }And update line 26:
- } else if (PLATFORM == PlatformWindows) { + } else if (PLATFORM === PlatformWindows) {Also applies to: 26-26
tsunami/frontend/src/util/platformutil.ts (1)
5-5: LGTM: Platform detection functions added with consistent comparisons.The
PlatformWindowsconstant andisWindows()function correctly implement Windows platform detection following the existing pattern. All platform comparisons in this file now consistently use==. However, consider using===for strict equality as a TypeScript/JavaScript best practice.Optional: Apply this diff to use strict equality:
export function isMacOS(): boolean { - return PLATFORM == PlatformMacOS; + return PLATFORM === PlatformMacOS; } export function isWindows(): boolean { - return PLATFORM == PlatformWindows; + return PLATFORM === PlatformWindows; }Also applies to: 12-18
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/util/platformutil.ts(2 hunks)tsunami/frontend/src/util/platformutil.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tsunami/frontend/src/util/platformutil.ts (1)
docs/src/components/platformcontext.tsx (1)
Platform(7-7)
frontend/util/platformutil.ts (1)
docs/src/components/platformcontext.tsx (1)
Platform(7-7)
⏰ 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 (3)
frontend/util/platformutil.ts (2)
1-3: LGTM: Copyright year updated.
5-5: LGTM: Windows platform constant added.The
PlatformWindowsconstant follows the same pattern asPlatformMacOSand correctly uses the NodeJS platform identifier "win32".tsunami/frontend/src/util/platformutil.ts (1)
4-18: Note: Code duplication across platform utility modules.This file shares identical constants and functions with
frontend/util/platformutil.ts. If these represent separate modules or codebases (main frontend vs tsunami), this duplication may be intentional. Otherwise, consider consolidating into a shared utility module.
## Overview This PR implements OSC 7 (current working directory reporting) support for Fish shell and PowerShell, completing the shell integration coverage alongside the existing Bash and Zsh implementations added in #2444. ## What is OSC 7? OSC 7 is an operating system command that allows shells to automatically report their current working directory to the terminal emulator using the format: ``` ESC]7;file://hostname/path BEL ``` This enables the terminal to track the current directory without manual commands, providing better context for AI features and ensuring accurate path information. ## Implementation Details ### Fish Shell (`fish_wavefish.sh`) Added shell integration functions using Fish-native features: - `_waveterm_si_blocked()` - Prevents OSC 7 in tmux/screen environments using `set -q` and `string match -q` - `_waveterm_si_osc7()` - Sends the OSC 7 sequence with built-in URL encoding - Uses `string escape --style=url` for UTF-8 percent-encoding - Hooked to `fish_prompt` event and `PWD` variable changes for automatic updates **Key Features:** - Fish-native checks (`set -q`, `string match`) instead of non-portable `test -o` - Built-in `string escape --style=url` for proper UTF-8 percent-encoding - Simple, portable, and maintainable (32 lines total) ### PowerShell (`pwsh_wavepwsh.sh`) Added simplified shell integration that leverages frontend normalization: - `_waveterm_si_blocked()` - Prevents OSC 7 in tmux/screen environments - `_waveterm_si_osc7()` - Sends OSC 7 with raw path encoding - Uses `[System.Uri]::EscapeDataString()` for proper percent-encoding - Integrated into the prompt function while preserving existing prompts **Key Features:** - **No path rewriting**: Sends raw paths (e.g., `C:\Users\Name` → `C%3A%5CUsers%5CName`) - **UNC support**: Network paths like `\\server\share` encoded as `%5C%5Cserver%5Cshare` - **Hostname fallback**: `$env:COMPUTERNAME` → `$env:HOSTNAME` → empty (produces `file:///path`) - **No DNS lookup**: Avoids potentially slow DNS calls - Simple and efficient (53 lines total) ## Implementation Benefits Both implementations use native, built-in features for maximum compatibility and maintainability: **Fish:** - ✅ Replaced `test -o` with fish-native `set -q` and `string match -q` for better portability - ✅ Replaced manual string replacements with `string escape --style=url` for proper UTF-8 support - ✅ Removed custom URL encoding function (19 lines removed) **PowerShell:** - ✅ Uses built-in `[System.Uri]::EscapeDataString()` for proper percent-encoding - ✅ No path rewriting - frontend handles normalization via `decodeURIComponent()` and backslash conversion - ✅ Proper UNC path support without special-casing - ✅ Removed DNS lookup to avoid slow operations - ✅ Removed custom URL encoding function ## Testing All implementations were tested for: - ✅ URL encoding of special characters (spaces, #, ?, &, ;, +, %) - ✅ Correct OSC 7 format generation - ✅ Fish-native checks work correctly (TMUX, STY, TERM patterns) - ✅ Raw path encoding (PowerShell - no rewriting) - ✅ UNC path support (PowerShell) - ✅ Unix path handling - ✅ Go package compilation - ✅ No security issues (CodeQL) ## Path Handling Examples **Fish:** - `/home/user` → `/home/user` - `/path with spaces` → `/path%20with%20spaces` - `/file#hash` → `/file%23hash` **PowerShell:** - Windows: `C:\Users\Name` → `file://HOST/C%3A%5CUsers%5CName` - UNC: `\\server\share\folder` → `file://HOST/%5C%5Cserver%5Cshare%5Cfolder` - Empty hostname: produces `file:///path` format ## Files Changed - `pkg/util/shellutil/shellintegration/fish_wavefish.sh` (+22 lines, -18 lines = net +4 lines, but 19 lines of custom code removed) - `pkg/util/shellutil/shellintegration/pwsh_wavepwsh.sh` (+26 lines, -51 lines = net -25 lines) Total: 38 fewer lines of code with better functionality and maintainability. Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sawka <2722291+sawka@users.noreply.github.com>
Merges 59 commits from wavetermdev/waveterm v0.12.0 release into a5af fork. Resolves 49 merge conflicts across frontend, backend, and configuration files. ## Major Upstream Features Integrated ### AI Enhancements (v0.12.0) - AI Response Feedback + Copy Buttons (wavetermdev#2457) - Reasoning Deltas Display (wavetermdev#2443) - Google AI File Summarization (wavetermdev#2455) - `wsh ai` Command Reimplementation (wavetermdev#2435) - Terminal Context Improvements (wavetermdev#2444) - Batch Tool Approval System - Enhanced AI Panel with welcome message - Context menu support for AI messages ### Infrastructure Updates - Mobile User Agent Emulation for web widgets (wavetermdev#2454) - OSC 7 Support for Fish & PowerShell shells (wavetermdev#2456) - Log Rotation System (wavetermdev#2432) - Onboarding improvements - React 19 compatibility updates - Tailwind v4 migration progress - Dependency updates (50+ commits) ## Fork Features Preserved ✅ **Horizontal Widget Bar** (tabbar.tsx) - Widgets remain in horizontal tab bar (not reverted to sidebar) - Fork-specific layout maintained ✅ **Optional Pane Title Labels** (blockframe.tsx) - Auto-generated pane titles preserved - Custom block rendering logic intact ✅ **Layout Model Modifications** (layoutModel.ts) - Fork's widget positioning logic maintained - Horizontal layout integration preserved ## Conflict Resolution Summary **Configuration (8 files):** - Accepted upstream: .golangci.yml, Taskfile.yml, package.json, go.mod, etc. - All dependencies updated to v0.12.0 levels **Backend AI (13 files):** - Accepted upstream: All pkg/aiusechat/ files - New AI tools: read_dir, screenshot, terminal context - Enhanced OpenAI backend with reasoning support **Frontend AI Panel (12 files):** - Accepted upstream: All frontend/app/aipanel/ files - New features: reasoning display, feedback buttons, welcome message - Enhanced message handling and UX **Backend Infrastructure (7 files):** - Accepted upstream: emain, pkg/telemetry, pkg/wcore, pkg/wshrpc - Updated RPC types and telemetry data structures **Frontend Fork Features (8 files):** - Preserved fork: blockframe.tsx, tabbar.tsx, layoutModel.ts - Accepted upstream: keymodel.ts, wshclientapi.ts, termwrap.ts, etc. **Deleted Files (1 file):** - Removed: frontend/app/modals/tos.tsx (deleted upstream) ## Files Changed - Configuration: 8 files - Backend: 20+ files - Frontend: 25+ files - Total staged: 135 files ## Testing Required 1. Verify AI panel functionality (reasoning, feedback, tools) 2. Test horizontal widget bar (fork feature) 3. Test pane title labels (fork feature) 4. Build verification: `npm install && npm run build:dev` 5. Backend build: `go build ./...` 6. Full test suite: `npm test` ## Known Issues⚠️ Widget bar integration may need review - upstream removed widget sidebar⚠️ Layout changes may conflict with horizontal widget positioning⚠️ React 19 compatibility should be tested thoroughly ## Rollback If issues arise, rollback available at: - Tag: fork-v0.11.6-pre-v0.12-merge - Branch: backup-pre-v0.12-merge ## Next Steps 1. Test all functionality thoroughly 2. Fix any runtime errors from merge 3. Review fork feature integration points 4. Update documentation if needed 5. Create PR to main after validation --- 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
