From 78bf2c23904164617f72f0f74beed24c4fa3d310 Mon Sep 17 00:00:00 2001 From: willbot Date: Tue, 18 Aug 2026 09:27:48 +0200 Subject: [PATCH] feat(cli-engine)!: make Runtime.outputStreamsShareDevice required MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This field decides whether a command stdout payload is written at all. Optional, it defaulted to "one screen" and silently dropped that payload for any host that did not know to set it — which is every host except this repo bin. The ORM toolchain bin in prisma/prisma hit exactly that: adopting engine 0.1.1 typechecked clean, because the required Runtime.host errored and this one did not, and `migration graph --dot` stopped printing DOT. Required, the compiler asks each host the question. Behaviour is unchanged everywhere: absent previously meant "same device" via `!== false`, and each call site now says true explicitly. The bin helper answers true when fstat cannot inspect the fds, which is the same answer the default gave it. The test harness gains an outputStreamsShareDevice option. It had no way to express separate sinks, so a test could not assert a stdout payload as a caller with two destinations receives it. Signed-off-by: willbot Signed-off-by: Will Madden --- packages/cli-engine/src/execution/rendering.ts | 5 ++--- packages/cli-engine/src/runtime.ts | 12 ++++++++---- packages/cli-engine/src/testing.ts | 6 ++++++ packages/cli-engine/tests/clack-isolation.test.ts | 1 + packages/cli-engine/tests/clack-prompts.test.ts | 1 + packages/cli-engine/tests/config.test.ts | 1 + packages/cli-engine/tests/engine.type-test.ts | 1 + .../tests/environment-credential-manager.test.ts | 1 + packages/cli-engine/tests/execution.test.ts | 3 +++ packages/cli-engine/tests/lifetimes.test.ts | 1 + packages/cli-engine/tests/management-api.test.ts | 1 + packages/cli-engine/tests/prompts.test.ts | 1 + packages/cli-engine/tests/spawn.test.ts | 1 + packages/cli/src/runtime.ts | 10 ++++++---- 14 files changed, 34 insertions(+), 11 deletions(-) diff --git a/packages/cli-engine/src/execution/rendering.ts b/packages/cli-engine/src/execution/rendering.ts index c93aeda8..5a1c1701 100644 --- a/packages/cli-engine/src/execution/rendering.ts +++ b/packages/cli-engine/src/execution/rendering.ts @@ -438,12 +438,11 @@ export function renderCompletedHuman( * (amends the 2026-08-09 "always" ruling; any redirection of either * stream keeps the mirror, so pipes still receive exactly the data * lines). A harness that allocates two separate PTYs reports - * outputStreamsShareDevice false and keeps its mirror; a host that - * cannot tell is treated as one terminal. */ + * outputStreamsShareDevice false and keeps its mirror. */ const oneScreen = runtime.isTty.stdout && runtime.isTty.stderr && - runtime.outputStreamsShareDevice !== false; + runtime.outputStreamsShareDevice; if (!oneScreen) { for (const line of presented.presentation.stdout) { runtime.stdout.write(`${line}\n`); diff --git a/packages/cli-engine/src/runtime.ts b/packages/cli-engine/src/runtime.ts index fbc06ac6..fd5c2b0f 100644 --- a/packages/cli-engine/src/runtime.ts +++ b/packages/cli-engine/src/runtime.ts @@ -40,11 +40,15 @@ export interface Runtime { * human blocks and the machine stdout mirror would draw on one screen * as visible duplication. Consulted only when both streams are TTYs: * `false` there means two separate terminals, so the mirror is kept - * for whatever is reading stdout. Absent means the host cannot tell, - * which is treated as "same" — the overwhelmingly common case for two - * TTYs is one terminal. + * for whatever is reading stdout. + * + * Required, and deliberately so. It decides whether a command's stdout + * payload is written at all, so a host that says nothing is choosing to + * drop that payload — a choice that belongs at the call site, in the open, + * not in a default the host never sees. A bin that cannot tell answers + * `true`: two TTYs are one terminal far more often than not. */ - readonly outputStreamsShareDevice?: boolean; + readonly outputStreamsShareDevice: boolean; /** * Forces the answer to "is this CI", where telemetry never reports. * Absent — the normal case — means the engine detects CI from `env` diff --git a/packages/cli-engine/src/testing.ts b/packages/cli-engine/src/testing.ts index 4f73671c..d27eeb63 100644 --- a/packages/cli-engine/src/testing.ts +++ b/packages/cli-engine/src/testing.ts @@ -130,6 +130,11 @@ export interface TestCli { readonly onSettled?: (summary: RunSummary) => void; readonly cwd?: string; readonly isTty?: { stdin?: boolean; stdout?: boolean; stderr?: boolean }; + /** Whether stdout and stderr are the same open device. Defaults to + * true — two TTYs are one terminal far more often than not — which + * suppresses the stdout mirror. Set false to assert a command's + * stdout payload the way a caller with separate sinks receives it. */ + readonly outputStreamsShareDevice?: boolean; /** Terminal width, as the stream would report it. Absent means * not a terminal, which is what ui.width reads as unbounded. */ readonly columns?: { stderr?: number }; @@ -361,6 +366,7 @@ export function createTestCli(spec: { stdout: opts?.isTty?.stdout ?? false, stderr: opts?.isTty?.stderr ?? false, }, + outputStreamsShareDevice: opts?.outputStreamsShareDevice ?? true, isCIOverride: opts?.isCI ?? spec.isCI, exit: (code: number): never => { throw new Error( diff --git a/packages/cli-engine/tests/clack-isolation.test.ts b/packages/cli-engine/tests/clack-isolation.test.ts index 9eb40e7b..4d08dfb8 100644 --- a/packages/cli-engine/tests/clack-isolation.test.ts +++ b/packages/cli-engine/tests/clack-isolation.test.ts @@ -115,6 +115,7 @@ describe("scripted and non-TTY paths are clack-free", () => { cwd: "/", env: {}, isTty: { stdin: true, stdout: true, stderr: true }, + outputStreamsShareDevice: true, exit: (code: number): never => { throw new Error(`runtime.exit(${code})`); }, diff --git a/packages/cli-engine/tests/clack-prompts.test.ts b/packages/cli-engine/tests/clack-prompts.test.ts index dfca2c26..fb46e794 100644 --- a/packages/cli-engine/tests/clack-prompts.test.ts +++ b/packages/cli-engine/tests/clack-prompts.test.ts @@ -103,6 +103,7 @@ async function runInteractive( cwd: "/", env: {}, isTty: { stdin: true, stdout: true, stderr: true }, + outputStreamsShareDevice: true, exit: (code: number): never => { throw new Error(`runtime.exit(${code})`); }, diff --git a/packages/cli-engine/tests/config.test.ts b/packages/cli-engine/tests/config.test.ts index a146e111..ef49533c 100644 --- a/packages/cli-engine/tests/config.test.ts +++ b/packages/cli-engine/tests/config.test.ts @@ -864,6 +864,7 @@ describe("needs.config", { timeout: 60_000 }, () => { cwd, env: {}, isTty: { stdin: false, stdout: false, stderr: false }, + outputStreamsShareDevice: true, exit: (code: number): never => { throw new Error(`runtime.exit(${code})`); }, diff --git a/packages/cli-engine/tests/engine.type-test.ts b/packages/cli-engine/tests/engine.type-test.ts index 86d31a60..cbb6a91a 100644 --- a/packages/cli-engine/tests/engine.type-test.ts +++ b/packages/cli-engine/tests/engine.type-test.ts @@ -474,6 +474,7 @@ export const runtimeShape: Runtime = { cwd: "/", env: { CI: "1" }, isTty: { stdin: true, stdout: true, stderr: true }, + outputStreamsShareDevice: true, exit: (code: number): never => { throw new Error(String(code)); }, diff --git a/packages/cli-engine/tests/environment-credential-manager.test.ts b/packages/cli-engine/tests/environment-credential-manager.test.ts index 29c4bbc6..4a4d60f7 100644 --- a/packages/cli-engine/tests/environment-credential-manager.test.ts +++ b/packages/cli-engine/tests/environment-credential-manager.test.ts @@ -167,6 +167,7 @@ describe("wired as a Runtime's manager", () => { cwd: "/", env, isTty: { stdin: false, stdout: false, stderr: false }, + outputStreamsShareDevice: true, exit: (code: number): never => { throw new Error(`runtime.exit(${code})`); }, diff --git a/packages/cli-engine/tests/execution.test.ts b/packages/cli-engine/tests/execution.test.ts index e6467183..c74ccd34 100644 --- a/packages/cli-engine/tests/execution.test.ts +++ b/packages/cli-engine/tests/execution.test.ts @@ -601,6 +601,7 @@ describe("needs preconditions", () => { cwd: process.cwd(), env: {}, isTty: { stdin: opts.interactive, stdout: false, stderr: false }, + outputStreamsShareDevice: true, exit: (code: number): never => { throw new Error(`runtime.exit(${code})`); }, @@ -821,6 +822,7 @@ describe("report() after the handler resolved", () => { cwd: "/", env: {}, isTty: { stdin: false, stdout: false, stderr: false }, + outputStreamsShareDevice: true, exit: (code: number): never => { throw new Error(`runtime.exit(${code})`); }, @@ -896,6 +898,7 @@ describe("credentials that cannot be read", () => { cwd: "/", env: {}, isTty: { stdin: false, stdout: false, stderr: false }, + outputStreamsShareDevice: true, exit: (code: number): never => { throw new Error(`runtime.exit(${code})`); }, diff --git a/packages/cli-engine/tests/lifetimes.test.ts b/packages/cli-engine/tests/lifetimes.test.ts index 842219ea..63c2963d 100644 --- a/packages/cli-engine/tests/lifetimes.test.ts +++ b/packages/cli-engine/tests/lifetimes.test.ts @@ -263,6 +263,7 @@ describe("the engine owns the double-signal policy", () => { cwd: "/", env: {}, isTty: { stdin: false, stdout: false, stderr: false }, + outputStreamsShareDevice: true, exit: (code: number): never => { exited.push(code); throw new Error(`runtime.exit(${code})`); diff --git a/packages/cli-engine/tests/management-api.test.ts b/packages/cli-engine/tests/management-api.test.ts index 224e89cf..a47b0e99 100644 --- a/packages/cli-engine/tests/management-api.test.ts +++ b/packages/cli-engine/tests/management-api.test.ts @@ -103,6 +103,7 @@ function makeRuntime(overrides?: { cwd: "/", env: {}, isTty: { stdin: false, stdout: false, stderr: false }, + outputStreamsShareDevice: true, exit: (code: number): never => { throw new Error(`runtime.exit(${code})`); }, diff --git a/packages/cli-engine/tests/prompts.test.ts b/packages/cli-engine/tests/prompts.test.ts index bb0044c7..ec72e46a 100644 --- a/packages/cli-engine/tests/prompts.test.ts +++ b/packages/cli-engine/tests/prompts.test.ts @@ -524,6 +524,7 @@ describe("stdin cleanup", () => { cwd: "/", env: {}, isTty: { stdin: true, stdout: true, stderr: true }, + outputStreamsShareDevice: true, exit: (code: number): never => { throw new Error(`runtime.exit(${code})`); }, diff --git a/packages/cli-engine/tests/spawn.test.ts b/packages/cli-engine/tests/spawn.test.ts index 2430a362..294c3f14 100644 --- a/packages/cli-engine/tests/spawn.test.ts +++ b/packages/cli-engine/tests/spawn.test.ts @@ -1773,6 +1773,7 @@ function controllableRuntime() { cwd: "/", env: {}, isTty: { stdin: false, stdout: false, stderr: false }, + outputStreamsShareDevice: true, exit: (code: number): never => { exited.push(code); throw new Error(`runtime.exit(${code})`); diff --git a/packages/cli/src/runtime.ts b/packages/cli/src/runtime.ts index 439426dd..7143c94d 100644 --- a/packages/cli/src/runtime.ts +++ b/packages/cli/src/runtime.ts @@ -83,15 +83,17 @@ function warnOnDeprecatedStateFileEnvVar(proc: HostProcess): void { /** Whether fd 1 and fd 2 are the same open device. Distinguishes one * terminal (mirror suppressed) from a harness that allocated separate - * PTYs for the two streams (mirror kept). Undefined when the fds - * cannot be inspected — the engine then assumes one terminal. */ -function outputStreamsShareDevice(): boolean | undefined { + * PTYs for the two streams (mirror kept). */ +function outputStreamsShareDevice(): boolean { try { const out = fstatSync(1); const err = fstatSync(2); return out.dev === err.dev && out.ino === err.ino && out.rdev === err.rdev; } catch { - return undefined; + // Cannot tell: answer "one terminal", which two TTYs are far more + // often than not. Same behaviour the engine used to apply to an + // absent field, now stated here rather than defaulted out of sight. + return true; } }