From b52b79d28fbe68e9d55bc692b5e0110573f17337 Mon Sep 17 00:00:00 2001 From: "opencode-agent[bot]" <219766164+opencode-agent[bot]@users.noreply.github.com> Date: Fri, 4 Sep 2026 19:47:57 +0800 Subject: [PATCH] fix(app): align file tool disclosures with Patch (#47269) Co-authored-by: Brendonovich <14191578+Brendonovich@users.noreply.github.com> --- .../component-tests/file-tool.spec.ts | 76 +++++++++ .../session-tool-projection.spec.ts | 23 ++- .../src/components/message-part.css | 96 ----------- .../src/timeline/file-changes.stories.tsx | 46 ++++++ .../session-ui/src/tools/tool-renderer.tsx | 155 ++++++++---------- 5 files changed, 203 insertions(+), 193 deletions(-) create mode 100644 packages/session-ui/component-tests/file-tool.spec.ts diff --git a/packages/session-ui/component-tests/file-tool.spec.ts b/packages/session-ui/component-tests/file-tool.spec.ts new file mode 100644 index 00000000000..4a207f54680 --- /dev/null +++ b/packages/session-ui/component-tests/file-tool.spec.ts @@ -0,0 +1,76 @@ +import { expect, story } from "../../storybook/playwright/story" + +for (const theme of ["light", "dark"]) { + story(`file tools share Patch's upfront file list in ${theme}`, async ({ mount }, info) => { + const root = await mount("current-session-research-agents--agent-research", { + args: { scenario: "workflow" }, + globals: { theme }, + }) + const group = root.locator('[data-component="collapsed-tool-group"]').filter({ hasText: "Patch" }) + const disclosure = group.getByRole("button", { name: /^Used .*Edit.*Write.*Patch$/ }) + await disclosure.click() + for (const name of ["edit", "write", "patch"]) { + const tool = group.locator(`[data-timeline-part-id="tool_family_${name}"]`) + const file = tool.locator('[data-slot="accordion-trigger"]') + await expect(file).toHaveAttribute("aria-expanded", "false") + await expect(tool.locator('[data-component="file"]')).toHaveCount(0) + await file.click() + await expect(file).toHaveAttribute("aria-expanded", "true") + await expect(tool.locator('[data-component="file"]')).toBeVisible() + await expect(file).toBeFocused() + await file.press("Space") + await expect(file).toHaveAttribute("aria-expanded", "false") + await file.press("Enter") + await expect(file).toHaveAttribute("aria-expanded", "true") + } + await disclosure.click() + await disclosure.click() + for (const name of ["edit", "write", "patch"]) { + const tool = group.locator(`[data-timeline-part-id="tool_family_${name}"]`) + await expect(tool.locator('[data-slot="accordion-trigger"]')).toHaveAttribute("aria-expanded", "true") + await expect(tool.locator('[data-component="file"]')).toBeVisible() + } + await root.screenshot({ path: info.outputPath(`file-tools-${theme}.png`) }) + }) +} + +for (const tool of ["edit", "write"]) { + for (const controlled of [false, true]) { + story(`${tool} supports forceOpen with ${controlled ? "controlled" : "local"} disclosure`, async ({ mount }) => { + const root = await mount("current-session-file-changes--file-tool-fallbacks", { + args: { tool, controlled, forceOpen: true }, + }) + await expect(root.getByRole("button", { name: /example\.ts/ })).toHaveAttribute("aria-expanded", "true") + await expect(root.locator('[data-component="file"]')).toBeVisible() + }) + } + + story(`${tool} preserves input fallback and disclosure through completion`, async ({ mount }) => { + const root = await mount("current-session-file-changes--file-tool-fallbacks", { args: { tool } }) + const file = root.getByRole("button", { name: /example\.ts/ }) + await expect(file).toBeVisible() + await expect(file).toHaveAttribute("aria-expanded", "false") + await expect(root.getByText("1 file", { exact: true })).toBeVisible() + await file.click() + await expect(root.locator('[data-component="file"]')).toContainText(tool === "edit" ? "after" : "written") + await root.getByRole("button", { name: "Complete file tool" }).click() + await expect(root.getByText("Example diagnostic")).toBeVisible() + await expect(file).toHaveAttribute("aria-expanded", "true") + await file.click() + await expect(file).toHaveAttribute("aria-expanded", "false") + await expect(root.getByText("Example diagnostic")).toBeVisible() + }) +} + +story("empty writes still show a file row", async ({ mount }) => { + const root = await mount("current-session-file-changes--file-tool-fallbacks", { + args: { tool: "write", empty: true }, + }) + await root.getByRole("button", { name: "Complete file tool" }).click() + const file = root.getByRole("button", { name: /example\.ts/ }) + await expect(file).toHaveAttribute("aria-expanded", "false") + await file.click() + await expect(file).toHaveAttribute("aria-expanded", "true") + await expect(root.locator('[data-component="file"]')).toBeAttached() + await expect(file).toBeVisible() +}) diff --git a/packages/session-ui/component-tests/session-tool-projection.spec.ts b/packages/session-ui/component-tests/session-tool-projection.spec.ts index dd54558d0f0..a898dfd847d 100644 --- a/packages/session-ui/component-tests/session-tool-projection.spec.ts +++ b/packages/session-ui/component-tests/session-tool-projection.spec.ts @@ -25,18 +25,17 @@ story("renders every admitted tool family and hides timeline-only exclusions", a ]) { await expect(timeline.locator(`[data-timeline-part-id="tool_family_${id}"]`), id).toBeVisible() } - const patch = timeline.locator('[data-timeline-part-id="tool_family_patch"]') - await expect(patch.getByText("1 file", { exact: true })).toBeVisible() - await expect(patch.getByRole("button", { name: "Patch 1 file", exact: true })).toHaveCount(0) - await expect(patch.getByRole("button")).toHaveCount(1) - await expect(patch.locator('[data-scope="apply-patch"] button')).toHaveAttribute("aria-expanded", "false") - await expect(patch.locator('[data-slot="message-part-title-filename"]')).toHaveCount(0) - await expect(patch.locator('[data-slot="message-part-actions"]')).toHaveCount(0) - await expect(patch.locator('[data-slot="basic-tool-tool-title"]')).toHaveCSS("font-size", "13px") - const edit = timeline.locator('[data-timeline-part-id="tool_family_edit"]') - await expect(edit).toContainText("Edit") - await expect(edit.locator('[data-slot="message-part-title"]')).toHaveCSS("font-size", "13px") - await expect(edit.locator('[data-slot="message-part-title"]')).toHaveCSS("line-height", "16px") + for (const name of ["edit", "write", "patch"]) { + const tool = timeline.locator(`[data-timeline-part-id="tool_family_${name}"]`) + await expect(tool.getByText("1 file", { exact: true })).toBeVisible() + await expect(tool.getByRole("button")).toHaveCount(1) + await expect(tool.locator('[data-scope="apply-patch"] button')).toHaveAttribute("aria-expanded", "false") + await expect(tool.locator('[data-slot="collapsible-trigger"]')).toHaveAttribute("data-locked", "") + await expect(tool.locator('[data-slot="message-part-title-filename"]')).toHaveCount(0) + await expect(tool.locator('[data-slot="message-part-actions"]')).toHaveCount(0) + await expect(tool.locator('[data-slot="basic-tool-tool-title"]')).toHaveCSS("font-size", "13px") + await expect(tool.locator('[data-slot="basic-tool-tool-title"]')).toHaveCSS("line-height", "16px") + } await expect(timeline.locator('[data-timeline-part-id="tool_family_todo"]')).toHaveCount(0) }) diff --git a/packages/session-ui/src/components/message-part.css b/packages/session-ui/src/components/message-part.css index a6626e94c84..eeebe0fab7d 100644 --- a/packages/session-ui/src/components/message-part.css +++ b/packages/session-ui/src/components/message-part.css @@ -466,102 +466,6 @@ --tool-content-gap: 8px; } -[data-component="edit-trigger"], -[data-component="write-trigger"] { - display: flex; - align-items: center; - justify-content: space-between; - gap: 12px; - width: 100%; - - [data-slot="message-part-title-area"] { - flex-grow: 1; - display: flex; - align-items: center; - gap: 8px; - min-width: 0; - } - - [data-slot="message-part-title"] { - flex: 1 1 auto; - display: flex; - align-items: center; - gap: 8px; - min-width: 0; - font-family: var(--font-family-sans); - font-size: 13px; - font-style: normal; - font-weight: var(--font-weight-medium); - line-height: var(--line-height-compact); - letter-spacing: var(--letter-spacing-normal); - color: var(--v2-text-text-muted); - } - - [data-slot="message-part-title-spinner"] { - margin-left: 4px; - width: 16px; - height: 16px; - display: inline-flex; - align-items: center; - justify-content: center; - flex-shrink: 0; - color: var(--v2-text-text-muted); - - [data-component="spinner"] { - width: 16px; - height: 16px; - } - } - - [data-slot="message-part-title-text"] { - flex-shrink: 0; - text-transform: capitalize; - color: var(--v2-text-text-base); - } - - [data-slot="message-part-title-filename"] { - /* No text-transform - preserve original filename casing */ - min-width: 0; - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - font-weight: var(--font-weight-regular); - direction: ltr; - unicode-bidi: isolate; - } - - [data-slot="message-part-path"] { - display: flex; - flex-grow: 1; - min-width: 0; - font-weight: var(--font-weight-regular); - direction: ltr; - unicode-bidi: isolate; - } - - [data-slot="message-part-directory"] { - color: var(--v2-text-text-muted); - text-overflow: ellipsis; - overflow: hidden; - white-space: nowrap; - direction: rtl; - text-align: left; - } - - [data-slot="message-part-filename"] { - color: var(--v2-text-text-base); - flex-shrink: 0; - } - - [data-slot="message-part-actions"] { - display: flex; - gap: 16px; - align-items: center; - justify-content: flex-end; - flex-shrink: 0; - } -} - [data-component="edit-content"] { border-radius: inherit; border-top: 0.5px solid var(--v2-border-border-muted); diff --git a/packages/session-ui/src/timeline/file-changes.stories.tsx b/packages/session-ui/src/timeline/file-changes.stories.tsx index 96e2f27c412..b9445a0e46d 100644 --- a/packages/session-ui/src/timeline/file-changes.stories.tsx +++ b/packages/session-ui/src/timeline/file-changes.stories.tsx @@ -11,6 +11,7 @@ import { } from "../storybook/current-session-fixtures" import { storyDocument, storyPatchFile, storyTool } from "../storybook/current-session-scenarios" import { SessionTimeline } from "./session-timeline" +import { ToolDisplay } from "../tools/tool-renderer" export default { title: "OpenCode/Work/File changes", @@ -261,3 +262,48 @@ export const CreatedANewFile = { /> ), } + +export const FileToolFallbacks = { + args: { tool: "edit", empty: false, forceOpen: false, controlled: true }, + argTypes: { + tool: { control: "select", options: ["edit", "write"] }, + empty: { control: "boolean" }, + forceOpen: { control: "boolean" }, + controlled: { control: "boolean" }, + }, + render: (args: { tool: string; empty: boolean; forceOpen: boolean; controlled: boolean }) => { + const [state, setState] = createStore({ completed: false, open: false }) + return ( +
+ + + setState("open", open)} + forceOpen={args.forceOpen} + /> + +
+ ) + }, +} diff --git a/packages/session-ui/src/tools/tool-renderer.tsx b/packages/session-ui/src/tools/tool-renderer.tsx index 9c0506f6c0c..a672949ec24 100644 --- a/packages/session-ui/src/tools/tool-renderer.tsx +++ b/packages/session-ui/src/tools/tool-renderer.tsx @@ -951,20 +951,66 @@ export const ToolRegistry = { render: getTool, } +function FileTool(props: ToolProps & { title: string; count: number; children: JSX.Element }) { + const i18n = useI18n() + return ( + 0 ? `${props.count} ${i18n.plural("ui.common.file", props.count)}` : "", + }} + > + {props.children} + + ) +} + function ToolFileAccordion(props: { path: string actions?: JSX.Element children: JSX.Element defaultOpen?: boolean + open?: boolean + onOpenChange?: (open: boolean) => void + forceOpen?: boolean + defer?: boolean }) { const value = createMemo(() => props.path || "tool-file") + const [expanded, setExpanded] = createSignal(props.defaultOpen ?? false) + const [visible, setVisible] = createSignal(false) + const open = () => props.open ?? expanded() + const change = (value: boolean) => { + if (props.open === undefined) setExpanded(value) + props.onOpenChange?.(value) + } + + createEffect(() => { + if (props.forceOpen && !open()) change(true) + }) + + createEffect(() => { + if (!open()) { + setVisible(false) + return + } + const frame = requestAnimationFrame(() => setVisible(true)) + onCleanup(() => cancelAnimationFrame(frame)) + }) return ( change(next.includes(value()))} > @@ -986,7 +1032,9 @@ function ToolFileAccordion(props: { - {props.children} + + {props.children} + ) @@ -1709,7 +1757,6 @@ ToolRegistry.register({ const value = diff() return typeof value?.file === "string" ? value.file : inputPath() }) - const filename = () => getFilename(inputPath()) const pending = () => props.status === "streaming" || props.status === "running" const diffSource = createMemo( () => { @@ -1749,39 +1796,15 @@ ToolRegistry.register({ return (
- -
-
- - - - - {filename()} - -
- -
- {displayDirectory(inputPath())} -
- -
-
- - {(diff) => } - -
-
- } - > + {(diff) => } @@ -1800,7 +1823,7 @@ ToolRegistry.register({ - + ) }, @@ -1814,38 +1837,18 @@ ToolRegistry.register({ const path = createMemo(() => (typeof props.input.path === "string" ? props.input.path : "")) const content = createMemo(() => (typeof props.input.content === "string" ? props.input.content : "")) const diagnostics = createMemo(() => getDiagnostics(props.metadata.diagnostics, path())) - const filename = () => getFilename(path()) - const pending = () => props.status === "streaming" || props.status === "running" return (
- -
-
- - - - - {filename()} - -
- -
- {displayDirectory(path())} -
- -
-
{/* */}
-
- } - > - - + + +
- +
) }, @@ -1891,27 +1894,9 @@ ToolRegistry.register({ files().forEach((file) => props.onFileOpenChange?.(file.path, next.includes(file.path))) } - const subtitle = createMemo(() => { - const count = files().length - if (count === 0) return "" - return `${count} ${i18n.plural("ui.common.file", count)}` - }) - return (
- + 0}> - +
) },