From 18ee91ef72ab37e9dbceaca1f74476786be31e72 Mon Sep 17 00:00:00 2001 From: drakeo338 Date: Wed, 14 Oct 2026 08:13:24 +0000 Subject: [PATCH 1/3] fix(tui): show every file in multi-file edit permission prompts The edit permission read only metadata.files[0], so approving an apply_patch touching several files showed one diff. Expose all files and render each one. --- packages/tui/src/mini/footer.permission.tsx | 65 +++++++++-------- packages/tui/src/mini/permission.shared.ts | 3 +- .../tui/src/routes/session/permission.tsx | 70 ++++++++++++------- packages/tui/src/util/permission.ts | 12 +++- .../tui/test/mini/permission.shared.test.ts | 19 +++++ packages/tui/test/util/permission.test.ts | 16 +++++ 6 files changed, 126 insertions(+), 59 deletions(-) diff --git a/packages/tui/src/mini/footer.permission.tsx b/packages/tui/src/mini/footer.permission.tsx index 6f11e8573a0b..daa82d4b751d 100644 --- a/packages/tui/src/mini/footer.permission.tsx +++ b/packages/tui/src/mini/footer.permission.tsx @@ -154,7 +154,6 @@ export function RunPermissionBody(props: { const [state, setState] = createSignal(createPermissionBodyState(props.request)) const stage = createMemo(() => state().stage) const info = createMemo(() => permissionInfo(props.request, props.directory?.(), props.mono)) - const ft = createMemo(() => toolFiletype(info().file)) let scroll: ScrollBoxRenderable | undefined const scrollbar = createMemo(() => ({ visible: !props.mono, @@ -452,36 +451,40 @@ export function RunPermissionBody(props: { } > - = 40} - fallback={ - - {info().diff} - - } - > - - + + {(item) => ( + = 40} + fallback={ + + {item.diff} + + } + > + + + )} + diff --git a/packages/tui/src/mini/permission.shared.ts b/packages/tui/src/mini/permission.shared.ts index 38d2de31354a..f61022474cec 100644 --- a/packages/tui/src/mini/permission.shared.ts +++ b/packages/tui/src/mini/permission.shared.ts @@ -62,10 +62,11 @@ export function permissionInfo(request: MiniPermissionRequest, directory?: strin ...info, icon: monoPrefix(info.icon, true), lines: [ - ...(info.diff || info.patch ? [info.diff ?? info.patch!] : []), + ...(info.files ? info.files.map((item) => item.diff) : info.diff || info.patch ? [info.diff ?? info.patch!] : []), ...info.lines.map((line) => monoPrefix(line, true)), ], diff: undefined, + files: undefined, patch: undefined, } } diff --git a/packages/tui/src/routes/session/permission.tsx b/packages/tui/src/routes/session/permission.tsx index 07338aba69d8..2af8b93f940d 100644 --- a/packages/tui/src/routes/session/permission.tsx +++ b/packages/tui/src/routes/session/permission.tsx @@ -19,15 +19,21 @@ import { useToast } from "../../ui/toast" type PermissionStage = "permission" | "reject" -function EditBody(props: { file?: string; diff?: string; patch?: string }) { +function EditBody(props: { + file?: string + diff?: string + files?: { file: string; diff: string }[] + patch?: string + formatPath: (value: string) => string +}) { const theme = useTheme() const themes = useThemes() const syntax = themes.currentSyntax const config = useConfig().data const dimensions = useTerminalDimensions() - const filepath = createMemo(() => props.file ?? "") const diff = createMemo(() => props.diff ?? "") + const files = createMemo(() => props.files ?? [{ file: props.file ?? "", diff: diff() }]) const view = createMemo(() => { const diffView = config.diffs?.view @@ -36,7 +42,6 @@ function EditBody(props: { file?: string; diff?: string; patch?: string }) { return dimensions().width > 120 ? "split" : "unified" }) - const ft = createMemo(() => filetype(filepath())) const scrollAcceleration = createMemo(() => getScrollAcceleration(config)) return ( @@ -52,26 +57,37 @@ function EditBody(props: { file?: string; diff?: string; patch?: string }) { }, }} > - + + {(item) => ( + + + + {props.formatPath(item.file)} + + + + + )} + @@ -167,7 +183,13 @@ export function PermissionPrompt(props: { request: PermissionRequest; directory? ) const presentationBody = () => props.request.action === "edit" ? ( - + ) : props.request.action === "external_directory" ? ( 0}> diff --git a/packages/tui/src/util/permission.ts b/packages/tui/src/util/permission.ts index 09d607ff373e..eb3e9e5eb366 100644 --- a/packages/tui/src/util/permission.ts +++ b/packages/tui/src/util/permission.ts @@ -8,6 +8,7 @@ export type PermissionPresentation = { title: string lines: string[] diff?: string + files?: { file: string; diff: string }[] patch?: string file?: string } @@ -31,13 +32,18 @@ export function permissionPresentation( if (action === "edit") { const file = text(input.path) || resources[0] || "" - const first = dict(Array.isArray(metadata.files) ? metadata.files[0] : undefined) - const diff = text(first.patch) || text(first.diff) || text(metadata.diff) || undefined + const files = (Array.isArray(metadata.files) ? metadata.files : []).flatMap((item, index) => { + const entry = dict(item) + const diff = text(entry.patch) || text(entry.diff) + return diff ? [{ file: text(entry.filePath) || resources[index] || "", diff }] : [] + }) + const diff = files[0]?.diff || text(metadata.diff) || undefined return { icon: "→", - title: `Edit ${formatPath(file)}`, + title: files.length > 1 ? `Edit ${files.length} files` : `Edit ${formatPath(file)}`, lines: [], diff, + files: files.length > 1 ? files : undefined, patch: diff ? undefined : text(input.patchText) || undefined, file, } diff --git a/packages/tui/test/mini/permission.shared.test.ts b/packages/tui/test/mini/permission.shared.test.ts index 0b0af972745e..ed5efdee0aef 100644 --- a/packages/tui/test/mini/permission.shared.test.ts +++ b/packages/tui/test/mini/permission.shared.test.ts @@ -128,6 +128,25 @@ describe("run permission shared", () => { }) }) + test("mono mode keeps every file diff of a multi-file edit", () => { + expect( + permissionInfo( + req({ + action: "edit", + resources: ["a.ts", "b.ts"], + metadata: { + files: [ + { filePath: "a.ts", patch: "a-diff" }, + { filePath: "b.ts", patch: "b-diff" }, + ], + }, + }), + undefined, + true, + ), + ).toMatchObject({ title: "Edit 2 files", lines: ["a-diff", "b-diff"] }) + }) + test("prefers canonical request metadata over source tool metadata", () => { expect( permissionInfo( diff --git a/packages/tui/test/util/permission.test.ts b/packages/tui/test/util/permission.test.ts index 4bd7c7cae42a..c05a10f27abc 100644 --- a/packages/tui/test/util/permission.test.ts +++ b/packages/tui/test/util/permission.test.ts @@ -21,3 +21,19 @@ test("preserves permission roots and self-contained metadata", () => { }, ) }) + +test("edit permission exposes every file in a multi-file patch", () => { + const result = permissionPresentation({ + action: "edit", + resources: ["src/alpha.ts", "src/beta.ts", "src/gamma.ts"], + metadata: { + files: [ + { filePath: "src/alpha.ts", patch: "alpha-diff" }, + { filePath: "src/beta.ts", patch: "beta-diff" }, + { filePath: "src/gamma.ts", patch: "gamma-diff" }, + ], + }, + }) + expect(result.files?.map((item) => item.diff)).toEqual(["alpha-diff", "beta-diff", "gamma-diff"]) + expect(result.title).toBe("Edit 3 files") +}) From c895fae5528ac13dd61dff929fa3ff5a8270f17c Mon Sep 17 00:00:00 2001 From: drakeo338 Date: Wed, 14 Oct 2026 08:19:25 +0000 Subject: [PATCH 2/3] fix(tui): label multi-file edit diffs by their own file Read the label from the file entry (file, filePath, relativePath) instead of the positional resource, which was wrong when a move made resources longer than files. Show the mini footer diffs with the same per-file header, and fall back to the combined diff when an entry has no patch. --- packages/tui/src/mini/footer.permission.tsx | 67 +++++++++------- packages/tui/src/util/permission.ts | 10 ++- .../tui/test/cli/tui/permission.edit.test.tsx | 79 +++++++++++++++++++ .../tui/test/mini/blocker.responsive.test.tsx | 49 ++++++++++++ .../tui/test/mini/permission.shared.test.ts | 4 +- packages/tui/test/util/permission.test.ts | 50 ++++++++++-- 6 files changed, 219 insertions(+), 40 deletions(-) create mode 100644 packages/tui/test/cli/tui/permission.edit.test.tsx diff --git a/packages/tui/src/mini/footer.permission.tsx b/packages/tui/src/mini/footer.permission.tsx index daa82d4b751d..47f4404ad97c 100644 --- a/packages/tui/src/mini/footer.permission.tsx +++ b/packages/tui/src/mini/footer.permission.tsx @@ -29,7 +29,7 @@ import { type PermissionOption, } from "./permission.shared" import { stringWidth } from "../util/string-width" -import { toolFiletype } from "./tool" +import { toolFiletype, toolPath } from "./tool" import { transparent, type RunBlockTheme, type RunFooterTheme } from "./theme" import type { MiniPermissionRequest, PermissionReply } from "./types" import { PatchDiff } from "../component/patch-diff" @@ -453,36 +453,43 @@ export function RunPermissionBody(props: { > {(item) => ( - = 40} - fallback={ - - {item.diff} + + + + {toolPath(item.file, { home: true, directory: props.directory?.() })} - } - > - - + + = 40} + fallback={ + + {item.diff} + + } + > + + + )} diff --git a/packages/tui/src/util/permission.ts b/packages/tui/src/util/permission.ts index eb3e9e5eb366..402131f7c6c8 100644 --- a/packages/tui/src/util/permission.ts +++ b/packages/tui/src/util/permission.ts @@ -32,15 +32,19 @@ export function permissionPresentation( if (action === "edit") { const file = text(input.path) || resources[0] || "" - const files = (Array.isArray(metadata.files) ? metadata.files : []).flatMap((item, index) => { + const entries = Array.isArray(metadata.files) ? metadata.files : [] + const parsed = entries.flatMap((item) => { const entry = dict(item) const diff = text(entry.patch) || text(entry.diff) - return diff ? [{ file: text(entry.filePath) || resources[index] || "", diff }] : [] + return diff ? [{ file: text(entry.file) || text(entry.filePath) || text(entry.relativePath), diff }] : [] }) + // An entry without a patch cannot be shown on its own; fall back to the combined diff instead of hiding it. + const files = parsed.length === entries.length || !text(metadata.diff) ? parsed : [] const diff = files[0]?.diff || text(metadata.diff) || undefined + const count = Math.max(entries.length, parsed.length) return { icon: "→", - title: files.length > 1 ? `Edit ${files.length} files` : `Edit ${formatPath(file)}`, + title: count > 1 ? `Edit ${count} files` : `Edit ${formatPath(file)}`, lines: [], diff, files: files.length > 1 ? files : undefined, diff --git a/packages/tui/test/cli/tui/permission.edit.test.tsx b/packages/tui/test/cli/tui/permission.edit.test.tsx new file mode 100644 index 000000000000..9547b227522d --- /dev/null +++ b/packages/tui/test/cli/tui/permission.edit.test.tsx @@ -0,0 +1,79 @@ +/** @jsxImportSource @opentui/solid */ +import type { PermissionRequest } from "@opencode/client" +import { testRender } from "@opentui/solid" +import { expect, test } from "bun:test" +import { ConfigProvider } from "../../../src/config" +import { ClientProvider } from "../../../src/context/client" +import { DataProvider } from "../../../src/context/data" +import { Keymap } from "../../../src/context/keymap" +import { InteractivityProvider } from "../../../src/context/interactivity" +import { LocationProvider } from "../../../src/context/location" +import { ThemeProvider } from "../../../src/context/theme" +import { PermissionPrompt } from "../../../src/routes/session/permission" +import { ToastProvider } from "../../../src/ui/toast" +import { emptyThemeSource, tmpdir } from "../../fixture/fixture" +import { createApi, createEventStream, createFetch } from "../../fixture/tui-client" +import { TestTuiContexts } from "../../fixture/tui-environment" +import { createTuiResolvedConfig } from "../../fixture/tui-runtime" + +const file = (name: string, patch: string) => ({ file: name, patch, additions: 1, deletions: 1, status: "modified" }) + +async function render(root: string, request: PermissionRequest) { + const transport = createFetch(() => undefined, createEventStream()) + const app = await testRender( + () => ( + + + + + + + + + + + + + + + + + + + + ), + { width: 100, height: 40, kittyKeyboard: true }, + ) + app.renderer.start() + await app.renderOnce() + await app.renderOnce() + return app +} + +test("multi-file edit prompt labels each diff with its own file, including a move target", async () => { + await using tmp = await tmpdir() + // A move lists source and destination in resources, so resources are longer than the file entries. + const app = await render(tmp.path, { + id: "per_edit", + sessionID: "ses_edit", + action: "edit", + resources: ["src/old.ts", "src/new.ts", "src/other.ts"], + metadata: { + files: [ + file("src/new.ts", "@@ -1 +1 @@\n-one\n+FIRST_DIFF\n"), + file("src/other.ts", "@@ -1 +1 @@\n-two\n+SECOND_DIFF\n"), + ], + }, + } satisfies PermissionRequest) + try { + const frame = app.captureCharFrame() + expect(frame).toContain("Edit 2 files") + expect(frame).toContain("src/new.ts") + expect(frame).toContain("src/other.ts") + expect(frame).not.toContain("old.ts") + expect(frame).toContain("FIRST_DIFF") + expect(frame).toContain("SECOND_DIFF") + } finally { + app.renderer.destroy() + } +}) diff --git a/packages/tui/test/mini/blocker.responsive.test.tsx b/packages/tui/test/mini/blocker.responsive.test.tsx index b851f1b8bebe..fb8c763c6224 100644 --- a/packages/tui/test/mini/blocker.responsive.test.tsx +++ b/packages/tui/test/mini/blocker.responsive.test.tsx @@ -497,6 +497,55 @@ test("form review scrolls from the first answer to the last", async () => { } }) +test("multi-file edit permissions label each diff with its file", async () => { + const app = await testRender( + () => ( + "/project"} + theme={RUN_THEME_FALLBACK.footer} + block={RUN_THEME_FALLBACK.block} + onReply={() => {}} + /> + ), + { width: 100, height: 30 }, + ) + try { + await settle(app) + const frame = app.captureCharFrame() + expect(frame).toContain("Edit 2 files") + expect(frame).toContain("src/new.ts") + expect(frame).toContain("src/other.ts") + expect(frame).not.toContain("old.ts") + expect(frame).toContain("FIRST_DIFF") + expect(frame).toContain("SECOND_DIFF") + } finally { + app.renderer.destroy() + } +}) + test("long permission paths, diffs and persistent scopes remain keyboard accessible", async () => { const app = await testRender( () => ( diff --git a/packages/tui/test/mini/permission.shared.test.ts b/packages/tui/test/mini/permission.shared.test.ts index ed5efdee0aef..7dcf6168bd10 100644 --- a/packages/tui/test/mini/permission.shared.test.ts +++ b/packages/tui/test/mini/permission.shared.test.ts @@ -136,8 +136,8 @@ describe("run permission shared", () => { resources: ["a.ts", "b.ts"], metadata: { files: [ - { filePath: "a.ts", patch: "a-diff" }, - { filePath: "b.ts", patch: "b-diff" }, + { file: "a.ts", patch: "a-diff", additions: 1, deletions: 0, status: "modified" }, + { file: "b.ts", patch: "b-diff", additions: 1, deletions: 0, status: "modified" }, ], }, }), diff --git a/packages/tui/test/util/permission.test.ts b/packages/tui/test/util/permission.test.ts index c05a10f27abc..9489999e6199 100644 --- a/packages/tui/test/util/permission.test.ts +++ b/packages/tui/test/util/permission.test.ts @@ -22,18 +22,58 @@ test("preserves permission roots and self-contained metadata", () => { ) }) +const info = (file: string, patch: string) => ({ file, patch, additions: 1, deletions: 1, status: "modified" }) + test("edit permission exposes every file in a multi-file patch", () => { const result = permissionPresentation({ action: "edit", resources: ["src/alpha.ts", "src/beta.ts", "src/gamma.ts"], + metadata: { + files: [info("src/alpha.ts", "alpha-diff"), info("src/beta.ts", "beta-diff"), info("src/gamma.ts", "gamma-diff")], + }, + }) + expect(result.files).toEqual([ + { file: "src/alpha.ts", diff: "alpha-diff" }, + { file: "src/beta.ts", diff: "beta-diff" }, + { file: "src/gamma.ts", diff: "gamma-diff" }, + ]) + expect(result.title).toBe("Edit 3 files") +}) + +test("edit permission labels files by their own path when resources include a move source", () => { + // A move lists both the source and the destination in resources, so resources outnumber files. + const result = permissionPresentation({ + action: "edit", + resources: ["src/old.ts", "src/new.ts", "src/other.ts"], + metadata: { files: [info("src/new.ts", "moved-diff"), info("src/other.ts", "other-diff")] }, + }) + expect(result.files).toEqual([ + { file: "src/new.ts", diff: "moved-diff" }, + { file: "src/other.ts", diff: "other-diff" }, + ]) +}) + +test("edit permission shows the combined diff when a file entry has no patch", () => { + const result = permissionPresentation({ + action: "edit", + resources: ["a.ts", "b.ts"], + metadata: { diff: "combined-diff", files: [info("a.ts", "a-diff"), info("b.ts", "")] }, + }) + expect(result.title).toBe("Edit 2 files") + expect(result.files).toBeUndefined() + expect(result.diff).toBe("combined-diff") +}) + +test("edit permission still reads the legacy filePath label", () => { + const result = permissionPresentation({ + action: "edit", + resources: [], metadata: { files: [ - { filePath: "src/alpha.ts", patch: "alpha-diff" }, - { filePath: "src/beta.ts", patch: "beta-diff" }, - { filePath: "src/gamma.ts", patch: "gamma-diff" }, + { filePath: "a.ts", patch: "a-diff" }, + { filePath: "b.ts", patch: "b-diff" }, ], }, }) - expect(result.files?.map((item) => item.diff)).toEqual(["alpha-diff", "beta-diff", "gamma-diff"]) - expect(result.title).toBe("Edit 3 files") + expect(result.files?.map((item) => item.file)).toEqual(["a.ts", "b.ts"]) }) From 6eb73744271ed8b15ba23343f8bf223433a7a133 Mon Sep 17 00:00:00 2001 From: drakeo338 Date: Wed, 21 Oct 2026 12:33:38 +0000 Subject: [PATCH 3/3] fix(tui): show each file path above its diff in mono permission prompts In mono mode the multi-file edit prompt listed every diff with nothing saying which file it belonged to. Put the file path before each diff, as the non-mono rendering does. --- packages/tui/src/mini/permission.shared.ts | 6 +++++- packages/tui/test/mini/blocker.responsive.test.tsx | 9 ++++++--- packages/tui/test/mini/permission.shared.test.ts | 4 ++-- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/tui/src/mini/permission.shared.ts b/packages/tui/src/mini/permission.shared.ts index f61022474cec..06e9d3a00c70 100644 --- a/packages/tui/src/mini/permission.shared.ts +++ b/packages/tui/src/mini/permission.shared.ts @@ -62,7 +62,11 @@ export function permissionInfo(request: MiniPermissionRequest, directory?: strin ...info, icon: monoPrefix(info.icon, true), lines: [ - ...(info.files ? info.files.map((item) => item.diff) : info.diff || info.patch ? [info.diff ?? info.patch!] : []), + ...(info.files + ? info.files.flatMap((item) => [toolPath(item.file, { home: true, directory }), item.diff]) + : info.diff || info.patch + ? [info.diff ?? info.patch!] + : []), ...info.lines.map((line) => monoPrefix(line, true)), ], diff: undefined, diff --git a/packages/tui/test/mini/blocker.responsive.test.tsx b/packages/tui/test/mini/blocker.responsive.test.tsx index fb8c763c6224..1c177a86c646 100644 --- a/packages/tui/test/mini/blocker.responsive.test.tsx +++ b/packages/tui/test/mini/blocker.responsive.test.tsx @@ -497,7 +497,7 @@ test("form review scrolls from the first answer to the last", async () => { } }) -test("multi-file edit permissions label each diff with its file", async () => { +test.each([false, true])("multi-file edit permissions label each diff with its file (mono=%s)", async (mono) => { const app = await testRender( () => ( { }, }} directory={() => "/project"} - theme={RUN_THEME_FALLBACK.footer} - block={RUN_THEME_FALLBACK.block} + theme={mono ? RUN_THEME_MONO.footer : RUN_THEME_FALLBACK.footer} + block={mono ? RUN_THEME_MONO.block : RUN_THEME_FALLBACK.block} + mono={mono} onReply={() => {}} /> ), @@ -541,6 +542,8 @@ test("multi-file edit permissions label each diff with its file", async () => { expect(frame).not.toContain("old.ts") expect(frame).toContain("FIRST_DIFF") expect(frame).toContain("SECOND_DIFF") + const order = ["src/new.ts", "FIRST_DIFF", "src/other.ts", "SECOND_DIFF"].map((value) => frame.indexOf(value)) + expect(order).toEqual([...order].sort((a, b) => a - b)) } finally { app.renderer.destroy() } diff --git a/packages/tui/test/mini/permission.shared.test.ts b/packages/tui/test/mini/permission.shared.test.ts index 7dcf6168bd10..8eacada19a77 100644 --- a/packages/tui/test/mini/permission.shared.test.ts +++ b/packages/tui/test/mini/permission.shared.test.ts @@ -128,7 +128,7 @@ describe("run permission shared", () => { }) }) - test("mono mode keeps every file diff of a multi-file edit", () => { + test("mono mode keeps every file diff of a multi-file edit, each under its path", () => { expect( permissionInfo( req({ @@ -144,7 +144,7 @@ describe("run permission shared", () => { undefined, true, ), - ).toMatchObject({ title: "Edit 2 files", lines: ["a-diff", "b-diff"] }) + ).toMatchObject({ title: "Edit 2 files", lines: ["a.ts", "a-diff", "b.ts", "b-diff"] }) }) test("prefers canonical request metadata over source tool metadata", () => {