Skip to content

fix: enable color output when --tty flag is set#1714

Merged
Mossaka merged 1 commit intomainfrom
fix/1427-tty-color-detection
Apr 6, 2026
Merged

fix: enable color output when --tty flag is set#1714
Mossaka merged 1 commit intomainfrom
fix/1427-tty-color-detection

Conversation

@Mossaka
Copy link
Copy Markdown
Collaborator

@Mossaka Mossaka commented Apr 6, 2026

Summary

  • Ties color behavior to the existing --tty CLI flag: when set, uses FORCE_COLOR=1, TERM=xterm-256color, COLUMNS=120 instead of unconditional NO_COLOR=1
  • Prevents host TERM from overriding the explicit xterm-256color when --tty is active
  • Adds tests for both tty and non-tty color environment behavior

Closes #1427

Test plan

  • All 1288 unit tests pass
  • Lint passes (0 errors)
  • Verify awf --tty --allow-domains example.com 'env | grep -E "FORCE_COLOR|TERM|COLUMNS|NO_COLOR"' shows correct env vars

🤖 Generated with Claude Code

When --tty is set, use FORCE_COLOR=1, TERM=xterm-256color, and
COLUMNS=120 instead of NO_COLOR=1 so tools get proper color detection
and terminal dimensions.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 6, 2026 17:39
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 86.20% 86.29% 📈 +0.09%
Statements 86.07% 86.16% 📈 +0.09%
Functions 87.41% 87.41% ➡️ +0.00%
Branches 78.57% 78.61% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/docker-manager.ts 86.6% → 87.0% (+0.39%) 86.1% → 86.5% (+0.38%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates agent container color/TTY behavior so that ANSI color output is enabled only when the existing --tty flag is used, while keeping default non-TTY runs free of escape codes that can break log/snapshot assertions.

Changes:

  • Switches agent environment color controls to FORCE_COLOR=1, TERM=xterm-256color, COLUMNS=120 when config.tty is true; otherwise sets NO_COLOR=1.
  • Prevents host TERM from overriding the explicit TERM=xterm-256color when --tty is active.
  • Adds unit tests covering both tty and non-tty environment variable behavior.
Show a summary per file
File Description
src/docker-manager.ts Implements conditional color/TERM env handling based on config.tty and avoids inheriting host TERM in tty mode.
src/docker-manager.test.ts Adds coverage verifying env var differences between tty and non-tty configurations.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

Smoke test results (run 24042861995)

✅ GitHub MCP — Last 2 merged PRs: "⚡ pelis-agent-factory-advisor: pre-fetch content..." (#1701), "chore: upgrade gh-aw to v0.67.0..." (#1686)
✅ Playwright — github.com title contains "GitHub"
✅ File write — /tmp/gh-aw/agent/smoke-test-claude-24042861995.txt created
✅ Bash verify — file content confirmed

Overall: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

🔥 Smoke Test Results

Test Status
GitHub MCP connectivity
GitHub.com HTTP
File write/read

PR: fix: enable color output when --tty flag is set
Author: @Mossaka

Overall: PASS

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

Smoke Test Status

PR Titles: ⚡ pelis-agent-factory-advisor: pre-fetch content, restrict tools, reduce prompt tokens (~21% token savings); chore: upgrade gh-aw to v0.67.0 and recompile all workflows

  1. GitHub MCP merged PR review: ✅
  2. safeinputs-gh PR query: ❌
  3. Playwright title contains "GitHub": ✅
  4. Tavily web search: ❌
  5. File write test: ✅
  6. Bash cat verification: ✅
  7. Discussion query + mystical comment: ❌
  8. npm ci && npm run build: ✅
    Overall: FAIL

🔮 The oracle has spoken through Smoke Codex

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

🧪 Chroot Version Comparison Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3 ❌ NO
Node.js v24.14.1 v20.20.2 ❌ NO
Go go1.22.12 go1.22.12 ✅ YES

Result: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot environment.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 6, 2026

Smoke Test: GitHub Actions Services Connectivity

Check Result Details
Redis PING (host.docker.internal:6379) ❌ FAILED redis-cli not installed; port unreachable (no sudo available to install)
PostgreSQL pg_isready (host.docker.internal:5432) ❌ FAILED no response — service not listening
PostgreSQL SELECT 1 (smoketest db) ❌ FAILED Cannot connect; pg_isready confirms port is down

Root cause: No GitHub Actions service containers (Redis, PostgreSQL) are running in this environment. The host.docker.internal entry resolves to 172.17.0.1 but no services are bound on those ports. Additionally, redis-cli is not pre-installed and sudo is unavailable for package installation.

All 3 checks failed — smoke-services label not applied.

🔌 Service connectivity validated by Smoke Services

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts how the agent container’s ANSI color behavior is configured by tying it to the existing --tty flag, addressing #1427 where lack of TTY breaks color detection and interactive output.

Changes:

  • When --tty is set, enable color-oriented env vars (FORCE_COLOR=1, TERM=xterm-256color, COLUMNS=120); otherwise default to NO_COLOR=1.
  • Prevent host TERM from being inherited when --tty is enabled (so TERM=xterm-256color remains effective).
  • Add unit tests for tty vs non-tty color environment behavior.
Show a summary per file
File Description
src/docker-manager.ts Updates agent container environment construction to toggle color-related env vars based on config.tty, and adjusts host TERM inheritance logic.
src/docker-manager.test.ts Adds assertions for the new tty/non-tty env behavior.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Comment on lines +543 to +549
...(config.tty ? {
FORCE_COLOR: '1',
TERM: 'xterm-256color',
COLUMNS: '120',
} : {
NO_COLOR: '1',
}),
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When config.envAll is enabled and config.tty is true, this block does not set NO_COLOR, so NO_COLOR from the host environment can be injected later by the --env-all passthrough and end up disabling colors even though --tty was requested. Consider explicitly excluding NO_COLOR (and possibly related variables) when config.tty is true, or deleting environment.NO_COLOR after the env-all merge to ensure tty reliably enables color output.

Copilot uses AI. Check for mistakes.
expect(env.COLUMNS).toBe('120');
expect(env.NO_COLOR).toBeUndefined();
});

Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current tests validate the default (tty=false) and tty=true env values, but they don’t cover common override scenarios that this PR is meant to address: (1) host TERM being present should not override TERM=xterm-256color when tty=true, and (2) with envAll: true, host NO_COLOR should not leak into the agent env when tty=true. Adding targeted tests for these cases would prevent regressions.

