Skip to content

Conversation

@rwoll
Copy link
Member

@rwoll rwoll commented Aug 28, 2025

When using the new workbench.action.chat.open.blockOnResponse feature, if the VSC instance was in auto-approve mode, it was observed that the command was resolving before the tool finished invocation.

This resulted in a race betwee sync and async code.

@connor4312 connor4312 enabled auto-merge (squash) August 28, 2025 21:14
@vs-code-engineering vs-code-engineering bot added this to the August 2025 milestone Aug 28, 2025
rwoll added 2 commits August 28, 2025 15:40
When using the new `workbench.action.chat.open.blockOnResponse`
feature, if the VSC instance was in auto-approve mode, it was observed
that the command was resolving before the tool finished invocation.

This resulted in a race betwee sync and async code:

```
		if (!this._confirmationMessages) {
			// No confirmation needed
			this._isConfirmed = { type: ToolConfirmKind.ConfirmationNotNeeded };
			this._confirmDeferred.complete(this._isConfirmed);
		}
```

and then the check for confirmation in `isConfirmed`.
@karthiknadig karthiknadig force-pushed the fix-block-on-response branch from 5290378 to 150cbd4 Compare August 28, 2025 22:40
@connor4312 connor4312 merged commit 6f24cee into microsoft:main Aug 28, 2025
17 checks passed
rwoll added a commit to rwoll/vscode that referenced this pull request Aug 29, 2025
This reverts commit 6f24cee.

It broke tool calling: microsoft#264023.

I also need to re-asses how I missed this during testing.
TylerLeonhardt pushed a commit that referenced this pull request Aug 29, 2025
This reverts commit 6f24cee.

It broke tool calling: #264023.

I also need to re-asses how I missed this during testing.
connor4312 added a commit that referenced this pull request Sep 2, 2025
This re-applies #263894 and fixes #264023 by removing a surprising
behavior in the DeferredPromise where calling complete() with additional
values will update the settled `deferred.value` without affecting the
original promise (which can of course only resolve once.)
connor4312 added a commit that referenced this pull request Sep 2, 2025
This re-applies #263894 and fixes #264023 by removing a surprising
behavior in the DeferredPromise where calling complete() with additional
values will update the settled `deferred.value` without affecting the
original promise (which can of course only resolve once.)
connor4312 added a commit that referenced this pull request Sep 5, 2025
This re-applies #263894 and fixes #264023 by removing a surprising
behavior in the DeferredPromise where calling complete() with additional
values will update the settled `deferred.value` without affecting the
original promise (which can of course only resolve once.)
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Oct 12, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants