feat(task): stream keep-order output in real-time per task#8164
Conversation
Summary of ChangesHello @jdx, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the user experience of the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a great user experience improvement by streaming keep-order output in real-time. The implementation, centered around the new KeepOrderState, is well-structured. My review focuses on a few areas to enhance correctness, performance, and user-facing documentation:
- The description for
keep-orderinsettings.tomlis no longer accurate with the new streaming behavior. - There's an opportunity to optimize performance in
task_executor.rsby avoiding repeated string cloning for task prefixes. - The use of
swap_removeintask_output_handler.rscould lead to a non-deterministic order for promoting tasks, which could be improved for predictability.
settings.toml
Outdated
| { value = "prefix", description = "(default if jobs > 1) print by line with the prefix of the task name" }, | ||
| { value = "interleave", description = "(default if jobs == 1 or all tasks run sequentially) print output as it comes in" }, | ||
| { value = "keep-order", description = "print output from tasks in the order they are defined" }, | ||
| { value = "keep-order", description = "buffer output per-task and print each task's output when it finishes" }, |
There was a problem hiding this comment.
The updated description for keep-order appears to describe the old behavior of buffering all output until tasks complete. The new implementation streams one task live while buffering others, which is a significant improvement not reflected here. Updating the description will help users understand the new, improved behavior.
{ value = "keep-order", description = "Stream one task's output live while buffering others, printing all output in order as tasks complete" },
| let state = self.output_handler.keep_order_state.clone(); | ||
| let task_clone = task.clone(); | ||
| let prefix_str = prefix.to_string(); | ||
| cmd = cmd.with_on_stdout(move |line| { | ||
| state | ||
| .lock() | ||
| .unwrap() | ||
| .on_stdout(&task_clone, prefix_str.clone(), line); | ||
| }); |
There was a problem hiding this comment.
In the with_on_stdout closure, prefix_str.clone() is called for every line of output. For tasks that produce a lot of output, this can be inefficient. A similar issue exists for with_on_stderr.
You could optimize this by using an Arc<String> for the prefix. This would involve a few changes across files:
-
In
src/task/task_executor.rs, create anArc<String>for the prefix outside the closure and clone theArcinside:let prefix_arc = std::sync::Arc::new(prefix.to_string()); cmd = cmd.with_on_stdout(move |line| { state .lock() .unwrap() .on_stdout(&task_clone, prefix_arc.clone(), line); });
-
In
src/task/task_output_handler.rs, updateKeepOrderLineto store anArc<String>:pub enum KeepOrderLine { Stdout(std::sync::Arc<String>, String), // (prefix, line) Stderr(std::sync::Arc<String>, String), // (prefix, line) }
-
Update
on_stdoutandon_stderrinKeepOrderStateto acceptArc<String>:pub fn on_stdout(&mut self, task: &Task, prefix: std::sync::Arc<String>, line: String) { // ... self.buffers .entry(task.clone()) .or_default() .push(KeepOrderLine::Stdout(prefix, line)); }
This would replace expensive string cloning with cheap Arc cloning.
src/task/task_output_handler.rs
Outdated
| self.buffers.swap_remove(task); | ||
| self.flush_finished(); | ||
| self.promote_next(); | ||
| } else { | ||
| // Non-active task finished — remember it for later flushing | ||
| self.finished.push(task.clone()); | ||
| } | ||
| } | ||
|
|
||
| /// Flush all buffered output for tasks that have already finished. | ||
| fn flush_finished(&mut self) { | ||
| let finished = std::mem::take(&mut self.finished); | ||
| for task in finished { | ||
| if let Some(lines) = self.buffers.swap_remove(&task) { |
There was a problem hiding this comment.
The use of swap_remove on self.buffers in on_task_finished (line 76) and flush_finished (line 89) can lead to unpredictable task promotion order. IndexMap::swap_remove is O(1) but reorders the remaining elements. Since promote_next picks self.buffers.first(), the next task to become active will not be deterministic based on the order they were started.
The comment for buffers says "insertion order preserved". To respect this and ensure predictable behavior, consider using shift_remove instead. It's O(n) but maintains the order of the remaining tasks. Given the number of parallel tasks is typically small, the performance impact should be acceptable.
You would need to change swap_remove to shift_remove on both lines 76 and 89.
f6bebfa to
7b9b083
Compare
There was a problem hiding this comment.
Pull request overview
Updates mise run keep-order output handling to support real-time streaming for one “active” task while buffering others, and to route metadata messages through the same ordering mechanism.
Changes:
- Replaces the old per-task buffered stdout/stderr map with a
KeepOrderStatestreaming state machine. - Routes keep-order stdout/stderr callbacks and metadata (
eprint) throughKeepOrderState. - Flushes/promotes keep-order state on task completion and at final results display.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/task/task_results_display.rs |
Switches final keep-order display to KeepOrderState::flush_all() safety-net flush. |
src/task/task_output_handler.rs |
Introduces KeepOrderState and updates OutputHandler to use it; routes keep-order metadata through state. |
src/task/task_executor.rs |
Updates keep-order stdout/stderr callbacks to stream/buffer via KeepOrderState. |
src/cli/run.rs |
Notifies keep-order state when a task finishes so buffers can flush/promote. |
settings.toml |
Updates the user-facing description string for task_output=keep-order. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.add_failed_task(task.clone(), status); | ||
| } | ||
| if let Some(oh) = &this.output_handler { | ||
| oh.keep_order_state.lock().unwrap().on_task_finished(&task); |
There was a problem hiding this comment.
on_task_finished is called unconditionally for every task, even when the selected output mode is not keep-order. Since init_task only populates keep_order_state in keep-order mode, this path will just grow the finished list and retain task clones unnecessarily. Gate this call behind oh.output(None) == TaskOutput::KeepOrder (or equivalent) so state is only updated when needed.
| oh.keep_order_state.lock().unwrap().on_task_finished(&task); | |
| if oh.output(None) == crate::output::TaskOutput::KeepOrder { | |
| oh.keep_order_state | |
| .lock() | |
| .unwrap() | |
| .on_task_finished(&task); | |
| } |
settings.toml
Outdated
| { value = "prefix", description = "(default if jobs > 1) print by line with the prefix of the task name" }, | ||
| { value = "interleave", description = "(default if jobs == 1 or all tasks run sequentially) print output as it comes in" }, | ||
| { value = "keep-order", description = "print output from tasks in the order they are defined" }, | ||
| { value = "keep-order", description = "stream one task at a time, buffering others to avoid interleaving" }, |
There was a problem hiding this comment.
The updated keep-order description no longer matches the behavior described in this PR (stream one task live while buffering others) and appears to conflict with existing CLI docs that say keep-order “keep[s] the order of the output”. Please update this description to reflect the new streaming semantics while clarifying what “order” means (definition order vs completion order).
| { value = "keep-order", description = "stream one task at a time, buffering others to avoid interleaving" }, | |
| { value = "keep-order", description = "preserve task definition order while streaming: show one task's output live, buffering later tasks and replaying them after earlier ones finish so lines never interleave" }, |
| if let Some((task, _)) = self.buffers.first() { | ||
| let task = task.clone(); | ||
| self.active = Some(task.clone()); | ||
| if let Some(lines) = self.buffers.get_mut(&task) { | ||
| let lines = std::mem::take(lines); | ||
| Self::print_lines(&lines); | ||
| } |
There was a problem hiding this comment.
promote_next promotes buffers.first() unconditionally, even if that task has an empty buffer (never produced output) or may not be running yet. This contradicts the comment about promoting the next running task with buffered output and can result in all other tasks being forced to buffer until the promoted task eventually emits output. Consider selecting the first task with a non-empty buffer (or tracking started/running tasks) and leaving active as None otherwise.
| if let Some((task, _)) = self.buffers.first() { | |
| let task = task.clone(); | |
| self.active = Some(task.clone()); | |
| if let Some(lines) = self.buffers.get_mut(&task) { | |
| let lines = std::mem::take(lines); | |
| Self::print_lines(&lines); | |
| } | |
| // Find the first task that actually has buffered output. | |
| if let Some((task, lines)) = self | |
| .buffers | |
| .iter_mut() | |
| .find(|(_, lines)| !lines.is_empty()) | |
| { | |
| let task = task.clone(); | |
| self.active = Some(task.clone()); | |
| let lines = std::mem::take(lines); | |
| Self::print_lines(&lines); |
src/task/task_output_handler.rs
Outdated
| if self.active.as_ref() == Some(task) { | ||
| // Active task finished — clear it, flush waiting tasks, promote next | ||
| self.active = None; | ||
| self.buffers.swap_remove(task); | ||
| self.flush_finished(); | ||
| self.promote_next(); | ||
| } else { |
There was a problem hiding this comment.
swap_remove on an IndexMap does not preserve insertion order, which can break keep-order semantics when you later use buffers.first() to pick the next task. Use an order-preserving removal method (e.g., shift_remove) or otherwise ensure the remaining task order is stable.
| fn flush_finished(&mut self) { | ||
| let finished = std::mem::take(&mut self.finished); | ||
| for task in finished { | ||
| if let Some(lines) = self.buffers.swap_remove(&task) { | ||
| Self::print_lines(&lines); | ||
| } | ||
| } |
There was a problem hiding this comment.
flush_finished removes buffers with swap_remove, which reorders the remaining IndexMap entries and can cause later promotion/printing to happen in the wrong task order. Prefer an order-preserving removal (shift_remove) or a different data structure that keeps the original task order intact.
| pub fn on_stdout(&mut self, task: &Task, prefix: String, line: String) { | ||
| if self.active.is_none() || self.active.as_ref() == Some(task) { | ||
| self.active = Some(task.clone()); | ||
| print_stdout(&prefix, &line); | ||
| } else { | ||
| self.buffers | ||
| .entry(task.clone()) | ||
| .or_default() | ||
| .push(KeepOrderLine::Stdout(prefix, line)); | ||
| } |
There was a problem hiding this comment.
When active is None, the first task to emit output becomes active and prints immediately. This can allow a later-defined task to print before earlier tasks, violating the stated keep-order behavior. Consider selecting the initial active task based on the pre-initialized task order (from init_task) and buffering output until that task is active, or otherwise ensure output cannot appear before earlier tasks in the configured order.
src/task/task_output_handler.rs
Outdated
| /// Flush all buffered output for tasks that have already finished. | ||
| fn flush_finished(&mut self) { | ||
| let finished = std::mem::take(&mut self.finished); | ||
| for task in finished { | ||
| if let Some(lines) = self.buffers.swap_remove(&task) { | ||
| Self::print_lines(&lines); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
finished is tracked in completion order and then flushed in that same order. If multiple non-active tasks finish while another task is active, this can print buffers out of the original keep-order sequence (e.g., task C finishes before task B). Flushing should follow the configured task order (e.g., iterate buffers in insertion order and flush those marked finished).
7b9b083 to
35f0db4
Compare
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.13 x -- echo |
24.3 ± 0.7 | 23.3 | 28.3 | 1.00 |
mise x -- echo |
24.6 ± 0.9 | 23.4 | 30.7 | 1.01 ± 0.05 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.13 env |
23.9 ± 0.8 | 22.7 | 28.3 | 1.00 |
mise env |
23.9 ± 0.7 | 22.6 | 26.2 | 1.00 ± 0.05 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.13 hook-env |
24.4 ± 0.7 | 23.4 | 26.9 | 1.00 |
mise hook-env |
24.8 ± 0.8 | 23.6 | 27.2 | 1.01 ± 0.04 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.13 ls |
22.7 ± 0.7 | 21.4 | 27.4 | 1.00 |
mise ls |
23.0 ± 0.7 | 21.8 | 26.0 | 1.01 ± 0.04 |
xtasks/test/perf
| Command | mise-2026.2.13 | mise | Variance |
|---|---|---|---|
| install (cached) | 128ms | 128ms | +0% |
| ls (cached) | 80ms | 80ms | +0% |
| bin-paths (cached) | 83ms | 84ms | -1% |
| task-ls (cached) | 843ms | 825ms | +2% |
35f0db4 to
1601835
Compare
1601835 to
af57b4f
Compare
| if self.is_active(task) { | ||
| // Active task finished — clear it, flush waiting tasks, promote next | ||
| self.active = None; | ||
| self.buffers.shift_remove(task); |
There was a problem hiding this comment.
Active task's buffered output discarded on finish
Low Severity
In on_task_finished, self.buffers.shift_remove(task) discards the removed buffer without printing it. This assumes the active task's buffer is empty, which is true for tasks initialized by init_task but not for tasks added to buffers dynamically via on_stdout/on_stderr's entry().or_default(). A task's first output line can get buffered (when is_active returns false because buffers.first() is None at that moment), yet a subsequent on_task_finished sees the task as active (since it's now first in buffers) and discards the buffer. Compare with promote_next, which correctly flushes via std::mem::take before streaming.
Additional Locations (1)
Previously, keep-order mode buffered ALL task output and printed everything after all tasks completed. Now, one task at a time streams its output live while other tasks buffer. When the active task finishes, buffered output from already-finished tasks is flushed, then the next running task is promoted to stream live. Closes #8136 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Gate on_task_finished behind KeepOrder check to avoid unnecessary work in non-keep-order modes - Update settings.toml description to reflect new streaming behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
af57b4f to
c2fe26c
Compare
|
bugbot run |
### 🚀 Features - **(task)** stream keep-order output in real-time per task by @jdx in [#8164](#8164) ### 🐛 Bug Fixes - **(aqua)** resolve lockfile artifacts for target platform (fix discussion #7479) by @mackwic in [#8183](#8183) - **(exec)** strip shims from PATH to prevent recursive shim execution by @jdx in [#8189](#8189) - **(hook-env)** preserve PATH reordering done after activation by @jdx in [#8190](#8190) - **(lockfile)** resolve version aliases before lockfile lookup by @jdx in [#8194](#8194) - **(registry)** set helm-diff archive bin name to diff by @jean-humann in [#8173](#8173) - **(task)** improve source freshness checks with dynamic task dirs by @rooperuu in [#8169](#8169) - **(task)** resolve global tasks when running from monorepo root by @jdx in [#8192](#8192) - **(task)** prevent wildcard glob `test:*` from matching parent task `test` by @jdx in [#8165](#8165) - **(task)** resolve task_config.includes relative to config root by @jdx in [#8193](#8193) - **(upgrade)** skip untrusted tracked configs during upgrade by @jdx in [#8195](#8195) ### 🚜 Refactor - use enum for npm.pacakge_manager by @risu729 in [#8180](#8180) ### 📚 Documentation - **(plugins)** replace node/asdf-nodejs examples with vfox plugins by @jdx in [#8191](#8191) ### ⚡ Performance - call npm view only once by @risu729 in [#8181](#8181) ### New Contributors - @jean-humann made their first contribution in [#8173](#8173) - @mackwic made their first contribution in [#8183](#8183) - @rooperuu made their first contribution in [#8169](#8169) ## 📦 Aqua Registry Updates #### New Packages (2) - [`BetterDiscord/cli`](https://github.com/BetterDiscord/cli) - [`glossia.ai/cli`](https://github.com/glossia.ai/cli)
## Summary - `keep-order` mode now streams the active task's output in real-time instead of buffering everything until all tasks complete - One task at a time is "active" and streams live; other tasks buffer. When the active task finishes, finished tasks' buffers are flushed, then the next running task is promoted to stream live - Metadata messages (`$ command` echo, `Finished in` timing) are also routed through the streaming state so they stay properly ordered with task output Closes jdx#8136 ## Test plan - [x] All 469 unit tests pass - [x] `test_task_keep_order` e2e test passes - [x] `test_task_sequence_keep_order` e2e test passes - [x] All lint checks pass - [ ] Manual testing with parallel tasks confirms streaming behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches core concurrent task output routing and ordering, so regressions could manifest as missing/reordered output or deadlocks under parallel runs. > > **Overview** > `keep-order` task output is reworked to **stream one “active” task in real time** while buffering other tasks and flushing buffers in *definition order* as tasks finish, instead of buffering everything until the end. > > This introduces a new `KeepOrderState` in `task_output_handler.rs`, routes stdout/stderr and metadata (`eprint`) through it, and notifies it on task completion from the scheduler; final result display now only does a safety-net `flush_all`. Documentation for `task_output` is updated, and the `test_task_keep_order` e2e test adds a parallel slow/fast case to assert ordering is preserved even when completion order differs. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 8e22c89. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
### 🚀 Features - **(task)** stream keep-order output in real-time per task by @jdx in [jdx#8164](jdx#8164) ### 🐛 Bug Fixes - **(aqua)** resolve lockfile artifacts for target platform (fix discussion jdx#7479) by @mackwic in [jdx#8183](jdx#8183) - **(exec)** strip shims from PATH to prevent recursive shim execution by @jdx in [jdx#8189](jdx#8189) - **(hook-env)** preserve PATH reordering done after activation by @jdx in [jdx#8190](jdx#8190) - **(lockfile)** resolve version aliases before lockfile lookup by @jdx in [jdx#8194](jdx#8194) - **(registry)** set helm-diff archive bin name to diff by @jean-humann in [jdx#8173](jdx#8173) - **(task)** improve source freshness checks with dynamic task dirs by @rooperuu in [jdx#8169](jdx#8169) - **(task)** resolve global tasks when running from monorepo root by @jdx in [jdx#8192](jdx#8192) - **(task)** prevent wildcard glob `test:*` from matching parent task `test` by @jdx in [jdx#8165](jdx#8165) - **(task)** resolve task_config.includes relative to config root by @jdx in [jdx#8193](jdx#8193) - **(upgrade)** skip untrusted tracked configs during upgrade by @jdx in [jdx#8195](jdx#8195) ### 🚜 Refactor - use enum for npm.pacakge_manager by @risu729 in [jdx#8180](jdx#8180) ### 📚 Documentation - **(plugins)** replace node/asdf-nodejs examples with vfox plugins by @jdx in [jdx#8191](jdx#8191) ### ⚡ Performance - call npm view only once by @risu729 in [jdx#8181](jdx#8181) ### New Contributors - @jean-humann made their first contribution in [jdx#8173](jdx#8173) - @mackwic made their first contribution in [jdx#8183](jdx#8183) - @rooperuu made their first contribution in [jdx#8169](jdx#8169) ## 📦 Aqua Registry Updates #### New Packages (2) - [`BetterDiscord/cli`](https://github.com/BetterDiscord/cli) - [`glossia.ai/cli`](https://github.com/glossia.ai/cli)
### 🚀 Features - **(task)** stream keep-order output in real-time per task by @jdx in [jdx#8164](jdx#8164) ### 🐛 Bug Fixes - **(aqua)** resolve lockfile artifacts for target platform (fix discussion jdx#7479) by @mackwic in [jdx#8183](jdx#8183) - **(exec)** strip shims from PATH to prevent recursive shim execution by @jdx in [jdx#8189](jdx#8189) - **(hook-env)** preserve PATH reordering done after activation by @jdx in [jdx#8190](jdx#8190) - **(lockfile)** resolve version aliases before lockfile lookup by @jdx in [jdx#8194](jdx#8194) - **(registry)** set helm-diff archive bin name to diff by @jean-humann in [jdx#8173](jdx#8173) - **(task)** improve source freshness checks with dynamic task dirs by @rooperuu in [jdx#8169](jdx#8169) - **(task)** resolve global tasks when running from monorepo root by @jdx in [jdx#8192](jdx#8192) - **(task)** prevent wildcard glob `test:*` from matching parent task `test` by @jdx in [jdx#8165](jdx#8165) - **(task)** resolve task_config.includes relative to config root by @jdx in [jdx#8193](jdx#8193) - **(upgrade)** skip untrusted tracked configs during upgrade by @jdx in [jdx#8195](jdx#8195) ### 🚜 Refactor - use enum for npm.pacakge_manager by @risu729 in [jdx#8180](jdx#8180) ### 📚 Documentation - **(plugins)** replace node/asdf-nodejs examples with vfox plugins by @jdx in [jdx#8191](jdx#8191) ### ⚡ Performance - call npm view only once by @risu729 in [jdx#8181](jdx#8181) ### New Contributors - @jean-humann made their first contribution in [jdx#8173](jdx#8173) - @mackwic made their first contribution in [jdx#8183](jdx#8183) - @rooperuu made their first contribution in [jdx#8169](jdx#8169) ## 📦 Aqua Registry Updates #### New Packages (2) - [`BetterDiscord/cli`](https://github.com/BetterDiscord/cli) - [`glossia.ai/cli`](https://github.com/glossia.ai/cli)
Summary
keep-ordermode now streams the active task's output in real-time instead of buffering everything until all tasks complete$ commandecho,Finished intiming) are also routed through the streaming state so they stay properly ordered with task outputCloses #8136
Test plan
test_task_keep_ordere2e test passestest_task_sequence_keep_ordere2e test passes🤖 Generated with Claude Code
Note
Medium Risk
Touches core concurrent task output routing and ordering, so regressions could manifest as missing/reordered output or deadlocks under parallel runs.
Overview
keep-ordertask output is reworked to stream one “active” task in real time while buffering other tasks and flushing buffers in definition order as tasks finish, instead of buffering everything until the end.This introduces a new
KeepOrderStateintask_output_handler.rs, routes stdout/stderr and metadata (eprint) through it, and notifies it on task completion from the scheduler; final result display now only does a safety-netflush_all. Documentation fortask_outputis updated, and thetest_task_keep_ordere2e test adds a parallel slow/fast case to assert ordering is preserved even when completion order differs.Written by Cursor Bugbot for commit 8e22c89. This will update automatically on new commits. Configure here.