fix: Windows path support and canonicalization (#13671)
Co-authored-by: LukeParkerDev <10430890+Hona@users.noreply.github.com>
This commit is contained in:
@@ -79,23 +79,23 @@ export namespace Patch {
|
|||||||
const line = lines[startIdx]
|
const line = lines[startIdx]
|
||||||
|
|
||||||
if (line.startsWith("*** Add File:")) {
|
if (line.startsWith("*** Add File:")) {
|
||||||
const filePath = line.split(":", 2)[1]?.trim()
|
const filePath = line.slice("*** Add File:".length).trim()
|
||||||
return filePath ? { filePath, nextIdx: startIdx + 1 } : null
|
return filePath ? { filePath, nextIdx: startIdx + 1 } : null
|
||||||
}
|
}
|
||||||
|
|
||||||
if (line.startsWith("*** Delete File:")) {
|
if (line.startsWith("*** Delete File:")) {
|
||||||
const filePath = line.split(":", 2)[1]?.trim()
|
const filePath = line.slice("*** Delete File:".length).trim()
|
||||||
return filePath ? { filePath, nextIdx: startIdx + 1 } : null
|
return filePath ? { filePath, nextIdx: startIdx + 1 } : null
|
||||||
}
|
}
|
||||||
|
|
||||||
if (line.startsWith("*** Update File:")) {
|
if (line.startsWith("*** Update File:")) {
|
||||||
const filePath = line.split(":", 2)[1]?.trim()
|
const filePath = line.slice("*** Update File:".length).trim()
|
||||||
let movePath: string | undefined
|
let movePath: string | undefined
|
||||||
let nextIdx = startIdx + 1
|
let nextIdx = startIdx + 1
|
||||||
|
|
||||||
// Check for move directive
|
// Check for move directive
|
||||||
if (nextIdx < lines.length && lines[nextIdx].startsWith("*** Move to:")) {
|
if (nextIdx < lines.length && lines[nextIdx].startsWith("*** Move to:")) {
|
||||||
movePath = lines[nextIdx].split(":", 2)[1]?.trim()
|
movePath = lines[nextIdx].slice("*** Move to:".length).trim()
|
||||||
nextIdx++
|
nextIdx++
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -105,7 +105,7 @@ export namespace Snapshot {
|
|||||||
.split("\n")
|
.split("\n")
|
||||||
.map((x) => x.trim())
|
.map((x) => x.trim())
|
||||||
.filter(Boolean)
|
.filter(Boolean)
|
||||||
.map((x) => path.join(Instance.worktree, x)),
|
.map((x) => path.join(Instance.worktree, x).replaceAll("\\", "/")),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -161,7 +161,7 @@ export const ApplyPatchTool = Tool.define("apply_patch", {
|
|||||||
// Build per-file metadata for UI rendering (used for both permission and result)
|
// Build per-file metadata for UI rendering (used for both permission and result)
|
||||||
const files = fileChanges.map((change) => ({
|
const files = fileChanges.map((change) => ({
|
||||||
filePath: change.filePath,
|
filePath: change.filePath,
|
||||||
relativePath: path.relative(Instance.worktree, change.movePath ?? change.filePath),
|
relativePath: path.relative(Instance.worktree, change.movePath ?? change.filePath).replaceAll("\\", "/"),
|
||||||
type: change.type,
|
type: change.type,
|
||||||
diff: change.diff,
|
diff: change.diff,
|
||||||
before: change.oldContent,
|
before: change.oldContent,
|
||||||
@@ -172,7 +172,7 @@ export const ApplyPatchTool = Tool.define("apply_patch", {
|
|||||||
}))
|
}))
|
||||||
|
|
||||||
// Check permissions if needed
|
// Check permissions if needed
|
||||||
const relativePaths = fileChanges.map((c) => path.relative(Instance.worktree, c.filePath))
|
const relativePaths = fileChanges.map((c) => path.relative(Instance.worktree, c.filePath).replaceAll("\\", "/"))
|
||||||
await ctx.ask({
|
await ctx.ask({
|
||||||
permission: "edit",
|
permission: "edit",
|
||||||
patterns: relativePaths,
|
patterns: relativePaths,
|
||||||
@@ -242,13 +242,13 @@ export const ApplyPatchTool = Tool.define("apply_patch", {
|
|||||||
// Generate output summary
|
// Generate output summary
|
||||||
const summaryLines = fileChanges.map((change) => {
|
const summaryLines = fileChanges.map((change) => {
|
||||||
if (change.type === "add") {
|
if (change.type === "add") {
|
||||||
return `A ${path.relative(Instance.worktree, change.filePath)}`
|
return `A ${path.relative(Instance.worktree, change.filePath).replaceAll("\\", "/")}`
|
||||||
}
|
}
|
||||||
if (change.type === "delete") {
|
if (change.type === "delete") {
|
||||||
return `D ${path.relative(Instance.worktree, change.filePath)}`
|
return `D ${path.relative(Instance.worktree, change.filePath).replaceAll("\\", "/")}`
|
||||||
}
|
}
|
||||||
const target = change.movePath ?? change.filePath
|
const target = change.movePath ?? change.filePath
|
||||||
return `M ${path.relative(Instance.worktree, target)}`
|
return `M ${path.relative(Instance.worktree, target).replaceAll("\\", "/")}`
|
||||||
})
|
})
|
||||||
let output = `Success. Updated the following files:\n${summaryLines.join("\n")}`
|
let output = `Success. Updated the following files:\n${summaryLines.join("\n")}`
|
||||||
|
|
||||||
@@ -264,7 +264,7 @@ export const ApplyPatchTool = Tool.define("apply_patch", {
|
|||||||
const limited = errors.slice(0, MAX_DIAGNOSTICS_PER_FILE)
|
const limited = errors.slice(0, MAX_DIAGNOSTICS_PER_FILE)
|
||||||
const suffix =
|
const suffix =
|
||||||
errors.length > MAX_DIAGNOSTICS_PER_FILE ? `\n... and ${errors.length - MAX_DIAGNOSTICS_PER_FILE} more` : ""
|
errors.length > MAX_DIAGNOSTICS_PER_FILE ? `\n... and ${errors.length - MAX_DIAGNOSTICS_PER_FILE} more` : ""
|
||||||
output += `\n\nLSP errors detected in ${path.relative(Instance.worktree, target)}, please fix:\n<diagnostics file="${target}">\n${limited.map(LSP.Diagnostic.pretty).join("\n")}${suffix}\n</diagnostics>`
|
output += `\n\nLSP errors detected in ${path.relative(Instance.worktree, target).replaceAll("\\", "/")}, please fix:\n<diagnostics file="${target}">\n${limited.map(LSP.Diagnostic.pretty).join("\n")}${suffix}\n</diagnostics>`
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -142,7 +142,11 @@ export const BashTool = Tool.define("bash", async () => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
if (directories.size > 0) {
|
if (directories.size > 0) {
|
||||||
const globs = Array.from(directories).map((dir) => path.join(dir, "*"))
|
const globs = Array.from(directories).map((dir) => {
|
||||||
|
// Preserve POSIX-looking paths with /s, even on Windows
|
||||||
|
if (dir.startsWith("/")) return `${dir.replace(/[\\/]+$/, "")}/*`
|
||||||
|
return path.join(dir, "*")
|
||||||
|
})
|
||||||
await ctx.ask({
|
await ctx.ask({
|
||||||
permission: "external_directory",
|
permission: "external_directory",
|
||||||
patterns: globs,
|
patterns: globs,
|
||||||
|
|||||||
@@ -50,7 +50,7 @@ Instructions here.
|
|||||||
const testSkill = skills.find((s) => s.name === "test-skill")
|
const testSkill = skills.find((s) => s.name === "test-skill")
|
||||||
expect(testSkill).toBeDefined()
|
expect(testSkill).toBeDefined()
|
||||||
expect(testSkill!.description).toBe("A test skill for verification.")
|
expect(testSkill!.description).toBe("A test skill for verification.")
|
||||||
expect(testSkill!.location).toContain("skill/test-skill/SKILL.md")
|
expect(testSkill!.location).toContain(path.join("skill", "test-skill", "SKILL.md"))
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
@@ -180,7 +180,7 @@ description: A skill in the .claude/skills directory.
|
|||||||
expect(skills.length).toBe(1)
|
expect(skills.length).toBe(1)
|
||||||
const claudeSkill = skills.find((s) => s.name === "claude-skill")
|
const claudeSkill = skills.find((s) => s.name === "claude-skill")
|
||||||
expect(claudeSkill).toBeDefined()
|
expect(claudeSkill).toBeDefined()
|
||||||
expect(claudeSkill!.location).toContain(".claude/skills/claude-skill/SKILL.md")
|
expect(claudeSkill!.location).toContain(path.join(".claude", "skills", "claude-skill", "SKILL.md"))
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
@@ -200,7 +200,7 @@ test("discovers global skills from ~/.claude/skills/ directory", async () => {
|
|||||||
expect(skills.length).toBe(1)
|
expect(skills.length).toBe(1)
|
||||||
expect(skills[0].name).toBe("global-test-skill")
|
expect(skills[0].name).toBe("global-test-skill")
|
||||||
expect(skills[0].description).toBe("A global skill from ~/.claude/skills for testing.")
|
expect(skills[0].description).toBe("A global skill from ~/.claude/skills for testing.")
|
||||||
expect(skills[0].location).toContain(".claude/skills/global-test-skill/SKILL.md")
|
expect(skills[0].location).toContain(path.join(".claude", "skills", "global-test-skill", "SKILL.md"))
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
} finally {
|
} finally {
|
||||||
@@ -245,7 +245,7 @@ description: A skill in the .agents/skills directory.
|
|||||||
expect(skills.length).toBe(1)
|
expect(skills.length).toBe(1)
|
||||||
const agentSkill = skills.find((s) => s.name === "agent-skill")
|
const agentSkill = skills.find((s) => s.name === "agent-skill")
|
||||||
expect(agentSkill).toBeDefined()
|
expect(agentSkill).toBeDefined()
|
||||||
expect(agentSkill!.location).toContain(".agents/skills/agent-skill/SKILL.md")
|
expect(agentSkill!.location).toContain(path.join(".agents", "skills", "agent-skill", "SKILL.md"))
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
@@ -279,7 +279,7 @@ This skill is loaded from the global home directory.
|
|||||||
expect(skills.length).toBe(1)
|
expect(skills.length).toBe(1)
|
||||||
expect(skills[0].name).toBe("global-agent-skill")
|
expect(skills[0].name).toBe("global-agent-skill")
|
||||||
expect(skills[0].description).toBe("A global skill from ~/.agents/skills for testing.")
|
expect(skills[0].description).toBe("A global skill from ~/.agents/skills for testing.")
|
||||||
expect(skills[0].location).toContain(".agents/skills/global-agent-skill/SKILL.md")
|
expect(skills[0].location).toContain(path.join(".agents", "skills", "global-agent-skill", "SKILL.md"))
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
} finally {
|
} finally {
|
||||||
|
|||||||
@@ -93,6 +93,13 @@ describe("tool.apply_patch freeform", () => {
|
|||||||
|
|
||||||
expect(result.title).toContain("Success. Updated the following files")
|
expect(result.title).toContain("Success. Updated the following files")
|
||||||
expect(result.output).toContain("Success. Updated the following files")
|
expect(result.output).toContain("Success. Updated the following files")
|
||||||
|
// Strict formatting assertions for slashes
|
||||||
|
expect(result.output).toMatch(/A nested\/new\.txt/)
|
||||||
|
expect(result.output).toMatch(/D delete\.txt/)
|
||||||
|
expect(result.output).toMatch(/M modify\.txt/)
|
||||||
|
if (process.platform === "win32") {
|
||||||
|
expect(result.output).not.toContain("\\")
|
||||||
|
}
|
||||||
expect(result.metadata.diff).toContain("Index:")
|
expect(result.metadata.diff).toContain("Index:")
|
||||||
expect(calls.length).toBe(1)
|
expect(calls.length).toBe(1)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user