Merge pull request #267 from PaulJPhilp/codex/ep-admin-tty-machine-consistency

fix(ep-admin): suppress progress noise in non-interactive mode

Author: PaulJPhilp

packages/ep-admin/src/services/execution/__tests__/helpers.test.ts

@@ -4,7 +4,7 @@
 
 import { Effect } from "effect";
 import { describe, expect, it } from "vitest";
-import { withSpinner } from "../helpers.js";
+import { shouldRenderProgress, withSpinner } from "../helpers.js";
 
 describe("Execution Helpers", () => {
 	describe("withSpinner", () => {
@@ -25,4 +25,59 @@ describe("Execution Helpers", () => {
 		});
 	});
 
-});
+	describe("shouldRenderProgress", () => {
+		it("returns true in interactive tty mode", () => {
+			expect(
+				shouldRenderProgress(undefined, {
+					isTTY: true,
+					ci: undefined,
+					term: "xterm-256color",
+				})
+			).toBe(true);
+		});
+
+		it("returns false in non-tty mode", () => {
+			expect(
+				shouldRenderProgress(undefined, {
+					isTTY: false,
+					ci: undefined,
+					term: "xterm-256color",
+				})
+			).toBe(false);
+		});
+
+		it("returns false in CI mode", () => {
+			expect(
+				shouldRenderProgress(undefined, {
+					isTTY: true,
+					ci: "true",
+					term: "xterm-256color",
+				})
+			).toBe(false);
+		});
+
+		it("returns false when terminal is dumb", () => {
+			expect(
+				shouldRenderProgress(undefined, {
+					isTTY: true,
+					ci: undefined,
+					term: "dumb",
+				})
+			).toBe(false);
+		});
+
+		it("returns false in verbose mode", () => {
+			expect(
+				shouldRenderProgress(
+					{ verbose: true },
+					{
+						isTTY: true,
+						ci: undefined,
+						term: "xterm-256color",
+					}
+				)
+			).toBe(false);
+		});
+	});
+
+});

packages/ep-admin/src/services/execution/helpers.ts

@@ -7,6 +7,33 @@ import { spawn } from "node:child_process";
 import { ExecutionError } from "./errors.js";
 import type { ExecutionOptions } from "./types.js";
 
+type ProgressSignal = {
+	readonly isTTY: boolean;
+	readonly ci?: string;
+	readonly term?: string;
+};
+
+const isTruthyEnvFlag = (value: string | undefined): boolean => {
+	if (!value) return false;
+	const normalized = value.trim().toLowerCase();
+	return normalized !== "0" && normalized !== "false" && normalized !== "no";
+};
+
+export const shouldRenderProgress = (
+	options?: ExecutionOptions,
+	signal: ProgressSignal = {
+		isTTY: Boolean(process.stdout.isTTY),
+		ci: process.env.CI,
+		term: process.env.TERM,
+	}
+): boolean => {
+	if (options?.verbose) return false;
+	if (!signal.isTTY) return false;
+	if (isTruthyEnvFlag(signal.ci)) return false;
+	if (signal.term?.toLowerCase() === "dumb") return false;
+	return true;
+};
+
 /**
  * Convert child process spawn to Effect
  * Returns void on success, Error on failure
@@ -65,8 +92,12 @@ export const spawnEffect = (
 export const withSpinner = <A, E, R>(
 	message: string,
 	effect: Effect.Effect<A, E, R>,
-	_options?: ExecutionOptions
+	options?: ExecutionOptions
 ): Effect.Effect<A, E, R> => {
+	if (!shouldRenderProgress(options)) {
+		return effect;
+	}
+
 	const showStart = Console.log(`⣾ ${message}...`);
 	const showEnd = Console.log(`✓ ${message} completed`);
 

packages/ep-admin/src/services/execution/service.ts

@@ -7,7 +7,7 @@ import { spawn } from "node:child_process";
 import { TUIService } from "../tui/service.js";
 import type { ExecutionService } from "./api.js";
 import { ExecutionError, ScriptExecutionError } from "./errors.js";
-import { spawnEffect, withSpinner } from "./helpers.js";
+import { shouldRenderProgress, spawnEffect, withSpinner } from "./helpers.js";
 import type { ExecutionOptions } from "./types.js";
 
 /**
@@ -25,24 +25,25 @@ export class Execution extends Effect.Service<Execution>()("Execution", {
 		) =>
 			Effect.gen(function* () {
 				const task = spawnEffect(scriptPath, options);
-
-				// Try to use TUI spinner if available
-				// biome-ignore lint/suspicious/noExplicitAny: Dynamic TUI module shape varies at runtime
-				const tui = yield* tuiService.load().pipe(Effect.catchAll(() => Effect.succeed(null))) as Effect.Effect<Record<string, any> | null>;
-				if (tui?.InkService && tui?.spinnerEffect) {
-					const maybeInk = yield* Effect.serviceOption(tui.InkService);
-					if (Opt.isSome(maybeInk) && !options?.verbose) {
-						// Use TUI spinner
-						yield* (tui.spinnerEffect(taskName, task, {
-							type: "dots",
-							color: "cyan",
-						}) as Effect.Effect<void, ExecutionError>);
-						return;
+				const showProgress = shouldRenderProgress(options);
+
+				if (showProgress) {
+					// Try to use TUI spinner if available
+					// biome-ignore lint/suspicious/noExplicitAny: Dynamic TUI module shape varies at runtime
+					const tui = yield* tuiService.load().pipe(Effect.catchAll(() => Effect.succeed(null))) as Effect.Effect<Record<string, any> | null>;
+					if (tui?.InkService && tui?.spinnerEffect) {
+						const maybeInk = yield* Effect.serviceOption(tui.InkService);
+						if (Opt.isSome(maybeInk)) {
+							// Use TUI spinner
+							yield* (tui.spinnerEffect(taskName, task, {
+								type: "dots",
+								color: "cyan",
+							}) as Effect.Effect<void, ExecutionError>);
+							return;
+						}
 					}
-				}
 
-				// Fallback to console with ora-style output
-				if (!options?.verbose) {
+					// Fallback to console with ora-style output
 					yield* Console.log(`⣾ ${taskName}...`);
 				}
 
@@ -64,7 +65,7 @@ export class Execution extends Effect.Service<Execution>()("Execution", {
 					})
 				);
 
-				if (!options?.verbose) {
+				if (showProgress) {
 					yield* Console.log(`✓ ${taskName} completed`);
 				}
 			});