fix(tasks): deduplicate shared deps across task delegation#8497
fix(tasks): deduplicate shared deps across task delegation#8497
Conversation
Summary of ChangesHello, 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 resolves an issue where tasks delegated via 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 resolves an issue with duplicate execution of shared dependencies in delegated tasks by passing parent dependency state to sub-graphs, allowing them to prune tasks that are already running or completed. However, a logic error was identified in src/task/deps.rs regarding how in-flight tasks are handled during sub-graph construction, which could lead to dependency bypass. Despite this, the overall implementation is robust in addressing concurrency concerns, and other changes in src/cli/run.rs and src/task/task_executor.rs were found to be secure.
96888cb to
67a1157
Compare
|
Confirmed this is a real bug. The root cause is clear: In the reproduction case:
The fix approach is sound, but I think you should try a simpler implementation first: just pass a This comment was generated by Claude Code. |
a175b08 to
4873cad
Compare
Greptile SummaryThis PR fixes a deduplication bug where shared task dependencies were re-executed when a task used Key changes:
The e2e test validates the specific scenario from the issue report and successfully verifies the fix. Confidence Score: 3/5
Important Files Changed
|
When a task delegates via `run = [{ task }]`, the sub-graph created in
`inject_and_wait` had its own `sent` HashSet, causing shared dependencies
to run multiple times. Pass a snapshot of completed task keys to sub-graphs
so they can prune tasks already started or completed in the parent graph.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
0080f84 to
1914e1c
Compare
### 🚀 Features - **(vfox)** add `RUNTIME.envType` for libc variant detection by @malept in [#8493](#8493) - store provenance verification results in lockfile by @jdx in [#8495](#8495) ### 🐛 Bug Fixes - **(env)** skip remote version fetching for "latest" in prefer-offline mode by @jdx in [#8500](#8500) - **(tasks)** deduplicate shared deps across task delegation by @vadimpiven in [#8497](#8497) - **(windows)** correctly identify mise binary without extension by @jdx in [#8503](#8503) ### 🚜 Refactor - **(core)** migrate cmd! callers to async with kill_on_drop by @jdx in [a63f7d2](a63f7d2) ### Chore - **(ci)** temporarily disable `mise up` in release-plz by @jdx in [#8504](#8504) - consolidate all linters into hk.pkl by @jdx in [#8498](#8498) ## 📦 Aqua Registry Updates #### New Packages (1) - [`apache/ant`](https://github.com/apache/ant)
When a task delegates via
run = [{ task }], the sub-graph created ininject_and_waithad its ownsentHashSet, causing shared dependencies to run multiple times. Pass the parentDepsto sub-graphs so they can prune tasks already started or completed in the parent graph.Here is the plan I used to make Claude implement the fix:
Details
# Task delegation (`run = [{ task }]`) bypasses dedupmise
2026.3.4 macos-arm64Reproduction
Expected:
setupruns once.Root cause
inject_and_wait(task_executor.rs) builds a freshDepsgraph foreach
run = [{ task }]entry. Each sub-graph has its ownsentHashSet, so it knows nothing about tasks the parent already ran.
Fix
Snapshot the parent graph's confirmed-complete (
removed) task keysbefore spawning the sub-graph. The sub-graph constructor prunes
matching nodes at construction time. Only completed tasks are pruned
-- in-flight tasks stay in the sub-graph to preserve dependency
ordering.
1.
src/task/deps.rsTwo additions to
impl Deps:new_pruned-- builds a sub-graph with completed tasks removed.Does not pre-populate
removed, keeping cycle detection intact(
sent.len() == removed.len()starts balanced at 0):handled_task_keys-- returns onlyremoved(confirmed-complete).Excludes in-flight (
sent) tasks so sub-graphs preserve dependencyordering for tasks still running in the parent:
2.
src/task/mod.rsRe-export
TaskKey(keepdepsmodule private):3.
src/task/task_executor.rsThread
completed_tasks: HashSet<TaskKey>through three methods.run_task_schedtakes ownership (value moved from spawn site):exec_task_run_entriesandinject_and_waitborrow it:4.
src/cli/run.rsspawn_sched_job-- snapshot before callingrun_task_sched:Run::run_task_schedwrapper -- forward the new parameter:E2e test
e2e/tasks/test_task_delegation_dedup-- serializes execution(
fix:securitydepends onlint) sosetupis confirmed completebefore the sub-graph is created. Asserts
setupruns exactly onceand standalone
mise run -f check:securitystill triggerssetup:Files changed
src/task/deps.rsnew_pruned,handled_task_keyssrc/task/mod.rsTaskKeyalongsideDepssrc/task/task_executor.rscompleted_tasks, usenew_prunedsrc/cli/run.rshandled_task_keys(), forwarde2e/tasks/test_task_delegation_dedup