diff --git a/src/core/prompts/tools/native-tools/read_command_output.ts b/src/core/prompts/tools/native-tools/read_command_output.ts index 5bd8bc4575..af5148cd10 100644 --- a/src/core/prompts/tools/native-tools/read_command_output.ts +++ b/src/core/prompts/tools/native-tools/read_command_output.ts @@ -49,7 +49,11 @@ export default { function: { name: "read_command_output", description: READ_COMMAND_OUTPUT_DESCRIPTION, - strict: true, + // Note: strict mode is intentionally disabled for this tool. + // With strict: true, OpenAI requires ALL properties to be in the 'required' array, + // which forces the LLM to always provide explicit values (even null) for optional params. + // This creates verbose tool calls and poor UX. By disabling strict mode, the LLM can + // omit optional parameters entirely, making the tool easier to use. parameters: { type: "object", properties: { @@ -58,21 +62,19 @@ export default { description: ARTIFACT_ID_DESCRIPTION, }, search: { - type: ["string", "null"], + type: "string", description: SEARCH_DESCRIPTION, }, offset: { - type: ["number", "null"], + type: "number", description: OFFSET_DESCRIPTION, }, limit: { - type: ["number", "null"], + type: "number", description: LIMIT_DESCRIPTION, }, }, - // With strict: true, ALL properties must be listed in required. - // Optional params use union type with null (e.g., ["string", "null"]). - required: ["artifact_id", "search", "offset", "limit"], + required: ["artifact_id"], additionalProperties: false, }, }, diff --git a/src/core/tools/ExecuteCommandTool.ts b/src/core/tools/ExecuteCommandTool.ts index 26f01fc736..bc16886ded 100644 --- a/src/core/tools/ExecuteCommandTool.ts +++ b/src/core/tools/ExecuteCommandTool.ts @@ -263,10 +263,10 @@ export async function executeCommandInTerminal( if (interceptor) { persistedResult = await interceptor.finalize() } - + // Continue using compressed output for UI display result = Terminal.compressTerminalOutput(output ?? "") - + task.say("command_output", result) completed = true } finally { diff --git a/src/integrations/terminal/OutputInterceptor.ts b/src/integrations/terminal/OutputInterceptor.ts index 0910697f83..d1725c6426 100644 --- a/src/integrations/terminal/OutputInterceptor.ts +++ b/src/integrations/terminal/OutputInterceptor.ts @@ -288,7 +288,10 @@ export class OutputInterceptor { /** * Finalize the interceptor and return the persisted output result. * - * Closes any open file streams and returns a summary object containing: + * Closes any open file streams and waits for them to fully flush before returning. + * This ensures the artifact file is completely written and ready for reading. + * + * Returns a summary object containing: * - A preview of the output (head + [omitted indicator] + tail) * - The total byte count of all output * - The path to the full output file (if truncated) @@ -298,7 +301,7 @@ export class OutputInterceptor { * * @example * ```typescript - * const result = interceptor.finalize(); + * const result = await interceptor.finalize(); * console.log(`Preview: ${result.preview}`); * console.log(`Total bytes: ${result.totalBytes}`); * if (result.truncated) { @@ -306,10 +309,14 @@ export class OutputInterceptor { * } * ``` */ - finalize(): PersistedCommandOutput { - // Close write stream if open + async finalize(): Promise { + // Close write stream if open and wait for it to fully flush. + // This ensures the artifact is completely written before we advertise the artifact_id. if (this.writeStream) { - this.writeStream.end() + await new Promise((resolve, reject) => { + this.writeStream!.end(() => resolve()) + this.writeStream!.on("error", reject) + }) } // Prepare preview: head + [omission indicator] + tail diff --git a/src/integrations/terminal/__tests__/OutputInterceptor.test.ts b/src/integrations/terminal/__tests__/OutputInterceptor.test.ts index b8ca5a251d..c91253d6ed 100644 --- a/src/integrations/terminal/__tests__/OutputInterceptor.test.ts +++ b/src/integrations/terminal/__tests__/OutputInterceptor.test.ts @@ -34,10 +34,14 @@ describe("OutputInterceptor", () => { storageDir = path.normalize("/tmp/test-storage") - // Setup mock write stream + // Setup mock write stream with callback support for end() mockWriteStream = { write: vi.fn(), - end: vi.fn(), + end: vi.fn((callback?: () => void) => { + // Immediately call the callback to simulate stream flush completing + if (callback) callback() + }), + on: vi.fn(), } vi.mocked(fs.existsSync).mockReturnValue(true) @@ -49,7 +53,7 @@ describe("OutputInterceptor", () => { }) describe("Buffering behavior", () => { - it("should keep small output in memory without spilling to disk", () => { + it("should keep small output in memory without spilling to disk", async () => { const interceptor = new OutputInterceptor({ executionId: "12345", taskId: "task-1", @@ -64,7 +68,7 @@ describe("OutputInterceptor", () => { expect(interceptor.hasSpilledToDisk()).toBe(false) expect(fs.createWriteStream).not.toHaveBeenCalled() - const result = interceptor.finalize() + const result = await interceptor.finalize() expect(result.preview).toBe(smallOutput) expect(result.truncated).toBe(false) expect(result.artifactPath).toBe(null) @@ -94,7 +98,7 @@ describe("OutputInterceptor", () => { expect(mockWriteStream.write).toHaveBeenCalled() }) - it("should truncate preview after spilling to disk using head/tail split", () => { + it("should truncate preview after spilling to disk using head/tail split", async () => { const interceptor = new OutputInterceptor({ executionId: "12345", taskId: "task-1", @@ -109,7 +113,7 @@ describe("OutputInterceptor", () => { expect(interceptor.hasSpilledToDisk()).toBe(true) - const result = interceptor.finalize() + const result = await interceptor.finalize() expect(result.truncated).toBe(true) expect(result.artifactPath).toBe(path.join(storageDir, "cmd-12345.txt")) // Preview is head (1024) + omission indicator + tail (1024) @@ -268,7 +272,7 @@ describe("OutputInterceptor", () => { }) describe("finalize() method", () => { - it("should return preview output for small commands", () => { + it("should return preview output for small commands", async () => { const interceptor = new OutputInterceptor({ executionId: "12345", taskId: "task-1", @@ -280,7 +284,7 @@ describe("OutputInterceptor", () => { const output = "Hello World\n" interceptor.write(output) - const result = interceptor.finalize() + const result = await interceptor.finalize() expect(result.preview).toBe(output) expect(result.totalBytes).toBe(Buffer.byteLength(output, "utf8")) @@ -288,7 +292,7 @@ describe("OutputInterceptor", () => { expect(result.truncated).toBe(false) }) - it("should return PersistedCommandOutput for large commands with head/tail preview", () => { + it("should return PersistedCommandOutput for large commands with head/tail preview", async () => { const interceptor = new OutputInterceptor({ executionId: "12345", taskId: "task-1", @@ -300,7 +304,7 @@ describe("OutputInterceptor", () => { const largeOutput = "x".repeat(5000) interceptor.write(largeOutput) - const result = interceptor.finalize() + const result = await interceptor.finalize() expect(result.truncated).toBe(true) expect(result.artifactPath).toBe(path.join(storageDir, "cmd-12345.txt")) @@ -310,7 +314,7 @@ describe("OutputInterceptor", () => { expect(result.preview).toContain("bytes omitted...]") }) - it("should close write stream when finalizing", () => { + it("should close write stream when finalizing", async () => { const interceptor = new OutputInterceptor({ executionId: "12345", taskId: "task-1", @@ -321,12 +325,12 @@ describe("OutputInterceptor", () => { // Trigger spill interceptor.write("x".repeat(3000)) - interceptor.finalize() + await interceptor.finalize() expect(mockWriteStream.end).toHaveBeenCalled() }) - it("should include correct metadata (artifactId, size, truncated flag)", () => { + it("should include correct metadata (artifactId, size, truncated flag)", async () => { const interceptor = new OutputInterceptor({ executionId: "12345", taskId: "task-1", @@ -338,7 +342,7 @@ describe("OutputInterceptor", () => { const output = "x".repeat(5000) interceptor.write(output) - const result = interceptor.finalize() + const result = await interceptor.finalize() expect(result).toHaveProperty("preview") expect(result).toHaveProperty("totalBytes", 5000) @@ -432,7 +436,7 @@ describe("OutputInterceptor", () => { }) describe("Head/Tail split behavior", () => { - it("should preserve first 50% and last 50% of output", () => { + it("should preserve first 50% and last 50% of output", async () => { const interceptor = new OutputInterceptor({ executionId: "12345", taskId: "task-1", @@ -450,7 +454,7 @@ describe("OutputInterceptor", () => { interceptor.write(middleContent) interceptor.write(tailContent) - const result = interceptor.finalize() + const result = await interceptor.finalize() // Should start with HEAD content (first 1024 bytes of head budget) expect(result.preview.startsWith("HEAD")).toBe(true) @@ -461,7 +465,7 @@ describe("OutputInterceptor", () => { expect(result.preview).toContain("bytes omitted...]") }) - it("should not add omission indicator when output fits in budget", () => { + it("should not add omission indicator when output fits in budget", async () => { const interceptor = new OutputInterceptor({ executionId: "12345", taskId: "task-1", @@ -473,14 +477,14 @@ describe("OutputInterceptor", () => { const smallOutput = "Hello World\n" interceptor.write(smallOutput) - const result = interceptor.finalize() + const result = await interceptor.finalize() // No omission indicator for small output expect(result.preview).toBe(smallOutput) expect(result.preview).not.toContain("[...") }) - it("should handle output that exactly fills head budget", () => { + it("should handle output that exactly fills head budget", async () => { const interceptor = new OutputInterceptor({ executionId: "12345", taskId: "task-1", @@ -493,14 +497,14 @@ describe("OutputInterceptor", () => { const exactHeadContent = "x".repeat(1024) interceptor.write(exactHeadContent) - const result = interceptor.finalize() + const result = await interceptor.finalize() // Should fit entirely in head, no truncation expect(result.preview).toBe(exactHeadContent) expect(result.truncated).toBe(false) }) - it("should split single large chunk across head and tail", () => { + it("should split single large chunk across head and tail", async () => { const interceptor = new OutputInterceptor({ executionId: "12345", taskId: "task-1", @@ -514,7 +518,7 @@ describe("OutputInterceptor", () => { const content = "A".repeat(1024) + "B".repeat(2000) + "C".repeat(1024) interceptor.write(content) - const result = interceptor.finalize() + const result = await interceptor.finalize() // Head should have A's expect(result.preview.startsWith("A")).toBe(true)