fix(log): no ANSI colour codes unless stderr is a terminal

[krassx]

Logs written to Xcode/Gradle build output, CI logs and the detached bugsee-cli.log contained ANSI escape codes, since tracing_subscriber::fmt() enables ANSI by default.

Colour is now enabled only when stderr is a terminal, and a non-empty NO_COLOR disables it (https://no-color.org).

Tested: cargo fmt --check, cargo clippy --all-targets -- -D warnings, cargo test all pass; unit tests for the decision function.

🤖 Generated with Claude Code

[claude]

Claude finished @krassx's task in 37s —— View job

---

Code review: PR #85 (no ANSI codes unless stderr is a terminal)

I reviewed the diff against origin/main and read the surrounding main.rs and the daemonize path. I did not run cargo build, cargo test or clippy.

Findings (2, both P3, posted inline)

• P3, src/main.rs:23: The new ansi_enabled fn was inserted between the // NOT #[tokio::main] comment block and fn main(). That comment, which guards the fork-before-runtime invariant, is now attached to the wrong function. Move the helper above or below it.
• P3, src/main.rs:74-77: The tests cover only the pure predicate. Nothing pins the .with_ansi(...) wiring through the compiled binary, which CLAUDE.md asks for. Add an assert_cmd test that asserts piped stderr contains no \x1b. tests/elf_upload.rs already captures stderr and has a strip_ansi helper that is now unnecessary.

Checked, no problem found

• Daemon path: The decision is evaluated after daemon::daemonize has redirected fd 2 to bugsee-cli.log, so is_terminal() is false there. The fix therefore works for the detached log. If the subscriber setup were ever moved ahead of the fork, it would break.
• stdout contract: Unchanged. Only the stderr writer is affected.
• NO_COLOR semantics: is_none_or(|v| v.is_empty()) matches no-color.org, where an empty value is ignored. Option::is_none_or is stable since Rust 1.82, so the 1.88 MSRV is fine.
• Exit codes and wire shapes: Untouched.
• Behaviour change: Output piped through less -R or forced colour in CI no longer gets colour, and there is no CLICOLOR_FORCE escape hatch. This is a reasonable trade-off and I'm not reporting it.

Overall risk: Low
Merge recommendation: Approve once the comment placement is fixed. The integration test is nice to have.
Most important issues:

1. Restore the // NOT #[tokio::main] comment next to fn main().
2. Add a binary-level no-\x1b assertion.

Positives: The change is small and uses the right criterion (stderr TTY plus NO_COLOR). It is a pure, testable decision function, and the daemon ordering is already correct.
· branch fix/no-ansi-when-not-terminal

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 155b8770-8c7d-42a6-b668-7cabbde5c332

📥 Commits

Reviewing files that changed from the base of the PR and between ac4b678 and 724c516.

📒 Files selected for processing: 2

• src/main.rs
• tests/no_ansi_when_piped.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

📝 Summary

Summary by CodeRabbit

• Improvements
  • Terminal output now uses ANSI colors when supported, unless NO_COLOR is set to a non-empty value.
  • Output remains uncolored when sent to a non-terminal, helping keep redirected output readable and compatible with tools that expect plain text.
  • These color rules apply to informational logging as well as other terminal output, while preserving plain text in piped output.

The tracing subscriber now enables ANSI output only when stderr is a terminal and NO_COLOR is absent or empty. Unit tests cover terminal status and NO_COLOR values. An integration test runs sourcemaps inject with piped stderr and checks that the completion log contains no ANSI escape byte.

Priority: ➖ Normal

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 724c5

No actionable merge-blocking issue is identified; the change is mergeable after normal checks.

Pre-merge checks: 5

Check name	Status	Explanation
Description check		The description directly explains the logging change, the affected environments, the NO_COLOR behavior, and the tests performed.
Title check		The title clearly and concisely describes the main change: disabling ANSI color codes unless stderr is a terminal.
Docstring Coverage		Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[claude]

Severity: P3 Low
Problem: The unit tests only cover the pure predicate ansi_enabled. Nothing checks that the wiring (.with_ansi(...), and the decision happening after daemonize so the redirected fd 2 is seen) actually produces escape-free output. CLAUDE.md says tests/ should pin behaviour through the compiled binary.
Scenario: Someone drops .with_ansi(...) or moves the subscriber setup ahead of daemonize. Every test still passes and the escape codes come back in Xcode, Gradle and CI logs.
Fix: Add an assert_cmd test that runs the binary with stderr piped (so it's not a TTY) and asserts the captured stderr contains no \x1b. tests/elf_upload.rs already captures stderr and has a strip_ansi helper. That helper is now unnecessary for piped stderr, and an assertion there would pin the fix cheaply.

[claude]

Severity: P3 Low
Problem: ansi_enabled was inserted between the existing // NOT #[tokio::main] comment block (lines 16-19) and fn main(). That block now sits above ansi_enabled, so it documents the wrong item. The new /// doc comment also runs on from it as one comment.
Impact: The comment guarding the fork-before-runtime invariant, which CLAUDE.md calls load-bearing, is no longer next to main. A future editor of main won't see it.
Fix: Move ansi_enabled above the // NOT #[tokio::main] block, or below main, so the block stays attached to fn main().

[claude]

Claude finished @krassx's task in 22s —— View job

---

Code review: PR #85 (no ANSI codes unless stderr is a terminal), re-review after push

I re-read the full diff against origin/main, plus tests/common/mod.rs and the sourcemaps inject log line the new test depends on. I did not run cargo build, cargo test or clippy.

Earlier findings: both addressed by 724c516

• Comment placement: ansi_enabled and its tests now sit below main. The // NOT #[tokio::main] block is attached to fn main() again.
• Binary-level test: tests/no_ansi_when_piped.rs runs the compiled binary with piped stderr and asserts no 0x1b byte. I checked that the test's dependencies exist:
  • common::cli() is there. It calls env_clear(), so the env_remove("NO_COLOR") is redundant but harmless.
  • sourcemaps inject complete is logged at info in src/cli/sourcemaps.rs:73.
  • The test also asserts that the log line is present, so it can't pass by accident with empty stderr.

New findings: none.

Checked, no problem found

• Daemon path: the subscriber is initialised after the fork and the fd 2 redirect, so is_terminal() is false for the detached bugsee-cli.log. The new test does not cover this path.
• stdout contract, exit codes, wire shapes: untouched.
• MSRV: Option::is_none_or is already used in src/symbols/elf.rs:269, so it adds no new floor.

Overall risk: Low
Merge recommendation: Approve. The CI gates (fmt, clippy, test) should be green before merging.
Most important issues: none outstanding.

Positives: The change is small. The decision is a pure function with unit tests. It is also pinned end to end through the binary. The NO_COLOR handling follows no-color.org.
· branch fix/no-ansi-when-not-terminal