From e193d44575104f345b6eff6fed291a9a55a144a4 Mon Sep 17 00:00:00 2001 From: Josh Pinkney <103940141+jpinkney-aws@users.noreply.github.com> Date: Thu, 5 Dec 2024 08:38:34 -0500 Subject: [PATCH] feat(amazonq): Create a common langauge server downloader (#6148) ## Problem - we need a way to download the manifest and install the correct mynah ui/language server code ## Solution - create a common lsp downloader that flare/workspace lsps can use --- packages/amazonq/src/lsp/activation.ts | 11 +- packages/amazonq/src/lsp/download.ts | 77 +++++ .../unit/amazonq/lsp/lspController.test.ts | 4 +- packages/core/src/amazonq/index.ts | 2 +- .../core/src/amazonq/lsp/lspController.ts | 284 ++++-------------- packages/core/src/shared/fetchLsp.ts | 214 +++++++++++++ packages/core/src/shared/index.ts | 1 + packages/core/src/shared/logger/logger.ts | 2 +- .../src/testInteg/perf/tryInstallLsp.test.ts | 11 +- 9 files changed, 375 insertions(+), 231 deletions(-) create mode 100644 packages/amazonq/src/lsp/download.ts create mode 100644 packages/core/src/shared/fetchLsp.ts diff --git a/packages/amazonq/src/lsp/activation.ts b/packages/amazonq/src/lsp/activation.ts index 7924096107d..44465f8659a 100644 --- a/packages/amazonq/src/lsp/activation.ts +++ b/packages/amazonq/src/lsp/activation.ts @@ -4,9 +4,18 @@ */ import vscode from 'vscode' +import { AmazonQLSPDownloader } from './download' export async function activate(ctx: vscode.ExtensionContext): Promise { + const serverPath = ctx.asAbsolutePath('resources/qdeveloperserver') + const clientPath = ctx.asAbsolutePath('resources/qdeveloperclient') + await new AmazonQLSPDownloader(serverPath, clientPath).tryInstallLsp() + /** - * download install and run the language server + * at this point the language server should be installed and available + * at serverPath and mynah ui should be available and serveable at + * clientPath + * + * TODO: actually hook up the language server */ } diff --git a/packages/amazonq/src/lsp/download.ts b/packages/amazonq/src/lsp/download.ts new file mode 100644 index 00000000000..29966b21495 --- /dev/null +++ b/packages/amazonq/src/lsp/download.ts @@ -0,0 +1,77 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + LspDownloader, + getLogger, + makeTemporaryToolkitFolder, + tryRemoveFolder, + fs, + Manifest, +} from 'aws-core-vscode/shared' + +const manifestURL = 'https://aws-toolkit-language-servers.amazonaws.com/codewhisperer/0/manifest.json' + +export class AmazonQLSPDownloader extends LspDownloader { + constructor( + private readonly serverPath: string, + private readonly clientPath: string + ) { + super(manifestURL) + } + + async isLspInstalled(): Promise { + return (await fs.exists(this.serverPath)) && (await fs.exists(this.clientPath)) + } + + async cleanup(): Promise { + if (await fs.exists(this.serverPath)) { + await tryRemoveFolder(this.serverPath) + } + + if (await fs.exists(this.clientPath)) { + await tryRemoveFolder(this.clientPath) + } + + return true + } + + async install(manifest: Manifest) { + const server = this.getDependency(manifest, 'servers') + const clients = this.getDependency(manifest, 'clients') + if (!server || !clients) { + getLogger('lsp').info(`Did not find LSP URL for ${process.platform} ${process.arch}`) + return false + } + + let tempFolder = undefined + + try { + tempFolder = await makeTemporaryToolkitFolder() + + // download and extract the business logic + await this.downloadAndExtractServer({ + content: server, + installLocation: this.serverPath, + name: 'qdeveloperserver', + tempFolder, + }) + + // download and extract mynah ui + await this.downloadAndExtractServer({ + content: clients, + installLocation: this.clientPath, + name: 'qdeveloperclient', + tempFolder, + }) + } finally { + if (tempFolder) { + await tryRemoveFolder(tempFolder) + } + } + + return true + } +} diff --git a/packages/amazonq/test/unit/amazonq/lsp/lspController.test.ts b/packages/amazonq/test/unit/amazonq/lsp/lspController.test.ts index d54551e433f..87111b97f1a 100644 --- a/packages/amazonq/test/unit/amazonq/lsp/lspController.test.ts +++ b/packages/amazonq/test/unit/amazonq/lsp/lspController.test.ts @@ -4,9 +4,9 @@ */ import assert from 'assert' import sinon from 'sinon' -import { Content, LspController } from 'aws-core-vscode/amazonq' +import { LspController } from 'aws-core-vscode/amazonq' import { createTestFile } from 'aws-core-vscode/test' -import { fs } from 'aws-core-vscode/shared' +import { fs, Content } from 'aws-core-vscode/shared' describe('Amazon Q LSP controller', function () { it('Download mechanism checks against hash, when hash matches', async function () { diff --git a/packages/core/src/amazonq/index.ts b/packages/core/src/amazonq/index.ts index 5bd20e4dfd0..c44878a18b6 100644 --- a/packages/core/src/amazonq/index.ts +++ b/packages/core/src/amazonq/index.ts @@ -15,7 +15,7 @@ export { walkthroughInlineSuggestionsExample, walkthroughSecurityScanExample, } from './onboardingPage/walkthrough' -export { LspController, Content } from './lsp/lspController' +export { LspController } from './lsp/lspController' export { LspClient } from './lsp/lspClient' export { api } from './extApi' export { AmazonQChatViewProvider } from './webview/webView' diff --git a/packages/core/src/amazonq/lsp/lspController.ts b/packages/core/src/amazonq/lsp/lspController.ts index 7a74318dd14..4c784eab163 100644 --- a/packages/core/src/amazonq/lsp/lspController.ts +++ b/packages/core/src/amazonq/lsp/lspController.ts @@ -5,23 +5,18 @@ import * as vscode from 'vscode' import * as path from 'path' -import * as crypto from 'crypto' -import { createWriteStream } from 'fs' // eslint-disable-line no-restricted-imports import { getLogger } from '../../shared/logger/logger' import { CurrentWsFolders, collectFilesForIndex } from '../../shared/utilities/workspaceUtils' -import fetch from 'node-fetch' -import request from '../../shared/request' import { LspClient } from './lspClient' -import AdmZip from 'adm-zip' import { RelevantTextDocument } from '@amzn/codewhisperer-streaming' -import { makeTemporaryToolkitFolder, tryRemoveFolder } from '../../shared/filesystemUtilities' import { activate as activateLsp } from './lspClient' import { telemetry } from '../../shared/telemetry' import { isCloud9 } from '../../shared/extensionUtilities' -import { fs, globals, ToolkitError } from '../../shared' -import { isWeb } from '../../shared/extensionGlobals' -import { getUserAgent } from '../../shared/telemetry/util' +import globals, { isWeb } from '../../shared/extensionGlobals' import { isAmazonInternalOs } from '../../shared/vscode/env' +import { LspDownloader, Manifest } from '../../shared/fetchLsp' +import { fs } from '../../shared/fs/fs' +import { makeTemporaryToolkitFolder, tryRemoveFolder } from '../../shared/filesystemUtilities' export interface Chunk { readonly filePath: string @@ -31,31 +26,6 @@ export interface Chunk { readonly programmingLanguage?: string } -export interface Content { - filename: string - url: string - hashes: string[] - bytes: number - serverVersion?: string -} - -export interface Target { - platform: string - arch: string - contents: Content[] -} - -export interface Manifest { - manifestSchemaVersion: string - artifactId: string - artifactDescription: string - isManifestDeprecated: boolean - versions: { - serverVersion: string - isDelisted: boolean - targets: Target[] - }[] -} const manifestUrl = 'https://aws-toolkit-language-servers.amazonaws.com/q-context/manifest.json' // this LSP client in Q extension is only going to work with these LSP server versions const supportedLspServerVersions = ['0.1.29'] @@ -79,202 +49,24 @@ export interface BuildIndexConfig { * Pre-process the input to Index Files API * Post-process the output from Query API */ -export class LspController { +export class LspController extends LspDownloader { static #instance: LspController private _isIndexingInProgress = false + private serverPath: string + private nodePath: string public static get instance() { return (this.#instance ??= new this()) } - constructor() {} - - isIndexingInProgress() { - return this._isIndexingInProgress - } - - async _download(localFile: string, remoteUrl: string) { - const res = await fetch(remoteUrl, { - headers: { - 'User-Agent': getUserAgent({ includePlatform: true, includeClientId: true }), - }, - }) - if (!res.ok) { - throw new ToolkitError(`Failed to download. Error: ${JSON.stringify(res)}`) - } - return new Promise((resolve, reject) => { - const file = createWriteStream(localFile) - res.body.pipe(file) - res.body.on('error', (err) => { - reject(err) - }) - file.on('finish', () => { - file.close(resolve) - }) - }) - } - async fetchManifest() { - try { - const resp = await request.fetch('GET', manifestUrl, { - headers: { - 'User-Agent': getUserAgent({ includePlatform: true, includeClientId: true }), - }, - }).response - if (!resp.ok) { - throw new ToolkitError(`Failed to fetch manifest. Error: ${resp.statusText}`) - } - return resp.json() - } catch (e: any) { - throw new ToolkitError(`Failed to fetch manifest. Error: ${JSON.stringify(e)}`) - } - } - - async getFileSha384(filePath: string): Promise { - const fileBuffer = await fs.readFileBytes(filePath) - const hash = crypto.createHash('sha384') - hash.update(fileBuffer) - return hash.digest('hex') - } - - async isLspInstalled(context: vscode.ExtensionContext) { - const localQServer = context.asAbsolutePath(path.join('resources', 'qserver')) - const localNodeRuntime = context.asAbsolutePath(path.join('resources', nodeBinName)) - return (await fs.exists(localQServer)) && (await fs.exists(localNodeRuntime)) - } - - getQserverFromManifest(manifest: Manifest): Content | undefined { - if (manifest.isManifestDeprecated) { - return undefined - } - for (const version of manifest.versions) { - if (version.isDelisted) { - continue - } - if (!supportedLspServerVersions.includes(version.serverVersion)) { - continue - } - for (const t of version.targets) { - if ( - (t.platform === process.platform || (t.platform === 'windows' && process.platform === 'win32')) && - t.arch === process.arch - ) { - for (const content of t.contents) { - if (content.filename.startsWith('qserver') && content.hashes.length > 0) { - content.serverVersion = version.serverVersion - return content - } - } - } - } - } - return undefined - } - - getNodeRuntimeFromManifest(manifest: Manifest): Content | undefined { - if (manifest.isManifestDeprecated) { - return undefined - } - for (const version of manifest.versions) { - if (version.isDelisted) { - continue - } - if (!supportedLspServerVersions.includes(version.serverVersion)) { - continue - } - for (const t of version.targets) { - if ( - (t.platform === process.platform || (t.platform === 'windows' && process.platform === 'win32')) && - t.arch === process.arch - ) { - for (const content of t.contents) { - if (content.filename.startsWith('node') && content.hashes.length > 0) { - content.serverVersion = version.serverVersion - return content - } - } - } - } - } - return undefined - } - - private async hashMatch(filePath: string, content: Content) { - const sha384 = await this.getFileSha384(filePath) - if ('sha384:' + sha384 !== content.hashes[0]) { - getLogger().error( - `LspController: Downloaded file sha ${sha384} does not match manifest ${content.hashes[0]}.` - ) - await fs.delete(filePath) - return false - } - return true - } - - async downloadAndCheckHash(filePath: string, content: Content) { - await this._download(filePath, content.url) - const match = await this.hashMatch(filePath, content) - if (!match) { - return false - } - return true + constructor() { + super(manifestUrl, supportedLspServerVersions) + this.serverPath = globals.context.asAbsolutePath(path.join('resources', 'qserver')) + this.nodePath = globals.context.asAbsolutePath(path.join('resources', nodeBinName)) } - async tryInstallLsp(context: vscode.ExtensionContext): Promise { - let tempFolder = undefined - try { - if (await this.isLspInstalled(context)) { - getLogger().info(`LspController: LSP already installed`) - return true - } - // clean up previous downloaded LSP - const qserverPath = context.asAbsolutePath(path.join('resources', 'qserver')) - if (await fs.exists(qserverPath)) { - await tryRemoveFolder(qserverPath) - } - // clean up previous downloaded node runtime - const nodeRuntimePath = context.asAbsolutePath(path.join('resources', nodeBinName)) - if (await fs.exists(nodeRuntimePath)) { - await fs.delete(nodeRuntimePath) - } - // fetch download url for qserver and node runtime - const manifest: Manifest = (await this.fetchManifest()) as Manifest - const qserverContent = this.getQserverFromManifest(manifest) - const nodeRuntimeContent = this.getNodeRuntimeFromManifest(manifest) - if (!qserverContent || !nodeRuntimeContent) { - getLogger().info(`LspController: Did not find LSP URL for ${process.platform} ${process.arch}`) - return false - } - - tempFolder = await makeTemporaryToolkitFolder() - - // download lsp to temp folder - const qserverZipTempPath = path.join(tempFolder, 'qserver.zip') - const downloadOk = await this.downloadAndCheckHash(qserverZipTempPath, qserverContent) - if (!downloadOk) { - return false - } - const zip = new AdmZip(qserverZipTempPath) - zip.extractAllTo(tempFolder) - await fs.rename(path.join(tempFolder, 'qserver'), qserverPath) - - // download node runtime to temp folder - const nodeRuntimeTempPath = path.join(tempFolder, nodeBinName) - const downloadNodeOk = await this.downloadAndCheckHash(nodeRuntimeTempPath, nodeRuntimeContent) - if (!downloadNodeOk) { - return false - } - await fs.chmod(nodeRuntimeTempPath, 0o755) - await fs.rename(nodeRuntimeTempPath, nodeRuntimePath) - return true - } catch (e) { - getLogger().error(`LspController: Failed to setup LSP server ${e}`) - return false - } finally { - // clean up temp folder - if (tempFolder) { - await tryRemoveFolder(tempFolder) - } - } + isIndexingInProgress() { + return this._isIndexingInProgress } async query(s: string): Promise { @@ -378,6 +170,54 @@ export class LspController { } } + async isLspInstalled(): Promise { + return (await fs.exists(this.serverPath)) && (await fs.exists(this.nodePath)) + } + + async cleanup(): Promise { + if (await fs.exists(this.serverPath)) { + await tryRemoveFolder(this.serverPath) + } + + if (await fs.exists(this.nodePath)) { + await fs.delete(this.nodePath) + } + + return true + } + + async install(manifest: Manifest) { + const server = this.getDependency(manifest, 'qserver') + const runtime = this.getDependency(manifest, 'node') + if (!server || !runtime) { + getLogger('lsp').info(`Did not find LSP URL for ${process.platform} ${process.arch}`) + return false + } + + let tempFolder = undefined + + try { + tempFolder = await makeTemporaryToolkitFolder() + await this.downloadAndExtractServer({ + content: server, + installLocation: this.serverPath, + name: 'qserver', + tempFolder, + extractToTempFolder: true, + }) + + const runtimeTempPath = path.join(tempFolder, nodeBinName) + await this.installRuntime(runtime, this.nodePath, runtimeTempPath) + } finally { + // clean up temp folder + if (tempFolder) { + await tryRemoveFolder(tempFolder) + } + } + + return true + } + async trySetupLsp(context: vscode.ExtensionContext, buildIndexConfig: BuildIndexConfig) { if (isCloud9() || isWeb() || isAmazonInternalOs()) { getLogger().warn('LspController: Skipping LSP setup. LSP is not compatible with the current environment. ') @@ -385,7 +225,7 @@ export class LspController { return } setImmediate(async () => { - const ok = await LspController.instance.tryInstallLsp(context) + const ok = await LspController.instance.tryInstallLsp() if (!ok) { return } diff --git a/packages/core/src/shared/fetchLsp.ts b/packages/core/src/shared/fetchLsp.ts new file mode 100644 index 00000000000..d57b05dabdc --- /dev/null +++ b/packages/core/src/shared/fetchLsp.ts @@ -0,0 +1,214 @@ +/*! + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +import path from 'path' +import * as crypto from 'crypto' +import fs from './fs/fs' +import { getLogger } from './logger/logger' +import request from './request' +import { getUserAgent } from './telemetry/util' +import { ToolkitError } from './errors' +import fetch from 'node-fetch' +// TODO remove +// eslint-disable-next-line no-restricted-imports +import { createWriteStream } from 'fs' +import AdmZip from 'adm-zip' + +export interface Content { + filename: string + url: string + hashes: string[] + bytes: number + serverVersion?: string +} + +export interface Target { + platform: string + arch: string + contents: Content[] +} + +export interface Manifest { + manifestSchemaVersion: string + artifactId: string + artifactDescription: string + isManifestDeprecated: boolean + versions: { + serverVersion: string + isDelisted: boolean + targets: Target[] + }[] +} + +export abstract class LspDownloader { + constructor( + private readonly manifestURL: string, + private readonly supportedLspServerVersions?: string[] + ) {} + + async fetchManifest() { + try { + const resp = await request.fetch('GET', this.manifestURL, { + headers: { + 'User-Agent': getUserAgent({ includePlatform: true, includeClientId: true }), + }, + }).response + if (!resp.ok) { + throw new ToolkitError(`Failed to fetch manifest. Error: ${resp.statusText}`) + } + return resp.json() + } catch (e: any) { + throw new ToolkitError(`Failed to fetch manifest. Error: ${JSON.stringify(e)}`) + } + } + + async _download(localFile: string, remoteUrl: string) { + const res = await fetch(remoteUrl, { + headers: { + 'User-Agent': getUserAgent({ includePlatform: true, includeClientId: true }), + }, + }) + if (!res.ok) { + throw new ToolkitError(`Failed to download. Error: ${JSON.stringify(res)}`) + } + return new Promise((resolve, reject) => { + const file = createWriteStream(localFile) + res.body.pipe(file) + res.body.on('error', (err) => { + reject(err) + }) + file.on('finish', () => { + file.close(resolve) + }) + }) + } + + async getFileSha384(filePath: string): Promise { + const fileBuffer = await fs.readFileBytes(filePath) + const hash = crypto.createHash('sha384') + hash.update(fileBuffer) + return hash.digest('hex') + } + + private async hashMatch(filePath: string, content: Content) { + const sha384 = await this.getFileSha384(filePath) + if ('sha384:' + sha384 !== content.hashes[0]) { + getLogger('lsp').error(`Downloaded file sha ${sha384} does not match manifest ${content.hashes[0]}.`) + await fs.delete(filePath) + return false + } + return true + } + + async downloadAndCheckHash(filePath: string, content: Content) { + await this._download(filePath, content.url) + const match = await this.hashMatch(filePath, content) + if (!match) { + return false + } + return true + } + + getDependency(manifest: Manifest, name: string): Content | undefined { + if (manifest.isManifestDeprecated) { + return undefined + } + for (const version of manifest.versions) { + if (version.isDelisted) { + continue + } + if (this.supportedLspServerVersions && !this.supportedLspServerVersions.includes(version.serverVersion)) { + continue + } + for (const t of version.targets) { + if ( + (t.platform === process.platform || (t.platform === 'windows' && process.platform === 'win32')) && + t.arch === process.arch + ) { + for (const content of t.contents) { + if (content.filename.startsWith(name) && content.hashes.length > 0) { + content.serverVersion = version.serverVersion + return content + } + } + } + } + } + return undefined + } + + async downloadAndExtractServer({ + content, + installLocation, + name, + tempFolder, + extractToTempFolder = false, + }: { + content: Content + installLocation: string + name: string + tempFolder: string + extractToTempFolder?: boolean + }) { + const serverZipTempPath = path.join(tempFolder, `${name}.zip`) + const downloadOk = await this.downloadAndCheckHash(serverZipTempPath, content) + if (!downloadOk) { + return false + } + + // load the zip contents + const extractPath = extractToTempFolder ? tempFolder : path.join(tempFolder, name) + new AdmZip(serverZipTempPath).extractAllTo(extractPath) + + await fs.rename(path.join(tempFolder, name), installLocation) + } + + async installRuntime(runtime: Content, installLocation: string, tempPath: string) { + const downloadNodeOk = await this.downloadAndCheckHash(tempPath, runtime) + if (!downloadNodeOk) { + return false + } + await fs.chmod(tempPath, 0o755) + await fs.rename(tempPath, installLocation) + } + + /** + * Detect if the lsps already exist on the filesystem + */ + abstract isLspInstalled(): Promise + + /** + * Cleanup any old LSPs or runtimes if they exist + */ + abstract cleanup(): Promise + + /** + * Given a manifest install any servers and runtimes that are required + */ + abstract install(manifest: Manifest): Promise + + async tryInstallLsp(): Promise { + try { + if (await this.isLspInstalled()) { + getLogger('lsp').info(`LSP already installed`) + return true + } + + const clean = await this.cleanup() + if (!clean) { + getLogger('lsp').error(`Failed to clean up old LSPs`) + return false + } + + // fetch download url for server and runtime + const manifest: Manifest = (await this.fetchManifest()) as Manifest + + return await this.install(manifest) + } catch (e) { + getLogger().error(`LspController: Failed to setup LSP server ${e}`) + return false + } + } +} diff --git a/packages/core/src/shared/index.ts b/packages/core/src/shared/index.ts index 5942f6ba6c2..f68221e7a4c 100644 --- a/packages/core/src/shared/index.ts +++ b/packages/core/src/shared/index.ts @@ -59,3 +59,4 @@ export { i18n } from './i18n-helper' export * from './icons' export * as textDocumentUtil from './utilities/textDocumentUtilities' export { TabTypeDataMap } from '../amazonq/webview/ui/tabs/constants' +export * from './fetchLsp' diff --git a/packages/core/src/shared/logger/logger.ts b/packages/core/src/shared/logger/logger.ts index 9cc04aa6585..a4f0ec2d6c5 100644 --- a/packages/core/src/shared/logger/logger.ts +++ b/packages/core/src/shared/logger/logger.ts @@ -5,7 +5,7 @@ import * as vscode from 'vscode' -export type LogTopic = 'crashMonitoring' | 'dev/beta' | 'notifications' | 'test' | 'unknown' +export type LogTopic = 'crashMonitoring' | 'dev/beta' | 'notifications' | 'test' | 'unknown' | 'lsp' class ErrorLog { constructor( diff --git a/packages/core/src/testInteg/perf/tryInstallLsp.test.ts b/packages/core/src/testInteg/perf/tryInstallLsp.test.ts index d84da8626d3..9fdd5ee8bdd 100644 --- a/packages/core/src/testInteg/perf/tryInstallLsp.test.ts +++ b/packages/core/src/testInteg/perf/tryInstallLsp.test.ts @@ -8,7 +8,7 @@ import { Content } from 'aws-sdk/clients/codecommit' import AdmZip from 'adm-zip' import path from 'path' import { LspController } from '../../amazonq' -import { fs, getRandomString, globals } from '../../shared' +import { fs, getRandomString } from '../../shared' import { createTestWorkspace } from '../../test/testUtil' import { getEqualOSTestOptions, performanceTest } from '../../shared/performance/performance' import { getFsCallsUpperBound } from './utilities' @@ -37,8 +37,11 @@ function createStubs(numberOfFiles: number, fileSize: number): sinon.SinonSpiedI // Avoid making HTTP request or mocking giant manifest, stub what we need directly from request. sinon.stub(LspController.prototype, 'fetchManifest') // Directly feed the runtime specifications. - sinon.stub(LspController.prototype, 'getQserverFromManifest').returns(fakeQServerContent) - sinon.stub(LspController.prototype, 'getNodeRuntimeFromManifest').returns(fakeNodeContent) + sinon + .stub(LspController.prototype, 'getDependency') + .withArgs(sinon.match.any, 'qserver') + .returns(fakeQServerContent) + sinon.stub(LspController.prototype, 'getDependency').withArgs(sinon.match.any, 'node').returns(fakeNodeContent) // avoid fetch call. sinon.stub(LspController.prototype, '_download').callsFake(getFakeDownload(numberOfFiles, fileSize)) // Hard code the hash since we are creating files on the spot, whose hashes can't be predicted. @@ -86,7 +89,7 @@ function performanceTestWrapper(numFiles: number, fileSize: number, message: str return createStubs(numFiles, fileSize) }, execute: async () => { - return await LspController.instance.tryInstallLsp(globals.context) + return await LspController.instance.tryInstallLsp() }, verify: async (fsSpy: sinon.SinonSpiedInstance, result: boolean) => { assert.ok(result)