fix(api): default TERM to xterm-256color for tmux PTY attach#249
Closed
oksusucha wants to merge 1 commit into
Closed
fix(api): default TERM to xterm-256color for tmux PTY attach#249oksusucha wants to merge 1 commit into
oksusucha wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to fix broken terminal rendering when attaching to tmux via the API WebSocket by ensuring the tmux attach subprocess has a usable TERM value (defaulting to xterm-256color) while preserving an explicitly configured TERM.
Changes:
- Copy the parent process environment before spawning
tmux attach-session. - Attempt to default
TERMtoxterm-256color. - Pass the patched environment to
subprocess.Popen(..., env=...)for tmux attach.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| # Start tmux attach inside the PTY | ||
| pty_env = os.environ.copy() | ||
| pty_env.setdefault("TERM", "xterm-256color") |
Comment on lines
+748
to
+757
| pty_env = os.environ.copy() | ||
| pty_env.setdefault("TERM", "xterm-256color") | ||
| proc = subprocess.Popen( | ||
| ["tmux", "-u", "attach-session", "-t", f"{session_name}:{window_name}"], | ||
| stdin=slave_fd, | ||
| stdout=slave_fd, | ||
| stderr=slave_fd, | ||
| close_fds=True, | ||
| preexec_fn=os.setsid, | ||
| env=pty_env, |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #249 +/- ##
=======================================
Coverage ? 92.84%
=======================================
Files ? 65
Lines ? 5507
Branches ? 0
=======================================
Hits ? 5113
Misses ? 394
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
When CAO runs inside Docker or devcontainers where TERM is unset or set to 'dumb', tmux attach inherits the broken value causing garbled terminal rendering. Default TERM to xterm-256color before spawning the subprocess. Fixes awslabs#150
b9b50b5 to
ce863d2
Compare
This was referenced May 23, 2026
haofeif
pushed a commit
that referenced
this pull request
May 23, 2026
Container and devcontainer environments routinely leave TERM unset or set to ``dumb``. tmux's ``attach-session`` subprocess inherited that value because the WebSocket PTY endpoint never passed ``env=`` to ``subprocess.Popen``, which broke colours, cursor positioning, and the Ink-based TUIs that Claude Code / Codex / Gemini render. The browser- side xterm.js then received text that was never written in a form it could render. Add a small ``_build_pty_env`` helper that copies the parent process environment and forces ``TERM`` to ``xterm-256color`` when the inherited value is unset, empty, or ``dumb``. Explicit non-dumb values (e.g. ``screen-256color``) are preserved verbatim. Resolves #150. Supersedes the abandoned #249, which used ``dict.setdefault`` and therefore only handled the unset case — not ``TERM=dumb`` — and shipped without unit tests (codecov flagged 0%). Tests cover the helper (unset / empty / dumb / xterm / custom value preserved / non-TERM env vars inherited) and a wiring guard that patches ``subprocess.Popen`` to assert the endpoint actually hands the corrected env to the tmux attach call.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Default
TERMtoxterm-256colorin the tmux attach subprocess environment. Usessetdefaultso an explicitly configuredTERMis preserved.Problem
When CAO runs inside Docker or devcontainers where
TERMis unset or set todumb, thetmux attach-sessionsubprocess inherits the broken value. This causes garbled terminal rendering — no colors, corrupted cursor positioning, and broken TUI output from agent CLIs (Claude Code, Kiro, Codex, etc.).Fix
setdefault("TERM", "xterm-256color")— only applies when TERM is missing or emptysubprocess.PopenNet change: +3 lines in
src/cli_agent_orchestrator/api/main.py.Testing
mainTERMis already set (setdefault is a no-op)Fixes #150