Skip to content

Conversation

@sawka
Copy link
Member

@sawka sawka commented Dec 12, 2025

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 12, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds 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:

  • pkg/blockcontroller/shellcontroller.go — local vs remote shell resolution, Git Bash variant and error propagation.
  • pkg/blockcontroller/blockcontroller.go — ResyncController connection-change logic and CheckConnStatus inversion for local connections.
  • pkg/util/shellutil/shellutil.go & pkg/utilds/synccache.go — Git Bash discovery, PATH heuristics, cache correctness and concurrency.
  • pkg/shellexec/shellexec.go — changed StartLocalShellProc signature; ensure all callers updated.
  • frontend/app/modals/conntypeahead.tsx & frontend/app/block/blockutil.tsx/blockframe.tsx — UI changes, new function signature, hasGitBash propagation, and local-display name logic.
  • frontend/app/store/connections-model.ts & frontend/app/store/global.ts — singleton atom initialization and RPC timeout/behavior.
  • RPC surface (pkg/wshrpc/*) and schema/settings.json — verify RPC wiring and new config key consistency.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning No pull request description was provided by the author; the description field is empty and therefore does not relate to the changeset. Add a description explaining the Git Bash support feature, how it works, and any important implementation details or testing notes for reviewers.
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding a new local connection option for Git Bash on Windows, which aligns with the extensive changes to support Git Bash detection and integration.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d0a3534 and b8ad630.

📒 Files selected for processing (1)
  • pkg/util/shellutil/shellutil.go (3 hunks)

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

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 outside registryLock (deadlock avoidance); consider logging Stop() 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 around undefined).
Given meta.connection is typically string | undefined, it’s cleaner if isLocalConnName (and isBlank) accept string | 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 use wshrpc.Command_FindGitBash instead 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:gitbash locals render null instead 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 that computeFn() runs under the lock. If computeFn can be slow (filesystem/network), consider a “singleflight”-style approach (compute once, unlock waiters) later.

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

156-167: Consider validating config.Settings.TermGitBashPath before returning it. If the configured path is stale, a quick os.Stat fallback to auto-discovery would be more resilient.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a5599e9 and d0a3534.

📒 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.go
  • pkg/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 rtInfo before accessing rtInfo["shell:integration"] prevents potential runtime errors if GetRTInfoCommand returns 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 when rtInfo is 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 in EnsureConnection is 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: Skipping CheckConnStatus for 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 hit CheckConnStatus.


365-368: Treating local connections as always-ready in CheckConnStatus is 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: connName matches 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) for connName to avoid silent mislabeling.

Now that StartLocalShellProc(..., connName string) is caller-driven, a "" / non-canonical value can silently propagate into ShellProc.ConnName and any downstream routing/UX logic that depends on it. Consider either if 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 other term:* 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.
omitempty on 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 (incl local:*).
This matches the new local-connection naming and avoids confusing prompts for Git Bash/local workflows.


599-614: Good: avoid ConnEnsure for 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; ensure isLocalConnName() is runtime-safe for unexpected values. If any call path can pass null/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 call loadGitBashPath(true) concurrently, consider a simple in-flight guard later.

frontend/app/modals/conntypeahead.tsx (6)

6-7: LGTM!

Import reorganization is clean. The ConnectionsModel import supports the new Git Bash feature, and the atoms import consolidation is appropriate.


102-102: Verify behavior consistency for local connection detection.

Using util.isBlank(connection) determines "current" status. This differs from the approach in getDisconnectItem (Line 244) which uses util.isLocalConnName(connection). The local suggestion item with value: "" should be marked current when the connection is blank (empty/null), which aligns with the logic here.

However, with the new local:gitbash variant, if a user is connected to local:gitbash, this item would show current: false since connection wouldn't be blank. This appears intentional since there's a separate Git Bash item at Line 182 that checks current: connection === "local:gitbash".


167-186: LGTM! Clean implementation of Git Bash suggestion item.

The logic correctly:

  1. Guards with hasGitBash flag
  2. Uses case-insensitive matching for the filter
  3. Creates a properly typed SuggestionConnectionItem with the local:gitbash value
  4. Correctly determines current state by comparing with connection === "local:gitbash"

The item is positioned between the local item and WSL items, which is a sensible ordering.


244-244: Good refactor to use isLocalConnName utility.

This correctly prevents showing disconnect option for local connections, including the new local:gitbash variant. The utility function (from pkg/remote/conncontroller/conncontroller.go) handles local:* prefix, local, and empty string cases.


355-355: LGTM!

Correctly reads the hasGitBashAtom from the ConnectionsModel singleton instance to determine Git Bash availability.


435-436: LGTM!

The hasGitBash parameter is correctly passed to getLocalSuggestions, completing the feature integration.

pkg/blockcontroller/shellcontroller.go (6)

14-14: LGTM!

The runtime import is needed for runtime.GOOS check in getLocalShellPath to 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, and local:* prefixes). This ensures the new local:gitbash variant is handled correctly under ConnType_Local.


619-623: LGTM! Proper error propagation.

The error returned from getLocalShellPath is 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:

  1. Prioritizes explicit term:localshellpath setting (Lines 655-658)
  2. Handles local:gitbash variant with Windows-only validation (Lines 660-675)
  3. Falls back to global settings, then auto-detection (Lines 677-681)

The error messages are descriptive and actionable, guiding users to configure term:localshellpath if 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 gitbash is currently the only variant, this is acceptable.


486-486: Function signature is correct. The StartLocalShellProc function at pkg/shellexec/shellexec.go:462 accepts connName string as its fifth parameter, matching the call with remoteName.

Comment on lines +80 to +82
func IsLocalConnName(connName string) bool {
return strings.HasPrefix(connName, "local:") || connName == "local" || connName == ""
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Suggested change
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.

Comment on lines +65 to 67
var gitBashCache = utilds.MakeSyncCache(findInstalledGitBash)

const DefaultShellPath = "/bin/bash"
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

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.

Comment on lines +57 to +67
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/"

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

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.

Comment on lines +200 to +218
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

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Find Taskfile.yml
find . -name "Taskfile.yml" -o -name "Taskfile.yaml" | head -20

Repository: wavetermdev/waveterm

Length of output: 78


🏁 Script executed:

# Get file size to understand what we're working with
wc -l ./Taskfile.yml

Repository: 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.yml

Repository: wavetermdev/waveterm

Length of output: 1443


🏁 Script executed:

# Read the build:server:windows task definition
sed -n '219,250p' ./Taskfile.yml

Repository: 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 -10

Repository: 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/*.yml

Repository: 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 -10

Repository: 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 -20

Repository: 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 -20

Repository: 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.yml

Repository: 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 -100

Repository: 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 -20

Repository: 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 -A

Repository: 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/null

Repository: 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.

Suggested change
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.

@sawka sawka merged commit fbb0c4d into main Dec 12, 2025
6 of 7 checks passed
@sawka sawka deleted the sawka/gitbash branch December 12, 2025 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants