fix(node:util): honor stream TTY/color support in styleText()

[sqdshguy]

Summary

• Update util.styleText() to honor stream TTY/color support when validating streams
• Adjust and extend util tests to cover TTY vs non-TTY behavior and keep colorized expectations stable

Why this works

• styleText() now relies on the shared stream validators and internal/util/colors logic. This matches Node's behavior for stream.isTTY and color depth
• Tests exercise both TTY and non-TTY streams plus the validateStream: false escape hatch

Fixes #25736

[coderabbitai]

Walkthrough

styleText in src/js/node/util.ts now accepts an optional third options object ({ validateStream = true, stream = process.stdout }). The function validates the provided stream when requested, respects stream isTTY via colors.shouldColorize(stream), normalizes single or multiple formats to an array, and applies ANSI codes for multiple formats or returns plain text when colorization is skipped.

Changes

Cohort / File(s)	Summary
Core Implementation src/js/node/util.ts	Signature changed to styleText(format, text, { validateStream = true, stream = process.stdout } = {}). Added imports for validateBoolean, colors, and stream utilities. Validates validateStream; when true validates stream type. Uses colors.shouldColorize(stream) to decide skipping colorization. Normalizes format to an array and accumulates multiple ANSI code pairs, assembles left/right sequences, and applies them or returns plain text early when skipped.
Unit Tests test/js/node/util/util.test.js	Tests updated to call styleText(..., options) and cover: TTY vs non-TTY behavior via provided stream, disabling validation with validateStream: false, handling of none/no-color formats, and invalid stream type raising ERR_INVALID_ARG_TYPE. Existing expectations adjusted to the new API.

Pre-merge checks

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: updating util.styleText() to respect stream TTY/color support, which is the core objective of this PR.
Description check	✅ Passed	The description covers the main changes and references the linked issue, but lacks detail on verification and testing methodology as specified in the template.
Linked Issues check	✅ Passed	The PR implementation successfully addresses issue #25736 by making util.styleText() respect stream.isTTY property and color support through shared validators.
Out of Scope Changes check	✅ Passed	All changes are scope-aligned: modifications to styleText() signature, stream validation logic, and test coverage directly address the TTY/color support objective.

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between fb17bc5 and 1052be4.

📒 Files selected for processing (2)

• src/js/node/util.ts
• test/js/node/util/util.test.js

🧰 Additional context used

📓 Path-based instructions (3)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

• test/js/node/util/util.test.js

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use .$call() and .$apply() instead of .call() and .apply() to prevent user tampering with function invocation
Use string literal require() statements only; dynamic requires are not permitted
Export modules using export default { ... } syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with $) such as $Array.from(), $isCallable(), and $newArrayWithSize() for performance-critical operations
Use private globals and methods with $ prefix (e.g., $Array, map.$set()) instead of public JavaScript globals
Use $debug() for debug logging and $assert() for assertions; both are stripped in release builds
Validate function arguments using validators from internal/validators and throw $ERR_* error codes for invalid arguments
Use process.platform and process.arch for platform detection; these values are inlined and dead-code eliminated at build time

Files:

• src/js/node/util.ts

src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

Builtin functions must include this parameter typing in TypeScript to enable direct method binding in C++

Files:

• src/js/node/util.ts

🧠 Learnings (8)

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always check exit codes and test error scenarios in error tests

Applied to files:

• test/js/node/util/util.test.js

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure

Applied to files:

• test/js/node/util/util.test.js

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output

Applied to files:

• test/js/node/util/util.test.js

