fix(web): construct apply_patch metadata before requesting permission (#10422)
This commit is contained in:
@@ -158,6 +158,19 @@ export const ApplyPatchTool = Tool.define("apply_patch", {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Build per-file metadata for UI rendering (used for both permission and result)
|
||||||
|
const files = fileChanges.map((change) => ({
|
||||||
|
filePath: change.filePath,
|
||||||
|
relativePath: path.relative(Instance.worktree, change.movePath ?? change.filePath),
|
||||||
|
type: change.type,
|
||||||
|
diff: change.diff,
|
||||||
|
before: change.oldContent,
|
||||||
|
after: change.newContent,
|
||||||
|
additions: change.additions,
|
||||||
|
deletions: change.deletions,
|
||||||
|
movePath: change.movePath,
|
||||||
|
}))
|
||||||
|
|
||||||
// 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))
|
||||||
await ctx.ask({
|
await ctx.ask({
|
||||||
@@ -167,6 +180,7 @@ export const ApplyPatchTool = Tool.define("apply_patch", {
|
|||||||
metadata: {
|
metadata: {
|
||||||
filepath: relativePaths.join(", "),
|
filepath: relativePaths.join(", "),
|
||||||
diff: totalDiff,
|
diff: totalDiff,
|
||||||
|
files,
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
|
|
||||||
@@ -253,19 +267,6 @@ export const ApplyPatchTool = Tool.define("apply_patch", {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Build per-file metadata for UI rendering
|
|
||||||
const files = fileChanges.map((change) => ({
|
|
||||||
filePath: change.filePath,
|
|
||||||
relativePath: path.relative(Instance.worktree, change.movePath ?? change.filePath),
|
|
||||||
type: change.type,
|
|
||||||
diff: change.diff,
|
|
||||||
before: change.oldContent,
|
|
||||||
after: change.newContent,
|
|
||||||
additions: change.additions,
|
|
||||||
deletions: change.deletions,
|
|
||||||
movePath: change.movePath,
|
|
||||||
}))
|
|
||||||
|
|
||||||
return {
|
return {
|
||||||
title: output,
|
title: output,
|
||||||
metadata: {
|
metadata: {
|
||||||
|
|||||||
@@ -18,7 +18,21 @@ type AskInput = {
|
|||||||
permission: string
|
permission: string
|
||||||
patterns: string[]
|
patterns: string[]
|
||||||
always: string[]
|
always: string[]
|
||||||
metadata: { diff: string }
|
metadata: {
|
||||||
|
diff: string
|
||||||
|
filepath: string
|
||||||
|
files: Array<{
|
||||||
|
filePath: string
|
||||||
|
relativePath: string
|
||||||
|
type: "add" | "update" | "delete" | "move"
|
||||||
|
diff: string
|
||||||
|
before: string
|
||||||
|
after: string
|
||||||
|
additions: number
|
||||||
|
deletions: number
|
||||||
|
movePath?: string
|
||||||
|
}>
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
type ToolCtx = typeof baseCtx & {
|
type ToolCtx = typeof baseCtx & {
|
||||||
@@ -60,7 +74,7 @@ describe("tool.apply_patch freeform", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
test("applies add/update/delete in one patch", async () => {
|
test("applies add/update/delete in one patch", async () => {
|
||||||
await using fixture = await tmpdir()
|
await using fixture = await tmpdir({ git: true })
|
||||||
const { ctx, calls } = makeCtx()
|
const { ctx, calls } = makeCtx()
|
||||||
|
|
||||||
await Instance.provide({
|
await Instance.provide({
|
||||||
@@ -81,6 +95,21 @@ describe("tool.apply_patch freeform", () => {
|
|||||||
expect(result.metadata.diff).toContain("Index:")
|
expect(result.metadata.diff).toContain("Index:")
|
||||||
expect(calls.length).toBe(1)
|
expect(calls.length).toBe(1)
|
||||||
|
|
||||||
|
// Verify permission metadata includes files array for UI rendering
|
||||||
|
const permissionCall = calls[0]
|
||||||
|
expect(permissionCall.metadata.files).toHaveLength(3)
|
||||||
|
expect(permissionCall.metadata.files.map((f) => f.type).sort()).toEqual(["add", "delete", "update"])
|
||||||
|
|
||||||
|
const addFile = permissionCall.metadata.files.find((f) => f.type === "add")
|
||||||
|
expect(addFile).toBeDefined()
|
||||||
|
expect(addFile!.relativePath).toBe("nested/new.txt")
|
||||||
|
expect(addFile!.after).toBe("created\n")
|
||||||
|
|
||||||
|
const updateFile = permissionCall.metadata.files.find((f) => f.type === "update")
|
||||||
|
expect(updateFile).toBeDefined()
|
||||||
|
expect(updateFile!.before).toContain("line2")
|
||||||
|
expect(updateFile!.after).toContain("changed")
|
||||||
|
|
||||||
const added = await fs.readFile(path.join(fixture.path, "nested", "new.txt"), "utf-8")
|
const added = await fs.readFile(path.join(fixture.path, "nested", "new.txt"), "utf-8")
|
||||||
expect(added).toBe("created\n")
|
expect(added).toBe("created\n")
|
||||||
expect(await fs.readFile(modifyPath, "utf-8")).toBe("line1\nchanged\n")
|
expect(await fs.readFile(modifyPath, "utf-8")).toBe("line1\nchanged\n")
|
||||||
@@ -89,6 +118,36 @@ describe("tool.apply_patch freeform", () => {
|
|||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test("permission metadata includes move file info", async () => {
|
||||||
|
await using fixture = await tmpdir({ git: true })
|
||||||
|
const { ctx, calls } = makeCtx()
|
||||||
|
|
||||||
|
await Instance.provide({
|
||||||
|
directory: fixture.path,
|
||||||
|
fn: async () => {
|
||||||
|
const original = path.join(fixture.path, "old", "name.txt")
|
||||||
|
await fs.mkdir(path.dirname(original), { recursive: true })
|
||||||
|
await fs.writeFile(original, "old content\n", "utf-8")
|
||||||
|
|
||||||
|
const patchText =
|
||||||
|
"*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/dir/name.txt\n@@\n-old content\n+new content\n*** End Patch"
|
||||||
|
|
||||||
|
await execute({ patchText }, ctx)
|
||||||
|
|
||||||
|
expect(calls.length).toBe(1)
|
||||||
|
const permissionCall = calls[0]
|
||||||
|
expect(permissionCall.metadata.files).toHaveLength(1)
|
||||||
|
|
||||||
|
const moveFile = permissionCall.metadata.files[0]
|
||||||
|
expect(moveFile.type).toBe("move")
|
||||||
|
expect(moveFile.relativePath).toBe("renamed/dir/name.txt")
|
||||||
|
expect(moveFile.movePath).toBe(path.join(fixture.path, "renamed/dir/name.txt"))
|
||||||
|
expect(moveFile.before).toBe("old content\n")
|
||||||
|
expect(moveFile.after).toBe("new content\n")
|
||||||
|
},
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
test("applies multiple hunks to one file", async () => {
|
test("applies multiple hunks to one file", async () => {
|
||||||
await using fixture = await tmpdir()
|
await using fixture = await tmpdir()
|
||||||
const { ctx } = makeCtx()
|
const { ctx } = makeCtx()
|
||||||
|
|||||||
@@ -605,7 +605,12 @@ PART_MAPPING["tool"] = function ToolPartDisplay(props) {
|
|||||||
|
|
||||||
const input = () => part.state?.input ?? emptyInput
|
const input = () => part.state?.input ?? emptyInput
|
||||||
// @ts-expect-error
|
// @ts-expect-error
|
||||||
const metadata = () => part.state?.metadata ?? emptyMetadata
|
const partMetadata = () => part.state?.metadata ?? emptyMetadata
|
||||||
|
const metadata = () => {
|
||||||
|
const perm = permission()
|
||||||
|
if (perm?.metadata) return { ...perm.metadata, ...partMetadata() }
|
||||||
|
return partMetadata()
|
||||||
|
}
|
||||||
|
|
||||||
const render = ToolRegistry.render(part.tool) ?? GenericTool
|
const render = ToolRegistry.render(part.tool) ?? GenericTool
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user