Skip to content

fix(progress): disable animated clx output in ci#9249

Merged
jdx merged 1 commit intomainfrom
codex/ci-clx-progress
Apr 18, 2026
Merged

fix(progress): disable animated clx output in ci#9249
jdx merged 1 commit intomainfrom
codex/ci-clx-progress

Conversation

@jdx
Copy link
Copy Markdown
Owner

@jdx jdx commented Apr 18, 2026

Summary

  • Compute the clx progress UI decision once from settings, stderr, force-progress, and CI state.
  • Keep text output as the default in CI even when stderr looks interactive because a CI runner allocated a PTY.
  • Store the resulting use_progress_ui decision on MultiProgressReport so later call sites reuse the same mode decision.

Why

Some CI systems expose stderr as a TTY so tools keep colors enabled, but their log capture strips cursor-control sequences. Animated progress frames can then become thousands of near-duplicate log rows. This keeps local interactive progress intact while making CI output quieter.

Validation

  • cargo fmt --check
  • cargo check -p mise
  • pre-commit hook suite during commit, including cargo check --all-features, cargo fmt --all -- --check, taplo, actionlint, markdownlint, and schema validation

Note

Low Risk
Low risk: changes only adjust progress UI enablement logic to prefer plain text in CI, with minimal behavioral impact outside CI unless CI detection is incorrect.

Overview
Disables clx’s animated progress UI when settings.ci is true, even if stderr appears interactive, to avoid CI logs being flooded by spinner frames.

Refactors MultiProgressReport to compute and store a single use_progress_ui flag during initialization (including logging ci) and uses that flag when selecting ProgressReport vs text-based reports and when creating the header job.

Reviewed by Cursor Bugbot for commit 13548a6. Bugbot is set up for automated code reviews on this repo. Configure here.

@jdx jdx marked this pull request as ready for review April 18, 2026 19:14
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit e2a0645. Configure here.

Comment thread src/ui/multi_progress_report.rs Outdated
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 18, 2026

Greptile Summary

Suppresses clx's animated spinner in CI by storing a use_progress_ui bool on MultiProgressReport at construction time, computed from settings.ci (the CI env var). The PR also simplifies the struct by folding verbose, raw, has_stderr, and force_progress into that single field; MISE_FORCE_PROGRESS continues to override CI detection.

  • The fix uses settings.ci (reads CI env var), but src/config/settings.rs:328 uses ci_info::is_ci() for color detection, which also covers Jenkins, TeamCity, and others without CI=true. A runner on those systems with a PTY would still trigger animated output.

Confidence Score: 5/5

Safe to merge; only a P2 CI-detection coverage gap remains

The logic change is correct and well-scoped. The only finding is that settings.ci misses a small set of CI environments detected by ci_info::is_ci() used elsewhere — a pre-existing inconsistency that this PR doesn't worsen, and a minor concern for rare runners. All P2.

No files require special attention

Important Files Changed

Filename Overview
src/ui/multi_progress_report.rs Folds verbose/raw/has_stderr/force_progress fields into a single use_progress_ui bool and adds CI check to suppress animated progress; minor inconsistency with the ci_info::is_ci() detection used elsewhere in settings.rs

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[MultiProgressReport::new] --> B{settings.raw or quiet or verbose?}
    B -- yes --> C[use_progress_ui = false]
    B -- no --> D{MISE_FORCE_PROGRESS?}
    D -- yes --> E[use_progress_ui = true]
    D -- no --> F{has_stderr AND NOT settings.ci?}
    F -- yes --> E
    F -- no --> C
    C --> G[progress::set_output ProgressOutput::Text]
    E --> H[Animated clx progress UI enabled]
    G --> I[Plain text output]
Loading

Fix All in Claude Code

Reviews (3): Last reviewed commit: "fix(progress): disable animated clx outp..." | Re-trigger Greptile

Comment thread src/ui/multi_progress_report.rs Outdated
Comment thread src/ui/multi_progress_report.rs Outdated
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces logic to disable the progress UI in CI environments to prevent log pollution from cursor controls, defaulting to text output instead. While the implementation correctly identifies CI environments using the is_ci crate, feedback indicates that the project already depends on ci_info for similar purposes. It is recommended to use the existing ci_info dependency to maintain consistency and avoid redundant crates.

Comment thread Cargo.toml Outdated
indexmap = { version = "2", features = ["serde"] }
clx = "2"
indoc = "2"
is_ci = "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.

