Skip to content

Async terminal for CLI in chat panel#312178

Draft
anthonykim1 wants to merge 27 commits intomainfrom
anthonykim1/supportAsyncTerminalInCopilotCLIchatPanel
Draft

Async terminal for CLI in chat panel#312178
anthonykim1 wants to merge 27 commits intomainfrom
anthonykim1/supportAsyncTerminalInCopilotCLIchatPanel

Conversation

@anthonykim1
Copy link
Copy Markdown
Contributor

@anthonykim1 anthonykim1 commented Apr 23, 2026

Resolves: #309290

*WIP almost there

  • Please see comments below (videos, screenshot, comments)
  • Feel free to fetch this branch locally and play around.

Both route with/without controller api should work.

anthonykim1 and others added 11 commits April 22, 2026 16:05
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
@anthonykim1 anthonykim1 self-assigned this Apr 23, 2026
Copilot AI review requested due to automatic review settings April 23, 2026 17:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a proposed chat sessions API for extensions to inject system-initiated “notification-style” turns into chat sessions they own, and wires it up for Copilot CLI sessions so async SDK/terminal completions can surface as new chat bubbles.

Changes:

  • Add vscode.chat.sendSystemInitiatedRequest(...) proposed API + options type.
  • Implement ext host ↔ main thread plumbing to enqueue a steering, system-initiated chat request with a label.
  • Update Copilot CLI chat sessions to forward SDK system.notification events into the chat panel and keep SDK sessions alive long enough for async completions to arrive.
Show a summary per file
File Description
src/vscode-dts/vscode.proposed.chatSessionsProvider.d.ts Adds proposed API surface for system-initiated chat requests.
src/vs/workbench/api/common/extHostChatSessions.ts Validates ownership + forwards requests to main thread via RPC.
src/vs/workbench/api/common/extHost.protocol.ts Extends main-thread chat sessions shape with $sendSystemInitiatedRequest.
src/vs/workbench/api/common/extHost.api.impl.ts Exposes chat.sendSystemInitiatedRequest to extensions under proposed API gate.
src/vs/workbench/api/browser/mainThreadChatSessions.ts Receives RPC and enqueues a steering system-initiated request in IChatService.
extensions/copilot/src/extension/chatSessions/vscode-node/copilotCLIChatSessions.ts Forwards CLI SDK system notifications into chat via the new API; passes through isSystemInitiated.
extensions/copilot/src/extension/chatSessions/copilotcli/node/test/testHelpers.ts Updates mock SDK session helper with an on(...) method to match usage.
extensions/copilot/src/extension/chatSessions/copilotcli/node/test/copilotcliSession.spec.ts Extends test mock with getBackgroundTasks() to satisfy new logging/inspection.
extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSessionService.ts Emits onDidReceiveSystemNotification and adds a post-turn keep-alive ref window.
extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSession.ts Translates SDK system.notification to a typed event; adds system-initiated request handling path.

Copilot's findings

Comments suppressed due to low confidence (1)

extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotcliSession.ts:357

  • There is new behavior for request.isSystemInitiated that changes how requests are handled (skipping _sdkSession.send() and waiting for the next assistant.turn_end). The existing unit tests for CopilotCLISession don't cover the isSystemInitiated path; please add a test that asserts the SDK send() is not called and that the handler resolves on assistant.turn_end / cancellation.
			}

			if (isAlreadyBusyWithAnotherRequest) {
				return this._handleRequestSteering(input, attachments, model, previousRequestSnapshot, token);
			} else {
				return this._handleRequestImpl(request, input, attachments, model, token, /*isSystemInitiated*/ false);
			}
		});
  • Files reviewed: 9/10 changed files
  • Comments generated: 5

Comment thread src/vs/workbench/api/browser/mainThreadChatSessions.ts
Comment thread src/vs/workbench/api/browser/mainThreadChatSessions.ts Outdated
@anthonykim1
Copy link
Copy Markdown
Contributor Author

anthonykim1 commented Apr 23, 2026

@DonJayamanne @rebornix @mattbierner

Don't think activeResponseCallback in