📚 Learning: 2025-10-26T01:32:04.844Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 24082
File: test/cli/test/coverage.test.ts:60-112
Timestamp: 2025-10-26T01:32:04.844Z
Learning: In the Bun repository test files (test/cli/test/*.test.ts), when spawning Bun CLI commands with Bun.spawnSync for testing, prefer using stdio: ["inherit", "inherit", "inherit"] to inherit stdio streams rather than piping them.

Applied to files:

• test/js/node/util/util.test.js

📚 Learning: 2025-11-24T18:37:11.466Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Validate function arguments using validators from `internal/validators` and throw `$ERR_*` error codes for invalid arguments

Applied to files:

• src/js/node/util.ts

📚 Learning: 2025-11-24T18:37:11.466Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `$debug()` for debug logging and `$assert()` for assertions; both are stripped in release builds

Applied to files:

• src/js/node/util.ts

📚 Learning: 2025-11-24T18:37:11.466Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time

Applied to files:

• src/js/node/util.ts

📚 Learning: 2025-11-24T18:37:11.466Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use string literal `require()` statements only; dynamic requires are not permitted

Applied to files:

• src/js/node/util.ts

🧬 Code graph analysis (1)

src/js/node/util.ts (2)

src/js/builtins/ReadableStreamInternals.ts (1)

• isReadableStream (658-663)

src/js/internal/util/inspect.js (2)

• codes (227-227)
• ObjectKeys (94-94)

🔇 Additional comments (7)

test/js/node/util/util.test.js (3)

345-349: LGTM! Tests updated to use new options API.

The tests correctly use the { validateStream: false } option to force colorization regardless of stream TTY status, which exercises the escape hatch behavior.

---

384-402: Excellent test coverage for stream TTY behavior.

The test comprehensively validates:

• TTY stream → ANSI codes applied
• Non-TTY stream → plain text output
• "none" format → plain text regardless of TTY
• validateStream: false → forced colorization
• Array format with non-TTY → plain text

The test implementation correctly creates custom Writable streams with isTTY property to exercise the stream validation logic.

---

404-409: Good addition - addresses the previous review comment.

This test validates the error path when an invalid stream type is passed, ensuring the stream type validation throws ERR_INVALID_ARG_TYPE as expected. This addresses the suggestion from the past review.

src/js/node/util.ts (4)

6-8: LGTM! Proper imports for new stream validation functionality.

The imports align with the coding guidelines by using internal modules for validation and color detection logic.

---

200-210: LGTM! Clean implementation of stream validation and TTY detection.

The function signature with default parameters is well-designed:

• validateStream = true by default ensures stream validation is performed
• stream = process.stdout provides sensible default for terminal output
• Validation logic correctly checks for readable/writable/node streams
• colors.shouldColorize(stream) properly honors stream TTY and color depth

The validateStream: false escape hatch allows forced colorization when needed (e.g., testing).

---

212-223: LGTM! Robust multi-format handling with proper validation.

The implementation correctly:

• Normalizes single format or array to uniform array representation using $isJSArray intrinsic
• Skips the special "none" format (which explicitly means no styling)
• Validates invalid format keys with helpful error message listing valid options
• Collects ANSI code pairs for each valid format
• Skips code collection when colorization should be suppressed

---

225-236: LGTM! Efficient ANSI code generation with proper nesting.

The implementation:

• Returns plain text early when colorization is disabled (performance optimization)
• Builds nested ANSI codes correctly by prepending opening codes (left) and prepending closing codes to achieve proper nesting order (right)
• Handles multiple formats properly (e.g., ["bold", "red"] → \u001b[1m\u001b[31mtext\u001b[39m\u001b[22m)

The right-to-left assembly of closing codes ensures proper ANSI nesting when multiple formats are applied.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6b5de25 and fb17bc5.

📒 Files selected for processing (2)

• src/js/node/util.ts
• test/js/node/util/util.test.js

🧰 Additional context used

📓 Path-based instructions (3)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

• test/js/node/util/util.test.js

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use .$call() and .$apply() instead of .call() and .apply() to prevent user tampering with function invocation
Use string literal require() statements only; dynamic requires are not permitted
Export modules using export default { ... } syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with $) such as $Array.from(), $isCallable(), and $newArrayWithSize() for performance-critical operations
Use private globals and methods with $ prefix (e.g., $Array, map.$set()) instead of public JavaScript globals
Use $debug() for debug logging and $assert() for assertions; both are stripped in release builds
Validate function arguments using validators from internal/validators and throw $ERR_* error codes for invalid arguments
Use process.platform and process.arch for platform detection; these values are inlined and dead-code eliminated at build time

Files:

• src/js/node/util.ts

src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

Builtin functions must include this parameter typing in TypeScript to enable direct method binding in C++

Files:

• src/js/node/util.ts

🧠 Learnings (6)

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always check exit codes and test error scenarios in error tests

Applied to files:

• test/js/node/util/util.test.js

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure

Applied to files:

• test/js/node/util/util.test.js

📚 Learning: 2025-11-24T18:37:11.466Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Validate function arguments using validators from `internal/validators` and throw `$ERR_*` error codes for invalid arguments

Applied to files:

• src/js/node/util.ts

📚 Learning: 2025-11-24T18:37:11.466Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `$debug()` for debug logging and `$assert()` for assertions; both are stripped in release builds

Applied to files:

• src/js/node/util.ts

📚 Learning: 2025-11-24T18:37:11.466Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use `process.platform` and `process.arch` for platform detection; these values are inlined and dead-code eliminated at build time

Applied to files:

• src/js/node/util.ts

📚 Learning: 2025-11-24T18:37:11.466Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/js/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:11.466Z
Learning: Applies to src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js} : Use string literal `require()` statements only; dynamic requires are not permitted

Applied to files:

• src/js/node/util.ts

🧬 Code graph analysis (1)

src/js/node/util.ts (2)

src/js/builtins/ReadableStreamInternals.ts (1)

• isReadableStream (658-663)

src/js/internal/util/inspect.js (2)

• codes (227-227)
• ObjectKeys (94-94)

🔇 Additional comments (5)

src/js/node/util.ts (3)

6-8: LGTM on expanded imports.

The imports correctly bring in the required validators and stream utilities needed for the new stream validation and colorization logic.

---

200-210: LGTM on stream validation and colorization control.

The implementation correctly:

• Validates the options parameter with proper boolean validation
• Validates stream type when validateStream is true (supporting both Web and Node.js streams)
• Provides an escape hatch via validateStream: false that bypasses stream checks and forces colorization (useful for testing)

The behavior where skipColorize remains undefined (falsy) when validateStream is false is intentional and results in colorization always being applied, matching the test expectations.

---

212-236: LGTM on format handling and ANSI code construction.

The implementation correctly:

• Uses $isJSArray intrinsic per coding guidelines
• Validates all formats even when colorization is skipped (ensures consistent error behavior)
• Handles "none" format via early continue, resulting in unstyled output
• Constructs nested ANSI codes properly with opening codes prepended and closing codes appended in reverse order

test/js/node/util/util.test.js (2)

345-348: LGTM on updated multiplecolors test.

The test correctly uses { validateStream: false } to bypass stream validation and ensure colorized output for testing purposes.

---

380-382: LGTM on styleText test update.

The test correctly verifies colorized output when stream validation is bypassed.