medium

The project already depends on ci_info (line 84), which is used for CI detection in src/config/settings.rs. Adding is_ci as a new direct dependency for the same purpose is redundant. It is recommended to use the existing ci_info crate to maintain consistency and keep the dependency list minimal.

Comment thread src/ui/multi_progress_report.rs Outdated
let settings = Settings::get();
let has_stderr = console::user_attended_stderr();
let force_progress = *env::MISE_FORCE_PROGRESS;
let is_ci = is_ci::cached();
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.

medium

Use ci_info::is_ci() instead of is_ci::cached() to avoid introducing a redundant dependency, as ci_info is already used elsewhere in the project (e.g., in src/config/settings.rs) for CI detection.

Suggested change
let is_ci = is_ci::cached();
let is_ci = ci_info::is_ci();

Comment thread src/ui/multi_progress_report.rs Outdated
!self.raw
&& !self.quiet
&& !self.verbose
&& (self.force_progress || (self.has_stderr && !is_ci::cached()))
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.

medium

Use ci_info::is_ci() for consistency with other parts of the codebase and to remove the need for the is_ci crate.

Suggested change
&& (self.force_progress || (self.has_stderr && !is_ci::cached()))
&& (self.force_progress || (self.has_stderr && !ci_info::is_ci()))

@jdx jdx force-pushed the codex/ci-clx-progress branch 2 times, most recently from a1da69c to da4b84a Compare April 18, 2026 19:29
@jdx jdx force-pushed the codex/ci-clx-progress branch from da4b84a to 13548a6 Compare April 18, 2026 19:31
@github-actions
Copy link
Copy Markdown

Hyperfine Performance

mise x -- echo

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.4.17 x -- echo 21.8 ± 0.8 20.6 28.9 1.00
mise x -- echo 22.2 ± 0.8 21.3 32.6 1.02 ± 0.05

mise env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.4.17 env 21.2 ± 0.9 20.2 28.4 1.00
mise env 21.8 ± 0.7 20.7 25.3 1.03 ± 0.05

mise hook-env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.4.17 hook-env 21.7 ± 0.4 20.9 23.6 1.00
mise hook-env 22.1 ± 0.5 21.4 25.9 1.02 ± 0.03

mise ls

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.4.17 ls 18.9 ± 0.3 18.2 20.6 1.00
mise ls 19.4 ± 0.4 18.7 22.8 1.03 ± 0.03

xtasks/test/perf

Command mise-2026.4.17 mise Variance
install (cached) 142ms 146ms -2%
ls (cached) 75ms 77ms -2%
bin-paths (cached) 79ms 80ms -1%
task-ls (cached) 793ms 802ms -1%

@jdx jdx merged commit e09eeb3 into main Apr 18, 2026
37 checks passed
@jdx jdx deleted the codex/ci-clx-progress branch April 18, 2026 19:53
jdx pushed a commit that referenced this pull request Apr 19, 2026
### 🚀 Features

- **(latest)** add --before flag to mise latest by @risu729 in
[#9168](#9168)
- **(npm)** add aube package manager support by @jdx in
[#9256](#9256)
- **(spm)** add filter_bins option to restrict built executables by @jdx
in [#9253](#9253)
- **(vfox)** support plugin-declared dependencies via metadata.lua by
@ahemon in [#9051](#9051)
- rename `mise prepare` to `mise deps` and add package management by
@jdx in [#9056](#9056)

### 🐛 Bug Fixes

- **(backend)** skip versions host for direct-source backends by @jdx in
[#9245](#9245)
- **(github)** route artifact attestation verification to custom api_url
by @jdx in [#9254](#9254)
- **(lockfile)** use unique temp file for atomic save to avoid
concurrent rename race by @jdx in
[#9250](#9250)
- **(log)** drop noisy third-party debug/trace logs by @jdx in
[#9248](#9248)
- **(progress)** disable animated clx output in ci by @jdx in
[#9249](#9249)
- **(use)** honor --quiet and --silent flags by @jdx in
[#9251](#9251)
- **(vfox)** opt backend plugins out of --locked URL check by @jdx in
[#9252](#9252)

### 📦 Registry

- add aqua backend for bitwarden-secrets-manager by @msuzoagu in
[#9255](#9255)

### New Contributors

- @ahemon made their first contribution in
[#9051](#9051)
- @msuzoagu made their first contribution in
[#9255](#9255)
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.

1 participant