/**
* Callback invoked by the editor for a currently running response. This allows the session to push items for the
* current response and stream these in as them come in. The current response will be considered complete once the
* callback resolved.
*
* If not provided, the chat session is assumed to not currently be running.
*/
readonly activeResponseCallback?: (stream: ChatResponseStream, token: CancellationToken) => Thenable<void>;
would do it since it would show the progress shimmer that the response is still in progress?

I think the biggest thing against activateResponseCallback is that ui would be stuck in perpetual shimmer/responding state with all notification glued onto the previous assistant turn as if the model never stopped talking, and user wouldn't be able to send follow ups without killing the very thing that delivers the notification.

VS.

(This PR with new proposed api):

The approach from the PR we'd have no stalling shimmer, but would have new compact "system" bubble appear for async output such as sleep 5 finished

  • Also user can type follow up anytime (input not blocked).
  • Multiple async shell should work via multiple discrete bubble (clearly its own event).

anthonykim1 and others added 3 commits April 23, 2026 13:00
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
…path

Co-authored-by: Copilot <copilot@github.com>
@anthonykim1
Copy link
Copy Markdown
Contributor Author

anthonykim1 commented Apr 23, 2026

@roblourens @TylerLeonhardt
Here is what it would look like as of Apr 23rd, 2:23 p.m. PST:

Screen.Recording.2026-04-23.at.2.22.09.PM.mov

Here is equivalent in Copilot CLI.
cliAsyncExample

We could improve to better distinguish the async terminal output but it seems like CLI themselves are not doing explicit labeling either. Maybe the color of the decorations are meant to signify something?

/cc @justschen For chat ui

anthonykim1 and others added 2 commits April 23, 2026 14:29
Co-authored-by: Copilot <copilot@github.com>
// task list only includes *detached* shells and background agents, not
// plain `mode: "async"` shells (they're tracked internally by
// `shellContext.currentExecutions`). A status-based window matches the
// existing `sessionTerminators` 5-minute lifetime and covers every
Copy link
Copy Markdown
Contributor Author

@anthonykim1 anthonykim1 Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DonJayamanne @roblourens I'm not so sure about this 5-minute lifetime thing. I can see it was existing previously but don't have context over what specific thing it was meant for, maybe we can do something better here for async terminals and keeping session alive.

Copy link
Copy Markdown
Contributor

@DonJayamanne DonJayamanne Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we try to use getBackgroundTaskProgress and refreshBackgroundTasks in conjuction with getBackgroundTasks (methods in the SDK)

If the current API isn't going to work, then I think we'll need new API from CLI.
Please try the above, perhaps try Opus 4.7, we might be able to make use of these (specially refreshBackgroundTasks).

I don't think we shoudl have any timeout be it 5minutes or 1 minute or 15 iminutes, at the end of the day, they'll all be wrong in one case or another.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we may need a new API to get this working since it seems like both getBackgroundTaskProgress and refreshBackgroundTasks excludes non-detached shells, meaning for async runs like sleep some time and get back, it wouldn't be tracked via the two getBackgroundTaskProgress and refreshBackgroundTasks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replied offline with possible solutions.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 23, 2026

Base: fe0c7703 Current: 1de21d9d

No screenshot changes.

Co-authored-by: Copilot <copilot@github.com>
const disposables = new DisposableStore();
try {
await new Promise<void>(resolve => {
const off = this._sdkSession.on('assistant.turn_end', () => resolve());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if the next turn also starts an async terminal call?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not getting the desired output - rather first round finishes and gets back successfully but I dont hear back from the second async call. Maybe permission related issue? The logs show permission is requested for the second run but nothing comes back afterwards. Super weird Screenshot 2026-04-24 at 9 46 28 AM

Copy link
Copy Markdown
Contributor Author

@anthonykim1 anthonykim1 Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Figured out what's happening: 283a8ee for description but we'd need something more proper to make sure the confirmation gets passed appropriately

Edit: Now should work
Screenshot 2026-04-25 at 11 16 25 PM

Comment thread src/vscode-dts/vscode.proposed.chatSessionsProvider.d.ts
@DonJayamanne DonJayamanne requested a review from meganrogge April 23, 2026 23:41
anthonykim1 and others added 9 commits April 23, 2026 22:40
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Async terminals don't resume the session

3 participants