getTask() and quickResolve() resolve tasks differently — side effect of an incorrect comparator?
- Dominant language
- TypeScript
- Stars
- 193k
- Forks
- 42.4k
- PR merge metrics
- PR metrics pending
Description
### Introduction
While working on a task runner extension, I ran into behavior that is difficult to understand and even harder to describe formally. It looks roughly like this: the same task can execute a different command depending on how it was launched, where it is defined, and whether it has collisions with tasks from other scopes.
Digging into the code, I found a comparator that violates the `Array.prototype.sort` contract — which is either a bug causing this behavior, or an undocumented design decision. I'd like clarification.
### Reproduction
**Note**: duplicate labels here are an **instrument for reproduction**, **not the core issue**. Duplicates make the violation observable — on unique labels the effect within a single scope is invisible, and I didn't want to complicate the setup with a multi-root project configuration.
The core issue is that there are two paths responsible for "find **this** task and run it", but they operate by different rules.
`.vscode/tasks.json`:
~~~json
{
"version": "2.0.0",
"tasks": [
{
"label": "PreTask",
"command": "echo 'first'",
"type": "shell",
"problemMatcher": []
},
{
"label": "PreTask",
"command": "echo 'second'",
"type": "shell",
"problemMatcher": []
},
{
"label": "PreTask",
"command": "echo 'third'",
"type": "shell",
"problemMatcher": []
},
{
"label": "Composite",
"dependsOn": ["PreTask"],
"problemMatcher": []
}
]
}
~~~
#### Observed behavior:
1. Running **any** of the three "PreTask" entries directly from the task picker: executes the task based on the **third (last)** definition of "PreTask" (`echo 'third'`).
2. Running "Composite" executes the dependency based on the **first** definition of "PreTask" (`echo 'first'`).
In other words: a directly launched PreTask ≠ PreTask launched as a dependency.
#### Environment
VS Code 1.137.0 (--disable-extensions)
#### Note
This reproduction demonstrates a specific case of a broader problem: VS Code itself decides which task gets executed, and its decision depends on where the task definition is located in the project/file and how the task was launched.
In a multi-root project, the same asymmetry applies at the cross-scope level — with User, Workspace, and folder scopes as additional variables.
I suspect some of these cases share a common root cause, but the repro above is intentionally kept minimal.
### Expected behavior
Given an identical task resolution request, the launch method should not change the resolution result.
If VS Code uses specific priority rules when resolving collisions, those rules should be applied consistently regardless of whether the task is launched directly or resolved as a dependsOn dependency.
Currently, that is not the case.
If this behavior is intentional, I — as an extension developer — would like to understand its semantics and know where it is documented.
If it is not intentional, it likely warrants a fix.
### What's happening in the code (as far as I can tell)
[`getTask()` (`abstractTaskService.ts`)](https://github.com/microsoft/vscode/blob/817a8d156d9e34e61e93d84f3c26be586f091c26/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts#L953) — re-resolution during direct user launch via the picker.
Two sequential paths inside:
[**Path 1**](https://github.com/microsoft/vscode/blob/817a8d156d9e34e61e93d84f3c26be586f091c26/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts#L971) — `_findWorkspaceTasks()`. Searches among tasks from `getWorkspaceTasks()` — these are `CustomTask` and `ConfiguringTask` from `.vscode/tasks.json`, .`code-workspace`, and user-level tasks. `ContributedTask` (extension tasks) are not included here. If matches are found, **sorting** occurs:
~~~ts
const matchedTasks = await this._findWorkspaceTasks((task, workspaceFolder) => {
const taskFolder = TaskMap.getKey(workspaceFolder);
if (taskFolder !== requestedFolder && taskFolder !== USER_TASKS_GROUP_KEY) {
return false;
}
return task.matches(key, compareId);
});
matchedTasks.sort(task => task._source.kind === TaskSourceKind.Extension ? 1 : -1); // <- !
return matchedTasks[0];
~~~
Since `matchedTasks` only contains `CustomTask` and `ConfiguringTask` (whose `_source.kind !== TaskSourceKind.Extension`), the comparator **always returns -1** — the condition inside it never triggers. The second argument is absent entirely, so the sort does not account for the relative order of elements. In practice this results in a **reversal** — though with certain caveats.
[**Path 2**](https://github.com/microsoft/vscode/blob/817a8d156d9e34e61e93d84f3c26be586f091c26/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts#L990) — fallback via `_getGroupedTasks()`, if Path 1 found nothing.
~~~ts
const map = await this._getGroupedTasks({ type });
let values = map.get(folder);
values = values.concat(map.get(USER_TASKS_GROUP_KEY));
if (!values) {
return undefined;
}
values = values.filter(task => task.matches(key, compareId)).sort(task => task._source.kind === TaskSourceKind.Extension ? 1 : -1);
return values.length > 0 ? values[0] : undefined;
~~~
Here `values` is expected to contain both `CustomTask`/`ConfiguringTask` and `ContributedTask`.
The same sort with the same comparator is applied. The intent is clearer here: [prioritize configured tasks over ContributedTask (Commit e526f16)](https://github.com/microsoft/vscode/commit/e526f16131e8dd700d1a1ff5ffdc618d098859cd), but the implementation raises questions.
For a pair of (`CustomTask`, `CustomTask`), the comparator always returns -1 — which again reverses the order of configured tasks. This looks like an unintentional side effect.
-----
`dependsOn` is resolved in [`quickResolve`](https://github.com/microsoft/vscode/blob/817a8d156d9e34e61e93d84f3c26be586f091c26/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts#L1971) or in [`fullResolve`](https://github.com/microsoft/vscode/blob/817a8d156d9e34e61e93d84f3c26be586f091c26/src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts#L2020) if `quickResolve` returned nothing. `quickResolve` does not touch the order of found tasks — it always uses the **first one**, no reordering:
~~~ts
const foundTasks = await that._findWorkspaceTasks(/* ... */);
if (foundTasks.length === 0) {
return undefined;
}
const task = foundTasks[0];
if (ConfiguringTask.is(task)) {
return that.tryResolveTask(task);
}
return task;
~~~
`fullResolve`, which builds a Map via `data.label.set(task._label, task)` when `getResolverData()` call, potentially returns the **last** task with a given label in case of a collision.
I'm not saying that this is a problem in and of itself — these semantics may well be intentional. But it's another example of ambiguous identifier resolution depending on which resolver path is taken.
Either way, neither `quickResolve` nor `fullResolve` reorders the tasks they receive.
-----
Summary of what I found: two paths for "find **this** task and run it", each with different behavior:
1. **Direct launch (`getTask()`):**
~~~
_findWorkspaceTasks → sort (reversal) → matchedTasks[0].
~~~
or
~~~
_getGroupedTasks → sort (reversal) → values[0]
~~~
2. **dependsOn (`quickResolve()`/`fullResolve`):**
~~~
_findWorkspaceTasks/_getGroupedTasks → (as is)
~~~
This difference alone is sufficient to produce the observed repro behavior:
1. direct run → last definition
2. dependsOn → first definition
The comparator in `getTask()` is also invalid as a comparator for `Array.prototype.sort()`.
In `getTask()` Path 2, the sort came from a commit with a clear intent (`CustomTask`/`ConfiguringTask` over `ContributedTask`). Apparently, the comparator was meant to push `ContributedTask` to the end. But due to the missing second argument, it reverses the entire array, changing the order within the configured group, which it should not do.
In `getTask()` Path 1, there are no `ContributedTask` at all — so the purpose of the same sort there is unclear to me. But either way, it violates the comparator contract.
-----
I don't claim to fully understand all the internal task resolver rules, which is precisely why I'd like some clarification:
**Is the difference between these paths intentional?**
If yes — is there documentation describing the collision resolution rules that extension developers can rely on?
If this is not intentional, there is at least one clear problem: the comparator implementation.
In `getTask()` Path 2, the comparator should probably compare Extension vs non-Extension without disturbing the relative order of tasks within the configured group:
~~~ts
values = values
.filter(task => task.matches(key, compareId))
.sort((a, b) => {
const aIsExt = a._source.kind === TaskSourceKind.Extension ? 1 : 0;
const bIsExt = b._source.kind === TaskSourceKind.Extension ? 1 : 0;
return aIsExt - bIsExt;
});
~~~
This groups `CustomTask`/`ConfiguringTask` before `ContributedTask` without changing the order within the groups themselves.
In `getTask()` Path 1, the sort appears to serve no purpose at all and looks like an attempt to maintain behavioral parity with Path 2 (reversal) — which has by now become an implicit contract of the function. It could be dropped, but only as part of the same change that fixes the comparator in Path 2.
-----
I was trying to determine the task-resolution rules from the implementation, but this is where I seem to be missing the specification: I cannot tell whether these differences are intentional behavior or implementation artifacts.
I am therefore not confident that the proposed change is complete, or even that it is the correct fix. If these resolution rules are intentional, some documentation of the resolver semantics would make them much easier for extension developers to understand and rely on. If they are not intentional, I hope the examples above help identify where the inconsistent behavior originates.
Contributor guide
Assessment
This issue has not been assessed yet.