Skip to content

fix(#2947): preserve TERM_PROGRAM for claude-teams across Go + Swift surfaces - #2999

Closed
lum wants to merge 5 commits into
manaflow-ai:mainfrom
lum:issue-2947-claude-teams-term-program
Closed

lum wants to merge 5 commits into
manaflow-ai:mainfrom
lum:issue-2947-claude-teams-term-program

Conversation

@lum

@lum lum commented Apr 19, 2026 •

Copy link
Copy Markdown

Summary

Fixes #2947 — cmux claude-teams crashes during teammate→team-lead permission escalation on Claude Code v2.1.112+. Root cause: cmux's agent-launch shims unconditionally unsetenv("TERM_PROGRAM") for every agent type, but Claude Code's new permission flow requires TERM_PROGRAM to be set or its cli.js crashes mid-prompt.

This PR fixes both surfaces — the Go remote daemon (cmuxd-remote) and the Swift local CLI — and adds the test coverage that was missing from the original report.

Why this supersedes #2960

PR #2960 (by @bobo-xxx) correctly identified the bug and patched the Go daemon. However, the original repro in #2947 is local-Mac (cmux claude-teams typed directly into a terminal pane), which goes through CLI/cmux.swift, not cmuxd-remote. So #2960 alone doesn't reach the reporter's crash path.

Rather than open a Swift-only companion PR (and leave maintainers to coordinate two reviews / two test plans), this PR includes:

Happy to defer to whatever the maintainers prefer — see the comment I'll leave on #2960.

What's in this PR

Bug fix (verified end-to-end)

  • Go (daemon/remote/cmd/cmuxd-remote/agent_launch.go) — gates os.Unsetenv("TERM_PROGRAM") on a per-agent flag. claude-teams preserves; opencode-family (omo/omx/omc) unsets.
  • Swift (CLI/cmux.swift) — same asymmetry on the local CLI path. Without this, the original reporter's local-Mac crash still reproduces.

Adversarial-review improvements

  1. preserveTermProgram (zero-value-safe naming) — CodeRabbit flagged the original unsetTermProgram field as having an unsafe zero value (false = preserve = the rare exception, not the historical default). Renamed across both surfaces; the zero value now matches the legacy "unset" behavior, so any new agent type defaults to the safe choice and only claude-teams opts in.

  2. infocmp xterm-ghostty probe — defaulting TERM to xterm-ghostty is the right call, but cmux's terminfo overlay installs asynchronously during shell bootstrap on remote SSH hosts (see tests_v2/test_ssh_remote_shell_integration.py:536-615 which documents that fresh containers don't have the entry preinstalled). If an agent process exec's before that install completes, it inherits TERM=xterm-ghostty without the matching terminfo entry → ncurses falls back to a dumb terminal. Both surfaces now probe infocmp xterm-ghostty and fall back to xterm-256color (universally supported).

  3. COLORTERM read alignment — the Swift implementation read from a processEnvironment snapshot taken before any env mutations; Go reads os.Getenv (live). Functionally equivalent today but fragile to re-orderings. Both surfaces now read live for parity.

  4. Comment accuracy — opencode reacts to any non-empty TERM_PROGRAM (treats it as an outer-host marker), not specifically =ghostty as the original comment implied. Updated in both files.

Tests

