From 53b26a6cc83950796bc1439e6e717b5fb55ed9d6 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Mon, 6 Jul 2026 01:22:29 -0400 Subject: [PATCH] merge: rebase onto main, combine Windows shell guidance with #476's MSYS text #476 merged into main and rewrote the same "Shell syntax: Windows cmd.exe..." guidance string in system_prompt.go and shellGuidanceForGOOS in shell_runtime.go that this branch also edits (this PR's fix for the broken single-quoted quoting examples). Combined both into one coherent string in each spot: the double-quote/single-quote metachar rule with correct examples, plus #476's MSYS coreutils warning and require_escalated mention. Merged the corresponding test assertions in loop_test.go the same way. internal/imageinput/clipboard.go, internal/tools/bash_tool_test.go, and internal/tools/exec_command_test.go applied cleanly with no overlap. --- internal/agent/loop_test.go | 13 ++++++++++++- internal/agent/system_prompt.go | 2 +- internal/imageinput/clipboard.go | 8 +++++++- internal/tools/bash_tool_test.go | 3 +++ internal/tools/exec_command_test.go | 9 +++++---- internal/tools/shell_runtime.go | 2 +- 6 files changed, 29 insertions(+), 8 deletions(-) diff --git a/internal/agent/loop_test.go b/internal/agent/loop_test.go index 06be5328..5555420a 100644 --- a/internal/agent/loop_test.go +++ b/internal/agent/loop_test.go @@ -2913,11 +2913,22 @@ func TestBuildSystemPromptInjectsHostShellContext(t *testing.T) { t.Fatalf("expected operating system in environment block, got %q", prompt) } if runtime.GOOS == "windows" { - for _, want := range []string{"Windows cmd.exe syntax", "cwd argument", "MSYS binaries", "grep", "require_escalated"} { + for _, want := range []string{"Windows cmd.exe syntax", "cwd argument", "MSYS binaries", "grep", "require_escalated", "use double quotes around the value"} { if !strings.Contains(prompt, want) { t.Fatalf("expected Windows shell guidance to mention %q, got %q", want, prompt) } } + // The examples must themselves use the safe (double-quoted) form; a + // single-quoted example here would teach the model the exact syntax + // that fails under cmd.exe. + for _, want := range []string{`--jq ".a | b"`, `-run "A|B"`} { + if !strings.Contains(prompt, want) { + t.Fatalf("expected double-quoted example %q in Windows shell guidance, got %q", want, prompt) + } + } + if strings.Contains(prompt, `'.a | b'`) || strings.Contains(prompt, `'A|B'`) { + t.Fatalf("Windows shell guidance must not show the unsafe single-quoted form, got %q", prompt) + } } else if !strings.Contains(prompt, "/bin/sh syntax") { t.Fatalf("expected POSIX shell guidance in prompt, got %q", prompt) } diff --git a/internal/agent/system_prompt.go b/internal/agent/system_prompt.go index 46c51b62..1d7118ed 100644 --- a/internal/agent/system_prompt.go +++ b/internal/agent/system_prompt.go @@ -286,7 +286,7 @@ func workspaceContext(cwd string) string { b.WriteString("Working directory: " + cwd + "\n") b.WriteString("Operating system: " + runtime.GOOS + "\n") if runtime.GOOS == "windows" { - b.WriteString("Shell syntax: Windows cmd.exe syntax for exec_command/bash tools; prefer the workdir/cwd argument instead of cd when changing directories. Do not pipe to or invoke POSIX coreutils from Git for Windows (usr\\bin head/grep/tail/cat/...): they are MSYS binaries and fail under the write-restricted sandbox; use native Zero tools (grep, read_file, list_directory, glob) or cmd.exe findstr/more instead, or sandbox_permissions require_escalated only when host-level execution is truly required.\n") + b.WriteString("Shell syntax: Windows cmd.exe syntax for exec_command/bash tools. To put | & > < etc inside an arg value, use double quotes around the value, not single quotes (single quotes do not protect metachars in cmd.exe): gh --jq \".a | b\", go test -run \"A|B\". Do not pipe to or invoke POSIX coreutils from Git for Windows (usr\\bin head/grep/tail/cat/...): they are MSYS binaries and fail under the write-restricted sandbox; use native Zero tools (grep, read_file, list_directory, glob) or cmd.exe findstr/more instead, or sandbox_permissions require_escalated only when host-level execution is truly required. Prefer the workdir/cwd argument over cd when changing directories.\n") } else { b.WriteString("Shell syntax: /bin/sh syntax for exec_command/bash tools; prefer the workdir/cwd argument instead of cd when changing directories.\n") } diff --git a/internal/imageinput/clipboard.go b/internal/imageinput/clipboard.go index 40db5ef3..eb092eb6 100644 --- a/internal/imageinput/clipboard.go +++ b/internal/imageinput/clipboard.go @@ -75,7 +75,13 @@ func readClipboardImageWindows() ([]byte, error) { tmpPath := tmpFile.Name() tmpFile.Close() defer os.Remove(tmpPath) - script := `Add-Type -AssemblyName System.Windows.Forms; Add-Type -AssemblyName System.Drawing; $img = [System.Windows.Forms.Clipboard]::GetImage(); if ($img -ne $null) { $img.Save('` + tmpPath + `', [System.Drawing.Imaging.ImageFormat]::Png) }` + + // Stay on -Command: -File is subject to script execution policy (default + // Restricted on Windows clients) and Windows PowerShell decodes BOM-less + // UTF-8 .ps1 files as ANSI. Doubling single quotes is all the escaping the + // single-quoted PowerShell literal needs. + escapedTmpPath := strings.ReplaceAll(tmpPath, "'", "''") + script := `Add-Type -AssemblyName System.Windows.Forms; Add-Type -AssemblyName System.Drawing; $img = [System.Windows.Forms.Clipboard]::GetImage(); if ($img -ne $null) { $img.Save('` + escapedTmpPath + `', [System.Drawing.Imaging.ImageFormat]::Png) }` cmd := exec.Command("powershell", "-NoProfile", "-Command", script) if err := cmd.Run(); err != nil { return nil, nil diff --git a/internal/tools/bash_tool_test.go b/internal/tools/bash_tool_test.go index 5fa7dfe4..44a81a67 100644 --- a/internal/tools/bash_tool_test.go +++ b/internal/tools/bash_tool_test.go @@ -126,6 +126,9 @@ func TestBashToolDescribesHostShellSyntax(t *testing.T) { if !strings.Contains(description, "cmd.exe") || !strings.Contains(description, "cwd") { t.Fatalf("expected Windows cmd.exe and cwd guidance in bash description, got %q", description) } + if !strings.Contains(description, "double quotes") || !strings.Contains(description, `--jq ".a | b"`) { + t.Fatalf("expected the double-quote metacharacter rule in bash description, got %q", description) + } return } if !strings.Contains(description, "/bin/sh") { diff --git a/internal/tools/exec_command_test.go b/internal/tools/exec_command_test.go index 92728526..3bbe81ef 100644 --- a/internal/tools/exec_command_test.go +++ b/internal/tools/exec_command_test.go @@ -541,10 +541,11 @@ func TestCollectRespectsDeadlineUnderContinuousOutput(t *testing.T) { elapsed := time.Since(start) // Generous slack over `wait` for scheduling jitter under a continuously - // writing goroutine — this must stay a small multiple of wait, not - // "however long the writer keeps going" (which is what the bug produced: - // this test would hang past the 30s test timeout without the fix). - if elapsed > 3*wait { + // writing goroutine (worse on Windows CI under load). This must stay a + // small multiple of wait, not "however long the writer keeps going" + // (which is what the bug produced: this test would hang past the 30s + // test timeout without the fix). + if elapsed > 5*wait { t.Fatalf("collect took %v under continuous output, want close to the %v deadline", elapsed, wait) } } diff --git a/internal/tools/shell_runtime.go b/internal/tools/shell_runtime.go index 72f9c7cc..4db6b2b7 100644 --- a/internal/tools/shell_runtime.go +++ b/internal/tools/shell_runtime.go @@ -55,7 +55,7 @@ func detectShellRuntime(goos string) shellRuntime { func shellGuidanceForGOOS(goos string) string { runtime := detectShellRuntime(goos) if goos == "windows" { - return "Uses " + runtime.Syntax + " syntax on Windows; prefer cwd over cd when changing directories. MSYS/Cygwin coreutils on PATH (Git for Windows usr\\bin) are not sandbox-compatible; prefer native Zero file tools." + return "Uses " + runtime.Syntax + " syntax on Windows; prefer cwd over cd when changing directories. To include | & > < or other metacharacters in an argument value, wrap the value in double quotes (e.g. --jq \".a | b\"); single quotes do not protect metacharacters in cmd.exe. MSYS/Cygwin coreutils on PATH (Git for Windows usr\\bin) are not sandbox-compatible; prefer native Zero file tools." } guidance := "Uses " + runtime.Syntax + " syntax." if goos == "darwin" {