fix: fix misc windows issues (#323)
Co-authored-by: Letta <noreply@letta.com>
This commit is contained in:
7
.github/workflows/ci.yml
vendored
7
.github/workflows/ci.yml
vendored
@@ -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:
|
||||
|
||||
57
WINDOWS_ISSUES.md
Normal file
57
WINDOWS_ISSUES.md
Normal file
@@ -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.
|
||||
@@ -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}": [
|
||||
|
||||
@@ -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)
|
||||
|
||||
119
src/tests/headless-windows.ts
Normal file
119
src/tests/headless-windows.ts
Normal file
@@ -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);
|
||||
});
|
||||
35
src/tests/session-context-windows.test.ts
Normal file
35
src/tests/session-context-windows.test.ts
Normal file
@@ -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");
|
||||
});
|
||||
}
|
||||
});
|
||||
@@ -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<typeof edit>[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");
|
||||
});
|
||||
});
|
||||
|
||||
81
src/tests/tools/shell-launchers.test.ts
Normal file
81
src/tests/tools/shell-launchers.test.ts
Normal file
@@ -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;
|
||||
}
|
||||
});
|
||||
});
|
||||
}
|
||||
});
|
||||
@@ -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") {
|
||||
|
||||
@@ -29,7 +29,9 @@ export async function edit(args: EditArgs): Promise<EditResult> {
|
||||
"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(
|
||||
|
||||
@@ -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];
|
||||
|
||||
@@ -17,12 +17,10 @@ function windowsLaunchers(command: string): string[][] {
|
||||
if (!trimmed) return [];
|
||||
const launchers: string[][] = [];
|
||||
const seen = new Set<string>();
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user