Skip to content

Commit b35bdf1

Browse files
committed
feat: add permission diff previews for write and edit tool calls
1 parent 71d6353 commit b35bdf1

5 files changed

Lines changed: 199 additions & 2 deletions

File tree

‎packages/cli/src/tests/permission-prompt.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,48 @@ function request(id: string, scopes: AskPermissionRequest["scopes"]): AskPermiss
9494
return { toolCallId: id, name: "Bash", command: id, scopes };
9595
}
9696

97+
test("permission diff appears above the question and follows the current request", async (t) => {
98+
const first: AskPermissionRequest = {
99+
...request("first", ["write-in-cwd"]),
100+
name: "edit",
101+
diff: {
102+
filePath: "/workspace/example.py",
103+
details: {
104+
added: 1,
105+
removed: 1,
106+
lines: [
107+
{ kind: "removed", content: "value = 1", lineNumber: 1 },
108+
{ kind: "added", content: "value = 2", lineNumber: 1 },
109+
],
110+
},
111+
},
112+
};
113+
const second: AskPermissionRequest = {
114+
...request("second", ["write-in-cwd"]),
115+
name: "write",
116+
diff: {
117+
filePath: "/workspace/new.py",
118+
details: { added: 1, removed: 0, lines: [{ kind: "added", content: "created = True", lineNumber: 1 }] },
119+
},
120+
};
121+
const h = await prompt(t, [first, second, request("third", ["network"])]);
122+
assert.match(h.frame(), /example\.py.*\+1 -1/);
123+
assert.match(h.frame(), /- value = 1/);
124+
assert.match(h.frame(), /\+ value = 2/);
125+
assert.ok(h.frame().indexOf("value = 2") < h.frame().indexOf("Do you want to proceed?"));
126+
await h.send("1");
127+
assert.doesNotMatch(h.frame(), /value = /);
128+
assert.match(h.frame(), /\+ created = True/);
129+
await h.send("4");
130+
assert.doesNotMatch(h.frame(), /created = True|new\.py/);
131+
await h.send("1");
132+
assert.deepEqual(h.results[0].permissions, [
133+
{ toolCallId: "first", permission: "allow" },
134+
{ toolCallId: "second", permission: "deny" },
135+
{ toolCallId: "third", permission: "allow" },
136+
]);
137+
});
138+
97139
for (const planMode of [false, true]) {
98140
test(`session option works in ${planMode ? "Plan" : "normal"} mode and skips repeated scopes`, async (t) => {
99141
const h = await prompt(t, [request("first", ["network"]), request("second", ["network"])], planMode);

‎packages/cli/src/ui/components/MessageView/index.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ function StatusLine({
215215
);
216216
}
217217

218-
function DiffPreview({
218+
export function DiffPreview({
219219
lines,
220220
filePath,
221221
added,

‎packages/cli/src/ui/views/PermissionPrompt.tsx‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import React, { useMemo, useRef, useState } from "react";
22
import { Box, Text } from "ink";
33
import { useTerminalInput } from "../hooks";
4+
import { DiffPreview } from "../components/MessageView";
45
import type { AskPermissionRequest, AskPermissionScope, PermissionScope } from "@vegamo/deepcode-core";
56
import {
67
buildScopePrompts,
@@ -92,6 +93,19 @@ function PermissionPromptForm({ requests, planMode = false, onSubmit, onCancel }
9293
<Text bold>{prompt.request.name}</Text>
9394
<Text>{prompt.request.command}</Text>
9495
{prompt.request.description ? <Text color="cyanBright">{prompt.request.description}</Text> : null}
96+
{prompt.request.diff ? (
97+
<Box marginTop={1}>
98+
<DiffPreview
99+
filePath={prompt.request.diff.filePath}
100+
added={prompt.request.diff.details.added}
101+
removed={prompt.request.diff.details.removed}
102+
lines={prompt.request.diff.details.lines.map((line) => ({
103+
...line,
104+
marker: line.kind === "added" ? "+" : line.kind === "removed" ? "-" : " ",
105+
}))}
106+
/>
107+
</Box>
108+
) : null}
95109
<Box marginTop={1}>
96110
<Text>Do you want to proceed?</Text>
97111
</Box>
@@ -127,7 +141,7 @@ function buildOptions(scope: AskPermissionScope, planMode: boolean): PromptOptio
127141
if (scope !== "unknown") {
128142
options.push({
129143
kind: "session",
130-
label: "Yes, during this session: ",
144+
label: "Yes, during this session allow ",
131145
scopeDescription: describeScope(scope),
132146
scopeColor: getScopeRiskColor(scope),
133147
});

‎packages/core/src/common/permissions.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import * as path from "path";
33
import type { DeepcodingSettings, PermissionScope, PermissionSettings } from "../settings";
44
import type { ToolExecutionFollowUpMessage } from "./tool-types";
55
import { isAbsoluteFilePath, normalizeFilePath } from "./state";
6+
import { buildFileDiff, readTextFileWithMetadata } from "./file-utils";
7+
import { getDiffDetails, type DiffDetails } from "./diff-preview";
68

79
export type BashPermissionScope = Exclude<PermissionScope, "mcp"> | "unknown";
810
type PermissionPolicySettings = Required<Omit<PermissionSettings, "addWorkingDirs">> &
@@ -34,6 +36,7 @@ export type AskPermissionRequest = {
3436
name: string;
3537
command: string;
3638
description?: string;
39+
diff?: { filePath: string; details: DiffDetails };
3740
};
3841

3942
export type PermissionToolCall = {
@@ -192,13 +195,61 @@ export function computeToolCallPermissions(options: ComputeToolCallPermissionsOp
192195
name: request.name,
193196
command: request.command,
194197
description: request.description,
198+
diff: buildPermissionDiff(toolCall, options),
195199
});
196200
}
197201
}
198202

199203
return { permissions, askPermissions };
200204
}
201205

206+
function buildPermissionDiff(
207+
toolCall: PermissionToolCall,
208+
options: ComputeToolCallPermissionsOptions
209+
): AskPermissionRequest["diff"] {
210+
const name = toolCall.function.name.toLowerCase();
211+
if (name !== "write" && name !== "edit") return undefined;
212+
const args = parseToolArgumentsForPermissions(toolCall.function.arguments);
213+
const filePath = normalizeFilePath(
214+
name === "edit"
215+
? resolveEditPermissionPath(options.sessionId, args, options.resolveSnippetPath)
216+
: typeof args.file_path === "string"
217+
? args.file_path
218+
: ""
219+
);
220+
if (!isAbsoluteFilePath(filePath)) return undefined;
221+
222+
try {
223+
let original: string | null;
224+
let updated: string;
225+
if (name === "edit") {
226+
if (typeof args.old_string !== "string" || typeof args.new_string !== "string") return undefined;
227+
original = args.old_string;
228+
updated = args.new_string;
229+
} else {
230+
const content = args.content;
231+
if (typeof content === "string") {
232+
updated = content;
233+
} else if (filePath.toLowerCase().endsWith(".json") && content !== null && typeof content === "object") {
234+
updated = JSON.stringify(content, null, 2);
235+
} else {
236+
return undefined;
237+
}
238+
try {
239+
original = readTextFileWithMetadata(filePath).content;
240+
} catch (error) {
241+
if ((error as NodeJS.ErrnoException).code !== "ENOENT") throw error;
242+
original = null;
243+
}
244+
}
245+
const { details } = buildFileDiff(filePath, original, updated);
246+
return details ? { filePath, details } : undefined;
247+
} catch {
248+
// A missing preview must not prevent the user from answering the permission request.
249+
return undefined;
250+
}
251+
}
252+
202253
function getAllowedForcedAskScopes(
203254
scopes: AskPermissionScope[],
204255
settings: PermissionPolicySettings | undefined,
@@ -615,12 +666,15 @@ export function normalizeAskPermissions(value: unknown): AskPermissionRequest[]
615666
const scopes = Array.isArray(record.scopes)
616667
? record.scopes.filter((scope): scope is AskPermissionScope => isAskPermissionScope(scope))
617668
: [];
669+
const diff = record.diff as AskPermissionRequest["diff"];
670+
const details = getDiffDetails({ diff_details: diff?.details });
618671
result.push({
619672
toolCallId: record.toolCallId,
620673
scopes,
621674
name: record.name,
622675
command: typeof record.command === "string" ? record.command : record.name,
623676
description: typeof record.description === "string" ? record.description : undefined,
677+
...(typeof diff?.filePath === "string" && details ? { diff: { filePath: diff.filePath, details } } : {}),
624678
});
625679
}
626680
return result.length > 0 ? result : undefined;

‎packages/core/src/tests/permissions.test.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,99 @@ import {
1111
getPermissionScopesRequiringAsk,
1212
hasUserPermissionReplies,
1313
isPathInAnyDirectory,
14+
normalizeAskPermissions,
1415
parseBashSideEffects,
1516
} from "../common/permissions";
1617
import type { PermissionScope, PermissionSettings } from "../settings";
18+
import { buildFileDiff } from "../common/file-utils";
1719

1820
const tempDirs: string[] = [];
1921

22+
test("permission previews cover write and edit without mutating files", () => {
23+
const projectRoot = createTempDir("deepcode-permission-diff-");
24+
const existing = path.join(projectRoot, "existing.txt");
25+
const created = path.join(projectRoot, "new", "file.txt");
26+
fs.writeFileSync(existing, "\ufeffbefore\r\n", "utf16le");
27+
const inputs = [
28+
{
29+
name: "write",
30+
args: { file_path: existing, content: "after\r\n" },
31+
old: "\ufeffbefore\n",
32+
next: "after\n",
33+
filePath: existing,
34+
},
35+
{ name: "Write", args: { file_path: created, content: "new\n" }, old: null, next: "new\n", filePath: created },
36+
{
37+
name: "write",
38+
args: { file_path: path.join(projectRoot, "data.json"), content: { a: 1 } },
39+
old: null,
40+
next: '{\n "a": 1\n}',
41+
filePath: path.join(projectRoot, "data.json"),
42+
},
43+
...[
44+
["old", "new"],
45+
["", "inserted"],
46+
["deleted", ""],
47+
].map(([old, next]) => ({
48+
name: "edit",
49+
args: { snippet_id: "snippet", old_string: old, new_string: next, replace_all: true },
50+
old,
51+
next,
52+
filePath: existing,
53+
})),
54+
];
55+
const snapshot = fs.readFileSync(existing);
56+
const plan = computeToolCallPermissions({
57+
sessionId: "diff-session",
58+
projectRoot,
59+
settings: { allow: [], deny: [], ask: [], defaultMode: "askAll" },
60+
resolveSnippetPath: () => existing,
61+
toolCalls: inputs.map(({ name, args }, index) => ({
62+
id: String(index),
63+
type: "function",
64+
function: { name, arguments: JSON.stringify(args) },
65+
})),
66+
});
67+
assert.equal(plan.askPermissions.length, inputs.length);
68+
inputs.forEach(({ filePath, old, next }, index) => {
69+
assert.deepEqual(plan.askPermissions[index].diff, {
70+
filePath,
71+
details: buildFileDiff(filePath, old, next).details,
72+
});
73+
});
74+
assert.deepEqual(fs.readFileSync(existing), snapshot);
75+
assert.equal(fs.existsSync(path.dirname(created)), false);
76+
assert.equal(fs.existsSync(path.join(projectRoot, "data.json")), false);
77+
assert.deepEqual(normalizeAskPermissions(JSON.parse(JSON.stringify(plan.askPermissions))), plan.askPermissions);
78+
});
79+
80+
test("missing permission previews do not affect approval decisions", () => {
81+
const projectRoot = createTempDir("deepcode-permission-diff-fallback-");
82+
const toolCalls = [
83+
{ name: "write", args: { file_path: projectRoot, content: "directory cannot be read" } },
84+
{ name: "write", args: { file_path: path.join(projectRoot, "file"), content: 123 } },
85+
{ name: "edit", args: { file_path: path.join(projectRoot, "file"), old_string: "same", new_string: "same" } },
86+
{ name: "edit", args: { file_path: path.join(projectRoot, "file"), new_string: "missing old_string" } },
87+
{ name: "bash", args: { command: "echo hello", sideEffects: ["network"] } },
88+
].map(({ name, args }, index) => ({
89+
id: String(index),
90+
type: "function",
91+
function: { name, arguments: JSON.stringify(args) },
92+
}));
93+
const options = { sessionId: "diff-session", projectRoot, toolCalls };
94+
const plan = computeToolCallPermissions({
95+
...options,
96+
settings: { allow: [], deny: [], ask: [], defaultMode: "askAll" },
97+
});
98+
assert.equal(plan.askPermissions.length, toolCalls.length);
99+
assert.ok(plan.askPermissions.every((request) => request.diff === undefined));
100+
assert.equal(computeToolCallPermissions(options).askPermissions.length, 0);
101+
const legacy = { toolCallId: "old", name: "write", command: "write file", scopes: [] };
102+
for (const diff of [undefined, null, { filePath: "file", details: { lines: [] } }]) {
103+
assert.equal(normalizeAskPermissions([{ ...legacy, diff }])?.[0].diff, undefined);
104+
}
105+
});
106+
20107
afterEach(() => {
21108
while (tempDirs.length > 0) {
22109
const dir = tempDirs.pop();

0 commit comments

Comments
 (0)