utils: Changes to enable re running commands with copilot #2136
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,34 +3,35 @@ | |
| * Licensed under the MIT License. See License.md in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import type { CopilotClient, CopilotSession } from "@github/copilot-sdk"; | ||
| import type * as vscode from "vscode"; | ||
| import { InvalidCopilotResponseError } from "../errors"; | ||
|
|
||
| let client: CopilotClient | undefined; | ||
| let session: CopilotSession | undefined; | ||
|
|
||
| async function loadCopilotSdk(): Promise<typeof import("@github/copilot-sdk")> { | ||
| return await import("@github/copilot-sdk"); | ||
| } | ||
|
|
||
| function getCopilotCliPath(): string { | ||
| return require.resolve(`@github/copilot-${process.platform}-${process.arch}`); | ||
| } | ||
|
|
||
| export function createPrimaryPromptToGetSingleQuickPickInput(picks: string[], placeholder?: string): string { | ||
| return ` | ||
| Task: choose one pick. | ||
|
|
||
| Input: | ||
| picks: ${JSON.stringify(picks)} | ||
| placeholder: ${placeholder ?? null} | ||
|
|
||
| Rules: | ||
| - If the picks represent container image tags, always prefer the newest available image tag. | ||
|
|
||
| Output: | ||
| Return ONLY the JSON object with fields "label" and "description". | ||
| `; | ||
| } | ||
|
|
||
| export function createPrimaryPromptToGetPickManyQuickPickInput(picks: string[], relevantContext?: string): string { | ||
|
|
@@ -63,35 +64,68 @@ export function createPrimaryPromptForWorkspaceFolderPick(folders: readonly vsco | |
| Respond with the workspace folder you have chosen. Do not respond in a conversational tone. `; | ||
| } | ||
|
|
||
| export async function doGithubCopilotInteraction(primaryPrompt: string, relevantContext?: string): Promise<string> { | ||
| const session = await getCopilotSession(relevantContext); | ||
| const response = await session.sendAndWait({ | ||
| prompt: primaryPrompt, | ||
| mode: "immediate" | ||
| }); | ||
|
|
||
| if (!response || !response.data) { | ||
| throw new InvalidCopilotResponseError(); | ||
| } | ||
|
|
||
| return response?.data.content; | ||
| } | ||
|
Comment on lines
+67
to
+79
There was a problem hiding this comment.
|
||
|
|
||
| export async function getCopilotSession(relevantContext?: string): Promise<CopilotSession> { | ||
| if (session) { | ||
| return session; | ||
| } | ||
|
|
||
| const { CopilotClient } = await loadCopilotSdk(); | ||
| client = new CopilotClient({ cliPath: getCopilotCliPath() }); | ||
| session = await client.createSession({ | ||
| onPermissionRequest: () => ({ kind: "approved" }) | ||
| }); | ||
| const activityChildren = extractActivityChildren(relevantContext || ''); | ||
| const subscriptionId = relevantContext ? extractSubscriptionIdFromContext(relevantContext) : undefined; | ||
|
Comment on lines
+81
to
+92
There was a problem hiding this comment.
|
||
|
|
||
| await session.sendAndWait({ | ||
| mode: "immediate", | ||
| prompt: `Picker. Choose one. Never explain your reasoning. Never use markdown. Output: {"label":"X","description":"Y"} | ||
|
|
||
| Rules (priority) | ||
| 1. Match activityChildren "Use X" → pick X | ||
| 2. Match subscriptionId → pick.data /subscriptions/{id} | ||
| 3. Else skipForNow | ||
|
|
||
| Context: ${JSON.stringify({ activityChildren, subscriptionId, relevantContext })}` | ||
| }); | ||
|
|
||
| return session; | ||
| } | ||
|
|
||
| export async function disposeCopilotSession(): Promise<void> { | ||
| if (client) { | ||
| await client.stop(); | ||
| client = undefined; | ||
| session = undefined; | ||
| } | ||
| } | ||
|
|
||
| function extractSubscriptionIdFromContext(context: string): string | undefined { | ||
|
Member
There was a problem hiding this comment. I think this is going to be too strict for any other cloud (government, China, etc.) It's a little looser matching, but I don't think we need to be too strict since we kind of know what we're parsing. |
||
| const regex = /https:\/\/management\.[^/]+\/subscriptions\/([0-9a-fA-F-]{36})/; | ||
| const match = context.match(regex); | ||
| return match ? match[1] : undefined; | ||
| } | ||
|
|
||
| function extractActivityChildren(context: string): string | undefined { | ||
| try { | ||
| const activityLog = JSON.parse(context) as { children?: unknown }; | ||
| const children = activityLog.children; | ||
| return JSON.stringify(children); | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.md in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
| import * as vscode from "vscode"; | ||
| import * as types from '../index'; | ||
|
|
||
| export function executeCommandWithAddedContext<T = unknown>(commandId: string, additionalContext: Partial<types.IActionContext>, ...args: unknown[]): Thenable<T> { | ||
| const metadata = { __injectedContext: additionalContext }; | ||
| return vscode.commands.executeCommand(commandId, ...args, metadata); | ||
| } | ||
|
Comment on lines
+8
to
+11
There was a problem hiding this comment. There’s no automated coverage for the new |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| /*--------------------------------------------------------------------------------------------- | ||
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.md in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
||
| import * as types from '../../index'; | ||
| import { AzureWizard } from '../wizard/AzureWizard'; | ||
|
|
||
| export async function runGenericPromptStep(context: types.PickExperienceContext, wizardOptions?: types.IWizardOptions<types.AzureResourceQuickPickWizardContext>): Promise<void> { | ||
| const wizard = new AzureWizard(context, { | ||
| ...wizardOptions, | ||
| hideStepCount: wizardOptions?.hideStepCount ?? true, | ||
| showLoadingPrompt: wizardOptions?.showLoadingPrompt ?? true | ||
| }); | ||
|
|
||
| await wizard.prompt(); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,20 @@ export function registerCommand(commandId: string, callback: (context: types.IAc | |
| return await callWithTelemetryAndErrorHandling( | ||
| telemetryId || commandId, | ||
| async (context: types.IActionContext) => { | ||
| let injectedContext: Partial<types.IActionContext> | undefined; | ||
| if (args.length > 0) { | ||
| const metadata = args.find( | ||
| (arg) => arg !== null && typeof arg === "object" && Object.prototype.hasOwnProperty.call(arg, "__injectedContext") | ||
| ); | ||
|
|
||
| if (metadata) { | ||
| const candidate = (metadata as Record<string, unknown>).__injectedContext; | ||
| if (candidate && typeof candidate === "object") { | ||
| injectedContext = candidate as Partial<types.IActionContext>; | ||
| } | ||
| args.splice(args.indexOf(metadata), 1); // remove only the metadata | ||
|
Comment on lines
+47
to
+56
There was a problem hiding this comment. The injected-context detection uses |
||
| } | ||
|
|
||
| try { | ||
| await setTelemetryProperties(context, args); | ||
| } catch (e: unknown) { | ||
|
|
@@ -52,8 +65,8 @@ export function registerCommand(commandId: string, callback: (context: types.IAc | |
| context.telemetry.properties.telemetryError = error.message; | ||
| } | ||
| } | ||
| const finalContext = injectedContext ? { ...context, ...injectedContext } : context; | ||
| return callback(finalContext, ...args); | ||
| } | ||
| ); | ||
| })); | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
relevantContextis ignored ifsessionalready exists due to the logic on line 68. Might want to redesign this so thatsessionisn't a global variable.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is by design we only want to pass in the relevant context when we create the session. In the subsequent calls it will be using that context and the prompt is changed through
createPrimaryPromptToGetSingleQuickPickInputso that the proper placeholder and picks get sent each time. If there is no session found then we need to send the relevantContext when we create the session.I designed it this way as it is much faster to not have create a new session on each quick pick call and send in all the relevant context everytime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of storing the Copilot session as a module-level global, it might be best to move the session ownership into the CopilotUserInput class itself.
Each CopilotUserInput instance could lazily create its own CopilotSession on the first interaction and reuse it for subsequent wizard steps within the same command so we keep the performance benefit of not recreating sessions for each step. But since each command already gets its own CopilotUserInput via context.ui, the session is naturally scoped to that command's lifecycle. This removes the need for callers to remember to call disposeCopilotSession(), and makes concurrent commands safe since they'd each have their own session. Also, the doGithubCopilotInteraction function would take a session as a parameter instead of managing the global itself.