fix(coding-agent): reserve stdout in print and json mode closes #2482
This commit is contained in:
@@ -14,6 +14,7 @@
|
|||||||
### Fixed
|
### Fixed
|
||||||
|
|
||||||
- Fixed `pi update` for git packages to skip destructive reset, clean, and reinstall steps when the fetched target already matches the local checkout ([#2503](https://github.com/badlogic/pi-mono/issues/2503))
|
- Fixed `pi update` for git packages to skip destructive reset, clean, and reinstall steps when the fetched target already matches the local checkout ([#2503](https://github.com/badlogic/pi-mono/issues/2503))
|
||||||
|
- Fixed print and JSON mode to take over stdout during non-interactive startup, keeping package-manager and other incidental chatter off protocol/output stdout ([#2482](https://github.com/badlogic/pi-mono/issues/2482))
|
||||||
|
|
||||||
## [0.61.1] - 2026-03-20
|
## [0.61.1] - 2026-03-20
|
||||||
|
|
||||||
|
|||||||
74
packages/coding-agent/src/core/output-guard.ts
Normal file
74
packages/coding-agent/src/core/output-guard.ts
Normal file
@@ -0,0 +1,74 @@
|
|||||||
|
interface StdoutTakeoverState {
|
||||||
|
rawStdoutWrite: (chunk: string, callback?: (error?: Error | null) => void) => boolean;
|
||||||
|
rawStderrWrite: (chunk: string, callback?: (error?: Error | null) => void) => boolean;
|
||||||
|
originalStdoutWrite: typeof process.stdout.write;
|
||||||
|
}
|
||||||
|
|
||||||
|
let stdoutTakeoverState: StdoutTakeoverState | undefined;
|
||||||
|
|
||||||
|
export function takeOverStdout(): void {
|
||||||
|
if (stdoutTakeoverState) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
const rawStdoutWrite = process.stdout.write.bind(process.stdout) as StdoutTakeoverState["rawStdoutWrite"];
|
||||||
|
const rawStderrWrite = process.stderr.write.bind(process.stderr) as StdoutTakeoverState["rawStderrWrite"];
|
||||||
|
const originalStdoutWrite = process.stdout.write;
|
||||||
|
|
||||||
|
process.stdout.write = ((
|
||||||
|
chunk: string | Uint8Array,
|
||||||
|
encodingOrCallback?: BufferEncoding | ((error?: Error | null) => void),
|
||||||
|
callback?: (error?: Error | null) => void,
|
||||||
|
): boolean => {
|
||||||
|
if (typeof encodingOrCallback === "function") {
|
||||||
|
return rawStderrWrite(String(chunk), encodingOrCallback);
|
||||||
|
}
|
||||||
|
return rawStderrWrite(String(chunk), callback);
|
||||||
|
}) as typeof process.stdout.write;
|
||||||
|
|
||||||
|
stdoutTakeoverState = {
|
||||||
|
rawStdoutWrite,
|
||||||
|
rawStderrWrite,
|
||||||
|
originalStdoutWrite,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
export function restoreStdout(): void {
|
||||||
|
if (!stdoutTakeoverState) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
process.stdout.write = stdoutTakeoverState.originalStdoutWrite;
|
||||||
|
stdoutTakeoverState = undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function isStdoutTakenOver(): boolean {
|
||||||
|
return stdoutTakeoverState !== undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function writeRawStdout(text: string): void {
|
||||||
|
if (stdoutTakeoverState) {
|
||||||
|
stdoutTakeoverState.rawStdoutWrite(text);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
process.stdout.write(text);
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function flushRawStdout(): Promise<void> {
|
||||||
|
if (stdoutTakeoverState) {
|
||||||
|
await new Promise<void>((resolve, reject) => {
|
||||||
|
stdoutTakeoverState?.rawStdoutWrite("", (err) => {
|
||||||
|
if (err) reject(err);
|
||||||
|
else resolve();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
await new Promise<void>((resolve, reject) => {
|
||||||
|
process.stdout.write("", (err) => {
|
||||||
|
if (err) reject(err);
|
||||||
|
else resolve();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
@@ -7,6 +7,7 @@ import ignore from "ignore";
|
|||||||
import { minimatch } from "minimatch";
|
import { minimatch } from "minimatch";
|
||||||
import { CONFIG_DIR_NAME } from "../config.js";
|
import { CONFIG_DIR_NAME } from "../config.js";
|
||||||
import { type GitSource, parseGitUrl } from "../utils/git.js";
|
import { type GitSource, parseGitUrl } from "../utils/git.js";
|
||||||
|
import { isStdoutTakenOver } from "./output-guard.js";
|
||||||
import type { PackageSource, SettingsManager } from "./settings-manager.js";
|
import type { PackageSource, SettingsManager } from "./settings-manager.js";
|
||||||
|
|
||||||
const NETWORK_TIMEOUT_MS = 10000;
|
const NETWORK_TIMEOUT_MS = 10000;
|
||||||
@@ -2091,7 +2092,7 @@ export class DefaultPackageManager implements PackageManager {
|
|||||||
return new Promise((resolvePromise, reject) => {
|
return new Promise((resolvePromise, reject) => {
|
||||||
const child = spawn(command, args, {
|
const child = spawn(command, args, {
|
||||||
cwd: options?.cwd,
|
cwd: options?.cwd,
|
||||||
stdio: "inherit",
|
stdio: isStdoutTakenOver() ? ["ignore", 2, 2] : "inherit",
|
||||||
shell: process.platform === "win32",
|
shell: process.platform === "win32",
|
||||||
});
|
});
|
||||||
child.on("error", reject);
|
child.on("error", reject);
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ import type { LoadExtensionsResult } from "./core/extensions/index.js";
|
|||||||
import { migrateKeybindingsConfigFile } from "./core/keybindings.js";
|
import { migrateKeybindingsConfigFile } from "./core/keybindings.js";
|
||||||
import { ModelRegistry } from "./core/model-registry.js";
|
import { ModelRegistry } from "./core/model-registry.js";
|
||||||
import { resolveCliModel, resolveModelScope, type ScopedModel } from "./core/model-resolver.js";
|
import { resolveCliModel, resolveModelScope, type ScopedModel } from "./core/model-resolver.js";
|
||||||
|
import { restoreStdout, takeOverStdout } from "./core/output-guard.js";
|
||||||
import { DefaultPackageManager } from "./core/package-manager.js";
|
import { DefaultPackageManager } from "./core/package-manager.js";
|
||||||
import { DefaultResourceLoader } from "./core/resource-loader.js";
|
import { DefaultResourceLoader } from "./core/resource-loader.js";
|
||||||
import { type CreateAgentSessionOptions, createAgentSession } from "./core/sdk.js";
|
import { type CreateAgentSessionOptions, createAgentSession } from "./core/sdk.js";
|
||||||
@@ -635,11 +636,15 @@ export async function main(args: string[]) {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Run migrations (pass cwd for project-local migrations)
|
|
||||||
const { migratedAuthProviders: migratedProviders, deprecationWarnings } = runMigrations(process.cwd());
|
|
||||||
|
|
||||||
// First pass: parse args to get --extension paths
|
// First pass: parse args to get --extension paths
|
||||||
const firstPass = parseArgs(args);
|
const firstPass = parseArgs(args);
|
||||||
|
const shouldTakeOverStdout = firstPass.mode !== undefined || firstPass.print || !process.stdin.isTTY;
|
||||||
|
if (shouldTakeOverStdout) {
|
||||||
|
takeOverStdout();
|
||||||
|
}
|
||||||
|
|
||||||
|
// Run migrations (pass cwd for project-local migrations)
|
||||||
|
const { migratedAuthProviders: migratedProviders, deprecationWarnings } = runMigrations(process.cwd());
|
||||||
|
|
||||||
// Early load extensions to discover their CLI flags
|
// Early load extensions to discover their CLI flags
|
||||||
const cwd = process.cwd();
|
const cwd = process.cwd();
|
||||||
@@ -884,9 +889,7 @@ export async function main(args: string[]) {
|
|||||||
initialImages,
|
initialImages,
|
||||||
});
|
});
|
||||||
stopThemeWatcher();
|
stopThemeWatcher();
|
||||||
if (process.stdout.writableLength > 0) {
|
restoreStdout();
|
||||||
await new Promise<void>((resolve) => process.stdout.once("drain", resolve));
|
return;
|
||||||
}
|
|
||||||
process.exit(0);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,6 +8,7 @@
|
|||||||
|
|
||||||
import type { AssistantMessage, ImageContent } from "@mariozechner/pi-ai";
|
import type { AssistantMessage, ImageContent } from "@mariozechner/pi-ai";
|
||||||
import type { AgentSession } from "../core/agent-session.js";
|
import type { AgentSession } from "../core/agent-session.js";
|
||||||
|
import { flushRawStdout, writeRawStdout } from "../core/output-guard.js";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Options for print mode.
|
* Options for print mode.
|
||||||
@@ -32,7 +33,7 @@ export async function runPrintMode(session: AgentSession, options: PrintModeOpti
|
|||||||
if (mode === "json") {
|
if (mode === "json") {
|
||||||
const header = session.sessionManager.getHeader();
|
const header = session.sessionManager.getHeader();
|
||||||
if (header) {
|
if (header) {
|
||||||
console.log(JSON.stringify(header));
|
writeRawStdout(`${JSON.stringify(header)}\n`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// Set up extensions for print mode (no UI)
|
// Set up extensions for print mode (no UI)
|
||||||
@@ -76,7 +77,7 @@ export async function runPrintMode(session: AgentSession, options: PrintModeOpti
|
|||||||
session.subscribe((event) => {
|
session.subscribe((event) => {
|
||||||
// In JSON mode, output all events
|
// In JSON mode, output all events
|
||||||
if (mode === "json") {
|
if (mode === "json") {
|
||||||
console.log(JSON.stringify(event));
|
writeRawStdout(`${JSON.stringify(event)}\n`);
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -107,7 +108,7 @@ export async function runPrintMode(session: AgentSession, options: PrintModeOpti
|
|||||||
// Output text content
|
// Output text content
|
||||||
for (const content of assistantMsg.content) {
|
for (const content of assistantMsg.content) {
|
||||||
if (content.type === "text") {
|
if (content.type === "text") {
|
||||||
console.log(content.text);
|
writeRawStdout(`${content.text}\n`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -115,10 +116,5 @@ export async function runPrintMode(session: AgentSession, options: PrintModeOpti
|
|||||||
|
|
||||||
// Ensure stdout is fully flushed before returning
|
// Ensure stdout is fully flushed before returning
|
||||||
// This prevents race conditions where the process exits before all output is written
|
// This prevents race conditions where the process exits before all output is written
|
||||||
await new Promise<void>((resolve, reject) => {
|
await flushRawStdout();
|
||||||
process.stdout.write("", (err) => {
|
|
||||||
if (err) reject(err);
|
|
||||||
else resolve();
|
|
||||||
});
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -18,6 +18,7 @@ import type {
|
|||||||
ExtensionUIDialogOptions,
|
ExtensionUIDialogOptions,
|
||||||
ExtensionWidgetOptions,
|
ExtensionWidgetOptions,
|
||||||
} from "../../core/extensions/index.js";
|
} from "../../core/extensions/index.js";
|
||||||
|
import { takeOverStdout, writeRawStdout } from "../../core/output-guard.js";
|
||||||
import { type Theme, theme } from "../interactive/theme/theme.js";
|
import { type Theme, theme } from "../interactive/theme/theme.js";
|
||||||
import { attachJsonlLineReader, serializeJsonLine } from "./jsonl.js";
|
import { attachJsonlLineReader, serializeJsonLine } from "./jsonl.js";
|
||||||
import type {
|
import type {
|
||||||
@@ -43,15 +44,10 @@ export type {
|
|||||||
* Listens for JSON commands on stdin, outputs events and responses on stdout.
|
* Listens for JSON commands on stdin, outputs events and responses on stdout.
|
||||||
*/
|
*/
|
||||||
export async function runRpcMode(session: AgentSession): Promise<never> {
|
export async function runRpcMode(session: AgentSession): Promise<never> {
|
||||||
const rawStdoutWrite = process.stdout.write.bind(process.stdout);
|
takeOverStdout();
|
||||||
const rawStderrWrite = process.stderr.write.bind(process.stderr);
|
|
||||||
|
|
||||||
process.stdout.write = ((
|
|
||||||
...args: Parameters<typeof process.stdout.write>
|
|
||||||
): ReturnType<typeof process.stdout.write> => rawStderrWrite(...args)) as typeof process.stdout.write;
|
|
||||||
|
|
||||||
const output = (obj: RpcResponse | RpcExtensionUIRequest | object) => {
|
const output = (obj: RpcResponse | RpcExtensionUIRequest | object) => {
|
||||||
rawStdoutWrite(serializeJsonLine(obj));
|
writeRawStdout(serializeJsonLine(obj));
|
||||||
};
|
};
|
||||||
|
|
||||||
const success = <T extends RpcCommand["type"]>(
|
const success = <T extends RpcCommand["type"]>(
|
||||||
|
|||||||
102
packages/coding-agent/test/stdout-cleanliness.test.ts
Normal file
102
packages/coding-agent/test/stdout-cleanliness.test.ts
Normal file
@@ -0,0 +1,102 @@
|
|||||||
|
import { spawn } from "node:child_process";
|
||||||
|
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
|
import { join, resolve } from "node:path";
|
||||||
|
import { afterEach, describe, expect, it } from "vitest";
|
||||||
|
import { ENV_AGENT_DIR } from "../src/config.js";
|
||||||
|
|
||||||
|
const cliPath = resolve(__dirname, "../src/cli.ts");
|
||||||
|
const tsxPath = resolve(__dirname, "../../../node_modules/tsx/dist/cli.mjs");
|
||||||
|
|
||||||
|
const tempDirs: string[] = [];
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
for (const dir of tempDirs.splice(0)) {
|
||||||
|
rmSync(dir, { recursive: true, force: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
function createTempDir(): string {
|
||||||
|
const dir = mkdtempSync(join(tmpdir(), "pi-stdout-clean-"));
|
||||||
|
tempDirs.push(dir);
|
||||||
|
return dir;
|
||||||
|
}
|
||||||
|
|
||||||
|
async function runCli(args: string[]): Promise<{ stdout: string; stderr: string; code: number | null }> {
|
||||||
|
const tempRoot = createTempDir();
|
||||||
|
const agentDir = join(tempRoot, "agent");
|
||||||
|
const projectDir = join(tempRoot, "project");
|
||||||
|
const projectConfigDir = join(projectDir, ".pi");
|
||||||
|
mkdirSync(agentDir, { recursive: true });
|
||||||
|
mkdirSync(projectConfigDir, { recursive: true });
|
||||||
|
|
||||||
|
const fakeNpmPath = join(tempRoot, "fake-npm.mjs");
|
||||||
|
writeFileSync(
|
||||||
|
fakeNpmPath,
|
||||||
|
[
|
||||||
|
'console.log("changed 1 package in 471ms");',
|
||||||
|
'console.log("found 0 vulnerabilities");',
|
||||||
|
"process.exit(0);",
|
||||||
|
].join("\n"),
|
||||||
|
"utf-8",
|
||||||
|
);
|
||||||
|
|
||||||
|
writeFileSync(
|
||||||
|
join(projectConfigDir, "settings.json"),
|
||||||
|
JSON.stringify(
|
||||||
|
{
|
||||||
|
packages: ["npm:fake-package"],
|
||||||
|
npmCommand: [process.execPath, fakeNpmPath],
|
||||||
|
},
|
||||||
|
null,
|
||||||
|
2,
|
||||||
|
),
|
||||||
|
"utf-8",
|
||||||
|
);
|
||||||
|
|
||||||
|
return await new Promise((resolvePromise, reject) => {
|
||||||
|
const child = spawn(process.execPath, [tsxPath, cliPath, ...args], {
|
||||||
|
cwd: projectDir,
|
||||||
|
env: {
|
||||||
|
...process.env,
|
||||||
|
[ENV_AGENT_DIR]: agentDir,
|
||||||
|
},
|
||||||
|
stdio: ["ignore", "pipe", "pipe"],
|
||||||
|
});
|
||||||
|
|
||||||
|
let stdout = "";
|
||||||
|
let stderr = "";
|
||||||
|
child.stdout.on("data", (chunk) => {
|
||||||
|
stdout += chunk.toString();
|
||||||
|
});
|
||||||
|
child.stderr.on("data", (chunk) => {
|
||||||
|
stderr += chunk.toString();
|
||||||
|
});
|
||||||
|
child.on("error", reject);
|
||||||
|
child.on("close", (code) => {
|
||||||
|
resolvePromise({ stdout, stderr, code });
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("stdout cleanliness in non-interactive modes", () => {
|
||||||
|
it("keeps stdout empty for --mode json --help while routing startup chatter to stderr", async () => {
|
||||||
|
const result = await runCli(["--mode", "json", "--help"]);
|
||||||
|
|
||||||
|
expect(result.code).toBe(0);
|
||||||
|
expect(result.stdout).toBe("");
|
||||||
|
expect(result.stderr).toContain("changed 1 package in 471ms");
|
||||||
|
expect(result.stderr).toContain("found 0 vulnerabilities");
|
||||||
|
expect(result.stderr).toContain("Usage:");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps stdout empty for -p --help while routing startup chatter to stderr", async () => {
|
||||||
|
const result = await runCli(["-p", "--help"]);
|
||||||
|
|
||||||
|
expect(result.code).toBe(0);
|
||||||
|
expect(result.stdout).toBe("");
|
||||||
|
expect(result.stderr).toContain("changed 1 package in 471ms");
|
||||||
|
expect(result.stderr).toContain("found 0 vulnerabilities");
|
||||||
|
expect(result.stderr).toContain("Usage:");
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user