-
Notifications
You must be signed in to change notification settings - Fork 685
windows, have a new "local" conn option for Git Bash if installed #2666
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds Git Bash support for Windows local shells and a Windows quick-dev Electron task. Changes include: new Taskfile entries for electron:winquickdev and build:backend:quickdev:windows; frontend UI and typeahead updates to surface a Git Bash local option; a ConnectionsModel exposing Git Bash path and hasGitBash atom; a new util isLocalConnName; RPC additions (findgitbash) and server/client handlers; shell utilities to discover Git Bash with a cached sync cache; local-connection handling and shell-start call sites updated to accept connection names; settings/schema fields for term:gitbashpath. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (6)
pkg/blockcontroller/blockcontroller.go (1)
84-99: Good: stop old controller outsideregistryLock(deadlock avoidance); consider loggingStop()errors.Right now
existingController.Stop(...)drops the returned error; at least logging it would help diagnose stuck/failed shutdowns.frontend/util/util.ts (1)
11-20: Widen input types to match real call sites (avoid TS friction aroundundefined).
Givenmeta.connectionis typicallystring | undefined, it’s cleaner ifisLocalConnName(andisBlank) acceptstring | null | undefined.-function isBlank(str: string): boolean { +function isBlank(str?: string | null): boolean { return str == null || str == ""; } -function isLocalConnName(connName: string): boolean { +function isLocalConnName(connName?: string | null): boolean { if (isBlank(connName)) { return true; } return connName === "local" || connName.startsWith("local:"); }Also applies to: 519-520
pkg/wshrpc/wshrpctypes.go (1)
132-133: Nice: central command constant added. Consider switching call sites to usewshrpc.Command_FindGitBashinstead of repeating"findgitbash".frontend/app/block/blockutil.tsx (1)
169-251: Consider always rendering a label for local connections (avoid blank name region). Right now, non-local:gitbashlocals rendernullinstead of something like “Local Machine”, which can look like missing UI text.
Suggested tweak:- } else { - titleText = "Connected to Local Machine"; - } + } else { + titleText = "Connected to Local Machine"; + connDisplayName = "Local Machine"; + }pkg/utilds/synccache.go (1)
8-33: Thread-safe and simple; note thatcomputeFn()runs under the lock. IfcomputeFncan be slow (filesystem/network), consider a “singleflight”-style approach (compute once, unlock waiters) later.pkg/util/shellutil/shellutil.go (1)
156-167: Consider validatingconfig.Settings.TermGitBashPathbefore returning it. If the configured path is stale, a quickos.Statfallback to auto-discovery would be more resilient.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
Taskfile.yml(2 hunks)frontend/app/block/blockframe.tsx(2 hunks)frontend/app/block/blockutil.tsx(3 hunks)frontend/app/modals/conntypeahead.tsx(7 hunks)frontend/app/store/connections-model.ts(1 hunks)frontend/app/store/global.ts(2 hunks)frontend/app/store/wshclientapi.ts(1 hunks)frontend/app/view/term/termwrap.ts(1 hunks)frontend/types/gotypes.d.ts(1 hunks)frontend/util/util.ts(2 hunks)pkg/blockcontroller/blockcontroller.go(4 hunks)pkg/blockcontroller/shellcontroller.go(6 hunks)pkg/remote/conncontroller/conncontroller.go(2 hunks)pkg/shellexec/shellexec.go(2 hunks)pkg/util/shellutil/shellutil.go(3 hunks)pkg/utilds/synccache.go(1 hunks)pkg/wconfig/metaconsts.go(1 hunks)pkg/wconfig/settingsconfig.go(1 hunks)pkg/wshrpc/wshclient/wshclient.go(1 hunks)pkg/wshrpc/wshrpctypes.go(2 hunks)pkg/wshrpc/wshserver/wshserver.go(4 hunks)schema/settings.json(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-17T18:58:09.756Z
Learnt from: sawka
Repo: wavetermdev/waveterm PR: 2444
File: frontend/app/view/term/termwrap.ts:0-0
Timestamp: 2025-10-17T18:58:09.756Z
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.
Applied to files:
frontend/app/view/term/termwrap.ts
📚 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.gopkg/wshrpc/wshclient/wshclient.go
🧬 Code graph analysis (9)
frontend/app/view/term/termwrap.ts (2)
frontend/app/store/global.ts (1)
globalStore(865-865)frontend/app/store/jotaiStore.ts (1)
globalStore(3-3)
pkg/wshrpc/wshclient/wshclient.go (2)
frontend/app/store/wshclientapi.ts (1)
FindGitBashCommand(286-288)pkg/wshutil/wshrpc.go (1)
WshRpc(47-61)
pkg/wshrpc/wshrpctypes.go (2)
frontend/app/store/wshclientapi.ts (1)
FindGitBashCommand(286-288)pkg/wshrpc/wshclient/wshclient.go (1)
FindGitBashCommand(348-351)
pkg/shellexec/shellexec.go (1)
pkg/waveobj/wtype.go (1)
TermSize(307-310)
pkg/util/shellutil/shellutil.go (2)
pkg/utilds/synccache.go (1)
MakeSyncCache(16-20)pkg/wconfig/settingsconfig.go (1)
FullConfigType(292-303)
frontend/app/store/connections-model.ts (3)
frontend/layout/lib/nodeRefMap.ts (1)
get(20-24)frontend/app/store/wshclientapi.ts (1)
RpcApi(692-692)frontend/app/store/wshrpcutil.ts (1)
TabRpcClient(37-37)
pkg/blockcontroller/blockcontroller.go (2)
pkg/waveobj/metaconsts.go (1)
MetaKey_Connection(19-19)pkg/remote/conncontroller/conncontroller.go (1)
IsLocalConnName(80-82)
frontend/app/modals/conntypeahead.tsx (1)
frontend/app/store/connections-model.ts (1)
ConnectionsModel(51-51)
pkg/blockcontroller/shellcontroller.go (5)
pkg/remote/conncontroller/conncontroller.go (1)
IsLocalConnName(80-82)pkg/shellexec/shellexec.go (1)
StartLocalShellProc(462-566)pkg/waveobj/wtype.go (1)
TermSize(307-310)pkg/waveobj/metaconsts.go (1)
MetaKey_TermLocalShellPath(109-109)pkg/util/shellutil/shellutil.go (2)
FindGitBash(156-167)DetectLocalShellPath(87-105)
⏰ 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 (34)
frontend/app/view/term/termwrap.ts (2)
529-534: LGTM! Good defensive null-safety guard.The null check for
rtInfobefore accessingrtInfo["shell:integration"]prevents potential runtime errors ifGetRTInfoCommandreturns null or undefined. This is particularly important for new connection types like Git Bash, which might have different runtime info availability during initialization.
536-537: LGTM! Proper null-safe access pattern.The ternary operator safely accesses
rtInfo["shell:lastcmd"]with a null fallback, preventing runtime errors whenrtInfois unavailable. This ensures the terminal initializes gracefully even when runtime information isn't available.pkg/remote/conncontroller/conncontroller.go (1)
839-842: Local-connection short-circuit inEnsureConnectionis consistent with “always ready” local execution.Please sanity-check any callers that relied on
EnsureConnection("")side effects (e.g., metrics/events) rather than just “can I proceed?”.pkg/blockcontroller/blockcontroller.go (3)
169-179: Connection-change handling looks correct (now logs from/to + refreshes controller reference).
212-223: SkippingCheckConnStatusfor local connections matches the new local/Git Bash flow.Verify
conncontroller.IsLocalConnName(connName)covers every local variant your UI can produce (e.g.,local:gitbash, etc.), so non-SSH locals never hitCheckConnStatus.
365-368: Treating local connections as always-ready inCheckConnStatusis consistent and keeps resync cheap.pkg/shellexec/shellexec.go (2)
565-565: ConnName propagation looks correct; ensure this is the intended “local vs gitbash” discriminator.Setting
ConnName: connNamematches the new signature and aligns with the PR’s “new local conn option” direction—just ensure the value passed is the one other layers expect for local/Git Bash selection.
462-462: Add a guard (or canonical default) forconnNameto avoid silent mislabeling.Now that
StartLocalShellProc(..., connName string)is caller-driven, a""/ non-canonical value can silently propagate intoShellProc.ConnNameand any downstream routing/UX logic that depends on it. Consider eitherif connName == "" { return nil, fmt.Errorf(...) }or normalizing via a single canonical helper/constant.pkg/wconfig/metaconsts.go (1)
43-43: Config key addition looks consistent.
Placing it alongside otherterm:*keys is sensible.frontend/types/gotypes.d.ts (1)
1085-1086: Typings update is correct.
Matches the new settings key shape and stays optional.pkg/wconfig/settingsconfig.go (1)
90-91: Backend settings field wiring looks good.
omitemptyon a string aligns with “unset/empty path” behavior.schema/settings.json (1)
101-103: Schema change is straightforward and consistent.
No extra constraints needed unless you want to enforce an absolute path format.frontend/app/block/blockframe.tsx (2)
257-259: Good: suppress “wsh not installed” UI for local connections (incllocal:*).
This matches the new local-connection naming and avoids confusing prompts for Git Bash/local workflows.
599-614: Good: avoidConnEnsurefor local connections.
This prevents sending a remote-connection lifecycle RPC for local shells like"local"/"local:*".frontend/app/store/wshclientapi.ts (1)
285-288: RPC contract is properly implemented and error handling is in place.The
"findgitbash"command is fully defined in the RPC contract (pkg/wshrpc/wshrpctypes.go:132), implemented server-side (pkg/wshrpc/wshserver/wshserver.go:838), and wrapped correctly in the frontend. The only call site (frontend/app/store/connections-model.ts:38) already handles RPC errors gracefully with a try-catch block that falls back to an empty string, so "Git Bash not available" is treated appropriately rather than surfacing a hard error.frontend/app/store/global.ts (2)
20-27: Import looks fine; ensureisLocalConnName()is runtime-safe for unexpected values. If any call path can passnull/undefined(despite TS types),isLocalConnName(conn)should guard internally.
736-777: Local conn synthetic “connected” status is consistent with the new local-conn concept. The atom initialization + caching pattern matches the existing AWS/disconnected branches.pkg/wshrpc/wshrpctypes.go (1)
278-279: RPC interface extension looks coherent (boolean rescan + string return).pkg/wshrpc/wshserver/wshserver.go (2)
645-652: Good: local-conn no-op guards prevent invalid remote operations.Also applies to: 672-679, 702-709
838-841: Server-side FindGitBash RPC wiring is clean and minimal.pkg/wshrpc/wshclient/wshclient.go (1)
347-351: Generated RPC stub looks consistent with the rest of the file.frontend/app/store/connections-model.ts (1)
10-49: Looks good as a minimal singleton state holder for Git Bash discovery. Only nit: if multiple UI paths can callloadGitBashPath(true)concurrently, consider a simple in-flight guard later.frontend/app/modals/conntypeahead.tsx (6)
6-7: LGTM!Import reorganization is clean. The
ConnectionsModelimport supports the new Git Bash feature, and theatomsimport consolidation is appropriate.
102-102: Verify behavior consistency for local connection detection.Using
util.isBlank(connection)determines "current" status. This differs from the approach ingetDisconnectItem(Line 244) which usesutil.isLocalConnName(connection). The local suggestion item withvalue: ""should be marked current when the connection is blank (empty/null), which aligns with the logic here.However, with the new
local:gitbashvariant, if a user is connected tolocal:gitbash, this item would showcurrent: falsesinceconnectionwouldn't be blank. This appears intentional since there's a separate Git Bash item at Line 182 that checkscurrent: connection === "local:gitbash".
167-186: LGTM! Clean implementation of Git Bash suggestion item.The logic correctly:
- Guards with
hasGitBashflag- Uses case-insensitive matching for the filter
- Creates a properly typed
SuggestionConnectionItemwith thelocal:gitbashvalue- Correctly determines
currentstate by comparing withconnection === "local:gitbash"The item is positioned between the local item and WSL items, which is a sensible ordering.
244-244: Good refactor to useisLocalConnNameutility.This correctly prevents showing disconnect option for local connections, including the new
local:gitbashvariant. The utility function (frompkg/remote/conncontroller/conncontroller.go) handleslocal:*prefix,local, and empty string cases.
355-355: LGTM!Correctly reads the
hasGitBashAtomfrom theConnectionsModelsingleton instance to determine Git Bash availability.
435-436: LGTM!The
hasGitBashparameter is correctly passed togetLocalSuggestions, completing the feature integration.pkg/blockcontroller/shellcontroller.go (6)
14-14: LGTM!The
runtimeimport is needed forruntime.GOOScheck ingetLocalShellPathto validate Windows-only Git Bash support.
48-50: LGTM!Clean constant definition for the Git Bash local connection variant. Using a constant instead of magic strings improves maintainability.
337-340: Good refactor for local connection detection.Using
conncontroller.IsLocalConnName(remoteName)properly identifies all local connection variants (empty string,local, andlocal:*prefixes). This ensures the newlocal:gitbashvariant is handled correctly underConnType_Local.
619-623: LGTM! Proper error propagation.The error returned from
getLocalShellPathis now correctly propagated. This ensures that errors (e.g., Git Bash not found on Windows, unsupported local variant) are surfaced to the caller rather than being silently ignored.
654-681: Well-structured Git Bash path resolution with clear error messages.The implementation correctly:
- Prioritizes explicit
term:localshellpathsetting (Lines 655-658)- Handles
local:gitbashvariant with Windows-only validation (Lines 660-675)- Falls back to global settings, then auto-detection (Lines 677-681)
The error messages are descriptive and actionable, guiding users to configure
term:localshellpathif Git Bash is not found.One minor observation: the error on Line 674 for unsupported variants could be more discoverable if it listed valid variants. However, since
gitbashis currently the only variant, this is acceptable.
486-486: Function signature is correct. TheStartLocalShellProcfunction atpkg/shellexec/shellexec.go:462acceptsconnName stringas its fifth parameter, matching the call withremoteName.
| func IsLocalConnName(connName string) bool { | ||
| return strings.HasPrefix(connName, "local:") || connName == "local" || connName == "" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a GoDoc comment for exported IsLocalConnName (possible lint failure).
+// IsLocalConnName reports whether connName represents a local (non-SSH) connection.
func IsLocalConnName(connName string) bool {
return strings.HasPrefix(connName, "local:") || connName == "local" || connName == ""
}📝 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.
| func IsLocalConnName(connName string) bool { | |
| return strings.HasPrefix(connName, "local:") || connName == "local" || connName == "" | |
| } | |
| // IsLocalConnName reports whether connName represents a local (non-SSH) connection. | |
| func IsLocalConnName(connName string) bool { | |
| return strings.HasPrefix(connName, "local:") || connName == "local" || connName == "" | |
| } |
🤖 Prompt for AI Agents
In pkg/remote/conncontroller/conncontroller.go around lines 80 to 82, the
exported function IsLocalConnName lacks a GoDoc comment which may trigger lint
failures; add a proper GoDoc comment immediately above the function starting
with "IsLocalConnName" and in one or two full sentences describe what the
function checks (e.g., that it returns true for connection names prefixed with
"local:", equal to "local", or empty), and keep it concise and idiomatic for Go
documentation.
| var gitBashCache = utilds.MakeSyncCache(findInstalledGitBash) | ||
|
|
||
| const DefaultShellPath = "/bin/bash" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid hard-coded C:\Program Files\...; use ProgramFiles env (and consider x86) for correctness. This can break on systems where Program Files isn’t on C:.
Proposed patch:
func findInstalledGitBash() (string, error) {
// Try PATH first (skip system32, and only accept if in a Git directory)
pathEnv := os.Getenv("PATH")
pathDirs := filepath.SplitList(pathEnv)
for _, dir := range pathDirs {
+ dir = strings.Trim(dir, `"`)
if hasDirPart(dir, "system32") {
continue
}
if !hasDirPart(dir, "git") {
continue
}
bashPath := filepath.Join(dir, "bash.exe")
if _, err := os.Stat(bashPath); err == nil {
return bashPath, nil
}
}
@@
- // Try C:\Program Files\Git\bin
- programFilesPath := filepath.Join("C:\\", "Program Files", "Git", "bin", "bash.exe")
- if _, err := os.Stat(programFilesPath); err == nil {
- return programFilesPath, nil
- }
+ // Try %ProgramFiles%\Git\bin (and x86 fallback)
+ for _, pf := range []string{os.Getenv("ProgramFiles"), os.Getenv("ProgramFiles(x86)")} {
+ if pf == "" {
+ continue
+ }
+ programFilesPath := filepath.Join(pf, "Git", "bin", "bash.exe")
+ if _, err := os.Stat(programFilesPath); err == nil {
+ return programFilesPath, nil
+ }
+ }
return "", nil
}Also applies to: 139-211
🤖 Prompt for AI Agents
In pkg/util/shellutil/shellutil.go at lines ~65-67 and also affecting lines
~139-211, remove any hard-coded "C:\Program Files\..." path and instead build
Windows Program Files paths from the environment: read %ProgramFiles% and, where
appropriate for 32-bit vs 64-bit considerations, fall back to
%ProgramFiles(x86)%. Validate non-empty env values and try both locations before
giving up; prefer ProgramFiles first and then ProgramFiles(x86) to locate Git
Bash binaries. Ensure path joins use filepath.Join so separators are correct
across platforms and keep the existing non-Windows default (/bin/bash) behavior
intact.
| electron:winquickdev: | ||
| desc: Run the Electron application via the Vite dev server (quick dev - Windows amd64 only, no generate, no wsh). | ||
| cmd: npm run dev | ||
| deps: | ||
| - npm:install | ||
| - build:backend:quickdev:windows | ||
| env: | ||
| WAVETERM_ENVFILE: "{{.ROOT_DIR}}/.env" | ||
| WCLOUD_ENDPOINT: "https://api-dev.waveterm.dev/central" | ||
| WCLOUD_WS_ENDPOINT: "wss://wsapi-dev.waveterm.dev/" | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gate electron:winquickdev to Windows to prevent accidental use on other platforms.
The task description explicitly states "Windows amd64 only", but it lacks a platforms: [windows] guard. Add it to match the pattern used by its dependency build:backend:quickdev:windows and other platform-specific tasks in the file.
electron:winquickdev:
desc: Run the Electron application via the Vite dev server (quick dev - Windows amd64 only, no generate, no wsh).
+ platforms: [windows]
cmd: npm run dev
deps:
- npm:install
- build:backend:quickdev:windows🤖 Prompt for AI Agents
In Taskfile.yml around lines 57 to 67, the electron:winquickdev task is flagged
as "Windows amd64 only" but lacks a platforms guard; add a platforms: [windows]
entry under the task (matching the format used by build:backend:quickdev:windows
and other platform-specific tasks) so the task is gated to Windows and cannot be
invoked on other platforms.
| build:backend:quickdev:windows: | ||
| desc: Build only the wavesrv component for quickdev (Windows amd64 only, no generate, no wsh). | ||
| platforms: [windows] | ||
| cmds: | ||
| - task: build:server:internal | ||
| vars: | ||
| ARCHS: amd64 | ||
| GO_ENV_VARS: CC="zig cc -target x86_64-windows-gnu" | ||
| deps: | ||
| - go:mod:tidy | ||
| sources: | ||
| - "cmd/server/*.go" | ||
| - "pkg/**/*.go" | ||
| - "pkg/**/*.json" | ||
| - "pkg/**/*.sh" | ||
| - "tsunami/**/*.go" | ||
| generates: | ||
| - dist/bin/wavesrv.x64.exe | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find Taskfile.yml
find . -name "Taskfile.yml" -o -name "Taskfile.yaml" | head -20Repository: wavetermdev/waveterm
Length of output: 78
🏁 Script executed:
# Get file size to understand what we're working with
wc -l ./Taskfile.ymlRepository: wavetermdev/waveterm
Length of output: 82
🏁 Script executed:
# Read lines 200-218 (build:backend:quickdev:windows)
sed -n '200,218p' ./Taskfile.yml
# Read around Line 250 (build:server:internal)
sed -n '245,260p' ./Taskfile.yml
# Find build:server:windows
grep -n "build:server:windows" ./Taskfile.ymlRepository: wavetermdev/waveterm
Length of output: 1443
🏁 Script executed:
# Read the build:server:windows task definition
sed -n '219,250p' ./Taskfile.ymlRepository: wavetermdev/waveterm
Length of output: 1741
🏁 Script executed:
# Look for any Task-related configuration files or comments about shell behavior
grep -r "shell\|cmd\.exe\|PowerShell\|bash" ./Taskfile.yml | head -20
# Check for any .taskfiles or includes
ls -la | grep -i task
# Look for any GitHub Actions or CI configuration that might reveal how Task is used on Windows
find . -name "*.yml" -o -name "*.yaml" | grep -E "(workflow|action|ci|github)" | head -10Repository: wavetermdev/waveterm
Length of output: 2122
🏁 Script executed:
# Check GitHub workflows for Windows builds
ls -la ./.github/workflows/
# Look for Windows-specific build workflows
grep -l "windows\|Windows" ./.github/workflows/*.ymlRepository: wavetermdev/waveterm
Length of output: 945
🏁 Script executed:
# Check build-helper.yml for Windows task execution
cat -n ./.github/workflows/build-helper.yml | grep -A 10 -B 10 "windows\|Windows\|task"Repository: wavetermdev/waveterm
Length of output: 9708
🏁 Script executed:
# Search for any issues or comments related to GO_ENV_VARS or Windows build issues
grep -n "GO_ENV_VARS\|shell\|zig cc" ./Taskfile.yml | head -30
# Look for any recent changes or git history comments
git log --oneline -20 2>/dev/null | head -10Repository: wavetermdev/waveterm
Length of output: 2605
🏁 Script executed:
# Search for Task documentation or any comments about shell behavior
grep -rn "task\|Task\|shell" . --include="*.md" --include="*.txt" 2>/dev/null | grep -i "windows\|shell\|bash" | head -20
# Check README for build instructions
head -100 ./README.md 2>/dev/null || echo "No README"
# Look for any task-related documentation
find . -name "*[Tt]ask*" -type f 2>/dev/null | grep -v node_modules | grep -v ".git"Repository: wavetermdev/waveterm
Length of output: 6905
🏁 Script executed:
# Check BUILD.md for build instructions
cat ./BUILD.md | head -150
# Look for any Task-related information in the repo
grep -rn "sh:" ./Taskfile.yml | head -20Repository: wavetermdev/waveterm
Length of output: 5129
🏁 Script executed:
# Search for any usage of build:backend:quickdev:windows
grep -rn "build:backend:quickdev:windows\|quickdev" . --include="*.yml" --include="*.yaml" --include="*.md" 2>/dev/null
# Look for any issue reports or comments about Windows builds
grep -rn "windows\|Windows" ./.github/workflows/ | grep -i "task\|build" | head -20Repository: wavetermdev/waveterm
Length of output: 3279
🏁 Script executed:
# Search for any GitHub issues or error reports related to Windows build
git log --all --grep="windows\|Windows\|GO_ENV_VARS" --oneline 2>/dev/null | head -20
# Check if there are any recent changes to the Taskfile
git log --oneline -30 -- Taskfile.yml 2>/dev/null
# Look for any comments or TODOs in the Taskfile
grep -n "TODO\|FIXME\|BUG\|HACK\|NOTE" ./Taskfile.ymlRepository: wavetermdev/waveterm
Length of output: 1370
🏁 Script executed:
# Check commit e6b4e36 for the Windows build error fix
git show e6b4e36 --stat 2>/dev/null | head -50
# Also check commit 79210f0
git show 79210f0 2>/dev/null | head -100Repository: wavetermdev/waveterm
Length of output: 1226
🏁 Script executed:
# Look for any GitHub issues mentioning the build task or quickdev
find ./.github -name "*.md" -o -name "*.txt" | xargs grep -l "quickdev\|GO_ENV_VARS" 2>/dev/null
# Check if there are any test workflows
ls -la ./.github/workflows/
# Look for any recent issues or PRs
git log --all --oneline --grep="quickdev\|GO_ENV_VARS\|backend:quickdev" 2>/dev/null | head -20Repository: wavetermdev/waveterm
Length of output: 792
🏁 Script executed:
# Get the exact structure of build:server:internal more carefully
sed -n '245,260p' ./Taskfile.yml | cat -ARepository: wavetermdev/waveterm
Length of output: 773
🏁 Script executed:
# Search for electron:winquickdev usage in workflows or documentation
grep -rn "electron:winquickdev\|build:backend:quickdev:windows" ./.github/ ./README.md ./BUILD.md 2>/dev/nullRepository: wavetermdev/waveterm
Length of output: 46
Use sh: echo for GO_ENV_VARS to ensure consistent Windows shell behavior.
build:backend:quickdev:windows hard-codes GO_ENV_VARS: CC="zig cc -target x86_64-windows-gnu", while build:server:windows (line 219+) uses sh: echo to compute it. On Windows, the hard-coded approach may fail because the cmd: field in build:server:internal (line 250) executes POSIX-style environment variable syntax (CGO_ENABLED=1 GOARCH={{.GOARCH}} {{.GO_ENV_VARS}}...), which cmd.exe/PowerShell don't reliably handle without explicit shell execution. The sh: echo pattern already proven in build:server:windows should be applied here for consistency:
build:backend:quickdev:windows:
desc: Build only the wavesrv component for quickdev (Windows amd64 only, no generate, no wsh).
platforms: [windows]
cmds:
- task: build:server:internal
vars:
ARCHS: amd64
- GO_ENV_VARS: CC="zig cc -target x86_64-windows-gnu"
+ GO_ENV_VARS:
+ sh: echo 'CC="zig cc -target x86_64-windows-gnu"'📝 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.
| build:backend:quickdev:windows: | |
| desc: Build only the wavesrv component for quickdev (Windows amd64 only, no generate, no wsh). | |
| platforms: [windows] | |
| cmds: | |
| - task: build:server:internal | |
| vars: | |
| ARCHS: amd64 | |
| GO_ENV_VARS: CC="zig cc -target x86_64-windows-gnu" | |
| deps: | |
| - go:mod:tidy | |
| sources: | |
| - "cmd/server/*.go" | |
| - "pkg/**/*.go" | |
| - "pkg/**/*.json" | |
| - "pkg/**/*.sh" | |
| - "tsunami/**/*.go" | |
| generates: | |
| - dist/bin/wavesrv.x64.exe | |
| build:backend:quickdev:windows: | |
| desc: Build only the wavesrv component for quickdev (Windows amd64 only, no generate, no wsh). | |
| platforms: [windows] | |
| cmds: | |
| - task: build:server:internal | |
| vars: | |
| ARCHS: amd64 | |
| GO_ENV_VARS: | |
| sh: echo 'CC="zig cc -target x86_64-windows-gnu"' | |
| deps: | |
| - go:mod:tidy | |
| sources: | |
| - "cmd/server/*.go" | |
| - "pkg/**/*.go" | |
| - "pkg/**/*.json" | |
| - "pkg/**/*.sh" | |
| - "tsunami/**/*.go" | |
| generates: | |
| - dist/bin/wavesrv.x64.exe |
🤖 Prompt for AI Agents
In Taskfile.yml around lines 200 to 218, the GO_ENV_VARS for
build:backend:quickdev:windows is hard-coded which can break on Windows shells;
change the vars entry to use the same sh: echo pattern used in
build:server:windows so the environment string is emitted by the shell
(preserving the CC="zig cc -target x86_64-windows-gnu" value and any quoting)
instead of embedding it directly, ensuring the downstream cmd: that sets
CGO_ENABLED/GOARCH can consume a POSIX-style env string reliably on Windows.
No description provided.