fix(acp): clean read tool display content (#30569)
This commit is contained in:
parent
e707e416ed
commit
8851e4de2b
@ -98,12 +98,14 @@ export function toLocations(toolName: string, input: ToolInput): ToolCallLocatio
|
||||
}
|
||||
|
||||
export function completedToolContent(toolName: string, state: CompletedToolState): ToolCallContent[] {
|
||||
const text =
|
||||
toolName.toLocaleLowerCase() === "read" ? (readDisplayText(state.metadata) ?? state.output) : state.output
|
||||
const content: ToolCallContent[] = [
|
||||
{
|
||||
type: "content",
|
||||
content: {
|
||||
type: "text",
|
||||
text: state.output,
|
||||
text,
|
||||
},
|
||||
},
|
||||
]
|
||||
@ -288,6 +290,18 @@ function diffContent(input: ToolInput): ToolCallContent[] {
|
||||
]
|
||||
}
|
||||
|
||||
function readDisplayText(metadata: unknown) {
|
||||
if (!metadata || typeof metadata !== "object") return undefined
|
||||
const display = (metadata as Record<string, unknown>).display
|
||||
if (!display || typeof display !== "object") return undefined
|
||||
const info = display as Record<string, unknown>
|
||||
if (info.type === "file") return stringValue(info.text)
|
||||
if (info.type === "directory" && Array.isArray(info.entries)) {
|
||||
return info.entries.filter((item): item is string => typeof item === "string").join("\n")
|
||||
}
|
||||
return undefined
|
||||
}
|
||||
|
||||
function dataUrlImage(attachment: ToolAttachment) {
|
||||
const match = stringValue(attachment.url)?.match(/^data:([^;,]+)(?:;[^,]*)*;base64,(.*)$/)
|
||||
const mime = match?.[1] ?? stringValue(attachment.mime)
|
||||
|
||||
@ -36,7 +36,37 @@ export const Parameters = Schema.Struct({
|
||||
}),
|
||||
})
|
||||
|
||||
export const ReadTool = Tool.define(
|
||||
type Display =
|
||||
| {
|
||||
type: "directory"
|
||||
path: string
|
||||
entries: string[]
|
||||
offset: number
|
||||
totalEntries: number
|
||||
truncated: boolean
|
||||
}
|
||||
| {
|
||||
type: "file"
|
||||
path: string
|
||||
text: string
|
||||
lineStart: number
|
||||
lineEnd: number
|
||||
totalLines: number
|
||||
truncated: boolean
|
||||
}
|
||||
|
||||
type Metadata = {
|
||||
preview: string
|
||||
truncated: boolean
|
||||
loaded: string[]
|
||||
display?: Display
|
||||
}
|
||||
|
||||
export const ReadTool = Tool.define<
|
||||
typeof Parameters,
|
||||
Metadata,
|
||||
FSUtil.Service | Instruction.Service | LSP.Service | Reference.Service | Scope.Scope
|
||||
>(
|
||||
"read",
|
||||
Effect.gen(function* () {
|
||||
const fs = yield* FSUtil.Service
|
||||
@ -200,7 +230,7 @@ export const ReadTool = Tool.define(
|
||||
|
||||
const run = Effect.fn("ReadTool.execute")(function* (
|
||||
params: Schema.Schema.Type<typeof Parameters>,
|
||||
ctx: Tool.Context,
|
||||
ctx: Tool.Context<Metadata>,
|
||||
) {
|
||||
const instance = yield* InstanceState.context
|
||||
let filepath = params.filePath
|
||||
@ -258,6 +288,14 @@ export const ReadTool = Tool.define(
|
||||
preview: sliced.slice(0, 20).join("\n"),
|
||||
truncated,
|
||||
loaded: [] as string[],
|
||||
display: {
|
||||
type: "directory" as const,
|
||||
path: filepath,
|
||||
entries: sliced,
|
||||
offset,
|
||||
totalEntries: items.length,
|
||||
truncated,
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
@ -328,6 +366,15 @@ export const ReadTool = Tool.define(
|
||||
preview: file.raw.slice(0, 20).join("\n"),
|
||||
truncated,
|
||||
loaded: loaded.map((item) => item.filepath),
|
||||
display: {
|
||||
type: "file" as const,
|
||||
path: filepath,
|
||||
text: file.raw.join("\n"),
|
||||
lineStart: file.offset,
|
||||
lineEnd: last,
|
||||
totalLines: file.count,
|
||||
truncated,
|
||||
},
|
||||
},
|
||||
}
|
||||
})
|
||||
@ -335,7 +382,7 @@ export const ReadTool = Tool.define(
|
||||
return {
|
||||
description: DESCRIPTION,
|
||||
parameters: Parameters,
|
||||
execute: (params: Schema.Schema.Type<typeof Parameters>, ctx: Tool.Context) =>
|
||||
execute: (params: Schema.Schema.Type<typeof Parameters>, ctx: Tool.Context<Metadata>) =>
|
||||
run(params, ctx).pipe(Effect.orDie),
|
||||
}
|
||||
}),
|
||||
|
||||
@ -251,6 +251,11 @@ function completedTool(
|
||||
callID: string,
|
||||
output = "done",
|
||||
attachments: Extract<ToolPart["state"], { status: "completed" }>["attachments"] = [],
|
||||
options: {
|
||||
readonly tool?: string
|
||||
readonly input?: Record<string, unknown>
|
||||
readonly metadata?: Record<string, unknown>
|
||||
} = {},
|
||||
) {
|
||||
return {
|
||||
id: `part_${callID}`,
|
||||
@ -258,13 +263,13 @@ function completedTool(
|
||||
messageID: `msg_${callID}`,
|
||||
type: "tool",
|
||||
callID,
|
||||
tool: "bash",
|
||||
tool: options.tool ?? "bash",
|
||||
state: {
|
||||
status: "completed",
|
||||
input: { cmd: "printf done" },
|
||||
input: options.input ?? { cmd: "printf done" },
|
||||
output,
|
||||
title: "bash",
|
||||
metadata: { exit: 0 },
|
||||
metadata: options.metadata ?? { exit: 0 },
|
||||
time: { start: Date.now() - 1, end: Date.now() },
|
||||
...(attachments.length ? { attachments } : {}),
|
||||
},
|
||||
@ -605,6 +610,55 @@ describe("acp event routing", () => {
|
||||
})
|
||||
})
|
||||
|
||||
it("emits clean read display content and preserves rawOutput", async () => {
|
||||
const harness = createHarness()
|
||||
await Effect.runPromise(harness.session.create({ id: "ses_read", cwd: "/workspace" }))
|
||||
const output = [
|
||||
"<path>/workspace/file.ts</path>",
|
||||
"<type>file</type>",
|
||||
"<content>",
|
||||
"1: import { value } from './value'",
|
||||
"2: export { value }",
|
||||
"",
|
||||
"(End of file - total 2 lines)",
|
||||
"</content>",
|
||||
].join("\n")
|
||||
const metadata = {
|
||||
display: {
|
||||
type: "file",
|
||||
path: "/workspace/file.ts",
|
||||
text: "import { value } from './value'\nexport { value }",
|
||||
lineStart: 1,
|
||||
lineEnd: 2,
|
||||
totalLines: 2,
|
||||
truncated: false,
|
||||
},
|
||||
}
|
||||
|
||||
await harness.subscription.handle(
|
||||
toolUpdated(
|
||||
completedTool("ses_read", "call_read", output, [], {
|
||||
tool: "read",
|
||||
input: { filePath: "/workspace/file.ts" },
|
||||
metadata,
|
||||
}),
|
||||
),
|
||||
)
|
||||
|
||||
expect(harness.updates.at(-1)?.update).toMatchObject({
|
||||
sessionUpdate: "tool_call_update",
|
||||
toolCallId: "call_read",
|
||||
status: "completed",
|
||||
content: [
|
||||
{
|
||||
type: "content",
|
||||
content: { type: "text", text: "import { value } from './value'\nexport { value }" },
|
||||
},
|
||||
],
|
||||
rawOutput: { output, metadata },
|
||||
})
|
||||
})
|
||||
|
||||
it("emits error tool output", async () => {
|
||||
const harness = createHarness()
|
||||
await Effect.runPromise(harness.session.create({ id: "ses_error", cwd: "/workspace" }))
|
||||
|
||||
@ -104,6 +104,46 @@ describe("acp tool conversion", () => {
|
||||
])
|
||||
})
|
||||
|
||||
test("uses clean read display text for completed content", () => {
|
||||
const output = [
|
||||
"<path>/tmp/file.ts</path>",
|
||||
"<type>file</type>",
|
||||
"<content>",
|
||||
"7: first",
|
||||
"8: second",
|
||||
"",
|
||||
"(End of file - total 8 lines)",
|
||||
"</content>",
|
||||
].join("\n")
|
||||
const state = {
|
||||
status: "completed" as const,
|
||||
input: { filePath: "/tmp/file.ts" },
|
||||
output,
|
||||
metadata: {
|
||||
display: {
|
||||
type: "file",
|
||||
path: "/tmp/file.ts",
|
||||
text: "first\nsecond",
|
||||
lineStart: 7,
|
||||
lineEnd: 8,
|
||||
totalLines: 8,
|
||||
truncated: false,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
expect(completedToolContent("read", state)).toEqual([
|
||||
{
|
||||
type: "content",
|
||||
content: { type: "text", text: "first\nsecond" },
|
||||
},
|
||||
])
|
||||
expect(completedToolRawOutput(state)).toEqual({
|
||||
output,
|
||||
metadata: state.metadata,
|
||||
})
|
||||
})
|
||||
|
||||
test("builds completed raw output with optional metadata and attachments", () => {
|
||||
const attachments = [
|
||||
{
|
||||
|
||||
@ -428,6 +428,15 @@ describe("tool.read truncation", () => {
|
||||
const result = yield* run({ filePath: path.join(test.directory, "small.txt") })
|
||||
expect(result.metadata.truncated).toBe(false)
|
||||
expect(result.output).toContain("End of file")
|
||||
expect(result.metadata.display).toMatchObject({
|
||||
type: "file",
|
||||
path: path.join(test.directory, "small.txt"),
|
||||
text: "hello world",
|
||||
lineStart: 1,
|
||||
lineEnd: 1,
|
||||
totalLines: 1,
|
||||
truncated: false,
|
||||
})
|
||||
}),
|
||||
)
|
||||
|
||||
@ -495,6 +504,14 @@ describe("tool.read truncation", () => {
|
||||
const result = yield* exec(dir, { filePath: path.join(dir, "dir"), offset: 6, limit: 5 })
|
||||
expect(result.metadata.truncated).toBe(false)
|
||||
expect(result.output).not.toContain("Showing 5 of 10 entries")
|
||||
expect(result.metadata.display).toMatchObject({
|
||||
type: "directory",
|
||||
path: path.join(dir, "dir"),
|
||||
entries: ["file-5.txt", "file-6.txt", "file-7.txt", "file-8.txt", "file-9.txt"],
|
||||
offset: 6,
|
||||
totalEntries: 10,
|
||||
truncated: false,
|
||||
})
|
||||
}),
|
||||
)
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user