shumengya/sproutclaw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(coding-agent,tui): stabilize windows shell/path handling

Browse Source 
fixes #1775

fixes #1769
This commit is contained in:
badlogic
2026-03-14 05:36:04 +01:00
parent 1a4d153d7a
commit 4f81c3c28d
10 changed files with 213 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
packages/coding-agent/test/auth-storage.test.ts
View File

@@ -30,6 +30,10 @@ describe("AuthStorage", () => {
writeFileSync(authJsonPath, JSON.stringify(data));
}
function toShPath(value: string): string {
return value.replace(/\\/g, "/").replace(/"/g, '\\"');
}
describe("API key resolution", () => {
test("literal API key is returned directly", async () => {
writeAuthJson({
@@ -161,7 +165,8 @@ describe("AuthStorage", () => {
const counterFile = join(tempDir, "counter");
writeFileSync(counterFile, "0");
const command = `!sh -c 'count=$(cat ${counterFile}); echo $((count + 1)) > ${counterFile}; echo "key-value"'`;
const counterPath = toShPath(counterFile);
const command = `!sh -c 'count=$(cat "${counterPath}"); echo $((count + 1)) > "${counterPath}"; echo "key-value"'`;
writeAuthJson({
anthropic: { type: "api_key", key: command },
});
@@ -182,7 +187,8 @@ describe("AuthStorage", () => {
const counterFile = join(tempDir, "counter");
writeFileSync(counterFile, "0");
const command = `!sh -c 'count=$(cat ${counterFile}); echo $((count + 1)) > ${counterFile}; echo "key-value"'`;
const counterPath = toShPath(counterFile);
const command = `!sh -c 'count=$(cat "${counterPath}"); echo $((count + 1)) > "${counterPath}"; echo "key-value"'`;
writeAuthJson({
anthropic: { type: "api_key", key: command },
});
@@ -203,7 +209,8 @@ describe("AuthStorage", () => {
const counterFile = join(tempDir, "counter");
writeFileSync(counterFile, "0");
const command = `!sh -c 'count=$(cat ${counterFile}); echo $((count + 1)) > ${counterFile}; echo "key-value"'`;
const counterPath = toShPath(counterFile);
const command = `!sh -c 'count=$(cat "${counterPath}"); echo $((count + 1)) > "${counterPath}"; echo "key-value"'`;
writeAuthJson({
anthropic: { type: "api_key", key: command },
});
@@ -239,7 +246,8 @@ describe("AuthStorage", () => {
const counterFile = join(tempDir, "counter");
writeFileSync(counterFile, "0");
const command = `!sh -c 'count=$(cat ${counterFile}); echo $((count + 1)) > ${counterFile}; exit 1'`;
const counterPath = toShPath(counterFile);
const command = `!sh -c 'count=$(cat "${counterPath}"); echo $((count + 1)) > "${counterPath}"; exit 1'`;
writeAuthJson({
anthropic: { type: "api_key", key: command },
});

7
packages/coding-agent/test/extensions-runner.test.ts
View File

@@ -127,9 +127,12 @@ describe("ExtensionRunner", () => {
});
it("warns but allows when extension uses non-reserved built-in shortcut", async () => {
const pasteImageKey = Array.isArray(DEFAULT_KEYBINDINGS.pasteImage)
? (DEFAULT_KEYBINDINGS.pasteImage[0] ?? "")
: DEFAULT_KEYBINDINGS.pasteImage;
const extCode = `
export default function(pi) {
pi.registerShortcut("ctrl+v", {
pi.registerShortcut("${pasteImageKey}", {
description: "Overrides non-reserved",
handler: async () => {},
});
@@ -144,7 +147,7 @@ describe("ExtensionRunner", () => {
const shortcuts = runner.getShortcuts(DEFAULT_KEYBINDINGS);
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("built-in shortcut for pasteImage"));
expect(shortcuts.has("ctrl+v")).toBe(true);
expect(shortcuts.has(pasteImageKey as KeyId)).toBe(true);
warnSpy.mockRestore();
});

16
packages/coding-agent/test/model-registry.test.ts
View File

@@ -57,6 +57,10 @@ describe("ModelRegistry", () => {
return registry.getAll().filter((m) => m.provider === provider);
}
function toShPath(value: string): string {
return value.replace(/\\/g, "/").replace(/"/g, '\\"');
}
/** Create a baseUrl-only override (no custom models) */
function overrideConfig(baseUrl: string, headers?: Record<string, string>) {
return { baseUrl, ...(headers && { headers }) };
@@ -897,7 +901,8 @@ describe("ModelRegistry", () => {
const counterFile = join(tempDir, "counter");
writeFileSync(counterFile, "0");
const command = `!sh -c 'count=$(cat ${counterFile}); echo $((count + 1)) > ${counterFile}; echo "key-value"'`;
const counterPath = toShPath(counterFile);
const command = `!sh -c 'count=$(cat "${counterPath}"); echo $((count + 1)) > "${counterPath}"; echo "key-value"'`;
writeRawModelsJson({
"custom-provider": providerWithApiKey(command),
});
@@ -918,7 +923,8 @@ describe("ModelRegistry", () => {
const counterFile = join(tempDir, "counter");
writeFileSync(counterFile, "0");
const command = `!sh -c 'count=$(cat ${counterFile}); echo $((count + 1)) > ${counterFile}; echo "key-value"'`;
const counterPath = toShPath(counterFile);
const command = `!sh -c 'count=$(cat "${counterPath}"); echo $((count + 1)) > "${counterPath}"; echo "key-value"'`;
writeRawModelsJson({
"custom-provider": providerWithApiKey(command),
});
@@ -939,7 +945,8 @@ describe("ModelRegistry", () => {
const counterFile = join(tempDir, "counter");
writeFileSync(counterFile, "0");
const command = `!sh -c 'count=$(cat ${counterFile}); echo $((count + 1)) > ${counterFile}; echo "key-value"'`;
const counterPath = toShPath(counterFile);
const command = `!sh -c 'count=$(cat "${counterPath}"); echo $((count + 1)) > "${counterPath}"; echo "key-value"'`;
writeRawModelsJson({
"custom-provider": providerWithApiKey(command),
});
@@ -975,7 +982,8 @@ describe("ModelRegistry", () => {
const counterFile = join(tempDir, "counter");
writeFileSync(counterFile, "0");
const command = `!sh -c 'count=$(cat ${counterFile}); echo $((count + 1)) > ${counterFile}; exit 1'`;
const counterPath = toShPath(counterFile);
const command = `!sh -c 'count=$(cat "${counterPath}"); echo $((count + 1)) > "${counterPath}"; exit 1'`;
writeRawModelsJson({
"custom-provider": providerWithApiKey(command),
});

60
packages/coding-agent/test/package-manager.test.ts
View File

@@ -5,12 +5,30 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
import { DefaultPackageManager, type ProgressEvent, type ResolvedResource } from "../src/core/package-manager.js";
import { SettingsManager } from "../src/core/settings-manager.js";
// Helper to check if a resource is enabled
const isEnabled = (r: ResolvedResource, pathMatch: string, matchFn: "endsWith" | "includes" = "endsWith") =>
matchFn === "endsWith" ? r.path.endsWith(pathMatch) && r.enabled : r.path.includes(pathMatch) && r.enabled;
function normalizeForMatch(value: string): string {
return value.replace(/\\/g, "/");
}
const isDisabled = (r: ResolvedResource, pathMatch: string, matchFn: "endsWith" | "includes" = "endsWith") =>
matchFn === "endsWith" ? r.path.endsWith(pathMatch) && !r.enabled : r.path.includes(pathMatch) && !r.enabled;
function pathEndsWith(actualPath: string, suffix: string): boolean {
return normalizeForMatch(actualPath).endsWith(normalizeForMatch(suffix));
}
// Helper to check if a resource is enabled
const isEnabled = (r: ResolvedResource, pathMatch: string, matchFn: "endsWith" | "includes" = "endsWith") => {
const normalizedPath = normalizeForMatch(r.path);
const normalizedMatch = normalizeForMatch(pathMatch);
return matchFn === "endsWith"
? normalizedPath.endsWith(normalizedMatch) && r.enabled
: normalizedPath.includes(normalizedMatch) && r.enabled;
};
const isDisabled = (r: ResolvedResource, pathMatch: string, matchFn: "endsWith" | "includes" = "endsWith") => {
const normalizedPath = normalizeForMatch(r.path);
const normalizedMatch = normalizeForMatch(pathMatch);
return matchFn === "endsWith"
? normalizedPath.endsWith(normalizedMatch) && !r.enabled
: normalizedPath.includes(normalizedMatch) && !r.enabled;
};
describe("DefaultPackageManager", () => {
let tempDir: string;
@@ -155,7 +173,7 @@ Content`,
);
// Should NOT find helper.ts (not declared in manifest)
expect(result.extensions.some((r) => r.path.endsWith("helper.ts"))).toBe(false);
expect(result.extensions.some((r) => pathEndsWith(r.path, "helper.ts"))).toBe(false);
});
});
@@ -331,8 +349,8 @@ Content`,
writeFileSync(join(pkgDir, "themes", "dark.json"), "{}");
const result = await packageManager.resolveExtensionSources([pkgDir]);
expect(result.extensions.some((r) => r.path.endsWith("main.ts") && r.enabled)).toBe(true);
expect(result.themes.some((r) => r.path.endsWith("dark.json") && r.enabled)).toBe(true);
expect(result.extensions.some((r) => pathEndsWith(r.path, "main.ts") && r.enabled)).toBe(true);
expect(result.themes.some((r) => pathEndsWith(r.path, "dark.json") && r.enabled)).toBe(true);
});
});
@@ -731,7 +749,7 @@ Content`,
const result = await packageManager.resolveExtensionSources([pkgDir]);
expect(result.extensions.some((r) => isEnabled(r, "local.ts"))).toBe(true);
expect(result.extensions.some((r) => isEnabled(r, "remote.ts"))).toBe(true);
expect(result.extensions.some((r) => r.path.endsWith("skip.ts"))).toBe(false);
expect(result.extensions.some((r) => pathEndsWith(r.path, "skip.ts"))).toBe(false);
});
it("should support glob patterns in manifest skills", async () => {
@@ -798,7 +816,7 @@ Content`,
// bar.ts should be excluded (by user)
expect(result.extensions.some((r) => isDisabled(r, "bar.ts"))).toBe(true);
// baz.ts should be excluded (by manifest)
expect(result.extensions.some((r) => r.path.endsWith("baz.ts"))).toBe(false);
expect(result.extensions.some((r) => pathEndsWith(r.path, "baz.ts"))).toBe(false);
});
it("should exclude extensions from package with ! pattern", async () => {
@@ -1194,11 +1212,11 @@ export default function(api) { api.registerTool({ name: "test", description: "te
const result = await packageManager.resolveExtensionSources([pkgDir]);
// Should find the index.ts and standalone.ts
expect(result.extensions.some((r) => r.path.endsWith("subagent/index.ts") && r.enabled)).toBe(true);
expect(result.extensions.some((r) => r.path.endsWith("standalone.ts") && r.enabled)).toBe(true);
expect(result.extensions.some((r) => pathEndsWith(r.path, "subagent/index.ts") && r.enabled)).toBe(true);
expect(result.extensions.some((r) => pathEndsWith(r.path, "standalone.ts") && r.enabled)).toBe(true);
// Should NOT find agents.ts as a standalone extension
expect(result.extensions.some((r) => r.path.endsWith("agents.ts"))).toBe(false);
expect(result.extensions.some((r) => pathEndsWith(r.path, "agents.ts"))).toBe(false);
});
it("should respect package.json pi.extensions manifest in subdirectories", async () => {
@@ -1220,10 +1238,10 @@ export default function(api) { api.registerTool({ name: "test", description: "te
const result = await packageManager.resolveExtensionSources([pkgDir]);
// Should find main.ts declared in manifest
expect(result.extensions.some((r) => r.path.endsWith("custom/main.ts") && r.enabled)).toBe(true);
expect(result.extensions.some((r) => pathEndsWith(r.path, "custom/main.ts") && r.enabled)).toBe(true);
// Should NOT find utils.ts (not declared in manifest)
expect(result.extensions.some((r) => r.path.endsWith("utils.ts"))).toBe(false);
expect(result.extensions.some((r) => pathEndsWith(r.path, "utils.ts"))).toBe(false);
});
it("should handle mixed top-level files and subdirectories", async () => {
@@ -1244,12 +1262,12 @@ export default function(api) { api.registerTool({ name: "test", description: "te
const result = await packageManager.resolveExtensionSources([pkgDir]);
// Should find simple.ts and complex/index.ts
expect(result.extensions.some((r) => r.path.endsWith("simple.ts") && r.enabled)).toBe(true);
expect(result.extensions.some((r) => r.path.endsWith("complex/index.ts") && r.enabled)).toBe(true);
expect(result.extensions.some((r) => pathEndsWith(r.path, "simple.ts") && r.enabled)).toBe(true);
expect(result.extensions.some((r) => pathEndsWith(r.path, "complex/index.ts") && r.enabled)).toBe(true);
// Should NOT find helper modules
expect(result.extensions.some((r) => r.path.endsWith("complex/a.ts"))).toBe(false);
expect(result.extensions.some((r) => r.path.endsWith("complex/b.ts"))).toBe(false);
expect(result.extensions.some((r) => pathEndsWith(r.path, "complex/a.ts"))).toBe(false);
expect(result.extensions.some((r) => pathEndsWith(r.path, "complex/b.ts"))).toBe(false);
// Total should be exactly 2
expect(result.extensions.filter((r) => r.enabled).length).toBe(2);
@@ -1269,7 +1287,7 @@ export default function(api) { api.registerTool({ name: "test", description: "te
const result = await packageManager.resolveExtensionSources([pkgDir]);
// Should only find the valid top-level extension
expect(result.extensions.some((r) => r.path.endsWith("valid.ts") && r.enabled)).toBe(true);
expect(result.extensions.some((r) => pathEndsWith(r.path, "valid.ts") && r.enabled)).toBe(true);
expect(result.extensions.filter((r) => r.enabled).length).toBe(1);
});
});
@@ -1299,7 +1317,7 @@ export default function(api) { api.registerTool({ name: "test", description: "te
const refreshTemporaryGitSourceSpy = vi.spyOn(packageManager as any, "refreshTemporaryGitSource");
const result = await packageManager.resolveExtensionSources([gitSource], { temporary: true });
expect(result.extensions.some((r) => r.path.endsWith("extensions/index.ts") && r.enabled)).toBe(true);
expect(result.extensions.some((r) => pathEndsWith(r.path, "extensions/index.ts") && r.enabled)).toBe(true);
expect(refreshTemporaryGitSourceSpy).not.toHaveBeenCalled();
});

4
packages/coding-agent/test/path-utils.test.ts
View File

@@ -1,6 +1,6 @@
import { mkdtempSync, readdirSync, rmdirSync, unlinkSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { join, resolve } from "node:path";
import { afterEach, beforeEach, describe, expect, it } from "vitest";
import { expandPath, resolveReadPath, resolveToCwd } from "../src/core/tools/path-utils.js";
@@ -32,7 +32,7 @@ describe("path-utils", () => {
it("should resolve relative paths against cwd", () => {
const result = resolveToCwd("relative/file.txt", "/some/cwd");
expect(result).toBe("/some/cwd/relative/file.txt");
expect(result).toBe(resolve("/some/cwd", "relative/file.txt"));
});
});

4
packages/coding-agent/test/session-info-modified-timestamp.test.ts
View File

@@ -1,7 +1,7 @@
import { writeFileSync } from "node:fs";
import { stat } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { dirname, join } from "node:path";
import { afterEach, beforeAll, describe, expect, it, vi } from "vitest";
import type { SessionHeader } from "../src/core/session-manager.js";
import { SessionManager } from "../src/core/session-manager.js";
@@ -74,7 +74,7 @@ describe("SessionInfo.modified", () => {
timestamp: msgTime,
});
const sessions = await SessionManager.list("/tmp", filePath.replace(/\/[^/]+$/, ""));
const sessions = await SessionManager.list("/tmp", dirname(filePath));
const s = sessions.find((x) => x.path === filePath);
expect(s).toBeDefined();
expect(s!.modified.getTime()).toBe(msgTime);