fix(task): run depends_post tasks even when parent task fails#8274
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 addresses a bug where 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
The pull request successfully addresses the issue where depends_post tasks were not running upon parent task failure. The implementation correctly updates the dependency graph to track post-dependencies and modifies the scheduler to allow these tasks to proceed even when the execution is stopping. I have identified a few performance-related improvements regarding redundant graph filtering and excessive allocations in the task identification logic that should be addressed to ensure the scheduler remains efficient with larger task graphs.
src/task/deps.rs
Outdated
| pub fn is_post_dep(&self, task: &Task) -> bool { | ||
| self.post_deps.contains(&task_key(task)) | ||
| } |
There was a problem hiding this comment.
The is_post_dep method calls task_key, which performs multiple string clones and vector allocations every time it is invoked. Since this check is used within filters and loops in the scheduler, it can lead to significant memory pressure and overhead in scenarios with many tasks. Consider precomputing this property on the Task struct during graph construction or using a more efficient identification method that avoids new allocations for every check.
src/task/task_scheduler.rs
Outdated
| let tasks_to_remove: Vec<Task> = deps | ||
| .all() | ||
| .filter(|t| !deps.is_post_dep(t)) | ||
| .cloned() | ||
| .collect(); | ||
| for task in tasks_to_remove { | ||
| deps.remove(&task); | ||
| } |
There was a problem hiding this comment.
This cleanup logic iterates over all tasks in the graph and performs expensive filtering (including the allocations in is_post_dep) on every iteration of the scheduler loop once a failure has occurred. Additionally, calling deps.remove in a loop triggers emit_leaves repeatedly, which can be
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where depends_post tasks (cleanup tasks) were not running when their parent task failed, which violated the documented behavior that states post-dependencies should run "whether successful or failed".
Changes:
- Added tracking of post-dependency tasks in the dependency graph to distinguish them from regular tasks
- Modified the scheduler to allow post-dependency tasks through even when stopping early due to failure
- Updated the job spawner to prevent post-dependency tasks from being skipped on failure
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/task/deps.rs | Added post_deps HashSet to track which tasks are post-dependencies and is_post_dep() method to check if a task should run on failure |
| src/task/task_scheduler.rs | Modified early-stop logic in the scheduler loop to filter out only non-post-dep tasks, allowing cleanup tasks to run |
| src/cli/run.rs | Updated spawn_sched_job to check is_post_dep() before skipping tasks on failure, in both pre-permit and post-permit checks |
| e2e/tasks/test_task_depends_post_on_failure | Added comprehensive test covering single post-dep failure, multiple post-deps on failure, and success case (regression) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Hyperfine Performance
|
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.17 x -- echo |
22.4 ± 0.3 | 21.8 | 23.7 | 1.00 |
mise x -- echo |
22.6 ± 0.5 | 21.8 | 26.3 | 1.01 ± 0.03 |
mise env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.17 env |
22.0 ± 0.9 | 21.2 | 29.8 | 1.00 |
mise env |
22.2 ± 0.6 | 21.4 | 29.0 | 1.01 ± 0.05 |
mise hook-env
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.17 hook-env |
22.9 ± 0.7 | 21.7 | 29.0 | 1.00 ± 0.04 |
mise hook-env |
22.8 ± 0.6 | 21.8 | 29.5 | 1.00 |
mise ls
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
mise-2026.2.17 ls |
20.0 ± 0.3 | 19.4 | 21.8 | 1.00 |
mise ls |
20.2 ± 0.4 | 19.6 | 23.9 | 1.01 ± 0.02 |
xtasks/test/perf
| Command | mise-2026.2.17 | mise | Variance |
|---|---|---|---|
| install (cached) | 123ms | 124ms | +0% |
| ls (cached) | 76ms | 76ms | +0% |
| bin-paths (cached) | 80ms | 80ms | +0% |
| task-ls (cached) | 807ms | 800ms | +0% |
depends_post tasks are intended as cleanup/finalization tasks that should run regardless of whether the parent task succeeds or fails, matching the documented behavior. Previously, the scheduler would stop all new task spawning on failure, preventing depends_post tasks from executing. Track post-dependency tasks in the dependency graph and allow them through the scheduler's stop checks so they run even after a task failure. Closes #5206 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
e16b3be to
b4cb620
Compare
Address review feedback: - Track parent→post-dep relationships so cleanup only preserves post-deps whose parent was at least scheduled to run - Use batch removal in cleanup to prevent intermediate emit_leaves from scheduling post-deps of never-started tasks - Run cleanup logic only once per failure instead of every loop iteration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
801cbe9 to
ca1d066
Compare
### 🚀 Features - **(install)** auto-lock all platforms after tool installation by @jdx in [#8277](#8277) ### 🐛 Bug Fixes - **(config)** respect --yes flag for config trust prompts by @jdx in [#8288](#8288) - **(exec)** strip shims from PATH on Unix to prevent infinite recursion by @jdx in [#8276](#8276) - **(install)** validate --locked before --dry-run short-circuit by @altendky in [#8290](#8290) - **(release)** refresh PATH after mise up in release-plz by @jdx in [#8292](#8292) - **(schema)** replace unevaluatedProperties with additionalProperties by @jdx in [#8285](#8285) - **(task)** avoid duplicated stderr on task failure in replacing mode by @jdx in [#8275](#8275) - **(task)** use process groups to kill child process trees on Unix by @jdx in [#8279](#8279) - **(task)** run depends_post tasks even when parent task fails by @jdx in [#8274](#8274) - **(task)** suggest similar commands when mistyping a CLI subcommand by @jdx in [#8286](#8286) - **(task)** execute monorepo subdirectory prepare steps from root by @jdx in [#8291](#8291) - **(upgrade)** don't force-reinstall already installed versions by @jdx in [#8282](#8282) - **(watch)** restore terminal state after watchexec exits by @jdx in [#8273](#8273) ### 📚 Documentation - clarify that MISE_CEILING_PATHS excludes the ceiling directory itself by @jdx in [#8283](#8283) ### Chore - replace gen-release-notes script with communique by @jdx in [#8289](#8289) ### New Contributors - @altendky made their first contribution in [#8290](#8290) ## 📦 Aqua Registry Updates #### New Packages (4) - [`Skarlso/crd-to-sample-yaml`](https://github.com/Skarlso/crd-to-sample-yaml) - [`kunobi-ninja/kunobi-releases`](https://github.com/kunobi-ninja/kunobi-releases) - [`swanysimon/markdownlint-rs`](https://github.com/swanysimon/markdownlint-rs) - [`tmux/tmux-builds`](https://github.com/tmux/tmux-builds) #### Updated Packages (2) - [`firecow/gitlab-ci-local`](https://github.com/firecow/gitlab-ci-local) - [`k1LoW/runn`](https://github.com/k1LoW/runn)
## [2026.2.18](https://github.com/jdx/mise/compare/v2026.2.17..v2026.2.18) - 2026-02-21 ### 🚀 Features - **(install)** auto-lock all platforms after tool installation by @jdx in [#8277](jdx/mise#8277) ### 🐛 Bug Fixes - **(config)** respect --yes flag for config trust prompts by @jdx in [#8288](jdx/mise#8288) - **(exec)** strip shims from PATH on Unix to prevent infinite recursion by @jdx in [#8276](jdx/mise#8276) - **(install)** validate --locked before --dry-run short-circuit by @altendky in [#8290](jdx/mise#8290) - **(release)** refresh PATH after mise up in release-plz by @jdx in [#8292](jdx/mise#8292) - **(schema)** replace unevaluatedProperties with additionalProperties by @jdx in [#8285](jdx/mise#8285) - **(task)** avoid duplicated stderr on task failure in replacing mode by @jdx in [#8275](jdx/mise#8275) - **(task)** use process groups to kill child process trees on Unix by @jdx in [#8279](jdx/mise#8279) - **(task)** run depends_post tasks even when parent task fails by @jdx in [#8274](jdx/mise#8274) - **(task)** suggest similar commands when mistyping a CLI subcommand by @jdx in [#8286](jdx/mise#8286) - **(task)** execute monorepo subdirectory prepare steps from root by @jdx in [#8291](jdx/mise#8291) - **(upgrade)** don't force-reinstall already installed versions by @jdx in [#8282](jdx/mise#8282) - **(watch)** restore terminal state after watchexec exits by @jdx in [#8273](jdx/mise#8273) ### 📚 Documentation - clarify that MISE_CEILING_PATHS excludes the ceiling directory itself by @jdx in [#8283](jdx/mise#8283) ### Chore - replace gen-release-notes script with communique by @jdx in [#8289](jdx/mise#8289) ### New Contributors - @altendky made their first contribution in [#8290](jdx/mise#8290) ### 📦 Aqua Registry Updates #### New Packages (4) - [`Skarlso/crd-to-sample-yaml`](https://github.com/Skarlso/crd-to-sample-yaml) - [`kunobi-ninja/kunobi-releases`](https://github.com/kunobi-ninja/kunobi-releases) - [`swanysimon/markdownlint-rs`](https://github.com/swanysimon/markdownlint-rs) - [`tmux/tmux-builds`](https://github.com/tmux/tmux-builds) #### Updated Packages (2) - [`firecow/gitlab-ci-local`](https://github.com/firecow/gitlab-ci-local) - [`k1LoW/runn`](https://github.com/k1LoW/runn) ## [2026.2.17](https://github.com/jdx/mise/compare/v2026.2.16..v2026.2.17) - 2026-02-19 ### 🚀 Features - **(prepare)** update mtime of outputs after command is run by @halms in [#8243](jdx/mise#8243) ### 🐛 Bug Fixes - **(install)** use backend bin paths for per-tool postinstall hooks by @jdx in [#8234](jdx/mise#8234) - **(use)** write to config.toml instead of config.local.toml by @jdx in [#8240](jdx/mise#8240) - default legacy .mise.backend installs to non-explicit by @jean-humann in [#8245](jdx/mise#8245) ### 🚜 Refactor - **(config)** consolidate flat task_* settings into nested task.* by @jdx in [#8239](jdx/mise#8239) ### Chore - **(prepare)** refactor common code into ProviderBase by @halms in [#8246](jdx/mise#8246) ### 📦 Aqua Registry Updates #### Updated Packages (1) - [`namespacelabs/foundation/nsc`](https://github.com/namespacelabs/foundation/nsc)
## Summary - `depends_post` tasks now run even when the parent task fails, matching the [documented behavior](https://github.com/jdx/mise/blob/main/docs/tasks/architecture.md#depends_post---cleanup-tasks) ("whether successful or failed") - The dependency graph now tracks which tasks are post-dependencies and allows them through the scheduler's stop checks - Non-post-dep tasks are still correctly skipped on failure (no change to that behavior) Closes jdx#5206 ## How it works Three changes: 1. **`Deps` (deps.rs)**: Tracks post-dep task keys in a `HashSet` during graph construction. Adds `is_post_dep()` method. 2. **Scheduler (task_scheduler.rs)**: When stopping due to failure, only removes non-post-dep tasks from the graph. Post-dep tasks are allowed through the drain loop and async recv. 3. **Job spawner (run.rs)**: `spawn_sched_job` checks `is_post_dep()` before skipping a task due to failure, allowing cleanup tasks to run. ## Test plan - [x] New e2e test `test_task_depends_post_on_failure` verifies: - Post-dep tasks run when parent fails - Multiple post-deps all run when parent fails - Post-dep tasks still work on success (no regression) - [x] Existing tests pass: `test_task_depends_post`, `test_task_depends_post_multiple`, `test_task_skip_deps` - [x] `mise run lint-fix` passes 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes core task scheduling/graph-pruning behavior on failure paths; regressions could affect which tasks run or whether the scheduler drains cleanly under concurrent execution. > > **Overview** > Fixes task execution so `depends_post` tasks (cleanup) still run even when a parent task fails and the scheduler enters stop mode. > > The dependency graph now records post-dep→parent relationships and exposes `Deps::is_runnable_post_dep()` plus batched removal (`remove_batch`) so failure cleanup prunes only non-runnable tasks while avoiding accidentally scheduling post-deps whose parent never started. The scheduler loop and `Run::spawn_sched_job` are updated to skip stop-aborts for runnable post-deps while still removing other tasks to ensure draining completes. > > Adds an e2e test `test_task_depends_post_on_failure` covering single and multiple post-deps on failure and a success-mode regression check. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit ca1d066. 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>
### 🚀 Features - **(install)** auto-lock all platforms after tool installation by @jdx in [jdx#8277](jdx#8277) ### 🐛 Bug Fixes - **(config)** respect --yes flag for config trust prompts by @jdx in [jdx#8288](jdx#8288) - **(exec)** strip shims from PATH on Unix to prevent infinite recursion by @jdx in [jdx#8276](jdx#8276) - **(install)** validate --locked before --dry-run short-circuit by @altendky in [jdx#8290](jdx#8290) - **(release)** refresh PATH after mise up in release-plz by @jdx in [jdx#8292](jdx#8292) - **(schema)** replace unevaluatedProperties with additionalProperties by @jdx in [jdx#8285](jdx#8285) - **(task)** avoid duplicated stderr on task failure in replacing mode by @jdx in [jdx#8275](jdx#8275) - **(task)** use process groups to kill child process trees on Unix by @jdx in [jdx#8279](jdx#8279) - **(task)** run depends_post tasks even when parent task fails by @jdx in [jdx#8274](jdx#8274) - **(task)** suggest similar commands when mistyping a CLI subcommand by @jdx in [jdx#8286](jdx#8286) - **(task)** execute monorepo subdirectory prepare steps from root by @jdx in [jdx#8291](jdx#8291) - **(upgrade)** don't force-reinstall already installed versions by @jdx in [jdx#8282](jdx#8282) - **(watch)** restore terminal state after watchexec exits by @jdx in [jdx#8273](jdx#8273) ### 📚 Documentation - clarify that MISE_CEILING_PATHS excludes the ceiling directory itself by @jdx in [jdx#8283](jdx#8283) ### Chore - replace gen-release-notes script with communique by @jdx in [jdx#8289](jdx#8289) ### New Contributors - @altendky made their first contribution in [jdx#8290](jdx#8290) ## 📦 Aqua Registry Updates #### New Packages (4) - [`Skarlso/crd-to-sample-yaml`](https://github.com/Skarlso/crd-to-sample-yaml) - [`kunobi-ninja/kunobi-releases`](https://github.com/kunobi-ninja/kunobi-releases) - [`swanysimon/markdownlint-rs`](https://github.com/swanysimon/markdownlint-rs) - [`tmux/tmux-builds`](https://github.com/tmux/tmux-builds) #### Updated Packages (2) - [`firecow/gitlab-ci-local`](https://github.com/firecow/gitlab-ci-local) - [`k1LoW/runn`](https://github.com/k1LoW/runn)
Summary
depends_posttasks now run even when the parent task fails, matching the documented behavior ("whether successful or failed")Closes #5206
How it works
Three changes:
Deps(deps.rs): Tracks post-dep task keys in aHashSetduring graph construction. Addsis_post_dep()method.spawn_sched_jobchecksis_post_dep()before skipping a task due to failure, allowing cleanup tasks to run.Test plan
test_task_depends_post_on_failureverifies:test_task_depends_post,test_task_depends_post_multiple,test_task_skip_depsmise run lint-fixpasses🤖 Generated with Claude Code
Note
Medium Risk
Changes core task scheduling/graph-pruning behavior on failure paths; regressions could affect which tasks run or whether the scheduler drains cleanly under concurrent execution.
Overview
Fixes task execution so
depends_posttasks (cleanup) still run even when a parent task fails and the scheduler enters stop mode.The dependency graph now records post-dep→parent relationships and exposes
Deps::is_runnable_post_dep()plus batched removal (remove_batch) so failure cleanup prunes only non-runnable tasks while avoiding accidentally scheduling post-deps whose parent never started. The scheduler loop andRun::spawn_sched_jobare updated to skip stop-aborts for runnable post-deps while still removing other tasks to ensure draining completes.Adds an e2e test
test_task_depends_post_on_failurecovering single and multiple post-deps on failure and a success-mode regression check.Written by Cursor Bugbot for commit ca1d066. This will update automatically on new commits. Configure here.