fix: disable ANSI colors when GitHub reporter is auto-enabled#9236
fix: disable ANSI colors when GitHub reporter is auto-enabled#9236slegarraga wants to merge 1 commit intobiomejs:mainfrom
Conversation
When `biome ci` runs in GitHub Actions, the GitHub reporter is automatically enabled via the GITHUB_ACTIONS environment variable. However, the color output was still being forced on, causing ANSI escape sequences (ESC[0m) to corrupt the GitHub reporter output and break GitHub's annotation parsing. This fix checks for the GITHUB_ACTIONS environment variable in the color resolution logic and disables colors when running in GitHub Actions, consistent with the existing behavior when --reporter=github is explicitly passed. Closes biomejs#9189
|
WalkthroughWhen running in CI mode without explicit color configuration, the code now detects GitHub Actions via the GITHUB_ACTIONS environment variable and disables colour output in that environment. If GitHub Actions is not detected, colours remain enabled in CI. A minor comment wording fix is also included. Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_cli/src/commands/mod.rs (1)
727-736: Add an explanatory comment above thecfg!(debug_assertions)guard to matchci.rs.
ci.rs(lines 155–156) already has a comment explaining why the guard exists ("This is funny, but we need to disable this at the moment, otherwise all our tests that runbiome ciIN OUR CI, will get false positives. Call it CI-ception"). The new code inmod.rshas none, leaving future readers guessing. Either add a similar short comment, or extract the check into a tiny helper function for consistency and testability:♻️ Optional refactor — extract helper for testability
+ fn is_github_actions() -> bool { + // Skip in debug builds to avoid test-suite env-var pollution + if cfg!(debug_assertions) { + return false; + } + std::env::var("GITHUB_ACTIONS") + .ok() + .is_some_and(|v| v == "true") + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_cli/src/commands/mod.rs` around lines 727 - 736, The code computing is_github_actions uses a cfg!(debug_assertions) guard without explanation; add a brief comment above that guard (similar to the one in ci.rs) describing why we skip the GITHUB_ACTIONS check in debug builds (to avoid CI runs in tests producing false positives / "CI-ception"), or alternatively extract the logic into a small helper function (e.g., is_github_actions()) and place the explanatory comment on that helper for clarity and testability; reference the existing variable name is_github_actions and the ColorsArg::Off return so the change is made where this check is performed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_cli/src/commands/mod.rs`:
- Around line 727-736: The code computing is_github_actions uses a
cfg!(debug_assertions) guard without explanation; add a brief comment above that
guard (similar to the one in ci.rs) describing why we skip the GITHUB_ACTIONS
check in debug builds (to avoid CI runs in tests producing false positives /
"CI-ception"), or alternatively extract the logic into a small helper function
(e.g., is_github_actions()) and place the explanatory comment on that helper for
clarity and testability; reference the existing variable name is_github_actions
and the ColorsArg::Off return so the change is made where this check is
performed.
| // However, when running inside GitHub Actions, we need to disable colors | ||
| // so the auto-enabled GitHub reporter output is not corrupted by ANSI escape codes | ||
| if matches!(self, Self::Ci { .. }) && cli_options.colors.is_none() { | ||
| let is_github_actions = if cfg!(debug_assertions) { |
There was a problem hiding this comment.
Please extract this code into a utility function. We have the same code in another place. Please keep the comment of the other code
|
Duplicate of #9215 |
|
Yeah we should go with the other |
Summary
When
biome ciruns in GitHub Actions, the GitHub reporter is automatically enabled via theGITHUB_ACTIONSenvironment variable (added in #9120). However, the color output was still being forced on in theget_color()method, causing ANSI escape sequences (ESC[0m) to wrap GitHub reporter output lines and break GitHub's annotation parsing.Fix
In the
get_color()method, before force-enabling colors for CI, check if we're running inside GitHub Actions. If so, disable colors — matching the existing behavior when--reporter=githubis explicitly passed.This implements option 1 from the issue: make
biome ci(whenGITHUB_ACTIONS=true) behave the same as--reporter=default --reporter=githubwith respect to color output.The
cfg!(debug_assertions)guard is included (consistent withci.rs) to avoid false positives in Biome's own CI test suite.Closes #9189