utils: Fixes for the copilot work in ACA #2337
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes issues with the “Deploy to Container app with Copilot…” flow by improving how the Copilot CLI binary is located/ensured, and updates vscode-tas-client to pick up fixes mentioned in microsoft/tas-client#95.
Changes:
- Improve Copilot CLI discovery by falling back to the globally-installed
copiloton PATH and prompting to install the CLI when missing. - Refine Copilot response validation and context parsing (
response?.datacheck andRegExp.execusage). - Upgrade
vscode-tas-clientfrom^0.1.84to^0.2.1(and lockfile updates), plus a minor type import cleanup inindex.d.ts.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| utils/src/errors.ts | Updates the Copilot-related error message to be more specific to CLI interaction failures. |
| utils/src/copilot/copilot.ts | Adds Copilot CLI installation check and introduces PATH-based CLI resolution fallback. |
| utils/package.json | Bumps vscode-tas-client dependency version. |
| utils/package-lock.json | Updates lockfile to reflect the new vscode-tas-client / tas-client versions. |
| utils/index.d.ts | Removes an unused vscode type import alias. |
Files not reviewed (1)
- utils/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| const fullPath = execFileSync( | ||
| process.platform === 'win32' ? 'where' : 'which', | ||
| ['copilot'], | ||
| { encoding: 'utf-8' }, | ||
| ).trim().split('\n')[0]; | ||
| if (fullPath) { | ||
| return fullPath; | ||
| } | ||
| } catch { | ||
| // which/where failed, fall back to bare command name | ||
| } | ||
| return 'copilot'; |
There was a problem hiding this comment.
Do you understand what it's concerned about here?
There was a problem hiding this comment.
Maybe it's referring to \r\n being the default line-ending sequence for Windows, while \n is the default for Linux and macOS
so split on \n has potential to leave a trailing \r in the path? something like that
|
I think we should be using the Also, should we extract this as a util method and put it with the other related util methods in utils/src/copilot/installCopilotCli.ts? |
…rrors # Conflicts: # utils/src/copilot/installCopilotCli.ts
I noticed the "Deploy to Container app with Copilot..." wasn't working. Not sure if it is because I switched to Mac but these changes seemed to fix it.
I also upgraded the "vscode-tas-client" as a fix was put in for this error that was occuring: microsoft/tas-client#95.