Repository navigation
feat(task)!: decouple task output style from verbosity - #10885
Conversation
|
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:
📝 WalkthroughWalkthroughThis PR adds a per-task output style field, separates output style from quiet/silent verbosity, updates output resolution logic, and refreshes schemas, docs, and end-to-end coverage for the new behavior. ChangesTask Output Style Decoupling
Estimated code review effort: 3 (Moderate) | ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Greptile SummaryThis PR decouples task output style (
Confidence Score: 3/5The stream-routing decoupling is sound, but the metadata-suppression path is not fully consistent with the documented per-task override semantics when a global task.output = silent is in effect. silent_bool() still checks Settings::get().task.output.is_some_and(|o| o.is_silent()), so when global task.output = silent is set and a task overrides with output = prefix, quiet(Some(task)) returns true via silent_bool(). This causes command-echo and timing lines to remain suppressed even though the task opted into prefix style. E2E test (f) only checks that the task stdout appears, so this gap passes CI. src/task/task_output_handler.rs — specifically silent_bool() at line 333 and how it feeds into quiet() for per-task context when a global task.output = silent is in effect. Important Files Changed
Reviews (7): Last reviewed commit: "fix(task): honor per-task keep-order/tim..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
b8a5589 to
bf74a00
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/task/mod.rs (1)
806-808: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider propagating deserialization errors for the
outputfield.Invalid values like
output = "prefxi"are silently swallowed by.ok()and fall back to the global setting with no diagnostic. This matches the existingsilentpattern (lines 802–804) but is inconsistent withconfirm(lines 781–787) which returns a clear error. Sinceoutputis a new field, this is a good opportunity to improve UX.♻️ Optional refactor: propagate parse errors
task.output = p .get_raw("output") - .and_then(|v| TaskOutput::deserialize(v.clone()).ok()); + .map(|v| { + TaskOutput::deserialize(v.clone()) + .map_err(|e| eyre!("failed to parse output field in task header: {e}")) + }) + .transpose()?;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/task/mod.rs` around lines 806 - 808, Propagate parsing failures for the new output field instead of swallowing them in task assignment. In the code that sets task.output via TaskOutput::deserialize, handle the error path explicitly and return a clear diagnostic like the confirm field logic does, so invalid values do not silently fall back to the global setting. Update the task parsing flow in the task mod implementation to keep the existing silent behavior only where intended and make output validation fail fast with a useful error.e2e/tasks/test_task_output_style_decoupling (1)
56-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding test coverage for
--output quietand globaltask.output = "silent".The test suite covers the key decoupling scenarios well. Two gaps worth considering:
--output quietCLI flag — the recommended replacement for old--quietbehavior (line 24 ofrunning-tasks.md). No test verifies it produces un-prefixed output with suppressed mise messages.- Global
task.output = "silent"+ raw task — this is the edge case wherestyle_with_raw(Silent, true)may returnInterleaveinstead ofSilent(see comment ontask_output.rs). A test here would catch the regression.🧪 Suggested additional test cases
# (f) --output quiet produces un-prefixed output with suppressed mise messages cat <<EOF >mise.toml [tasks.hello] run = 'echo "hello world"' EOF assert_not_contains "mise run --output quiet hello 2>&1" "[hello] hello world" assert_contains "mise run --output quiet hello 2>&1" "hello world" assert_not_contains "mise run --output quiet hello 2>&1" '$ echo' # (g) global task.output = "silent" + raw task -> still silent cat <<EOF >mise.toml [settings] task.output = "silent" [tasks.rawed] run = 'echo "should not appear"' raw = true EOF assert_empty "mise run rawed 2>&1"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/tasks/test_task_output_style_decoupling` around lines 56 - 65, Add coverage for the missing task output decoupling cases in the task output tests: extend the existing task output style test around the relevant shell assertions to verify `mise run --output quiet` emits un-prefixed task output while suppressing mise command echoes, and add a separate case for `task.output = "silent"` with a raw task to ensure the output stays fully silent. Use the existing task-run test helpers and the `task.output` / `raw` task setup so the new cases exercise the same behavior paths as the current `style_with_raw` and `output quiet` handling.settings.toml (1)
2497-2517: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDoc update reads well and accurately reflects the style/verbosity decoupling.
Minor: the
replacingentry has a typo — "printed-this" should be "printed - this" or "printed. This".✏️ Typo fix
- { value = "replacing", description = "replace stdout each time a line is printed-this uses similar logic as `mise install`" }, + { value = "replacing", description = "replace stdout each time a line is printed - this uses similar logic as `mise install`" },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@settings.toml` around lines 2497 - 2517, Fix the typo in the task output docs enum entry for task.output so the `replacing` description reads naturally; update the description in the `task.output` enum to change “printed-this” to a proper sentence break such as “printed - this” or “printed. This”, keeping the rest of the wording unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/tasks/test_task_output_style_decoupling`:
- Around line 56-65: Add coverage for the missing task output decoupling cases
in the task output tests: extend the existing task output style test around the
relevant shell assertions to verify `mise run --output quiet` emits un-prefixed
task output while suppressing mise command echoes, and add a separate case for
`task.output = "silent"` with a raw task to ensure the output stays fully
silent. Use the existing task-run test helpers and the `task.output` / `raw`
task setup so the new cases exercise the same behavior paths as the current
`style_with_raw` and `output quiet` handling.
In `@settings.toml`:
- Around line 2497-2517: Fix the typo in the task output docs enum entry for
task.output so the `replacing` description reads naturally; update the
description in the `task.output` enum to change “printed-this” to a proper
sentence break such as “printed - this” or “printed. This”, keeping the rest of
the wording unchanged.
In `@src/task/mod.rs`:
- Around line 806-808: Propagate parsing failures for the new output field
instead of swallowing them in task assignment. In the code that sets task.output
via TaskOutput::deserialize, handle the error path explicitly and return a clear
diagnostic like the confirm field logic does, so invalid values do not silently
fall back to the global setting. Update the task parsing flow in the task mod
implementation to keep the existing silent behavior only where intended and make
output validation fail fast with a useful error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: a54a9a9c-ffa6-4789-8329-39569c9f4ed7
📒 Files selected for processing (10)
docs/tasks/running-tasks.mddocs/tasks/task-configuration.mde2e/tasks/test_task_output_style_decouplingschema/mise-task.jsonschema/mise.jsonsettings.tomlsrc/task/mod.rssrc/task/task_executor.rssrc/task/task_output.rssrc/task/task_output_handler.rs
bf74a00 to
5535ada
Compare
5535ada to
2e1ed9a
Compare
`task.output` now controls only the output *style* (prefix/interleave/ keep-order/replacing/timed); verbosity (quiet/silent) is a separate axis, so the two combine — e.g. `output = "prefix"` + `quiet = true` keeps the task-name prefixes while suppressing mise's own messages. Also adds a per-task `output` field so an individual task can pick its own style. BREAKING CHANGE: `--quiet` / `quiet = true` / `MISE_QUIET=1` no longer collapse task output to un-prefixed interleave; they now preserve the resolved style (prefix when jobs > 1). Use `--output quiet` / `-o interleave` for the old un-prefixed behavior. The `task.output = "quiet"` / `"silent"` enum values are unchanged. Resolves jdx#4308 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2e1ed9a to
3c365f6
Compare
|
Thanks for putting this together. The direction makes sense to me: separating output style from quiet/silent is the right shape, and the docs/tests cover a lot of the intended behavior. I do think this needs a small follow-up before merge because a couple of run-wide output gates still miss per-task
[tasks.a]
run = 'sleep 0.2; echo A'
output = "keep-order"
[tasks.b]
run = 'echo B'
output = "keep-order"mise run a ::: bObserved output was only task
[tasks.slow]
run = 'echo first; sleep 1.2; echo second; sleep 1.2'
output = "timed"mise run slowObserved output had the command/timing metadata, but none of the timed stdout lines. I verified the PR builds locally with This comment was generated by an AI coding assistant. |
…de gates Run-wide output gates resolved the mode via output(None) (global only) instead of each task's effective mode, so a per-task output = "keep-order" or output = "timed" override dropped that task's output: - keep-order task completion (src/cli/run.rs) and the final flush safety net (src/task/task_results_display.rs) now use the task's effective mode / flush unconditionally. - the timed background printer now spawns when any task resolves to Timed, not only when the run-wide default is Timed. Adds e2e coverage for both per-task overrides. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks for the thorough review — both were real bugs. Pushed a fix in cb6665c. The root cause was exactly what you diagnosed: a few run-wide gates resolved the mode via
I verified both of your repros now behave correctly:
I also extended Note: I couldn't run This comment was generated by an AI coding assistant. |
… again (#13718) `mise run --no-timings` is documented to "hide the elapsed time printed after each task completes", but since v2025.11.2 it has only hidden the run's total. Each task still printed its own timing line (mise 2026.9.12): ```console $ mise run --no-timings a ::: b [a] $ echo a-out [b] $ echo b-out [b] b-out [a] a-out [b] Finished in 1.6ms [a] Finished in 1.8ms ``` With this fix, `--no-timings` hides both kinds of timing line again: ```console $ mise run --no-timings a ::: b [b] $ echo b-out [a] $ echo a-out [a] a-out [b] b-out ``` `--no-timings` also overrides `MISE_TASK_TIMINGS=1` and the hidden `--timings` flag, as it did before the regression, and nested `mise run` calls inside a task get `MISE_TASK_TIMINGS=0` even when both flags are passed. `MISE_TASK_TIMINGS=0` is unchanged: it hides the per-task lines and keeps the run's total, which #5175 made deliberate. ## Cause When the executor was split out of `run.rs` (#6852), the per-task gate went from `self.timings() && …` to `self.timings || …`. `timings()` is the method that checks `--no-timings` and `--quiet`; `timings` is the field that holds the `--timings` flag. `--quiet` was wired back in by #10885, but `--no-timings` never reached the executor. This PR passes it through and checks it first. ## Testing `e2e/tasks/test_task_no_timings` covers the parallel run with and without `--no-timings`, `--no-timings` against `MISE_TASK_TIMINGS=1` and `--timings` (directly and through a nested `mise run`), and `MISE_TASK_TIMINGS=0` keeping the total. Each `--no-timings` assertion fails without the matching executor change. The existing output-style, raw-output and keep-order task e2e tests pass. A manual comparison across `-j 8` and `-j 1`, `a ::: b` and a `depends` task, and each `MISE_TASK_TIMINGS` value changed only the `--no-timings` cases. *AI-assisted — Tool: Claude Code; model: anthropic/claude-opus-5-5; version: 2.1.281.* 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Narrow CLI-to-executor wiring and output gating for task timing lines; covered by new e2e tests with no security or data-handling impact. > > **Overview** > **`mise run --no-timings`** again suppresses each task’s **“Finished in …”** line (not only the run total), matching documented behavior after the task executor split stopped honoring the flag. > > The CLI **`no_timings`** value is passed into **`TaskExecutorConfig`** / **`TaskExecutor`**. **`task_timings()`** returns false when **`no_timings`** is set, so it overrides **`--timings`**, **`MISE_TASK_TIMINGS=1`**, and per-task timing defaults—including for nested **`mise run`** via **`MISE_TASK_TIMINGS=0`** in the child env. **`MISE_TASK_TIMINGS=0`** behavior (hide per-task lines, keep run total) is unchanged. > > Adds e2e **`e2e/tasks/test_task_no_timings`** for parallel runs, flag/env precedence, nested tasks, and **`MISE_TASK_TIMINGS=0`**. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit d8f9dd6. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Resolves #4308.
task.outputcurrently conflates two concerns in one enum: the output style(
prefix/interleave/keep-order/replacing/timed) and verbosity(
quiet/silent). Because they collapse to a single value, you can't get"quiet and prefixed" — choosing
quietforces un-prefixed output.This decouples the two axes:
task.output(and--output/MISE_TASK_OUTPUT) now resolves a style only; it never collapses toQuiet.quiet/silentknobs (global settings,--quiet/--silent, and the per-taskquiet/silentfields), which already gate mise's own metadata prints. So styles and quietness combine freely — e.g.output = "prefix"+quiet = trueprints prefixed task lines with none of mise's own chatter.outputfield so an individual task can pick its own style. It overrides the globaltask.outputsetting — including a globaltask.output = "silent"(a task can opt back into visible output).output = "interleave"opts out,output = "prefix"opts in), instead of the global default.--quiet/quiet = true/MISE_QUIET=1no longer collapse task output toun-prefixed interleave — they now preserve the resolved style (prefix when
jobs > 1). Users who relied on--quietalso dropping prefixes should use--output quiet(or-o interleave). Thetask.output = "quiet"/"silent"enum values are unchanged and remain the backward-compatible escape hatch
(
quiet→ interleave + suppressed mise output).Changes
src/task/task_output_handler.rs—output()resolves style-only; keeps full-silent distinct from partialsilent="stdout"/"stderr"; a globaltask.output = "silent"is treated as a style default (overridable by a per-taskoutput) rather than an unconditional short-circuit;quiet()/silent()extended for the per-task field.src/task/task_output.rs—style_only/style_with_rawhelpers (map legacyQuiet→Interleave;style_with_rawnever un-silences aSilentvalue underraw).src/task/task_executor.rs—task_timings()takes the task context and respects per-taskoutput+quietso the "Finished in …" line no longer leaks under--quietor mismatches a per-task style.src/task/mod.rs— per-taskoutput: Option<TaskOutput>field (decl/parse/merge/default).settings.toml+schema/mise.json(→ regeneratedschema/mise-task.json) — document the split; clarify the legacyquiet/silentvalues.docs/tasks/{task-configuration,running-tasks}.md— document the new field and decoupling; fixed a stale--interleavereference (that flag doesn't exist →--output interleave).e2e/tasks/test_task_output_style_decoupling— covers prefix+quiet, the legacyquietvalue,silent, per-task override (incl. overriding a globalsilent),rawprecedence,--quietkeeping prefixes (and not leaking timing), and per-task timing opt-in/opt-out.Testing
CI is green:
build-ubuntu(compile),unit-macos+nightly(unit tests), and the fulle2e-0…e2e-7sweep all pass. The regression guards (test_task_run_output,test_task_silent_streams) confirm the legacyquietvalue and partial-silent behavior are unchanged. Localsettings.toml/schema JSON were validated and the docs pass prettier + markdownlint.This PR was generated by an AI coding assistant.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
outputstyle overrides (prefix,interleave,keep-order,timed, etc.) independent ofquiet/silent.outputfield.Bug Fixes
silentsuppresses task output and legacytask.output="quiet"behavior is preserved.Documentation
--output interleaveand how style combines with--quiet/--silent.Tests