-
Notifications
You must be signed in to change notification settings - Fork 716
feat(amazonq): Bundle LSP with the extension as fallback. #7421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
588bfec
1cdcf1e
048195b
632953c
d2bcaa4
51aec30
4931885
7a06a07
9b077d9
ce41a61
5182ba6
5927053
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
{ | ||
"type": "Feature", | ||
"description": "Launch LSP with bundled artifacts as fallback" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ import { | |
AmazonQPromptSettings, | ||
LanguageServerResolver, | ||
amazonqMark, | ||
getLogger, | ||
} from 'aws-core-vscode/shared' | ||
import { AuthUtil, RegionProfile } from 'aws-core-vscode/codewhisperer' | ||
import { featureConfig } from 'aws-core-vscode/amazonq' | ||
|
@@ -44,9 +45,12 @@ export class AmazonQChatViewProvider implements WebviewViewProvider { | |
) { | ||
const lspDir = Uri.file(LanguageServerResolver.defaultDir()) | ||
const dist = Uri.joinPath(globals.context.extensionUri, 'dist') | ||
|
||
const resourcesRoots = [lspDir, dist] | ||
|
||
const bundledResources = Uri.joinPath(globals.context.extensionUri, 'resources/language-server') | ||
let resourcesRoots = [lspDir, dist] | ||
if (this.mynahUIPath?.startsWith(globals.context.extensionUri.fsPath)) { | ||
getLogger('amazonqLsp').info(`Using bundled webview resources ${bundledResources.fsPath}`) | ||
Comment on lines
+49
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this decision being made here instead of in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same reason as I mentioned above. When it failed to launch, there are cases when the anti virus modified/removed the downloaded artifacts later on. The There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Can it do a step that verifies the contents after unzipping? I.e. whatever causes the AV to quarantine the artifacts, the resolver should do itself before continuing. |
||
resourcesRoots = [bundledResources, dist] | ||
} | ||
/** | ||
* if the mynah chat client is defined, then make sure to add it to the resource roots, otherwise | ||
* it will 401 when trying to load | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,181 @@ | ||
import * as https from 'https' | ||
import * as fs from 'fs' | ||
import * as crypto from 'crypto' | ||
import * as path from 'path' | ||
import * as os from 'os' | ||
import AdmZip from 'adm-zip' | ||
|
||
interface ManifestContent { | ||
filename: string | ||
url: string | ||
hashes: string[] | ||
bytes: number | ||
} | ||
|
||
interface ManifestTarget { | ||
platform: string | ||
arch: string | ||
contents: ManifestContent[] | ||
} | ||
|
||
interface ManifestVersion { | ||
serverVersion: string | ||
isDelisted: boolean | ||
targets: ManifestTarget[] | ||
} | ||
|
||
interface Manifest { | ||
versions: ManifestVersion[] | ||
} | ||
async function verifyFileHash(filePath: string, expectedHash: string): Promise<boolean> { | ||
return new Promise((resolve, reject) => { | ||
const hash = crypto.createHash('sha384') | ||
const stream = fs.createReadStream(filePath) | ||
|
||
stream.on('data', (data) => { | ||
hash.update(data) | ||
}) | ||
|
||
stream.on('end', () => { | ||
const fileHash = hash.digest('hex') | ||
// Remove 'sha384:' prefix from expected hash if present | ||
const expectedHashValue = expectedHash.replace('sha384:', '') | ||
resolve(fileHash === expectedHashValue) | ||
}) | ||
|
||
stream.on('error', reject) | ||
}) | ||
} | ||
|
||
async function ensureDirectoryExists(dirPath: string): Promise<void> { | ||
if (!fs.existsSync(dirPath)) { | ||
await fs.promises.mkdir(dirPath, { recursive: true }) | ||
} | ||
} | ||
|
||
export async function downloadLanguageServer(): Promise<void> { | ||
const tempDir = path.join(os.tmpdir(), 'amazonq-download-temp') | ||
const resourcesDir = path.join(__dirname, '../packages/amazonq/resources/language-server') | ||
|
||
// clear previous cached language server | ||
leigaol marked this conversation as resolved.
Show resolved
Hide resolved
|
||
try { | ||
if (fs.existsSync(resourcesDir)) { | ||
fs.rmdirSync(resourcesDir, { recursive: true }) | ||
} | ||
} catch (e) { | ||
throw Error(`Failed to clean up language server ${resourcesDir}`) | ||
} | ||
|
||
await ensureDirectoryExists(tempDir) | ||
await ensureDirectoryExists(resourcesDir) | ||
|
||
return new Promise((resolve, reject) => { | ||
const manifestUrl = 'https://aws-toolkit-language-servers.amazonaws.com/qAgenticChatServer/0/manifest.json' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
||
https | ||
.get(manifestUrl, (res) => { | ||
let data = '' | ||
|
||
res.on('data', (chunk) => { | ||
data += chunk | ||
}) | ||
|
||
res.on('end', async () => { | ||
try { | ||
const manifest: Manifest = JSON.parse(data) | ||
|
||
const latestVersion = manifest.versions | ||
.filter((v) => !v.isDelisted) | ||
.sort((a, b) => b.serverVersion.localeCompare(a.serverVersion))[0] | ||
|
||
if (!latestVersion) { | ||
throw new Error('No valid version found in manifest') | ||
} | ||
|
||
const darwinArm64Target = latestVersion.targets.find( | ||
(t) => t.platform === 'darwin' && t.arch === 'arm64' | ||
) | ||
|
||
if (!darwinArm64Target) { | ||
throw new Error('No darwin arm64 target found') | ||
} | ||
|
||
for (const content of darwinArm64Target.contents) { | ||
const fileName = content.filename | ||
const fileUrl = content.url | ||
const expectedHash = content.hashes[0] | ||
const tempFilePath = path.join(tempDir, fileName) | ||
const fileFolderName = content.filename.replace('.zip', '') | ||
|
||
console.log(`Downloading ${fileName} from ${fileUrl} ...`) | ||
|
||
await new Promise((downloadResolve, downloadReject) => { | ||
https | ||
.get(fileUrl, (fileRes) => { | ||
const fileStream = fs.createWriteStream(tempFilePath) | ||
fileRes.pipe(fileStream) | ||
|
||
fileStream.on('finish', () => { | ||
fileStream.close() | ||
downloadResolve(void 0) | ||
}) | ||
|
||
fileStream.on('error', (err) => { | ||
fs.unlink(tempFilePath, () => {}) | ||
downloadReject(err) | ||
}) | ||
}) | ||
.on('error', (err) => { | ||
fs.unlink(tempFilePath, () => {}) | ||
downloadReject(err) | ||
}) | ||
}) | ||
|
||
console.log(`Verifying hash for ${fileName}...`) | ||
leigaol marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const isHashValid = await verifyFileHash(tempFilePath, expectedHash) | ||
|
||
if (!isHashValid) { | ||
fs.unlinkSync(tempFilePath) | ||
throw new Error(`Hash verification failed for ${fileName}`) | ||
} | ||
|
||
console.log(`Extracting ${fileName}...`) | ||
const zip = new AdmZip(tempFilePath) | ||
zip.extractAllTo(path.join(resourcesDir, fileFolderName), true) // true for overwrite | ||
|
||
// Clean up temp file | ||
fs.unlinkSync(tempFilePath) | ||
console.log(`Successfully processed ${fileName}`) | ||
} | ||
|
||
// Clean up temp directory | ||
fs.rmdirSync(tempDir) | ||
fs.rmdirSync(path.join(resourcesDir, 'servers', 'indexing'), { recursive: true }) | ||
fs.rmdirSync(path.join(resourcesDir, 'servers', 'ripgrep'), { recursive: true }) | ||
fs.rmSync(path.join(resourcesDir, 'servers', 'node')) | ||
if (!fs.existsSync(path.join(resourcesDir, 'servers', 'aws-lsp-codewhisperer.js'))) { | ||
throw new Error(`Extracting aws-lsp-codewhisperer.js failure`) | ||
} | ||
if (!fs.existsSync(path.join(resourcesDir, 'clients', 'amazonq-ui.js'))) { | ||
throw new Error(`Extracting amazonq-ui.js failure`) | ||
} | ||
console.log('Download and extraction completed successfully') | ||
resolve() | ||
} catch (err) { | ||
// Clean up temp directory on error | ||
if (fs.existsSync(tempDir)) { | ||
fs.rmdirSync(tempDir, { recursive: true }) | ||
} | ||
reject(err) | ||
} | ||
}) | ||
}) | ||
.on('error', (err) => { | ||
// Clean up temp directory on error | ||
if (fs.existsSync(tempDir)) { | ||
fs.rmdirSync(tempDir, { recursive: true }) | ||
} | ||
reject(err) | ||
}) | ||
}) | ||
} |
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.
the fallback logic should live in the resolver, just like all the other fallback cases. it doesn't make sense to add a fallback at this level.
this fallback is no different than the other fallback cases in the resolver. it's just yet another fallback scenario.
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.
No. In the case of a firewall/anti virus, the resolver believes it successfully finished downloading and it then proceed to start the language server with the download path, but at that moment anti virus kicked in and broke it.
This outmost try catch can handle such cases.
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.
isn't that a bug in the resolver that should be fixed / redesigned ?