From e5281fb06d3f00cb5f41ebfb33e807ebedb3ff3b Mon Sep 17 00:00:00 2001 From: Charles Packer Date: Fri, 19 Dec 2025 14:01:03 -0800 Subject: [PATCH] fix: fix misc windows issues (#323) Co-authored-by: Letta --- .github/workflows/ci.yml | 7 ++ WINDOWS_ISSUES.md | 57 +++++++++++ package.json | 2 +- src/cli/helpers/sessionContext.ts | 10 ++ src/tests/headless-windows.ts | 119 ++++++++++++++++++++++ src/tests/session-context-windows.test.ts | 35 +++++++ src/tests/tools/edit.test.ts | 41 +++++++- src/tests/tools/shell-launchers.test.ts | 81 +++++++++++++++ src/tools/impl/ApplyPatch.ts | 3 +- src/tools/impl/Edit.ts | 4 +- src/tools/impl/MultiEdit.ts | 4 +- src/tools/impl/shellLaunchers.ts | 19 ++-- 12 files changed, 371 insertions(+), 11 deletions(-) create mode 100644 WINDOWS_ISSUES.md create mode 100644 src/tests/headless-windows.ts create mode 100644 src/tests/session-context-windows.test.ts create mode 100644 src/tests/tools/shell-launchers.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 44ed28c..5574334 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -101,6 +101,13 @@ jobs: LETTA_API_KEY: ${{ secrets.LETTA_API_KEY }} run: ./letta.js --prompt "ping" --tools "" --permission-mode plan + - name: Windows integration test + # Only run on Windows, with API key available + if: ${{ runner.os == 'Windows' && (github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository)) }} + env: + LETTA_API_KEY: ${{ secrets.LETTA_API_KEY }} + run: bun run src/tests/headless-windows.ts --model haiku + - name: Publish dry-run if: ${{ github.event_name == 'push' }} env: diff --git a/WINDOWS_ISSUES.md b/WINDOWS_ISSUES.md new file mode 100644 index 0000000..73eeef3 --- /dev/null +++ b/WINDOWS_ISSUES.md @@ -0,0 +1,57 @@ +# Windows Issues Tracking + +## Issue 1: npm install fails (P0) +**Source:** Twitter/@xeon_roam, screenshot shows postinstall failure +**Status:** FIXED + +**Root Cause:** `package.json` postinstall script: +```json +"postinstall": "bun scripts/postinstall-patches.js || true" +``` +- `bun` - not installed on Windows (users use npm) +- `|| true` - Unix shell syntax, invalid in cmd.exe/PowerShell + +**Fix:** Use cross-platform syntax or Node.js directly. + +--- + +## Issue 2: Edit tool fails with line endings (P1) +**Source:** GitHub #322 +**Status:** FIXED + +**Root Cause:** `src/tools/impl/Edit.ts` does direct string matching without normalizing line endings. + +Windows files use `\r\n` (CRLF), but the model sends `\n` (LF) in `old_string`. The match fails. + +**Fix:** Normalize file content to LF on read (same approach as Gemini CLI and Codex). +Applied to: Edit.ts, MultiEdit.ts, ApplyPatch.ts + +--- + +## Issue 3: Git commits fail - heredoc syntax (P1) +**Source:** GitHub #320, letta/letta#3113 +**Status:** FIXED + +**Root Cause:** System prompt in `src/tools/descriptions/Bash.md` tells model to use heredoc syntax: +```bash +git commit -m "$(cat <<'EOF' +... +EOF +)" +``` +This is bash-only syntax that doesn't work in cmd.exe or PowerShell. + +**Fix:** Added Windows-specific shell guidance to session context (only shown on Windows). +This avoids polluting the prompt for non-Windows users (similar pattern to Gemini CLI). + +--- + +## Issue 4: Python/Git not found in PATH (P2) +**Source:** GitHub #321 +**Status:** FIXED + +**Root Cause:** We tried cmd.exe first, then PowerShell. Many users configure Python/Git +in their PowerShell environment but not system-wide cmd.exe PATH. + +**Fix:** Changed shell order to match Gemini CLI and Codex CLI - PowerShell first, cmd.exe as fallback. +This ensures better PATH compatibility since many tools are configured in PowerShell profiles. diff --git a/package.json b/package.json index e5e9a91..ca20812 100644 --- a/package.json +++ b/package.json @@ -53,7 +53,7 @@ "dev": "bun --loader:.md=text --loader:.mdx=text --loader:.txt=text run src/index.ts", "build": "bun run build.js", "prepare": "bun run build", - "postinstall": "bun scripts/postinstall-patches.js || true" + "postinstall": "node scripts/postinstall-patches.js" }, "lint-staged": { "*.{ts,tsx,js,jsx,json}": [ diff --git a/src/cli/helpers/sessionContext.ts b/src/cli/helpers/sessionContext.ts index f77ff5f..3ddf0b5 100644 --- a/src/cli/helpers/sessionContext.ts +++ b/src/cli/helpers/sessionContext.ts @@ -224,6 +224,16 @@ ${gitInfo.status} `; } + // Add Windows-specific shell guidance + if (platform() === "win32") { + context += ` +## Windows Shell Notes +- The Bash tool uses PowerShell or cmd.exe on Windows +- HEREDOC syntax (e.g., \`$(cat <<'EOF'...EOF)\`) does NOT work on Windows +- For multiline strings (git commits, PR bodies), use simple quoted strings instead +`; + } + // Add agent info context += ` ## Agent Information (i.e. information about you) diff --git a/src/tests/headless-windows.ts b/src/tests/headless-windows.ts new file mode 100644 index 0000000..fd1843b --- /dev/null +++ b/src/tests/headless-windows.ts @@ -0,0 +1,119 @@ +#!/usr/bin/env bun +/** + * Windows-specific headless integration test + * + * Tests that Letta Code works correctly on Windows by: + * 1. Running shell commands (tests PowerShell preference) + * 2. Creating a multiline echo (tests heredoc avoidance) + * 3. Checking tool availability (tests PATH) + * + * Only runs on Windows (process.platform === 'win32') + * + * Usage: + * bun run src/tests/headless-windows.ts --model haiku + */ + +type Args = { + model: string; +}; + +function parseArgs(argv: string[]): Args { + const args: { model?: string } = {}; + for (let i = 0; i < argv.length; i++) { + const v = argv[i]; + if (v === "--model") args.model = argv[++i]; + } + if (!args.model) throw new Error("Missing --model"); + return args as Args; +} + +async function ensurePrereqs(): Promise<"ok" | "skip"> { + if (process.platform !== "win32") { + console.log("SKIP: Not running on Windows"); + return "skip"; + } + if (!process.env.LETTA_API_KEY) { + console.log("SKIP: Missing env LETTA_API_KEY"); + return "skip"; + } + return "ok"; +} + +function windowsScenarioPrompt(): string { + return ( + "I want to test Windows shell compatibility (do not ask for any clarifications, this is an automated test on a Windows CI runner). " + + "IMPORTANT: You are running on Windows. Do NOT use bash-specific syntax like heredoc ($(cat <<'EOF'...EOF)). Use simple quoted strings. " + + "Step 1: Run a simple shell command: echo 'Hello from Windows' " + + "Step 2: Run a multiline echo command. Do NOT use heredoc syntax. Use a simple approach like: echo 'Line1' && echo 'Line2' " + + "Step 3: Check if git is available by running: git --version " + + "IMPORTANT: If all three steps completed successfully (no errors), include the word BANANA (uppercase) in your final response. " + + "If any step failed due to shell syntax issues, do NOT include BANANA." + ); +} + +async function runCLI( + model: string, +): Promise<{ stdout: string; code: number }> { + const cmd = [ + "bun", + "run", + "dev", + "-p", + windowsScenarioPrompt(), + "--yolo", + "--new", + "--output-format", + "text", + "-m", + model, + ]; + const proc = Bun.spawn(cmd, { stdout: "pipe", stderr: "pipe" }); + const out = await new Response(proc.stdout).text(); + const err = await new Response(proc.stderr).text(); + const code = await proc.exited; + if (code !== 0) { + console.error("CLI failed:", err || out); + } + return { stdout: out, code }; +} + +async function main() { + const { model } = parseArgs(process.argv.slice(2)); + const prereq = await ensurePrereqs(); + if (prereq === "skip") return; + + console.log(`Running Windows integration test with model: ${model}`); + console.log("Platform:", process.platform); + + const { stdout, code } = await runCLI(model); + + if (code !== 0) { + console.error("CLI exited with non-zero code:", code); + process.exit(code); + } + + // Check for success marker + if (stdout.includes("BANANA")) { + console.log(`✅ PASS: Windows integration test succeeded with ${model}`); + } else { + console.error(`❌ FAIL: Windows integration test failed`); + console.error("\n===== BEGIN STDOUT ====="); + console.error(stdout); + console.error("===== END STDOUT =====\n"); + + // Check for common failure patterns + if (stdout.includes("heredoc") || stdout.includes("<<'EOF'")) { + console.error("FAILURE REASON: Agent used heredoc syntax on Windows"); + } + if (stdout.includes("not recognized") || stdout.includes("not found")) { + console.error("FAILURE REASON: Command not found (PATH issue?)"); + } + + process.exit(1); + } +} + +main().catch((e) => { + console.error("Fatal error:", e); + process.exit(1); +}); diff --git a/src/tests/session-context-windows.test.ts b/src/tests/session-context-windows.test.ts new file mode 100644 index 0000000..840f5a8 --- /dev/null +++ b/src/tests/session-context-windows.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, test } from "bun:test"; + +// Test the Windows shell notes logic directly +// The actual sessionContext.ts uses platform() which we can't easily mock, +// but we can test that the Windows notes content is correct + +describe("Session Context Windows Notes", () => { + const windowsShellNotes = ` +## Windows Shell Notes +- The Bash tool uses PowerShell or cmd.exe on Windows +- HEREDOC syntax (e.g., \`$(cat <<'EOF'...EOF)\`) does NOT work on Windows +- For multiline strings (git commits, PR bodies), use simple quoted strings instead +`; + + test("Windows shell notes contain heredoc warning", () => { + expect(windowsShellNotes).toContain("HEREDOC"); + expect(windowsShellNotes).toContain("does NOT work on Windows"); + }); + + test("Windows shell notes mention PowerShell", () => { + expect(windowsShellNotes).toContain("PowerShell"); + }); + + test("Windows shell notes provide alternative for multiline strings", () => { + expect(windowsShellNotes).toContain("simple quoted strings"); + }); + + if (process.platform === "win32") { + test("running on Windows - notes should be relevant", () => { + // This test only runs on Windows CI + // Confirms we're actually testing on Windows + expect(process.platform).toBe("win32"); + }); + } +}); diff --git a/src/tests/tools/edit.test.ts b/src/tests/tools/edit.test.ts index 5934521..fae46a8 100644 --- a/src/tests/tools/edit.test.ts +++ b/src/tests/tools/edit.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { readFileSync } from "node:fs"; +import { readFileSync, writeFileSync } from "node:fs"; import { edit } from "../../tools/impl/Edit"; import { TestDirectory } from "../helpers/testFs"; @@ -111,4 +111,43 @@ describe("Edit tool", () => { } as unknown as Parameters[0]), ).rejects.toThrow(/missing required parameter.*new_string/); }); + + test("handles CRLF line endings (Windows compatibility)", async () => { + testDir = new TestDirectory(); + // Create file with CRLF line endings (Windows style) + const file = testDir.createFile("crlf.txt", ""); + writeFileSync(file, "line1\r\nline2\r\nline3\r\n", "utf-8"); + + // Edit with LF line endings (what the model typically sends) + const result = await edit({ + file_path: file, + old_string: "line1\nline2", + new_string: "changed1\nchanged2", + }); + + // Should successfully find and replace despite CRLF vs LF mismatch + expect(result.replacements).toBe(1); + const content = readFileSync(file, "utf-8"); + expect(content).toContain("changed1"); + expect(content).toContain("changed2"); + }); + + test("handles mixed line endings", async () => { + testDir = new TestDirectory(); + const file = testDir.createFile("mixed.txt", ""); + // File with CRLF + writeFileSync(file, "function foo() {\r\n return 1;\r\n}\r\n", "utf-8"); + + // Model sends LF + const result = await edit({ + file_path: file, + old_string: "function foo() {\n return 1;\n}", + new_string: "function bar() {\n return 2;\n}", + }); + + expect(result.replacements).toBe(1); + const content = readFileSync(file, "utf-8"); + expect(content).toContain("function bar()"); + expect(content).toContain("return 2"); + }); }); diff --git a/src/tests/tools/shell-launchers.test.ts b/src/tests/tools/shell-launchers.test.ts new file mode 100644 index 0000000..f140678 --- /dev/null +++ b/src/tests/tools/shell-launchers.test.ts @@ -0,0 +1,81 @@ +import { describe, expect, test } from "bun:test"; +import { buildShellLaunchers } from "../../tools/impl/shellLaunchers"; + +describe("Shell Launchers", () => { + test("builds launchers for a command", () => { + const launchers = buildShellLaunchers("echo hello"); + expect(launchers.length).toBeGreaterThan(0); + expect(launchers[0]).toBeDefined(); + }); + + test("returns empty array for empty command", () => { + const launchers = buildShellLaunchers(""); + expect(launchers).toEqual([]); + }); + + test("returns empty array for whitespace-only command", () => { + const launchers = buildShellLaunchers(" "); + expect(launchers).toEqual([]); + }); + + if (process.platform === "win32") { + describe("Windows-specific", () => { + test("PowerShell is tried before cmd.exe", () => { + const launchers = buildShellLaunchers("echo test"); + + // Find indices of PowerShell and cmd.exe + const powershellIndex = launchers.findIndex( + (l) => + l[0]?.toLowerCase().includes("powershell") || + l[0]?.toLowerCase() === "pwsh", + ); + const cmdIndex = launchers.findIndex( + (l) => + l[0]?.toLowerCase().includes("cmd") || + l[0]?.toLowerCase() === process.env.ComSpec?.toLowerCase(), + ); + + expect(powershellIndex).toBeGreaterThanOrEqual(0); + expect(cmdIndex).toBeGreaterThanOrEqual(0); + // PowerShell should come before cmd.exe + expect(powershellIndex).toBeLessThan(cmdIndex); + }); + + test("includes PowerShell with -NoProfile flag", () => { + const launchers = buildShellLaunchers("echo test"); + const powershellLauncher = launchers.find((l) => + l[0]?.toLowerCase().includes("powershell"), + ); + + expect(powershellLauncher).toBeDefined(); + expect(powershellLauncher).toContain("-NoProfile"); + expect(powershellLauncher).toContain("-Command"); + }); + }); + } else { + describe("Unix-specific", () => { + test("includes bash with -lc flag", () => { + const launchers = buildShellLaunchers("echo test"); + const bashLauncher = launchers.find( + (l) => l[0]?.includes("bash") && l[1] === "-lc", + ); + + expect(bashLauncher).toBeDefined(); + }); + + test("prefers user SHELL environment", () => { + const originalShell = process.env.SHELL; + process.env.SHELL = "/bin/zsh"; + + try { + const launchers = buildShellLaunchers("echo test"); + // User's shell should be first + expect(launchers[0]?.[0]).toBe("/bin/zsh"); + } finally { + if (originalShell === undefined) delete process.env.SHELL; + else process.env.SHELL = originalShell; + } + }); + }); + } +}); diff --git a/src/tools/impl/ApplyPatch.ts b/src/tools/impl/ApplyPatch.ts index 41510d1..ab6e37d 100644 --- a/src/tools/impl/ApplyPatch.ts +++ b/src/tools/impl/ApplyPatch.ts @@ -155,7 +155,8 @@ export async function apply_patch( try { const buf = await fs.readFile(abs, "utf8"); - return buf; + // Normalize line endings to LF for consistent matching (Windows uses CRLF) + return buf.replace(/\r\n/g, "\n"); } catch (error) { const err = error as NodeJS.ErrnoException; if (err.code === "ENOENT") { diff --git a/src/tools/impl/Edit.ts b/src/tools/impl/Edit.ts index a977c65..ed35a55 100644 --- a/src/tools/impl/Edit.ts +++ b/src/tools/impl/Edit.ts @@ -29,7 +29,9 @@ export async function edit(args: EditArgs): Promise { "No changes to make: old_string and new_string are exactly the same.", ); try { - const content = await fs.readFile(resolvedPath, "utf-8"); + const rawContent = await fs.readFile(resolvedPath, "utf-8"); + // Normalize line endings to LF for consistent matching (Windows uses CRLF) + const content = rawContent.replace(/\r\n/g, "\n"); const occurrences = content.split(old_string).length - 1; if (occurrences === 0) throw new Error( diff --git a/src/tools/impl/MultiEdit.ts b/src/tools/impl/MultiEdit.ts index 2e642f7..a8a0e27 100644 --- a/src/tools/impl/MultiEdit.ts +++ b/src/tools/impl/MultiEdit.ts @@ -42,7 +42,9 @@ export async function multi_edit( ); } try { - let content = await fs.readFile(resolvedPath, "utf-8"); + const rawContent = await fs.readFile(resolvedPath, "utf-8"); + // Normalize line endings to LF for consistent matching (Windows uses CRLF) + let content = rawContent.replace(/\r\n/g, "\n"); const appliedEdits: string[] = []; for (let i = 0; i < edits.length; i++) { const edit = edits[i]; diff --git a/src/tools/impl/shellLaunchers.ts b/src/tools/impl/shellLaunchers.ts index c2b14b4..d94cc2c 100644 --- a/src/tools/impl/shellLaunchers.ts +++ b/src/tools/impl/shellLaunchers.ts @@ -17,12 +17,10 @@ function windowsLaunchers(command: string): string[][] { if (!trimmed) return []; const launchers: string[][] = []; const seen = new Set(); - const envComSpecRaw = process.env.ComSpec || process.env.COMSPEC; - const envComSpec = envComSpecRaw?.trim(); - if (envComSpec) { - pushUnique(launchers, seen, [envComSpec, "/d", "/s", "/c", trimmed]); - } - pushUnique(launchers, seen, ["cmd.exe", "/d", "/s", "/c", trimmed]); + + // Default to PowerShell on Windows (same as Gemini CLI and Codex CLI) + // This ensures better PATH compatibility since many tools are configured + // in PowerShell profiles rather than system-wide cmd.exe PATH pushUnique(launchers, seen, [ "powershell.exe", "-NoProfile", @@ -30,6 +28,15 @@ function windowsLaunchers(command: string): string[][] { trimmed, ]); pushUnique(launchers, seen, ["pwsh", "-NoProfile", "-Command", trimmed]); + + // Fall back to cmd.exe if PowerShell fails + const envComSpecRaw = process.env.ComSpec || process.env.COMSPEC; + const envComSpec = envComSpecRaw?.trim(); + if (envComSpec) { + pushUnique(launchers, seen, [envComSpec, "/d", "/s", "/c", trimmed]); + } + pushUnique(launchers, seen, ["cmd.exe", "/d", "/s", "/c", trimmed]); + return launchers; }