File What it covers
tests/test_cli_claude_teams_env.py Asserts the new TERM=xterm-ghostty + preserved TERM_PROGRAM contract for claude-teams (was: asserted the old contract; would have broken on this PR). Updated, not new.
tests/test_cli_omo_env.py New. Sibling test for cmux omo asserting TERM_PROGRAM is unset (regression guard for #2516 — opencode's light-theme switch on any non-empty TERM_PROGRAM), TERM=xterm-ghostty, and COLORTERM=truecolor fallback.
daemon/remote/cmd/cmuxd-remote/agent_launch_test.go New. 7 Go unit tests covering the full configureAgentEnvironment matrix (preserve / unset / TERM override / COLORTERM preservation / extraEnv) plus 2 tests for resolveDefaultTerm (terminfo race success + fallback).
daemon/remote/cmd/cmuxd-remote/tmux_compat_test.go Added a TERM_PROGRAM unset assertion to the existing TestConfigureAgentEnvironment (the test set TERM_PROGRAM=should-be-removed but never asserted the result).

Per CLAUDE.md's regression-test policy, the original test/fix pair (commits 8369e3c2 → 3b2b37b2) is a two-commit failing-test-then-fix sequence so CI on the test commit alone goes red — proving the test catches the bug.

Test plan

# Go matrix (8 tests, all green)
cd daemon/remote && go test ./cmd/cmuxd-remote/

# Python: claude-teams contract (phase 1 covers our changes; phase 2 has a
# pre-existing Node-24 incompatibility flagged below)
python3 tests/test_cli_claude_teams_env.py

# Python: OMO regression (#2516 guard)
python3 tests/test_cli_omo_env.py

# Manual GUI repro of #2947
./scripts/reload.sh --tag fix-2960-supersede --launch
# In the launched DEV app: cmux claude-teams → teamCreate → teammate triggers
# permission escalation. Should NOT crash (was: cli.js stack trace ending in
# `ov (/$bunfs/root/src/entrypoints/cli.js:477:76101)`).

# Manual GUI sanity for opencode-family theme:
# cmux omo → confirm dark theme intact (regression #2516 guard)

I've verified the GUI repro locally on macOS 26.4.1 / Apple Silicon. The original crash is gone; opencode's dark theme is intact.

Out of scope (flagged for follow-up)

  • Phase 2 of tests/test_cli_claude_teams_env.py (the --max-old-space-size 2048 --trace-warnings heap-flag scenario) fails on Node 24 because Node 24 rejects the space-separated form (--max-old-space-size 2048) and now requires --max-old-space-size=2048. Pre-existing; unrelated to this PR. CI's macOS VM probably runs an older Node where it still passes.

Related

🤖 Generated with Claude Code


Summary by cubic

Preserves TERM_PROGRAM for claude-teams across the Go daemon and Swift CLI to fix the permission-escalation crash in Claude Code v2.1.112+ (Fixes #2947). Also hardens terminal env defaults and adds coverage to prevent regressions.

  • Bug Fixes

    • Go daemon: add per‑agent preserveTermProgram; claude-teams preserves, opencode‑family (omo/omx/omc) unsets.
    • Swift CLI: mirror the preserve/unset behavior on the local path used by cmux claude-teams.
    • Default TERM via probe: use xterm-ghostty when infocmp xterm-ghostty succeeds, else fall back to xterm-256color.
    • Set COLORTERM=truecolor when absent to ensure truecolor rendering.
    • Tests: new Go unit tests for env matrix and terminfo probe; new tests/test_cli_omo_env.py; updated tests/test_cli_claude_teams_env.py.
  • Refactors

    • Rename flag to preserveTermProgram (safe default: unset); read COLORTERM from live env; clarify comments about opencode reacting to any non‑empty TERM_PROGRAM.

Written for commit 4f02421. Summary will update on new commits.

Summary by CodeRabbit

  • Bug Fixes

    • Improved terminal detection to dynamically resolve terminal type at runtime instead of using fixed defaults
    • Fixed terminal program preservation for Claude Teams
    • Enhanced color terminal support by automatically detecting and setting truecolor when appropriate
  • Tests

    • Added tests for terminal environment configuration
    • Added tests for OpenCode family integration with terminal settings

Steve Lum and others added 5 commits April 18, 2026 20:06
Pins the new contract for cmux claude-teams's tmux-compat env: TERM
defaults to xterm-ghostty (was screen-256color) and TERM_PROGRAM is
preserved (was unconditionally unset). Without the matching Swift CLI
fix, this test fails — exposing the Claude Code v2.1.112 crash from
issue manaflow-ai#2947 during permission escalation.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…#2947)

Mirrors PR manaflow-ai#2960's Go-daemon fix into the local-Mac Swift CLI path,
which is what `cmux claude-teams` actually goes through on the user's
machine. Without this, the Go-only fix doesn't reach the reporter's
crash.

- Add `unsetTermProgram: Bool` to configureTmuxCompatEnvironment.
- claude-teams: pass false (Claude Code v2.1.112+ needs TERM_PROGRAM
  set during permission escalation; missing it triggers the cli.js
  crash in manaflow-ai#2947).
- omo / omx / omc: pass true (opencode flips to a light theme on
  TERM_PROGRAM=ghostty; preserves prior behavior).
- Default TERM is now xterm-ghostty instead of screen-256color so
  agents render with full Ghostty fidelity, and COLORTERM=truecolor
  is set when the parent env didn't already supply one.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Fixes cmux crash when using claude-teams with teamCreate (issue manaflow-ai#2947).

Root cause: Claude Code v2.1.112 changed how it handles permission
escalation based on terminal environment variables. The previous
code unconditionally unset TERM_PROGRAM which caused Claude Code's
cli.js to crash during permissionExplainerEnabled checks.

Changes:
- Add unsetTermProgram flag to agentConfig to control TERM_PROGRAM
  handling per agent type
- claude-teams: preserve TERM_PROGRAM (needed for Ghostty detection)
- omo/omx/omc: unset TERM_PROGRAM (opencode switches to light theme)
- Change default TERM from screen-256color to xterm-ghostty
- Add COLORTERM=truecolor for proper truecolor support

(cherry picked from commit 4c6f9b2)
…Swift

Polish on top of the cherry-picked Go fix (manaflow-ai#2960) and our matching Swift
fix, addressing CodeRabbit + adversarial-review feedback.

Changes:

- Rename agentConfig.unsetTermProgram (Go) and the matching Swift
  parameter to preserveTermProgram. The zero/default value now matches
  the historically common case (TERM_PROGRAM unset, the legacy
  behavior), so any future agent type defaults to the safe choice and
  only claude-teams opts in to preservation. CodeRabbit flagged the
  original naming as zero-value-unsafe; both adversarial reviewers
  concurred.

- Add a resolveDefaultTerm() helper in both surfaces that probes
  `infocmp xterm-ghostty` and falls back to `xterm-256color` when the
  terminfo entry is missing. cmux's terminfo overlay installs
  asynchronously during shell bootstrap on remote SSH hosts (see
  tests_v2/test_ssh_remote_shell_integration.py for the documented
  race), so a child process exec'd before the install completes would
  otherwise inherit TERM=xterm-ghostty without the matching terminfo
  entry — landing it at a dumb terminal. xterm-256color is universally
  available.

- Read COLORTERM from the live process env (getenv) instead of the
  Swift-side processEnvironment snapshot, mirroring the Go daemon's
  behavior in agent_launch.go. Functionally equivalent today (nothing
  mutates COLORTERM between snapshot and check) but more robust to
  future re-orderings and keeps the two surfaces aligned.

- Tighten the asymmetry comment in both files. opencode reacts to any
  non-empty TERM_PROGRAM (treats it as an outer-host marker), not
  specifically TERM_PROGRAM=ghostty as the previous comment implied.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add comprehensive coverage for configureAgentEnvironment's behavior
matrix in cmuxd-remote, plus a sibling Python test for the local Swift
CLI's `cmux omo` path.

Go (daemon/remote/cmd/cmuxd-remote/agent_launch_test.go):

- TestConfigureAgentEnvironment_PreservesTermProgramWhenFlagSet — locks
  in claude-teams's preserve contract that prevents the Claude Code
  v2.1.112+ permission-escalation crash from manaflow-ai#2947.
- TestConfigureAgentEnvironment_OpencodeFamilyUnsetsTermProgram —
  guards against silent regression of manaflow-ai#2516 (opencode flips to a light
  theme on any non-empty TERM_PROGRAM); without this assertion a
  future PR could remove the unset side of the asymmetry undetected.
- TestConfigureAgentEnvironment_TermEnvVarOverride — asserts the
  per-agent TERM override env var (e.g. CMUX_CLAUDE_TEAMS_TERM) wins
  over resolveDefaultTerm's fallback.
- TestConfigureAgentEnvironment_COLORTERMPreservedWhenSet — asserts
  the truecolor fallback never downgrades a caller-provided COLORTERM.
- TestConfigureAgentEnvironment_AppliesExtraEnv — sanity check that
  the extraEnv map applies (regression guard for the field).
- TestResolveDefaultTerm_FallsBackWhenInfocmpAbsent — simulates the
  remote-SSH-bootstrap race (PATH stripped of infocmp) and asserts
  resolveDefaultTerm returns "xterm-256color" instead of leaving a
  child process with an unresolvable TERM.
- TestResolveDefaultTerm_PrefersXtermGhosttyWhenInfocmpSucceeds —
  happy-path check, gracefully skips on hosts without the terminfo
  entry installed.

The existing TestConfigureAgentEnvironment in tmux_compat_test.go now
also asserts TERM_PROGRAM is unset by default (zero-value
preserveTermProgram = legacy behavior), filling a previously-empty
assertion gap in the existing test.

Python (tests/test_cli_omo_env.py):

- New file mirroring tests/test_cli_claude_teams_env.py's fake-shim
  pattern but for `cmux omo`. Asserts TERM_PROGRAM is unset (the
  manaflow-ai#2516 guard), TERM defaults to xterm-ghostty, and COLORTERM=truecolor
  is set when the parent had no COLORTERM. Uses the cmux DEV CLI from
  /tmp/cmux-last-cli-path so it runs against the user's most recent
  reload.sh build.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@vercel

vercel Bot commented Apr 19, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Manaflow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Apr 19, 2026 •

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR fixes a cmux claude-teams crash by refactoring terminal environment handling: it adds dynamic TERM resolution via infocmp, sets COLORTERM to truecolor when empty, and introduces a preserveTermProgram flag to conditionally preserve TERM_PROGRAM for claude-teams while unsetting it for other agents.

Changes

Cohort / File(s) Summary
Swift Terminal Environment Setup
CLI/cmux.swift
Added resolveDefaultTerm() function to dynamically probe infocmp xterm-ghostty with fallback to xterm-256color. Updated configureTmuxCompatEnvironment() to use dynamic TERM resolution and set COLORTERM=truecolor when empty. Introduced preserveTermProgram: Bool = false parameter; updated configureClaudeTeamsEnvironment() to pass preserveTermProgram: true, preserving TERM_PROGRAM specifically for claude-teams.
Go Daemon Terminal Environment Setup
daemon/remote/cmd/cmuxd-remote/agent_launch.go
Added resolveDefaultTerm() function mirroring Swift behavior. Added preserveTermProgram field to agentConfig struct. Updated configureAgentEnvironment() to conditionally unset TERM_PROGRAM based on flag value. Updated runClaudeTeamsRelay() to pass preserveTermProgram: true.
Go Unit Tests
daemon/remote/cmd/cmuxd-remote/agent_launch_test.go, daemon/remote/cmd/cmuxd-remote/tmux_compat_test.go
New test file with envSnapshot helper and comprehensive tests for environment configuration, TERM_PROGRAM preservation semantics (claude-teams vs. opencode-family), resolveDefaultTerm() fallback behavior, and agent-specific TERM override variables. Updated existing test to assert TERM_PROGRAM is unset for non-claude-teams agents.
Python Regression Tests
tests/test_cli_claude_teams_env.py, tests/test_cli_omo_env.py
Updated claude-teams test to expect TERM=xterm-ghostty and TERM_PROGRAM preservation. Added new omo test verifying omo agent unsets TERM_PROGRAM and sets COLORTERM=truecolor; includes isolated shim environment for testing.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 A rabbit hops through terminal trees,
Where TERM_PROGRAM now bends to please,
Claude-teams keeps its ghostty shore,
While OMO unsets what came before,
Dynamic terms in infocmp's breeze! 🌬️✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 58.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed Title accurately summarizes the main change: fixing TERM_PROGRAM preservation for claude-teams across both Go and Swift surfaces to resolve the reported crash.
Description check ✅ Passed Description is comprehensive and well-structured, covering summary, rationale, what's included, testing approach, and out-of-scope items. Matches required template sections.
Linked Issues check ✅ Passed The PR fully addresses issue #2947 requirements: identifies TERM_PROGRAM as the root cause, provides fixes across both Go daemon and Swift CLI surfaces (the reporter's local-Mac path), adds comprehensive test coverage, and includes manual verification steps.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing TERM_PROGRAM preservation and related environment variable handling. No unrelated refactoring or feature scope-creep detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.4)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Apr 19, 2026 •

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes cmux claude-teams crashing during permission escalation on Claude Code v2.1.112+ by conditionally preserving TERM_PROGRAM for claude-teams while still unsetting it for opencode-family agents (omo/omx/omc), applied symmetrically across the Go daemon and Swift CLI. It also introduces a resolveDefaultTerm() helper that probes infocmp xterm-ghostty and falls back to xterm-256color, and aligns COLORTERM handling between the two surfaces.

  • Both Python tests (test_cli_claude_teams_env.py and test_cli_omo_env.py) hard-assert TERM=xterm-ghostty but neither pins CMUX_CLAUDE_TEAMS_TERM/CMUX_OMO_TERM in the test env, so on any CI host where infocmp xterm-ghostty fails (Linux runners, Macs without Ghostty), the tests will reliably fail. Setting the respective override env var before the subprocess call is a one-liner fix.

Confidence Score: 4/5

Core fix is correct and well-tested in Go; two Python tests will reliably fail on CI hosts without xterm-ghostty terminfo and need a one-liner fix before merge.

The implementation logic across both Go and Swift surfaces is sound, the Go unit tests are thorough and correct, and the preserveTermProgram zero-value design is safe. The only blocking issue is that both updated/new Python integration tests hard-assert TERM=xterm-ghostty without pinning the override env var, making them non-deterministic on any host where infocmp xterm-ghostty fails — which includes standard Linux CI runners.

tests/test_cli_claude_teams_env.py (line 205) and tests/test_cli_omo_env.py (line 100) — both need the term override env var pinned before the subprocess call.

Important Files Changed

Filename Overview
daemon/remote/cmd/cmuxd-remote/agent_launch.go Adds preserveTermProgram flag to agentConfig, wires it for claude-teams (true) and opencode-family (false/zero), adds resolveDefaultTerm() with infocmp probe, and adds COLORTERM truecolor fallback — all correct.
CLI/cmux.swift Mirrors Go changes: preserveTermProgram parameter (default false), resolveDefaultTerm() via /usr/bin/env infocmp, live-env COLORTERM fallback, preserveTermProgram: true for claude-teams — implementation is consistent with the Go side.
daemon/remote/cmd/cmuxd-remote/agent_launch_test.go New tests covering preserve/unset TERM_PROGRAM, TERM override, COLORTERM preservation, extraEnv, and resolveDefaultTerm happy/fallback paths — well-structured with proper env snapshots.
daemon/remote/cmd/cmuxd-remote/tmux_compat_test.go Added TERM_PROGRAM unset assertion to existing TestConfigureAgentEnvironment; env-cleanup still uses os.Getenv (not LookupEnv), leaving a minor correctness gap for keys set to empty string.
tests/test_cli_claude_teams_env.py Updated to assert TERM_PROGRAM is preserved (correct), but now hard-asserts TERM=xterm-ghostty without pinning CMUX_CLAUDE_TEAMS_TERM — will fail on hosts where infocmp xterm-ghostty is unavailable.
tests/test_cli_omo_env.py New regression guard for TERM_PROGRAM unset on opencode-family agents — good intent, but hard-asserts TERM=xterm-ghostty without pinning CMUX_OMO_TERM, causing intermittent CI failures on hosts without Ghostty terminfo.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[cmux agent launch] --> B{agent type?}
    B -->|claude-teams| C[preserveTermProgram = true]
    B -->|omo / omx / omc| D[preserveTermProgram = false]

    C --> E[configureAgentEnvironment]
    D --> E

    E --> F[Set PATH / TMUX / TMUX_PANE]
    F --> G{termEnvVar set?}
    G -->|yes| H[TERM = override value]
    G -->|no| I[resolveDefaultTerm]
    I --> J{infocmp xterm-ghostty exits 0?}
    J -->|yes| K[TERM = xterm-ghostty]
    J -->|no| L[TERM = xterm-256color]
    H --> M[COLORTERM fallback]
    K --> M
    L --> M
    M --> N{preserveTermProgram?}
    N -->|true| O[TERM_PROGRAM preserved]
    N -->|false| P[unsetenv TERM_PROGRAM]
    O --> Q[exec agent]
    P --> Q
Loading

Comments Outside Diff (1)

  1. daemon/remote/cmd/cmuxd-remote/tmux_compat_test.go, line 307-325 (link)

    P2 Env-cleanup uses os.Getenv instead of os.LookupEnv

    The save loop uses os.Getenv, which returns "" for both an unset key and a key explicitly set to the empty string. The defer restore block then calls os.Unsetenv for any saved "" value, so a key that was set to "" before the test runs will be unset after it, not restored. This pre-existed, but TERM_PROGRAM and COLORTERM were added to envKeys in this PR, making the gap more material. The new agent_launch_test.go correctly uses the envSnapshot helper with os.LookupEnv — the existing test could follow the same pattern.

Reviews (1): Last reviewed commit: "test: cover claude-teams + opencode env ..." | Re-trigger Greptile

Comment on lines 205 to 208
term_value = read_text(term_log)
if term_value != "screen-256color":
print(f"FAIL: expected TERM=screen-256color, got {term_value!r}")
if term_value != "xterm-ghostty":
print(f"FAIL: expected TERM=xterm-ghostty, got {term_value!r}")
raise SystemExit(1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 TERM=xterm-ghostty assertion is non-deterministic

The test unconditionally expects xterm-ghostty, but resolveDefaultTerm() returns xterm-256color whenever infocmp xterm-ghostty fails — which is the case on any Linux CI runner or Mac without Ghostty installed. The test provides no fake infocmp in real_bin, so whether the assertion passes depends entirely on test-host config rather than code correctness.

The simplest fix is to pin the override env var so the test bypasses resolveDefaultTerm() and stays deterministic:

Suggested change
term_value = read_text(term_log)
if term_value != "screen-256color":
print(f"FAIL: expected TERM=screen-256color, got {term_value!r}")
if term_value != "xterm-ghostty":
print(f"FAIL: expected TERM=xterm-ghostty, got {term_value!r}")
raise SystemExit(1)
env["CMUX_CLAUDE_TEAMS_TERM"] = "xterm-ghostty"
env["TERM"] = "xterm-256color"

Alternatively, accept both valid values:

if term_value not in ("xterm-ghostty", "xterm-256color"):
    print(f"FAIL: expected TERM=xterm-ghostty or xterm-256color, got {term_value!r}")
    raise SystemExit(1)

Comment thread tests/test_cli_omo_env.py
Comment on lines +99 to +104
term_value = read_text(term_log)
if term_value != "xterm-ghostty":
print(
f"FAIL: expected TERM=xterm-ghostty for cmux omo, got {term_value!r}"
)
raise SystemExit(1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 TERM=xterm-ghostty assertion will fail on hosts without Ghostty terminfo

Same issue as the sibling test: the check hard-asserts xterm-ghostty, but resolveDefaultTerm() falls back to xterm-256color on hosts where infocmp xterm-ghostty exits non-zero (any Linux CI runner, any Mac without Ghostty). No fake infocmp is placed in real_bin, so the result is host-dependent.

Pin CMUX_OMO_TERM in the test env to force the override path and make the assertion deterministic:

Suggested change
term_value = read_text(term_log)
if term_value != "xterm-ghostty":
print(
f"FAIL: expected TERM=xterm-ghostty for cmux omo, got {term_value!r}"
)
raise SystemExit(1)
env["CMUX_OMO_TERM"] = "xterm-ghostty"
env["TERM"] = "xterm-256color"

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🧹 Nitpick comments (2)
daemon/remote/cmd/cmuxd-remote/agent_launch.go (1)

441-449: Consider caching resolveDefaultTerm() across a single process lifetime.

The infocmp probe spawns a subprocess on every agent launch. It's only called when CMUX_*_TERM isn't set, so the cost is bounded (one launch → one spawn), but if this function ends up being called from other code paths later it would be cheap insurance to memoize via sync.Once. Not needed today — just flagging for the next caller.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@daemon/remote/cmd/cmuxd-remote/agent_launch.go` around lines 441 - 449, The
resolveDefaultTerm function currently runs infocmp on every call; memoize its
result for the process lifetime by adding a package-level cached variable (e.g.,
defaultTerm string) and a sync.Once (e.g., defaultTermOnce) and change
resolveDefaultTerm to call defaultTermOnce.Do(func(){ ...compute and set
defaultTerm... }) and return defaultTerm; keep the same probe logic
(exec.Command("infocmp", "xterm-ghostty") etc.) inside the once body so the
subprocess is spawned only once per process.
daemon/remote/cmd/cmuxd-remote/agent_launch_test.go (1)

160-177: Test doesn't actually verify the "applied LAST" contract in its docstring.

The comment states extraEnv "is applied LAST (after all other env mutations) so callers can override anything the function sets", but the assertion only checks that CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS=1 is set — which would pass regardless of ordering since nothing else writes that variable. To actually pin the ordering contract, have extraEnv collide with a value the function sets earlier (e.g., override TERM or TMUX) and assert the extraEnv value wins.

♻️ Proposed strengthening
 func TestConfigureAgentEnvironment_AppliesExtraEnv(t *testing.T) {
 	t.Cleanup(envSnapshot(t,
 		"COLORTERM", "TERM", "PATH", "TMUX", "TMUX_PANE",
 		"CMUX_CLAUDE_TEAMS_CMUX_BIN", "CMUX_SOCKET_PATH", "CMUX_SOCKET",
 		"CMUX_CLAUDE_TEAMS_TERM", "TERM_PROGRAM",
 		"CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS",
 	))
 
-	configureAgentEnvironment(claudeTeamsConfig())
+	cfg := claudeTeamsConfig()
+	// Collide with a value set earlier in configureAgentEnvironment to
+	// prove extraEnv wins (i.e., is applied LAST).
+	cfg.extraEnv["TERM"] = "extraenv-override"
+	configureAgentEnvironment(cfg)
 
 	if got := os.Getenv("CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS"); got != "1" {
 		t.Errorf("CLAUDE_CODE_EXPERIMENTAL_AGENT_TEAMS = %q, want 1", got)
 	}
+	if got := os.Getenv("TERM"); got != "extraenv-override" {
+		t.Errorf("TERM = %q, want %q (extraEnv must be applied LAST and override earlier writes)", got, "extraenv-override")
+	}
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@daemon/remote/cmd/cmuxd-remote/agent_launch_test.go` around lines 160 - 177,
The test TestConfigureAgentEnvironment_AppliesExtraEnv documents that extraEnv
is applied last but doesn't verify ordering; update the test to make extraEnv
collide with a variable that configureAgentEnvironment mutates (for example TERM
or TMUX) by using claudeTeamsConfig() or constructing an extraEnv map that sets
that variable to a distinct sentinel value, call configureAgentEnvironment, and
then assert the environment variable equals the sentinel (verifying extraEnv
wins). Ensure references to configureAgentEnvironment, claudeTeamsConfig (or the
test's extraEnv construction), and the chosen env var (e.g., "TERM" or "TMUX")
are used so the test actually checks the "applied LAST" contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@CLI/cmux.swift`:
- Around line 10381-10385: The code currently sets process.standardOutput and
process.standardError to Pipe() then calls try process.run() and
process.waitUntilExit(), which can block because the pipes are never read;
change those assignments to use FileHandle.nullDevice for both standardOutput
and standardError (replace the Pipe() uses), so output is discarded instead of
creating unread pipes before calling process.run() and process.waitUntilExit();
update references around process.standardOutput, process.standardError,
process.run(), and process.waitUntilExit() accordingly.

In `@tests/test_cli_claude_teams_env.py`:
- Around line 205-207: The test currently asserts term_value == "xterm-ghostty"
unconditionally; change it to accept the documented fallback by mirroring the
production resolveDefaultTerm logic: determine whether the runtime prefers
"xterm-ghostty" or falls back to "xterm-256color" (either by calling/importing
the existing resolveDefaultTerm helper or implementing the same detection in the
test) and then assert term_value equals the resolved default instead of
hard-coding "xterm-ghostty" (refer to term_log/term_value and resolveDefaultTerm
in the codebase).

In `@tests/test_cli_omo_env.py`:
- Around line 99-104: The test unconditionally asserts TERM == "xterm-ghostty"
which flakes when the terminfo entry is missing; change the test to either set
CMUX_OMO_TERM (or CMUX_CLAUDE_TEAMS_TERM) to a known value before launching the
agent so the produced term file (read_text -> term_value) is deterministic, or
probe for the terminfo entry and skip the assertion if it's absent (mirror Go's
resolveDefaultTerm probe-and-skip). Locate the block that reads term_value
(term_value = read_text(term_log)) and implement one of these fixes: export
os.environ["CMUX_OMO_TERM"]="xterm-ghostty" for the test run, or run a quick
check (e.g., subprocess call to infocmp or use shutil.which) to detect the
xterm-ghostty terminfo and only perform the equality assertion when present.
Ensure references remain to term_value, read_text, and CMUX_OMO_TERM so the
change is easy to find.
- Around line 33-60: The test currently allows real plugin installation because
run_omo (and the Swift runOMO path) calls omoEnsurePlugin unconditionally and
PATH still contains the host's npm/bun; fix by ensuring the plugin setup is
short-circuited during tests: either pre-seed a fake plugin installation
directory under the test's fake_home (create fake_home/.config/opencode with
expected files) before invoking run_omo, or export an opt-out env var (e.g.,
CMUX_OMO_SKIP_PLUGIN=1) in base_env so omoEnsurePlugin returns early; locate
calls to run_omo/runOMO and omoEnsurePlugin to implement the pre-seed or opt-out
behavior and ensure fake_home and base_env are used consistently so the test
cannot reach the network to install plugins.

---

Nitpick comments:
In `@daemon/remote/cmd/cmuxd-remote/agent_launch_test.go`:
- Around line 160-177: The test TestConfigureAgentEnvironment_AppliesExtraEnv
documents that extraEnv is applied last but doesn't verify ordering; update the
test to make extraEnv collide with a variable that configureAgentEnvironment
mutates (for example TERM or TMUX) by using claudeTeamsConfig() or constructing
an extraEnv map that sets that variable to a distinct sentinel value, call
configureAgentEnvironment, and then assert the environment variable equals the
sentinel (verifying extraEnv wins). Ensure references to
configureAgentEnvironment, claudeTeamsConfig (or the test's extraEnv
construction), and the chosen env var (e.g., "TERM" or "TMUX") are used so the
test actually checks the "applied LAST" contract.

In `@daemon/remote/cmd/cmuxd-remote/agent_launch.go`:
- Around line 441-449: The resolveDefaultTerm function currently runs infocmp on
every call; memoize its result for the process lifetime by adding a
package-level cached variable (e.g., defaultTerm string) and a sync.Once (e.g.,
defaultTermOnce) and change resolveDefaultTerm to call
defaultTermOnce.Do(func(){ ...compute and set defaultTerm... }) and return
defaultTerm; keep the same probe logic (exec.Command("infocmp", "xterm-ghostty")
etc.) inside the once body so the subprocess is spawned only once per process.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 44e634fa-aadc-464e-a9f0-09af9fd289b2

📥 Commits

Reviewing files that changed from the base of the PR and between 3698f13 and 4f02421.

📒 Files selected for processing (6)
  • CLI/cmux.swift
  • daemon/remote/cmd/cmuxd-remote/agent_launch.go
  • daemon/remote/cmd/cmuxd-remote/agent_launch_test.go
  • daemon/remote/cmd/cmuxd-remote/tmux_compat_test.go
  • tests/test_cli_claude_teams_env.py
  • tests/test_cli_omo_env.py

Comment thread CLI/cmux.swift
Comment on lines +10381 to +10385
process.standardOutput = Pipe()
process.standardError = Pipe()
do {
try process.run()
process.waitUntilExit()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
python3 - <<'PY'
from pathlib import Path

for path in Path(".").rglob("cmux.swift"):
    text = path.read_text(errors="ignore")
    marker = "private static func resolveDefaultTerm()"
    start = text.find(marker)
    if start == -1:
        continue
    end = text.find("\n    }\n", start)
    body = text[start:end if end != -1 else start + 1200]
    print(path)
    print("uses Pipe():", "Pipe()" in body)
    print("drains pipe data:", "readDataToEndOfFile" in body or ".read(" in body)
    break
else:
    raise SystemExit("cmux.swift with resolveDefaultTerm() not found")
PY

Repository: manaflow-ai/cmux

Length of output: 116


🏁 Script executed:

#!/bin/bash

# View the exact code context at lines 10381-10385 and surrounding
sed -n '10369,10395p' CLI/cmux.swift

echo "---"
echo "Searching for FileHandle.nullDevice usage patterns in Swift files:"
rg "FileHandle\.nullDevice" -A 2 -B 2 CLI/cmux.swift | head -30

echo "---"
echo "Checking for other Pipe() + waitUntilExit patterns without draining:"
rg -A 5 'process\.standardOutput = Pipe\(\)' CLI/cmux.swift

Repository: manaflow-ai/cmux

Length of output: 1878


Discard probe output instead of piping it.

Lines 10382–10383 create pipes that are never drained before waitUntilExit(). Use FileHandle.nullDevice to discard output and avoid potential blocking, matching both the Go-side implementation and the pattern used elsewhere in this file.

🐛 Proposed fix
-        process.standardOutput = Pipe()
-        process.standardError = Pipe()
+        process.standardOutput = FileHandle.nullDevice
+        process.standardError = FileHandle.nullDevice
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
process.standardOutput = Pipe()
process.standardError = Pipe()
do {
try process.run()
process.waitUntilExit()
process.standardOutput = FileHandle.nullDevice
process.standardError = FileHandle.nullDevice
do {
try process.run()
process.waitUntilExit()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@CLI/cmux.swift` around lines 10381 - 10385, The code currently sets
process.standardOutput and process.standardError to Pipe() then calls try
process.run() and process.waitUntilExit(), which can block because the pipes are
never read; change those assignments to use FileHandle.nullDevice for both
standardOutput and standardError (replace the Pipe() uses), so output is
discarded instead of creating unread pipes before calling process.run() and
process.waitUntilExit(); update references around process.standardOutput,
process.standardError, process.run(), and process.waitUntilExit() accordingly.

Comment on lines 205 to +207
term_value = read_text(term_log)
if term_value != "screen-256color":
print(f"FAIL: expected TERM=screen-256color, got {term_value!r}")
if term_value != "xterm-ghostty":
print(f"FAIL: expected TERM=xterm-ghostty, got {term_value!r}")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check whether this host should expect xterm-ghostty or the fallback.
if command -v infocmp >/dev/null 2>&1 && infocmp xterm-ghostty >/dev/null 2>&1; then
  printf 'expected TERM=xterm-ghostty\n'
else
  printf 'expected TERM=xterm-256color\n'
fi

Repository: manaflow-ai/cmux

Length of output: 88


🏁 Script executed:

cat -n tests/test_cli_claude_teams_env.py | sed -n '200,215p'

Repository: manaflow-ai/cmux

Length of output: 949


🏁 Script executed:

# Search for TERM-related code to understand the fallback logic
rg -n "xterm-ghostty|xterm-256color" --type py

Repository: manaflow-ai/cmux

Length of output: 2524


🏁 Script executed:

# Search for infocmp usage in the codebase
rg -n "infocmp" --type py

Repository: manaflow-ai/cmux

Length of output: 903


🏁 Script executed:

# Search for where TERM default is set (likely in main claude-teams launch code)
rg -n "TERM.*=|DEFAULT.*TERM|fallback.*TERM" --type py | head -20

Repository: manaflow-ai/cmux

Length of output: 2597


🏁 Script executed:

# Look for claude_teams or run_claude_teams function definition to understand the env setup
rg -n "def.*claude_teams|def.*run_claude_teams" --type py

Repository: manaflow-ai/cmux

Length of output: 119


🏁 Script executed:

# Check what the test file imports and setup to see if it mocks infocmp
rg -n "import|mock|patch|monkeypatch" tests/test_cli_claude_teams_env.py | head -20

Repository: manaflow-ai/cmux

Length of output: 231


🏁 Script executed:

# Find the claude-teams main script/implementation
fd "claude[_-]teams" --type f | grep -v test | head -10

Repository: manaflow-ai/cmux

Length of output: 102


🏁 Script executed:

# Look for TERM fallback logic in non-test code
rg -n "xterm-ghostty|xterm-256color" --type py | grep -v test

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Read more of the test to understand the full context
cat -n tests/test_cli_claude_teams_env.py | sed -n '1,50p'

Repository: manaflow-ai/cmux

Length of output: 1943


🏁 Script executed:

# Search for claude-teams implementation (might be shell script, Python, or compiled)
find . -name "*claude*teams*" -type f ! -path "./.git/*" ! -path "*/node_modules/*" ! -path "*/__pycache__/*" 2>/dev/null

Repository: manaflow-ai/cmux

Length of output: 457


🏁 Script executed:

# Check if there's a cmux binary or shell script
fd "cmux" --type f ! -path "./.git" ! -path "*/node_modules" | head -20

Repository: manaflow-ai/cmux

Length of output: 228


🏁 Script executed:

# Look for environment variable handling in the codebase
rg -n "TERM.*=" --type sh | head -20

Repository: manaflow-ai/cmux

Length of output: 1456


🏁 Script executed:

# Check the claude_teams_test_utils to understand test setup
cat tests/claude_teams_test_utils.py

Repository: manaflow-ai/cmux

Length of output: 771


🏁 Script executed:

# Look for where the claude-teams subprocess's TERM is actually set
# Maybe in the cmux CLI code itself
find . -name "*.py" -type f ! -path "./.git/*" ! -path "*/node_modules/*" ! -path "*/__pycache__/*" | xargs grep -l "claude.teams" | grep -v test

Repository: manaflow-ai/cmux

Length of output: 42


🏁 Script executed:

# Search for TERM setting in shell scripts or config
rg -n "TERM.*ghostty" --type sh

Repository: manaflow-ai/cmux

Length of output: 142


🏁 Script executed:

# The test calls a cmux CLI binary. Let's see what the test is actually testing
cat -n tests/test_cli_claude_teams_env.py | sed -n '100,180p'

Repository: manaflow-ai/cmux

Length of output: 3695


🏁 Script executed:

# Look for any implementation that sets TERM in the claude-teams context
rg -n "infocmp.*xterm-ghostty|ghostty.*terminfo" -A 3 -B 3

Repository: manaflow-ai/cmux

Length of output: 14991


🏁 Script executed:

# Check if there's any reference to fallback behavior in the PR or comments
rg -n "fallback|xterm-256color" tests/test_cli_claude_teams_env.py

Repository: manaflow-ai/cmux

Length of output: 102


Account for the documented xterm-256color fallback.

This assertion hard-codes xterm-ghostty, but the runtime implementation (in both Swift and Go) prefers xterm-ghostty only when its terminfo entry is available, otherwise falls back to xterm-256color. On CI images without that terminfo entry (like this sandbox), this test will fail even when the fallback works correctly.

The proposed fix adds a helper function that mirrors the actual resolveDefaultTerm() logic already implemented in the codebase:

🐛 Proposed fix
 def read_text(path: Path) -> str:
     if not path.exists():
         return ""
     return path.read_text(encoding="utf-8").strip()
 
 
+def expected_default_term() -> str:
+    try:
+        proc = subprocess.run(
+            ["infocmp", "xterm-ghostty"],
+            stdout=subprocess.DEVNULL,
+            stderr=subprocess.DEVNULL,
+            check=False,
+        )
+    except FileNotFoundError:
+        return "xterm-256color"
+    return "xterm-ghostty" if proc.returncode == 0 else "xterm-256color"
+
+
 def run_claude_teams(
         term_value = read_text(term_log)
-        if term_value != "xterm-ghostty":
-            print(f"FAIL: expected TERM=xterm-ghostty, got {term_value!r}")
+        expected_term = expected_default_term()
+        if term_value != expected_term:
+            print(f"FAIL: expected TERM={expected_term}, got {term_value!r}")
             raise SystemExit(1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_cli_claude_teams_env.py` around lines 205 - 207, The test
currently asserts term_value == "xterm-ghostty" unconditionally; change it to
accept the documented fallback by mirroring the production resolveDefaultTerm
logic: determine whether the runtime prefers "xterm-ghostty" or falls back to
"xterm-256color" (either by calling/importing the existing resolveDefaultTerm
helper or implementing the same detection in the test) and then assert
term_value equals the resolved default instead of hard-coding "xterm-ghostty"
(refer to term_log/term_value and resolveDefaultTerm in the codebase).

Comment thread tests/test_cli_omo_env.py
Comment on lines +33 to +60
def run_omo(cli_path: str, base_env: dict[str, str]) -> subprocess.CompletedProcess[str]:
with tempfile.TemporaryDirectory(prefix="cmux-omo-env-") as td:
tmp = Path(td)
real_bin = tmp / "real-bin"
real_bin.mkdir(parents=True, exist_ok=True)

term_log = tmp / "term.log"
term_program_log = tmp / "term-program.log"
colorterm_log = tmp / "colorterm.log"
cmux_bin_log = tmp / "cmux-bin.log"
argv_log = tmp / "argv.log"
fake_home = tmp / "home"
fake_home.mkdir(parents=True, exist_ok=True)

# Fake opencode shim — logs env on every invocation (the final `--version`
# call wins, overwriting earlier writes from any plugin-install probes).
make_executable(
real_bin / "opencode",
"""#!/usr/bin/env bash
set -euo pipefail
printf '%s\\n' "${TERM-__UNSET__}" > "$FAKE_TERM_LOG"
printf '%s\\n' "${TERM_PROGRAM-__UNSET__}" > "$FAKE_TERM_PROGRAM_LOG"
printf '%s\\n' "${COLORTERM-__UNSET__}" > "$FAKE_COLORTERM_LOG"
printf '%s\\n' "${CMUX_OMO_CMUX_BIN-__UNSET__}" > "$FAKE_CMUX_BIN_LOG"
printf '%s\\n' "$@" > "$FAKE_ARGV_LOG"
exit 0
""",
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look at the omo code path in CLI/cmux.swift to see whether --version bypasses plugin setup.
rg -nP --type=swift -C4 'runOMO|omoEnsurePlugin|oh-my-opencode'

Repository: manaflow-ai/cmux

Length of output: 8754


🏁 Script executed:

#!/bin/bash
# Check if there's early --version handling before runOMO() is called
# Look around line 1783 where runOMO is called
sed -n '1770,1790p' CLI/cmux.swift | cat -n

Repository: manaflow-ai/cmux

Length of output: 733


🏁 Script executed:

#!/bin/bash
# Check the beginning of runOMO function and whether it handles --version
sed -n '11073,11120p' CLI/cmux.swift | cat -n

Repository: manaflow-ai/cmux

Length of output: 2605


🏁 Script executed:

#!/bin/bash
# Check the test to see if it pre-seeds the plugin or sets env vars to prevent install
sed -n '1,100p' tests/test_cli_omo_env.py | cat -n

Repository: manaflow-ai/cmux

Length of output: 4382


🏁 Script executed:

#!/bin/bash
# Check omoEnsurePlugin signature and if it receives commandArgs or checks arguments
sed -n '10896,10910p' CLI/cmux.swift | cat -n

Repository: manaflow-ai/cmux

Length of output: 979


🏁 Script executed:

#!/bin/bash
# Check if commandArgs (containing --version) are used anywhere to short-circuit plugin setup
rg -n 'commandArgs' CLI/cmux.swift | grep -i 'omo\|11073' | head -20

Repository: manaflow-ai/cmux

Length of output: 277


🏁 Script executed:

#!/bin/bash
# Check if there's any env var that controls plugin installation
rg -n 'OPENCODE\|omoEnsure' CLI/cmux.swift | head -30

Repository: manaflow-ai/cmux

Length of output: 42


The test will trigger a real plugin installation whenever bun or npm are available on the test host.

The Swift cmux omo path does not gate off or short-circuit plugin setup based on --version. The runOMO() function calls omoEnsurePlugin() unconditionally at line 11103, before any argument parsing. The function receives commandArgs: [String] but does not pass it to omoEnsurePlugin(), so there is no mechanism to bypass plugin installation when --version is requested.

Since the test sets PATH to prepend a fake bin directory but still includes the original PATH, any bun or npm installed on the test host will be found and used by omoEnsurePlugin() (line 10956–10989) to install oh-my-opencode into fake_home/.config/opencode/. This makes the test slow, network-dependent, and flaky on CI.

Pre-seeding a fake plugin directory in fake_home or exporting an opt-out environment variable would harden the test.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_cli_omo_env.py` around lines 33 - 60, The test currently allows
real plugin installation because run_omo (and the Swift runOMO path) calls
omoEnsurePlugin unconditionally and PATH still contains the host's npm/bun; fix
by ensuring the plugin setup is short-circuited during tests: either pre-seed a
fake plugin installation directory under the test's fake_home (create
fake_home/.config/opencode with expected files) before invoking run_omo, or
export an opt-out env var (e.g., CMUX_OMO_SKIP_PLUGIN=1) in base_env so
omoEnsurePlugin returns early; locate calls to run_omo/runOMO and
omoEnsurePlugin to implement the pre-seed or opt-out behavior and ensure
fake_home and base_env are used consistently so the test cannot reach the
network to install plugins.

Comment thread tests/test_cli_omo_env.py
Comment on lines +99 to +104
term_value = read_text(term_log)
if term_value != "xterm-ghostty":
print(
f"FAIL: expected TERM=xterm-ghostty for cmux omo, got {term_value!r}"
)
raise SystemExit(1)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Host-dependent assertion may cause CI flakiness.

The Go tests (agent_launch_test.go:217-235) deliberately skip the xterm-ghostty expectation when the terminfo entry isn't installed, because resolveDefaultTerm() falls back to xterm-256color. This Python test asserts TERM == "xterm-ghostty" unconditionally and will fail on any host where the xterm-ghostty terminfo entry isn't present (fresh Linux CI, minimal containers, etc.).

Consider mirroring the Go test's probe-and-skip pattern, or set CMUX_CLAUDE_TEAMS_TERM/CMUX_OMO_TERM explicitly so the test doesn't depend on the host's terminfo database.

🔧 Probe-and-skip sketch
+import shutil
+
+def has_xterm_ghostty_terminfo() -> bool:
+    if shutil.which("infocmp") is None:
+        return False
+    return subprocess.run(
+        ["infocmp", "xterm-ghostty"],
+        stdout=subprocess.DEVNULL,
+        stderr=subprocess.DEVNULL,
+    ).returncode == 0
+
@@
-        term_value = read_text(term_log)
-        if term_value != "xterm-ghostty":
-            print(
-                f"FAIL: expected TERM=xterm-ghostty for cmux omo, got {term_value!r}"
-            )
-            raise SystemExit(1)
+        term_value = read_text(term_log)
+        expected_term = "xterm-ghostty" if has_xterm_ghostty_terminfo() else "xterm-256color"
+        if term_value != expected_term:
+            print(
+                f"FAIL: expected TERM={expected_term} for cmux omo, got {term_value!r}"
+            )
+            raise SystemExit(1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_cli_omo_env.py` around lines 99 - 104, The test unconditionally
asserts TERM == "xterm-ghostty" which flakes when the terminfo entry is missing;
change the test to either set CMUX_OMO_TERM (or CMUX_CLAUDE_TEAMS_TERM) to a
known value before launching the agent so the produced term file (read_text ->
term_value) is deterministic, or probe for the terminfo entry and skip the
assertion if it's absent (mirror Go's resolveDefaultTerm probe-and-skip). Locate
the block that reads term_value (term_value = read_text(term_log)) and implement
one of these fixes: export os.environ["CMUX_OMO_TERM"]="xterm-ghostty" for the
test run, or run a quick check (e.g., subprocess call to infocmp or use
shutil.which) to detect the xterm-ghostty terminfo and only perform the equality
assertion when present. Ensure references remain to term_value, read_text, and
CMUX_OMO_TERM so the change is easy to find.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

2 issues found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="CLI/cmux.swift">

<violation number="1" location="CLI/cmux.swift:10381">
P2: Use `FileHandle.nullDevice` instead of `Pipe()` for stdout/stderr. These pipes are never drained before `waitUntilExit()`, which can deadlock if the subprocess writes enough to fill the pipe buffer (~64KB). Since the output of `infocmp` is discarded anyway, `FileHandle.nullDevice` is the correct choice — and matches the Go-side pattern (`cmd.Stdout = nil`).</violation>
</file>

<file name="tests/test_cli_claude_teams_env.py">

<violation number="1" location="tests/test_cli_claude_teams_env.py:206">
P2: This assertion is host-dependent and will fail on CI runners (or any host) without the `xterm-ghostty` terminfo entry installed. The runtime's `resolveDefaultTerm()` falls back to `xterm-256color` when `infocmp xterm-ghostty` fails, but this test doesn't account for the fallback. The Go-side tests already handle this correctly by probing and skipping. Either probe `infocmp xterm-ghostty` to determine the expected value, or pin `CMUX_CLAUDE_TEAMS_TERM` in the test env to make the assertion deterministic.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread CLI/cmux.swift
Comment on lines +10381 to +10382
process.standardOutput = Pipe()
process.standardError = Pipe()

@cubic-dev-ai cubic-dev-ai Bot Apr 19, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Use FileHandle.nullDevice instead of Pipe() for stdout/stderr. These pipes are never drained before waitUntilExit(), which can deadlock if the subprocess writes enough to fill the pipe buffer (~64KB). Since the output of infocmp is discarded anyway, FileHandle.nullDevice is the correct choice — and matches the Go-side pattern (cmd.Stdout = nil).

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At CLI/cmux.swift, line 10381:

<comment>Use `FileHandle.nullDevice` instead of `Pipe()` for stdout/stderr. These pipes are never drained before `waitUntilExit()`, which can deadlock if the subprocess writes enough to fill the pipe buffer (~64KB). Since the output of `infocmp` is discarded anyway, `FileHandle.nullDevice` is the correct choice — and matches the Go-side pattern (`cmd.Stdout = nil`).</comment>

<file context>
@@ -10366,6 +10366,29 @@ struct CMUXCLI {
+        let process = Process()
+        process.executableURL = URL(fileURLWithPath: "/usr/bin/env")
+        process.arguments = ["infocmp", "xterm-ghostty"]
+        process.standardOutput = Pipe()
+        process.standardError = Pipe()
+        do {
</file context>
Suggested change
process.standardOutput = Pipe()
process.standardError = Pipe()
process.standardOutput = FileHandle.nullDevice
process.standardError = FileHandle.nullDevice
Fix with Cubic

term_value = read_text(term_log)
if term_value != "screen-256color":
print(f"FAIL: expected TERM=screen-256color, got {term_value!r}")
if term_value != "xterm-ghostty":

@cubic-dev-ai cubic-dev-ai Bot Apr 19, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: This assertion is host-dependent and will fail on CI runners (or any host) without the xterm-ghostty terminfo entry installed. The runtime's resolveDefaultTerm() falls back to xterm-256color when infocmp xterm-ghostty fails, but this test doesn't account for the fallback. The Go-side tests already handle this correctly by probing and skipping. Either probe infocmp xterm-ghostty to determine the expected value, or pin CMUX_CLAUDE_TEAMS_TERM in the test env to make the assertion deterministic.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/test_cli_claude_teams_env.py, line 206:

<comment>This assertion is host-dependent and will fail on CI runners (or any host) without the `xterm-ghostty` terminfo entry installed. The runtime's `resolveDefaultTerm()` falls back to `xterm-256color` when `infocmp xterm-ghostty` fails, but this test doesn't account for the fallback. The Go-side tests already handle this correctly by probing and skipping. Either probe `infocmp xterm-ghostty` to determine the expected value, or pin `CMUX_CLAUDE_TEAMS_TERM` in the test env to make the assertion deterministic.</comment>

<file context>
@@ -203,13 +203,16 @@ def run_claude_teams(
         term_value = read_text(term_log)
-        if term_value != "screen-256color":
-            print(f"FAIL: expected TERM=screen-256color, got {term_value!r}")
+        if term_value != "xterm-ghostty":
+            print(f"FAIL: expected TERM=xterm-ghostty, got {term_value!r}")
             raise SystemExit(1)
</file context>
Fix with Cubic

@lum lum closed this Apr 19, 2026
@lum
lum deleted the issue-2947-claude-teams-term-program branch April 19, 2026 05:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cmux claude-teams crash

2 participants