[Test Improver] fix: suppress ANSI escape codes in non-TTY contexts

[danielmeppiel]

🤖 This is an automated change from Test Improver, an AI assistant.

Summary

_get_console() in src/apm_cli/utils/console.py created Console() with no arguments. Rich's auto-detection uses sys.__stdout__.isatty() (the original stdout), not the stream it actually writes to. This caused ANSI escape sequences (bold \x1b[1m, italic \x1b[3m, reset \x1b[0m) to be emitted even when writing to pipes, files, or test-runner capture buffers.

This broke 7 tests in tests/unit/commands/test_policy_status.py that were added in the most recent commit to enforce the repo's ASCII-only encoding rule.

Root cause

# Before: Console() uses sys.__stdout__.isatty() for detection,
# ignoring the actual write target (which may be a non-TTY buffer).
return Console()

Fix

# After: explicitly set force_terminal based on sys.stdout.isatty(),
# which IS replaced by CliRunner/pytest capture, so detection is accurate.
is_tty = getattr(sys.stdout, "isatty", lambda: False)()
return Console(force_terminal=is_tty)
```

**Behaviour change:**
- In a real terminal (`isatty()` → True): ANSI codes emitted as before — no visible change for users
- In non-TTY contexts (pipes, files, test runners): ANSI codes suppressed
- Box-drawing characters (Rich table borders) still emitted in both modes — `_ascii_only()` explicitly permits these

## Test Status

| Suite | Before | After |
|---|---|---|
| `tests/unit/commands/test_policy_status.py` | 7 failed, 33 passed | **40 passed** |
| Full unit suite (`tests/unit tests/test_console.py`) | 5262 passed | **5302 passed** |

```
uv run pytest tests/unit tests/test_console.py -x -q
# 5302 passed, 1 warning, 26 subtests passed

Scope

Only src/apm_cli/utils/console.py is modified (9 lines — 7 added, 2 changed). No test files changed, no new dependencies.

Generated by Daily Test Improver · ◷

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/daily-test-improver.md@b87234850bf9664d198f28a02df0f937d0447295

[github-actions]

APM Review Panel Verdict

Disposition: APPROVE (with two required actions before merge)

---

Per-persona findings

Python Architect: Clean, minimal, proportional fix. getattr(sys.stdout, "isatty", lambda: False)() is a correct defensive pattern — handles I/O objects that lack isatty and correctly reads sys.stdout (the replaced stream in CliRunner/pytest capture), not sys.__stdout__ (the original). Each call to _get_console() creates a fresh Console instance, which means TTY state is re-evaluated per helper invocation; this is pre-existing behavior and is actually correct for programs that may redirect stdout mid-execution. No architectural concerns. PASS.

CLI Logging Expert: This is the right fix. Rich's Console() defaulting to sys.__stdout__.isatty() is a well-known footgun — it ignores redirected or captured streams, emitting ANSI bold/italic/color sequences into piped output, test captures, and CI logs. force_terminal=is_tty resolves it cleanly. One issue with the new docstring: "Box-drawing characters used in Rich tables are still emitted; those are acceptable Unicode output" overstates what the repo encoding rule permits. The _ascii_only() test helper tolerates Rich box-drawing in rendered output (it cannot suppress library-level rendering), but the repo rule says source/CLI output strings we author must stay ASCII. The docstring should say box-drawing is "not suppressed by this change" rather than "acceptable." Minor but inaccurate. PASS on code, docstring needs minor wording fix.

DevX UX Expert: No command surface changes. The fix aligns APM with standard practice for all major package managers (npm, pip, cargo) which suppress ANSI when stdout is not a TTY. Piped output that previously carried stray \x1b[1m/\x1b[0m sequences would have broken downstream parsers and scripts. This is the correct UX behavior. No CLI reference doc updates needed — no user-facing command surface changed. PASS.

Supply Chain Security Expert: No dependency changes, no auth/token/lockfile surface, no path operations. Single-file utility fix. No new attack surface introduced. PASS.

Auth Expert: Not applicable to this change.

OSS Growth Hacker: No conversion surface impact. Fixing test failures and ANSI leakage is a positive quality signal for external contributors who run the test suite — fewer mysterious failures on first checkout. No WIP/growth-strategy.md update needed for this scale of change.

---

CEO arbitration

This is a focused, correct, and low-risk bug fix generated by the Daily Test Improver agentic workflow. The core change (Console(force_terminal=is_tty)) is unambiguously right — Rich's auto-detection relies on sys.__stdout__ which is not replaced by test runners, so ANSI codes were leaking into captured output and breaking the recently-added ASCII-only assertions. The fix resolves 40 tests cleanly with zero behavior change for real-terminal users.

Two items block a clean merge under repo conventions: a missing CHANGELOG entry (required for every code-touching PR per CHANGELOG.md rules) and a docstring clause that slightly misrepresents the repo's encoding policy. Both are trivial to address — the maintainer can amend before merging. The automated-PR format is fine; no naming or positioning concerns.

---

Required actions before merge

1. Add CHANGELOG entry. Per repo convention, every PR that touches code requires an entry in ## [Unreleased]. Suggested under ### Fixed:

   - Fix: suppress ANSI escape codes (`\x1b[...m`) in non-TTY contexts by passing `force_terminal=is_tty` to Rich `Console()`, resolving ANSI bleed into pipes, files, and test-runner capture buffers. (#890)
   

2. Correct the docstring wording. Replace:

   "Box-drawing characters used in Rich tables are still emitted; those are acceptable Unicode output."

   With something like:

   "Rich table box-drawing characters are not suppressed by this change; they originate from the library renderer, not from CLI strings we author."

   This avoids implying the repo encoding rule explicitly endorses box-drawing in CLI output.

---

Optional follow-ups

• Consider whether _get_console() should be memoized per-process (with a reset hook for tests) to avoid constructing a new Console instance on every _rich_echo / _rich_info / _rich_panel call. This is a pre-existing pattern and not a blocker, but at scale it produces many short-lived objects.
• The _ascii_only() helper in test_policy_status.py silently tolerates Rich box-drawing characters (U+2500-U+257F). If the repo encoding rule is intended to apply to rendered CLI output too, a follow-up PR could configure a Console(ascii_only=True) in non-TTY mode — but this is out of scope for this fix.

Generated by PR Review Panel for issue #890 · ● 513.8K · ◷