Suggested change
it('should not let host TERM override TERM=xterm-256color when tty is true', () => {
const originalTerm = process.env.TERM;
process.env.TERM = 'screen-256color';
try {
const ttyConfig = { ...mockConfig, tty: true };
const result = generateDockerCompose(ttyConfig, mockNetworkConfig);
const agent = result.services.agent;
const env = agent.environment as Record<string, string>;
expect(env.FORCE_COLOR).toBe('1');
expect(env.TERM).toBe('xterm-256color');
expect(env.COLUMNS).toBe('120');
expect(env.NO_COLOR).toBeUndefined();
} finally {
if (originalTerm === undefined) {
delete process.env.TERM;
} else {
process.env.TERM = originalTerm;
}
}
});
it('should not leak host NO_COLOR into agent env when envAll is true and tty is true', () => {
const originalNoColor = process.env.NO_COLOR;
process.env.NO_COLOR = '1';
try {
const ttyEnvAllConfig = { ...mockConfig, tty: true, envAll: true };
const result = generateDockerCompose(ttyEnvAllConfig, mockNetworkConfig);
const agent = result.services.agent;
const env = agent.environment as Record<string, string>;
expect(env.FORCE_COLOR).toBe('1');
expect(env.TERM).toBe('xterm-256color');
expect(env.COLUMNS).toBe('120');
expect(env.NO_COLOR).toBeUndefined();
} finally {
if (originalNoColor === undefined) {
delete process.env.NO_COLOR;
} else {
process.env.NO_COLOR = originalNoColor;
}
}
});

Copilot uses AI. Check for mistakes.
@Mossaka Mossaka merged commit 2c81abd into main Apr 6, 2026
74 of 77 checks passed
@Mossaka Mossaka deleted the fix/1427-tty-color-detection branch April 6, 2026 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Agent container lacks TTY, breaking color detection and interactive output

2 participants