Repository navigation
fix(coding-agent): keep Bun console output out of the RPC/JSON stdout stream #284
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
32ec94a
fix(coding-agent): keep Bun console output out of the RPC/JSON stdout…
opensec-intelligence befba93
fix(coding-agent): use a top-level type import in the RPC console test
opensec-intelligence 27d6744
fix: read the Windows selftest extension's console.log from stderr
opensec-intelligence File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
40 changes: 40 additions & 0 deletions
40
packages/coding-agent/test/fixtures/rpc-console-session.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| // Run by test/suite/regressions/rpc-console-stdout.test.ts under Bun: an RPC session (faux provider) whose extension | ||
| // prints with every console method that writes to stdout. Commands come on stdin; the session ends when stdin closes. | ||
| import { fauxAssistantMessage } from "@earendil-works/pi-ai"; | ||
| import type { AgentSessionRuntime } from "../../src/core/agent-session-runtime.ts"; | ||
| import type { ExtensionAPI } from "../../src/core/extensions/types.ts"; | ||
| import { takeOverStdout } from "../../src/core/output-guard.ts"; | ||
| import { runRpcMode } from "../../src/modes/rpc/rpc-mode.ts"; | ||
| import { createHarness } from "../suite/harness.ts"; | ||
|
|
||
| // As main.ts does: stdout is taken over before extensions load. | ||
| takeOverStdout(); | ||
| const harness = await createHarness({ | ||
| extensionFactories: [ | ||
| (pi: ExtensionAPI) => { | ||
| console.log("console-marker extension loaded", { a: 1 }); | ||
| pi.on("agent_start", () => { | ||
| console.log("console-marker log %s", "x"); | ||
| console.info("console-marker info"); | ||
| console.debug("console-marker debug"); | ||
| console.dir({ marker: "console-marker dir" }); | ||
| console.dirxml("console-marker dirxml"); | ||
| console.table([{ marker: "console-marker table" }]); | ||
| console.trace("console-marker trace"); | ||
| console.count("console-marker count"); | ||
| console.group("console-marker group"); | ||
| console.log("console-marker in group"); | ||
| console.groupEnd(); | ||
| (console as { write?: (text: string) => void }).write?.("console-marker write\n"); | ||
| }); | ||
| }, | ||
| ], | ||
| }); | ||
| harness.setResponses([fauxAssistantMessage("done")]); | ||
| process.on("exit", () => harness.cleanup()); | ||
|
|
||
| await runRpcMode({ | ||
| session: harness.session, | ||
| dispose: async () => {}, | ||
| setRebindSession: () => {}, | ||
| } as unknown as AgentSessionRuntime); |
134 changes: 134 additions & 0 deletions
134
packages/coding-agent/test/suite/regressions/rpc-console-stdout.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| import { spawn, spawnSync } from "node:child_process"; | ||
| import { Console } from "node:console"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import { fauxAssistantMessage } from "@earendil-works/pi-ai"; | ||
| import { afterEach, describe, expect, test, vi } from "vitest"; | ||
| import type { AgentSessionRuntime } from "../../../src/core/agent-session-runtime.ts"; | ||
| import { restoreStdout } from "../../../src/core/output-guard.ts"; | ||
| import { runRpcMode } from "../../../src/modes/rpc/rpc-mode.ts"; | ||
| import { createHarness, type Harness } from "../harness.ts"; | ||
|
|
||
| // Bun's console methods print to fd 1 without calling a replaced process.stdout.write | ||
| // (https://github.com/oven-sh/bun/issues/47982), so an extension's console.log went into the RPC stream. | ||
|
|
||
| const rpcInput = vi.hoisted(() => ({ lineHandler: undefined as ((line: string) => void) | undefined })); | ||
|
|
||
| vi.mock("../../../src/modes/rpc/jsonl.js", async (importOriginal) => ({ | ||
| ...(await importOriginal<typeof import("../../../src/modes/rpc/jsonl.ts")>()), | ||
| attachJsonlLineReader: vi.fn((_stream: NodeJS.ReadableStream, onLine: (line: string) => void) => { | ||
| rpcInput.lineHandler = onLine; | ||
| return () => { | ||
| rpcInput.lineHandler = undefined; | ||
| }; | ||
| }), | ||
| })); | ||
|
|
||
| const fixture = fileURLToPath(new URL("../../fixtures/rpc-console-session.ts", import.meta.url)); | ||
| const hasBun = spawnSync("bun", ["--version"]).status === 0; | ||
|
|
||
| function expectJsonLines(stdout: string): Array<Record<string, unknown>> { | ||
| const lines = stdout.split("\n").filter((line) => line.length > 0); | ||
| expect(lines.length).toBeGreaterThan(0); | ||
| return lines.map((line) => JSON.parse(line) as Record<string, unknown>); | ||
| } | ||
|
|
||
| describe("console output in RPC mode", () => { | ||
| const harnesses: Harness[] = []; | ||
| afterEach(() => { | ||
| for (const harness of harnesses.splice(0)) harness.cleanup(); | ||
| }); | ||
|
|
||
| test("an extension's console.log goes to stderr and stdout stays JSON lines (Node)", async () => { | ||
| const stdout: string[] = []; | ||
| const stderr: string[] = []; | ||
| const originalStdoutWrite = process.stdout.write; | ||
| const originalStderrWrite = process.stderr.write; | ||
| const originalLog = console.log; | ||
| const originalInfo = console.info; | ||
| // Vitest replaces the global console with its own; use one over process.stdout as Node's global console is. | ||
| const nodeConsole = new Console({ stdout: process.stdout, stderr: process.stderr }); | ||
| console.log = nodeConsole.log.bind(nodeConsole); | ||
| console.info = nodeConsole.info.bind(nodeConsole); | ||
| process.stdout.write = ((chunk: string, ...rest: unknown[]) => { | ||
| stdout.push(String(chunk)); | ||
| (rest.find((arg) => typeof arg === "function") as (() => void) | undefined)?.(); | ||
| return true; | ||
| }) as typeof process.stdout.write; | ||
| process.stderr.write = ((chunk: string, ...rest: unknown[]) => { | ||
| stderr.push(String(chunk)); | ||
| (rest.find((arg) => typeof arg === "function") as (() => void) | undefined)?.(); | ||
| return true; | ||
| }) as typeof process.stderr.write; | ||
|
|
||
| try { | ||
| const harness = await createHarness({ | ||
| extensionFactories: [ | ||
| (pi) => { | ||
| pi.on("agent_start", () => { | ||
| console.log("console-marker log", { a: 1 }); | ||
| console.info("console-marker info"); | ||
| }); | ||
| }, | ||
| ], | ||
| }); | ||
| harnesses.push(harness); | ||
| harness.setResponses([fauxAssistantMessage("done")]); | ||
| void runRpcMode({ | ||
| session: harness.session, | ||
| dispose: async () => {}, | ||
| setRebindSession: () => {}, | ||
| } as unknown as AgentSessionRuntime); | ||
| await vi.waitFor(() => expect(rpcInput.lineHandler).toBeDefined()); | ||
| rpcInput.lineHandler?.(JSON.stringify({ id: "p", type: "prompt", message: "hi" })); | ||
| await vi.waitFor(() => expect(stdout.join("")).toContain('"type":"agent_end"')); | ||
| } finally { | ||
| restoreStdout(); | ||
| process.stdout.write = originalStdoutWrite; | ||
| process.stderr.write = originalStderrWrite; | ||
| console.log = originalLog; | ||
| console.info = originalInfo; | ||
| } | ||
|
|
||
| expect(stdout.join("")).not.toContain("console-marker"); | ||
| expectJsonLines(stdout.join("")); | ||
| expect(stderr.join("")).toContain("console-marker log { a: 1 }\n"); | ||
| expect(stderr.join("")).toContain("console-marker info\n"); | ||
| }); | ||
|
|
||
| test.skipIf(!hasBun)("every stdout console method of an extension goes to stderr under Bun", async () => { | ||
| const child = spawn("bun", [fixture], { stdio: ["pipe", "pipe", "pipe"] }); | ||
| let stdout = ""; | ||
| let stderr = ""; | ||
| child.stdout.setEncoding("utf8"); | ||
| child.stderr.setEncoding("utf8"); | ||
| child.stdout.on("data", (chunk: string) => { | ||
| stdout += chunk; | ||
| if (stdout.includes('"type":"agent_end"')) child.stdin.end(); | ||
| }); | ||
| child.stderr.on("data", (chunk: string) => { | ||
| stderr += chunk; | ||
| }); | ||
| const exitCode = new Promise<number | null>((resolve) => child.on("exit", resolve)); | ||
| child.stdin.write(`${JSON.stringify({ id: "p", type: "prompt", message: "hi" })}\n`); | ||
| expect(await exitCode).toBe(0); | ||
|
|
||
| const events = expectJsonLines(stdout); | ||
| expect(events.some((event) => event.type === "agent_end")).toBe(true); | ||
| expect(stdout).not.toContain("console-marker"); | ||
| for (const marker of [ | ||
| "console-marker extension loaded { a: 1 }\n", | ||
| "console-marker log x\n", | ||
| "console-marker info\n", | ||
| "console-marker debug\n", | ||
| "console-marker dir", | ||
| "console-marker dirxml\n", | ||
| "console-marker table", | ||
| "Trace: console-marker trace\n", | ||
| "console-marker count: 1\n", | ||
| "console-marker group\n console-marker in group\n", | ||
| "console-marker write\n", | ||
| ]) { | ||
| expect(stderr).toContain(marker); | ||
| } | ||
| }); | ||
| }); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.