Keep prompt colors when switching local TERM to xterm-256color#2613
Keep prompt colors when switching local TERM to xterm-256color#2613lawrencecchen merged 16 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughManaged terminal identity values (TERM, COLORTERM, TERM_PROGRAM) are injected and protected during Ghostty surface creation. During interactive zsh startup, the original TERM is saved to Changes
Sequence Diagram(s)sequenceDiagram
participant App as CMux App
participant Shell as zsh Shell
participant Surface as Ghostty Surface
App->>App: configureGhosttyEnvironment() (use TerminalSurface.managed* constants)
App->>Shell: launch interactive shell (startup env may include TERM=xterm-ghostty)
Shell->>Shell: early .zshenv saves original TERM -> CMUX_ZSH_RESTORE_TERM
Shell->>Shell: .zshenv temporarily sets TERM="xterm-ghostty" when conditions match
App->>Surface: createSurface() (apply managed terminal identity env, protect keys)
Surface->>Surface: surface created
Shell->>Shell: _cmux_restore_terminal_identity_after_startup invoked (preexec)
Shell->>Shell: restore TERM from CMUX_ZSH_RESTORE_TERM and unset it
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48f18958b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
Sources/cmuxApp.swift (1)
231-240: Normalize blank inheritedTERM*vars before defaulting.These branches only run when
getenv(...)returnsnil. If the process inheritsTERM="",COLORTERM="", orTERM_PROGRAM="", the managed defaults are skipped even though the value is effectively missing. A tiny helper that trims and treats blank env vars as unset would make the startup handoff more robust.♻️ Suggested refactor
+ private static func nonEmptyEnvironmentValue(_ key: String) -> String? { + guard let raw = getenv(key) else { return nil } + let value = String(cString: raw).trimmingCharacters(in: .whitespacesAndNewlines) + return value.isEmpty ? nil : value + } + private static func configureGhosttyEnvironment() { let fileManager = FileManager.default let ghosttyAppResources = "/Applications/Ghostty.app/Contents/Resources/ghostty" let bundledGhosttyURL = Bundle.main.resourceURL?.appendingPathComponent("ghostty") var resolvedResourcesDir: String? @@ - if getenv("TERM") == nil { + if nonEmptyEnvironmentValue("TERM") == nil { setenv("TERM", TerminalSurface.managedTerminalType, 1) } - if getenv("COLORTERM") == nil { + if nonEmptyEnvironmentValue("COLORTERM") == nil { setenv("COLORTERM", TerminalSurface.managedColorTerm, 1) } if getenv("TERM_PROGRAM") == nil { + if nonEmptyEnvironmentValue("TERM_PROGRAM") == nil { setenv("TERM_PROGRAM", TerminalSurface.managedTerminalProgram, 1) }Based on learnings: treat empty or whitespace-only values from
getenvas unset before applying environment defaults.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/cmuxApp.swift` around lines 231 - 240, The getenv checks for TERM, COLORTERM, and TERM_PROGRAM skip normalization for blank values, so update the logic to treat empty or whitespace-only getenv results as unset: replace direct getenv(...) == nil checks with a small normalized helper (e.g., a private function that calls getenv, trims whitespace, and returns nil for empty strings) and use that helper when deciding to call setenv with TerminalSurface.managedTerminalType, TerminalSurface.managedColorTerm, and TerminalSurface.managedTerminalProgram so blank inherited env vars are overwritten with the managed defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ghostty`:
- Line 1: Revert the submodule pointer update in the parent repo (undo the
change that set the submodule to commit
ae3cc5d298d6a913297fc4dc7cd8b08283e1fa01), then push that commit to the fork
manaflow-ai/ghostty main branch so the hash is reachable from origin/main; once
pushed, verify with git fetch/origin and git ls-remote that ae3cc5d2 is listed,
update docs/ghostty-fork.md to replace the old fork head (0b231db94) with
ae3cc5d2 and add a short change summary, and finally re-commit the submodule
pointer update in the parent repo so it references the now-pushed commit.
In `@Resources/shell-integration/.zshenv`:
- Around line 26-31: The TERM handoff should only happen when restoration will
be performed; update the conditional around setting CMUX_ZSH_RESTORE_TERM and
TERM so it also checks that shell integration is enabled and available (e.g.
CMUX_SHELL_INTEGRATION != "0" and CMUX_SHELL_INTEGRATION_DIR is set and/or the
expected integration file exists). Concretely, modify the existing
interactive/TERM check that sets CMUX_ZSH_RESTORE_TERM and TERM="xterm-ghostty"
to include guards for CMUX_SHELL_INTEGRATION and CMUX_SHELL_INTEGRATION_DIR (or
a test for the integration file), so TERM is only rewritten when restoration via
cmux-zsh-integration.zsh is guaranteed.
In `@Resources/shell-integration/cmux-zsh-integration.zsh`:
- Around line 1235-1242: The unguarded add-zle-hook-widget invocation may still
emit errors if the autoloaded function isn't available at runtime; update the
block that checks for add-zle-hook-widget (the condition using
$+functions[add-zle-hook-widget]) so the call to add-zle-hook-widget line-init
_cmux_restore_terminal_identity_after_startup is executed with stderr suppressed
and failures ignored (same pattern used elsewhere: redirect errors to /dev/null
and || true) to prevent startup noise when the function file is missing.
---
Nitpick comments:
In `@Sources/cmuxApp.swift`:
- Around line 231-240: The getenv checks for TERM, COLORTERM, and TERM_PROGRAM
skip normalization for blank values, so update the logic to treat empty or
whitespace-only getenv results as unset: replace direct getenv(...) == nil
checks with a small normalized helper (e.g., a private function that calls
getenv, trims whitespace, and returns nil for empty strings) and use that helper
when deciding to call setenv with TerminalSurface.managedTerminalType,
TerminalSurface.managedColorTerm, and TerminalSurface.managedTerminalProgram so
blank inherited env vars are overwritten with the managed defaults.
🪄 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: c961b67b-cc1a-4ecb-9088-8b8cb04b3cf0
📒 Files selected for processing (9)
Resources/shell-integration/.zshenvResources/shell-integration/cmux-zsh-integration.zshResources/terminfo-overlay/78/xterm-256colorResources/terminfo-overlay/README.mdSources/GhosttyTerminalView.swiftSources/cmuxApp.swiftcmuxTests/GhosttyConfigTests.swiftghosttyvendor/bonsplit
Greptile SummaryThis PR switches cmux's reported
Confidence Score: 4/5Core mechanism is sound and well-tested, but one P1 leaves TERM=xterm-ghostty permanently when shell integration is disabled The terminfo overlay, managed env triple (TERM/COLORTERM/TERM_PROGRAM), and the two-phase startup handoff are all implemented correctly and covered by a good behavioral test. One P1 exists: the TERM swap in .zshenv does not check whether the integration that would restore TERM will actually be loaded, so CMUX_SHELL_INTEGRATION=0 users get a permanently wrong TERM value. Resources/shell-integration/.zshenv — the TERM swap condition needs the same CMUX_SHELL_INTEGRATION guard that gates the integration load on line 55 Important Files Changed
Sequence DiagramsequenceDiagram
participant App as cmux App
participant zshenv as .zshenv
participant zshrc as user .zshrc
participant integ as cmux-zsh-integration
participant ZLE as zsh ZLE / precmd
App->>zshenv: spawn shell (TERM=xterm-256color)
zshenv->>zshenv: detect TERM=xterm-256color → set CMUX_ZSH_RESTORE_TERM, TERM=xterm-ghostty
zshenv->>zshrc: source user .zshrc (TERM=xterm-ghostty)
zshrc->>zshrc: theme init sees xterm-ghostty → enables Ghostty-specific features
zshenv->>integ: source cmux-zsh-integration.zsh
integ->>ZLE: register _cmux_restore_terminal_identity_after_startup on precmd + line-init
ZLE->>integ: first precmd or line-init fires
integ->>integ: restore TERM=xterm-256color, unset CMUX_ZSH_RESTORE_TERM
integ->>ZLE: deregister hook (one-shot)
Note over zshenv,ZLE: If CMUX_SHELL_INTEGRATION=0, integ step is skipped and TERM stays xterm-ghostty
Reviews (1): Last reviewed commit: "fix: restore GhosttyKit-compatible submo..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 9 files
You’re at about 94% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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="Resources/shell-integration/.zshenv">
<violation number="1" location="Resources/shell-integration/.zshenv:26">
P2: In `zsh -i -c '<cmd>'` mode, the `precmd`/`line-init` hooks that restore `TERM` to `xterm-256color` never fire before the command executes. The command runs with `TERM=xterm-ghostty` and `CMUX_ZSH_RESTORE_TERM` remains set, causing inconsistent terminal identity for tooling that shells out through `zsh -ic`.</violation>
<violation number="2" location="Resources/shell-integration/.zshenv:26">
P2: TERM is switched to `xterm-ghostty` even when cmux shell integration is disabled, so it may never be restored.</violation>
</file>
<file name="ghostty">
<violation number="1" location="ghostty:1">
P0: Submodule pointer references a commit (`ae3cc5d…`) that is not reachable from `manaflow-ai/ghostty` `origin/main`. Anyone cloning or updating submodules will fail to resolve this commit. Push the commit to the fork's main branch, verify reachability, and update `docs/ghostty-fork.md` (which still shows the old fork head `0b231db94`) before merging.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4194e9b8a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8211f3397d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 468ced0bc6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@cmuxTests/GhosttyConfigTests.swift`:
- Around line 2811-2830: The two tests are inconsistent because interactive
command mode (-i -c) causes the .zshenv spoof path to run (ZSH_EXECUTION_STRING
is unset), so update the failing test to match actual behavior: change the
assertion in
testShellIntegrationDoesNotSpoofManagedTermForInteractiveCommandMode to expect
TERM to be spoofed (xterm-ghostty / corresponding CMUX_STARTUP_* value) or
alternatively modify that test to invoke runInteractiveZsh with
ZSH_EXECUTION_STRING set; reference the test name
testShellIntegrationDoesNotSpoofManagedTermForInteractiveCommandMode and the
CMUX_STARTUP_TERM/CMUX_STARTUP_THEME_TERM variables when making the change.
🪄 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: 50dbdc79-c034-464b-aa46-408e7dc9d7c7
📒 Files selected for processing (3)
Resources/shell-integration/.zshenvResources/shell-integration/cmux-zsh-integration.zshcmuxTests/GhosttyConfigTests.swift
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmuxTests/GhosttyConfigTests.swift (1)
3474-3514: Consider extracting shared setup logic.Both
runInteractiveZshandrunPromptInteractiveZshhave ~40 lines of duplicated setup code (zdotdir creation,.zshenv/.zshrcwriting, environment configuration). Consider extracting this into a shared helper method to reduce duplication.That said, keeping test helpers self-contained is a valid choice for readability—this is a minor refactor opportunity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@cmuxTests/GhosttyConfigTests.swift` around lines 3474 - 3514, Tests runInteractiveZsh and runPromptInteractiveZsh duplicate ~40 lines that create a temporary root, a zdotdir, assemble userZshEnvFileContents (PATH and extra contents), write .zshenv and .zshrc, and cleanup; extract that logic into a shared helper (e.g., createZshUserEnvironment or prepareZdotdir) that returns the temporary root URL and userZdotdir and accepts parameters for extraEnvironment, userZshEnvContents, and userZshRCContents, ensure the helper performs the createDirectory/write calls and exposes a cleanup/defer strategy so both runInteractiveZsh and runPromptInteractiveZsh call the helper, use the same symbols (userZdotdir, userZshEnvFileContents, ".zshenv", ".zshrc") inside the helper, and update the two callers to remove their duplicated setup/teardown code and call the new helper instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@cmuxTests/GhosttyConfigTests.swift`:
- Around line 3474-3514: Tests runInteractiveZsh and runPromptInteractiveZsh
duplicate ~40 lines that create a temporary root, a zdotdir, assemble
userZshEnvFileContents (PATH and extra contents), write .zshenv and .zshrc, and
cleanup; extract that logic into a shared helper (e.g., createZshUserEnvironment
or prepareZdotdir) that returns the temporary root URL and userZdotdir and
accepts parameters for extraEnvironment, userZshEnvContents, and
userZshRCContents, ensure the helper performs the createDirectory/write calls
and exposes a cleanup/defer strategy so both runInteractiveZsh and
runPromptInteractiveZsh call the helper, use the same symbols (userZdotdir,
userZshEnvFileContents, ".zshenv", ".zshrc") inside the helper, and update the
two callers to remove their duplicated setup/teardown code and call the new
helper instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e350fb30-2c7f-401c-8ef8-4ecf18c86c77
📒 Files selected for processing (1)
cmuxTests/GhosttyConfigTests.swift
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
You’re at about 95% of the monthly review limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
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="cmuxTests/GhosttyConfigTests.swift">
<violation number="1" location="cmuxTests/GhosttyConfigTests.swift:3527">
P2: Guard `openpty` failure explicitly; the current assert-only check can continue with invalid file descriptors and crash tests.</violation>
<violation number="2" location="cmuxTests/GhosttyConfigTests.swift:3579">
P2: Exit the helper after timeout; continuing after `XCTFail` can write to a terminated PTY and cause noisy secondary failures.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
1 participant
Summary
TERM=xterm-256colorwithTERM_PROGRAM=ghosttyandCOLORTERM=truecolorxterm-256colorterminfo entry so cmux keeps the same bright color behavior as the Ghostty pathxterm-256colorafter init, and add regression coverage for the startup handoffTesting
./scripts/reload.sh --tag feat-term-xterm-256colorxterm-ghosttyand restoresxterm-256colorbefore the first real commandSummary by cubic
Switch local shells to
TERM=xterm-256colorwithCOLORTERM=truecolorandTERM_PROGRAM=ghosttyto keep Ghostty color behavior and fix zsh prompt visibility. During interactive zsh startup, temporarily reportxterm-ghosttyfor theme selection, then restorexterm-256coloron the first command.xterm-256colorterminfo so bright colors use 256-color indexes;zsh-autosuggestionsstay readable.CMUX_SHELL_INTEGRATION=0; restore inpreexec.zlehook errors when the widget isn’t available.TERM,COLORTERM,TERM_PROGRAM) in startup env; add app-level defaults.preexecrestore, user.zshenvdisables, PTY-driven prompt coverage, and improved PTY output buffering.Written for commit d6853e1. Summary will update on new commits.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores