From 749f8ed09fceda62df61ac726a71c7c5caac2e74 Mon Sep 17 00:00:00 2001 From: Nicolas Will Date: Tue, 13 Oct 2026 17:50:09 +0100 Subject: [PATCH 1/5] Add internal support for a custom variant analysis server Adds hidden, user/machine-scoped settings that send variant analysis requests to a self-hosted server implementing the GitHub variant analysis REST endpoints, authenticated with a bearer token supplied by a named companion extension. GitHub sign-in, the github-enterprise.uri setting and database downloads are unaffected. Internal and unsupported; see docs/variant-analysis-custom-server.md. Also tolerates repository tasks without a database commit SHA or source location prefix, and implements inspect() in the vscode-tests config mock. --- docs/variant-analysis-custom-server.md | 73 ++++++ extensions/ql-vscode/package.json | 2 +- extensions/ql-vscode/src/config.ts | 74 ++++++ .../src/variant-analysis/export-results.ts | 21 +- .../gh-api/variant-analysis-server-client.ts | 123 ++++++++++ .../ql-vscode/src/variant-analysis/ghec-dr.ts | 11 +- .../src/variant-analysis/run-remote-query.ts | 7 + .../variant-analysis/variant-analysis-api.ts | 134 +++++++++++ .../variant-analysis-manager.ts | 12 +- .../variant-analysis-monitor.ts | 2 +- .../variant-analysis-results-manager.ts | 10 +- .../variant-analysis-api.test.ts | 222 ++++++++++++++++++ .../test/vscode-tests/test-config.ts | 14 +- 13 files changed, 681 insertions(+), 24 deletions(-) create mode 100644 docs/variant-analysis-custom-server.md create mode 100644 extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts create mode 100644 extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts create mode 100644 extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts diff --git a/docs/variant-analysis-custom-server.md b/docs/variant-analysis-custom-server.md new file mode 100644 index 00000000000..fb3213c2a2b --- /dev/null +++ b/docs/variant-analysis-custom-server.md @@ -0,0 +1,73 @@ +# Custom variant analysis server (internal) + +> [!WARNING] +> **Internal and unsupported.** This is not a public feature. The settings below are not declared in +> `package.json`, do not appear in the Settings UI, and may change or be removed without notice. +> Do not document them in user-facing docs or the changelog. + +By default, multi-repository variant analysis (MRVA) uses the GitHub REST API and the user's GitHub sign-in. +An organization can instead send variant analyses to its own server. That server must implement the +GitHub variant analysis REST endpoints and authenticate requests with a bearer token. A separate +extension, owned by the organization, supplies the token. + +## Settings + +Both settings are only read from **user** or **machine** (remote) settings. Workspace and folder values are ignored, +so a repository cannot redirect queries or tokens. + +| Setting | Value | +| --- | --- | +| `codeQL.variantAnalysis.serverUrl` | Base URL of the server. Must be `https:` and must not contain credentials. A trailing `/` is removed. | +| `codeQL.variantAnalysis.serverTokenProvider` | ID (`publisher.name`) of the installed extension that provides the token. | + +If `serverUrl` is set but invalid, variant analysis fails instead of falling back to GitHub. + +## Token provider extension + +The extension named in `serverTokenProvider` must return the following from `activate()`: + +```ts +export interface VariantAnalysisServerTokenProvider { + getVariantAnalysisServerToken(serverUrl: string): Promise; +} +``` + +CodeQL activates the extension if it isn't already running, and calls `getVariantAnalysisServerToken` +before every request. The provider is responsible for caching and refreshing the token. It should return +`undefined` if no token is available, so that CodeQL can show a clear error instead of sending an +unauthenticated request. The token is never logged. + +Only the extension that the user or administrator names is called. Extensions cannot register +themselves as providers. + +## Behavior when a server is configured + +- MRVA is enabled regardless of `github-enterprise.uri`. The GitHub sign-in, GitHub database + downloads and other extensions that read `github-enterprise.uri` are not affected. +- No controller repository is needed. Requests use controller repository ID `0`. +- The GitHub token is never sent to the server, and results are never uploaded to GitHub: + "Export to gist" and "View logs" (GitHub Actions) are unavailable. +- Result artifacts are downloaded from `artifact_url` without authentication, as on GitHub. +- If a repository task has no `database_commit_sha`, file links point to `HEAD`. + +## Server contract + +All paths are relative to `serverUrl`. Requests send `Authorization: Bearer ` and JSON bodies. +Request and response shapes match `src/variant-analysis/gh-api/variant-analysis.ts`. + +| Operation | Request | +| --- | --- | +| Submit | `POST /repositories/0/code-scanning/codeql/variant-analyses` | +| Get status | `GET /repositories/0/code-scanning/codeql/variant-analyses/{id}` | +| Get repository task | `GET /repositories/0/code-scanning/codeql/variant-analyses/{id}/repositories/{repoId}` | +| Cancel | `POST /repos/{controller_repo.full_name}/actions/runs/{actions_workflow_run_id}/cancel` | + +Variant analysis responses must include `controller_repo` and, for cancellation to work, +`actions_workflow_run_id`. Errors should be returned as non-2xx responses with a JSON body +`{ "message": "..." }`. CodeQL shows the status and that message to the user. + +## Running CodeQL in a remote workspace + +To run the CodeQL extension on the remote machine alongside the token provider, set the VS Code +setting `remote.extensionKind` to `{ "github.vscode-codeql": ["workspace"] }`. No change to the +extension is needed. diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index aee0433d86c..cec9ce8fcd3 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -2034,7 +2034,7 @@ { "view": "codeQLVariantAnalysisRepositories", "contents": "Set up a controller repository to start using variant analysis. [Learn more](https://codeql.github.com/docs/codeql-for-visual-studio-code/running-codeql-queries-at-scale-with-mrva#controller-repository) about controller repositories. \n[Set up controller repository](command:codeQLVariantAnalysisRepositories.setupControllerRepository)", - "when": "!config.codeQL.variantAnalysis.controllerRepo" + "when": "!config.codeQL.variantAnalysis.controllerRepo && !config.codeQL.variantAnalysis.serverUrl" } ] }, diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index aabd1061b92..0bcec47c18a 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -605,6 +605,80 @@ export async function setRemoteControllerRepo(repo: string | undefined) { await REMOTE_CONTROLLER_REPO.updateValue(repo, ConfigurationTarget.Global); } +/** + * INTERNAL AND UNSUPPORTED: these settings are intentionally not declared in + * package.json and are not documented publicly. See + * docs/variant-analysis-custom-server.md. + * + * `serverUrl` points variant analysis at a self-hosted server that implements + * the GitHub variant analysis REST endpoints, instead of GitHub. + * `serverTokenProvider` is the ID of an installed extension that supplies the + * bearer token for that server. + * + * Both are only read from user or machine (global) settings, so a workspace + * cannot redirect queries or tokens. + */ +const VARIANT_ANALYSIS_SERVER_URL = new Setting( + "serverUrl", + VARIANT_ANALYSIS_SETTING, +); +const VARIANT_ANALYSIS_SERVER_TOKEN_PROVIDER = new Setting( + "serverTokenProvider", + VARIANT_ANALYSIS_SETTING, +); + +export interface VariantAnalysisServerConfig { + /** The server's base URL, always `https:` and without a trailing slash. */ + url: string; + /** ID of the extension that provides the bearer token. */ + tokenProviderExtensionId: string | undefined; +} + +function getGlobalStringSetting(setting: Setting): string | undefined { + const value = workspace + .getConfiguration(setting.parent!.qualifiedName) + .inspect(setting.name)?.globalValue; + return typeof value === "string" && value.trim() ? value.trim() : undefined; +} + +/** + * Returns the custom variant analysis server configuration, or `undefined` + * if variant analysis should use GitHub. Throws if the server URL is invalid, + * rather than silently falling back to GitHub. + */ +export function getVariantAnalysisServerConfig(): + VariantAnalysisServerConfig | undefined { + const rawUrl = getGlobalStringSetting(VARIANT_ANALYSIS_SERVER_URL); + if (!rawUrl) { + return undefined; + } + + let url: URL; + try { + url = new URL(rawUrl); + } catch { + throw new Error( + `Invalid ${VARIANT_ANALYSIS_SERVER_URL.qualifiedName} setting: ${rawUrl}`, + ); + } + if (url.protocol !== "https:" || url.username || url.password) { + throw new Error( + `${VARIANT_ANALYSIS_SERVER_URL.qualifiedName} must be an https URL without credentials.`, + ); + } + + return { + url: url.toString().replace(/\/+$/, ""), + tokenProviderExtensionId: getGlobalStringSetting( + VARIANT_ANALYSIS_SERVER_TOKEN_PROVIDER, + ), + }; +} + +export function isVariantAnalysisServerConfigured(): boolean { + return getGlobalStringSetting(VARIANT_ANALYSIS_SERVER_URL) !== undefined; +} + export interface VariantAnalysisConfig { controllerRepo: string | undefined; showSystemDefinedRepositoryLists: boolean; diff --git a/extensions/ql-vscode/src/variant-analysis/export-results.ts b/extensions/ql-vscode/src/variant-analysis/export-results.ts index 17d1997a47d..bfecd36cca4 100644 --- a/extensions/ql-vscode/src/variant-analysis/export-results.ts +++ b/extensions/ql-vscode/src/variant-analysis/export-results.ts @@ -10,7 +10,8 @@ import { } from "../common/vscode/progress"; import { showInformationMessageWithAction } from "../common/vscode/dialog"; import { extLogger } from "../common/logging/vscode"; -import { createGist } from "./gh-api/gh-api-client"; +import { createGist } from "./variant-analysis-api"; +import { isVariantAnalysisServerConfigured } from "../config"; import type { MarkdownFile, RepositorySummary } from "./markdown-generation"; import { generateVariantAnalysisMarkdown } from "./markdown-generation"; import { pluralize } from "../common/word"; @@ -208,14 +209,16 @@ async function determineExportFormat(): Promise<"gist" | "local" | undefined> { label: "$(markdown) Save as markdown", }; - const exportFormat = await window.showQuickPick( - [gistOption, localMarkdownOption], - { - placeHolder: "Select export format", - canPickMany: false, - ignoreFocusOut: true, - }, - ); + // Results must not be sent to GitHub when using a custom server. + const options = isVariantAnalysisServerConfigured() + ? [localMarkdownOption] + : [gistOption, localMarkdownOption]; + + const exportFormat = await window.showQuickPick(options, { + placeHolder: "Select export format", + canPickMany: false, + ignoreFocusOut: true, + }); if (!exportFormat || !exportFormat.label) { throw new UserCancellationException("No export format selected", true); } diff --git a/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts b/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts new file mode 100644 index 00000000000..a8866b4a939 --- /dev/null +++ b/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts @@ -0,0 +1,123 @@ +import { extensions } from "vscode"; +import { RequestError } from "@octokit/request-error"; +import type { VariantAnalysisServerConfig } from "../../config"; + +/** + * INTERNAL AND UNSUPPORTED: client for a self-hosted variant analysis server + * that implements the GitHub variant analysis REST endpoints. See + * docs/variant-analysis-custom-server.md. + */ + +/** + * The export that the configured token provider extension must implement. + */ +export interface VariantAnalysisServerTokenProvider { + getVariantAnalysisServerToken(serverUrl: string): Promise; +} + +/** The server does not use controller repositories, so this ID is sent instead. */ +export const SERVER_CONTROLLER_REPO_ID = 0; + +const MAX_ERROR_BODY_LENGTH = 500; + +async function getToken(server: VariantAnalysisServerConfig): Promise { + const extensionId = server.tokenProviderExtensionId; + if (!extensionId) { + throw new Error( + "No token provider is configured for the variant analysis server. Set codeQL.variantAnalysis.serverTokenProvider in your user settings.", + ); + } + + const extension = + extensions.getExtension>( + extensionId, + ); + if (!extension) { + throw new Error( + `The variant analysis server token provider extension '${extensionId}' is not installed.`, + ); + } + + const api = extension.isActive + ? extension.exports + : await extension.activate(); + if (typeof api?.getVariantAnalysisServerToken !== "function") { + throw new Error( + `The extension '${extensionId}' does not provide a variant analysis server token.`, + ); + } + + const token = await api.getVariantAnalysisServerToken(server.url); + if (typeof token !== "string" || !token) { + throw new Error( + `The extension '${extensionId}' did not return a variant analysis server token. Check that you are signed in.`, + ); + } + return token; +} + +/** + * Sends a request to the variant analysis server. Neither the request body + * nor the token is logged or included in errors. + */ +export async function requestVariantAnalysisServer( + server: VariantAnalysisServerConfig, + method: "GET" | "POST", + path: string, + body?: unknown, +): Promise { + const token = await getToken(server); + const url = `${server.url}${path}`; + + const response = await fetch(url, { + method, + headers: { + Accept: "application/json", + Authorization: `Bearer ${token}`, + ...(body === undefined ? {} : { "Content-Type": "application/json" }), + }, + body: body === undefined ? undefined : JSON.stringify(body), + }); + + const text = await response.text(); + let data: unknown = undefined; + if (text) { + try { + data = JSON.parse(text); + } catch { + data = text; + } + } + + if (!response.ok) { + throw new RequestError( + `Variant analysis server request failed: ${method} ${path}: ${response.status} ${response.statusText}${describeErrorBody(data)}`, + response.status, + { + request: { method, url, headers: {} }, + response: { + status: response.status, + url, + headers: {}, + data, + retryCount: 0, + }, + }, + ); + } + + return data as T; +} + +function describeErrorBody(data: unknown): string { + const message = + typeof data === "object" && + data !== null && + "message" in data && + typeof data.message === "string" + ? data.message + : typeof data === "string" + ? data + : undefined; + return message ? `: ${message.slice(0, MAX_ERROR_BODY_LENGTH)}` : ""; +} diff --git a/extensions/ql-vscode/src/variant-analysis/ghec-dr.ts b/extensions/ql-vscode/src/variant-analysis/ghec-dr.ts index c5a8ed06c62..131a2df72c9 100644 --- a/extensions/ql-vscode/src/variant-analysis/ghec-dr.ts +++ b/extensions/ql-vscode/src/variant-analysis/ghec-dr.ts @@ -1,10 +1,17 @@ -import { hasEnterpriseUri, hasGhecDrUri } from "../config"; +import { + hasEnterpriseUri, + hasGhecDrUri, + isVariantAnalysisServerConfigured, +} from "../config"; /** * Determines whether MRVA should be enabled or not for the current GitHub host. * MRVA is enabled on github.com and GHEC-DR. * This is based on the `github-enterprise.uri` setting. + * It is also enabled when an internal custom variant analysis server is configured. */ export function isVariantAnalysisEnabledForGitHubHost(): boolean { - return !hasEnterpriseUri() || hasGhecDrUri(); + return ( + !hasEnterpriseUri() || hasGhecDrUri() || isVariantAnalysisServerConfigured() + ); } diff --git a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts index f2670d914ae..da6084dd85b 100644 --- a/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts +++ b/extensions/ql-vscode/src/variant-analysis/run-remote-query.ts @@ -12,6 +12,7 @@ import { extLogger } from "../common/logging/vscode"; import { getActionBranch, getRemoteControllerRepo, + isVariantAnalysisServerConfigured, setRemoteControllerRepo, } from "../config"; import type { ProgressCallback } from "../common/vscode/progress"; @@ -20,6 +21,7 @@ import type { RequestError } from "@octokit/types/dist-types"; import type { QueryMetadata } from "../common/interface-types"; import { getErrorMessage, REPO_REGEX } from "../common/helpers-pure"; import { getRepositoryFromNwo } from "./gh-api/gh-api-client"; +import { SERVER_CONTROLLER_REPO_ID } from "./gh-api/variant-analysis-server-client"; import type { RepositorySelection } from "./repository-selection"; import { getRepositorySelection, @@ -326,6 +328,11 @@ export function getQueryName( export async function getControllerRepo( credentials: Credentials, ): Promise { + // A custom variant analysis server does not use controller repositories. + if (isVariantAnalysisServerConfigured()) { + return { id: SERVER_CONTROLLER_REPO_ID, fullName: "", private: true }; + } + // Get the controller repo from the config, if it exists. // If it doesn't exist, prompt the user to enter it, check // whether the repo exists, and save the nwo to the config. diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts new file mode 100644 index 00000000000..b1f20ed024b --- /dev/null +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts @@ -0,0 +1,134 @@ +import type { Credentials } from "../common/authentication"; +import { getVariantAnalysisServerConfig } from "../config"; +import { cancelVariantAnalysis as cancelGitHubVariantAnalysis } from "./gh-api/gh-actions-api-client"; +import { + createGist as createGitHubGist, + getVariantAnalysis as getGitHubVariantAnalysis, + getVariantAnalysisRepo as getGitHubVariantAnalysisRepo, + submitVariantAnalysis as submitGitHubVariantAnalysis, +} from "./gh-api/gh-api-client"; +import type { + VariantAnalysis as ApiVariantAnalysis, + VariantAnalysisRepoTask as ApiVariantAnalysisRepoTask, + VariantAnalysisSubmissionRequest, +} from "./gh-api/variant-analysis"; +import { requestVariantAnalysisServer } from "./gh-api/variant-analysis-server-client"; +import type { + VariantAnalysis, + VariantAnalysisSubmission, +} from "./shared/variant-analysis"; + +/** + * Variant analysis API calls. These go to GitHub, unless an internal custom + * variant analysis server is configured (see config.ts), in which case they + * go to that server and the GitHub credentials are not used. + */ + +function variantAnalysesPath(controllerRepoId: number): string { + return `/repositories/${controllerRepoId}/code-scanning/codeql/variant-analyses`; +} + +export async function submitVariantAnalysis( + credentials: Credentials, + submission: VariantAnalysisSubmission, +): Promise { + const server = getVariantAnalysisServerConfig(); + if (!server) { + return submitGitHubVariantAnalysis(credentials, submission); + } + + const { actionRepoRef, language, pack, databases, controllerRepoId } = + submission; + const data: VariantAnalysisSubmissionRequest = { + action_repo_ref: actionRepoRef, + language, + query_pack: pack, + repositories: databases.repositories, + repository_lists: databases.repositoryLists, + repository_owners: databases.repositoryOwners, + }; + return requestVariantAnalysisServer( + server, + "POST", + variantAnalysesPath(controllerRepoId), + data, + ); +} + +export async function getVariantAnalysis( + credentials: Credentials, + controllerRepoId: number, + variantAnalysisId: number, +): Promise { + const server = getVariantAnalysisServerConfig(); + if (!server) { + return getGitHubVariantAnalysis( + credentials, + controllerRepoId, + variantAnalysisId, + ); + } + + return requestVariantAnalysisServer( + server, + "GET", + `${variantAnalysesPath(controllerRepoId)}/${variantAnalysisId}`, + ); +} + +export async function getVariantAnalysisRepo( + credentials: Credentials, + controllerRepoId: number, + variantAnalysisId: number, + repoId: number, +): Promise { + const server = getVariantAnalysisServerConfig(); + if (!server) { + return getGitHubVariantAnalysisRepo( + credentials, + controllerRepoId, + variantAnalysisId, + repoId, + ); + } + + return requestVariantAnalysisServer( + server, + "GET", + `${variantAnalysesPath(controllerRepoId)}/${variantAnalysisId}/repositories/${repoId}`, + ); +} + +export async function cancelVariantAnalysis( + credentials: Credentials, + variantAnalysis: VariantAnalysis, +): Promise { + const server = getVariantAnalysisServerConfig(); + if (!server) { + return cancelGitHubVariantAnalysis(credentials, variantAnalysis); + } + + const { + actionsWorkflowRunId, + controllerRepo: { fullName }, + } = variantAnalysis; + await requestVariantAnalysisServer( + server, + "POST", + `/repos/${fullName}/actions/runs/${actionsWorkflowRunId}/cancel`, + ); +} + +export async function createGist( + credentials: Credentials, + description: string, + files: { [key: string]: { content: string } }, +): Promise { + // Results from a custom server must not be uploaded to GitHub. + if (getVariantAnalysisServerConfig()) { + throw new Error( + "Exporting to a gist is not supported when using a custom variant analysis server.", + ); + } + return createGitHubGist(credentials, description, files); +} diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts index ad5dee850bf..68961dd4dbb 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts @@ -1,9 +1,10 @@ import { join } from "path"; import { + cancelVariantAnalysis, submitVariantAnalysis, getVariantAnalysisRepo, -} from "./gh-api/gh-api-client"; +} from "./variant-analysis-api"; import type { VariantAnalysis as ApiVariantAnalysis } from "./gh-api/variant-analysis"; import type { AuthenticationSessionsChangeEvent, @@ -52,7 +53,6 @@ import PQueue from "p-queue"; import { createTimestampFile, saveBeforeStart } from "../run-queries-shared"; import { readFile, remove, pathExists } from "fs-extra"; import { EOL } from "os"; -import { cancelVariantAnalysis } from "./gh-api/gh-actions-api-client"; import type { ProgressCallback } from "../common/vscode/progress"; import { UserCancellationException, @@ -98,7 +98,7 @@ import { resolveCodeScanningQueryPack } from "./code-scanning-pack"; import { isSarifResultsQueryKind } from "../common/query-metadata"; import { isVariantAnalysisEnabledForGitHubHost } from "./ghec-dr"; import type { VariantAnalysisConfig } from "../config"; -import { getEnterpriseUri } from "../config"; +import { getEnterpriseUri, isVariantAnalysisServerConfigured } from "../config"; const maxRetryCount = 3; @@ -959,6 +959,12 @@ export class VariantAnalysisManager throw new Error(`No variant analysis with id: ${variantAnalysisId}`); } + if (isVariantAnalysisServerConfigured()) { + throw new Error( + "Viewing logs is not supported when using a custom variant analysis server.", + ); + } + const actionsWorkflowRunUrl = getActionsWorkflowRunUrl( variantAnalysis, this.config.githubUrl, diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts index 8592f84353d..ed25228c431 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts @@ -1,5 +1,5 @@ import { env, EventEmitter } from "vscode"; -import { getVariantAnalysis } from "./gh-api/gh-api-client"; +import { getVariantAnalysis } from "./variant-analysis-api"; import { RequestError } from "@octokit/request-error"; import type { diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-results-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-results-manager.ts index 01e27410ff0..b3edf3bb4f7 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-results-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-results-manager.ts @@ -200,13 +200,11 @@ export class VariantAnalysisResultsManager extends DisposableObject { const repoTask: VariantAnalysisRepositoryTask = await readRepoTask(storageDirectory); - if (!repoTask.databaseCommitSha || !repoTask.sourceLocationPrefix) { - throw new Error("Missing database commit SHA"); - } - + // Some servers do not report these, so fall back to linking to the + // default branch and to the legacy source location handling. const fileLinkPrefix = this.createGitHubFileLinkPrefix( repoTask.repository.fullName, - repoTask.databaseCommitSha, + repoTask.databaseCommitSha || "HEAD", ); const resultsDirectory = join( @@ -233,7 +231,7 @@ export class VariantAnalysisResultsManager extends DisposableObject { rawResults = await this.readBqrsResults( bqrsPath, fileLinkPrefix, - repoTask.sourceLocationPrefix, + repoTask.sourceLocationPrefix ?? "", ); } diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts new file mode 100644 index 00000000000..ca7ad016689 --- /dev/null +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts @@ -0,0 +1,222 @@ +import { ConfigurationTarget, extensions, workspace } from "vscode"; +import type { Extension } from "vscode"; +import { RequestError } from "@octokit/request-error"; +import { getVariantAnalysisServerConfig } from "../../../../src/config"; +import { + cancelVariantAnalysis, + createGist, + getVariantAnalysis, + getVariantAnalysisRepo, + submitVariantAnalysis, +} from "../../../../src/variant-analysis/variant-analysis-api"; +import { isVariantAnalysisEnabledForGitHubHost } from "../../../../src/variant-analysis/ghec-dr"; +import { createMockSubmission } from "../../../factories/variant-analysis/shared/variant-analysis-submission"; +import { createMockVariantAnalysis } from "../../../factories/variant-analysis/shared/variant-analysis"; +import { mockedObject } from "../../utils/mocking.helpers"; +import { testCredentialsWithStub } from "../../../factories/authentication"; +import type { VariantAnalysisServerTokenProvider } from "../../../../src/variant-analysis/gh-api/variant-analysis-server-client"; + +const SERVER_URL = "https://mrva.example.com"; +const PROVIDER_ID = "example.token-provider"; + +async function setSetting( + name: string, + value: string, + target = ConfigurationTarget.Global, +) { + await workspace + .getConfiguration("codeQL.variantAnalysis") + .update(name, value, target); +} + +describe("variant analysis custom server", () => { + let fetchSpy: jest.SpiedFunction; + let getVariantAnalysisServerToken: jest.Mock< + Promise, + [string] + >; + let credentials: ReturnType; + + beforeEach(() => { + credentials = testCredentialsWithStub(); + fetchSpy = jest + .spyOn(global, "fetch") + .mockImplementation( + async () => new Response(JSON.stringify({ id: 123 })), + ); + getVariantAnalysisServerToken = jest.fn().mockResolvedValue("the-token"); + jest.spyOn(extensions, "getExtension").mockImplementation((id: string) => + id === PROVIDER_ID + ? mockedObject>({ + isActive: true, + exports: { getVariantAnalysisServerToken }, + }) + : undefined, + ); + }); + + describe("configuration", () => { + it("is not configured by default", () => { + expect(getVariantAnalysisServerConfig()).toBeUndefined(); + }); + + it("reads the server URL and token provider from user settings", async () => { + await setSetting("serverUrl", `${SERVER_URL}/`); + await setSetting("serverTokenProvider", PROVIDER_ID); + + expect(getVariantAnalysisServerConfig()).toEqual({ + url: SERVER_URL, + tokenProviderExtensionId: PROVIDER_ID, + }); + expect(isVariantAnalysisEnabledForGitHubHost()).toBe(true); + }); + + it("ignores workspace settings", async () => { + await setSetting("serverUrl", SERVER_URL, ConfigurationTarget.Workspace); + await setSetting( + "serverUrl", + SERVER_URL, + ConfigurationTarget.WorkspaceFolder, + ); + + expect(getVariantAnalysisServerConfig()).toBeUndefined(); + }); + + it("ignores a workspace token provider", async () => { + await setSetting("serverUrl", SERVER_URL); + await setSetting( + "serverTokenProvider", + PROVIDER_ID, + ConfigurationTarget.Workspace, + ); + + expect(getVariantAnalysisServerConfig()?.tokenProviderExtensionId).toBe( + undefined, + ); + }); + + it.each(["http://mrva.example.com", "https://user:pw@mrva.example.com"])( + "rejects %s", + async (url) => { + await setSetting("serverUrl", url); + expect(() => getVariantAnalysisServerConfig()).toThrow(); + }, + ); + }); + + describe("when not configured", () => { + it("does not call the server", async () => { + await expect( + submitVariantAnalysis(credentials, createMockSubmission()), + ).rejects.toThrow("no stub was provided"); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + }); + + describe("when configured", () => { + beforeEach(async () => { + await setSetting("serverUrl", SERVER_URL); + await setSetting("serverTokenProvider", PROVIDER_ID); + }); + + it("submits to the server with the provided token", async () => { + const submission = createMockSubmission(); + submission.controllerRepoId = 0; + + const result = await submitVariantAnalysis(credentials, submission); + + expect(result).toEqual({ id: 123 }); + expect(getVariantAnalysisServerToken).toHaveBeenCalledWith(SERVER_URL); + expect(fetchSpy).toHaveBeenCalledTimes(1); + const [url, init] = fetchSpy.mock.calls[0]; + expect(url).toBe( + `${SERVER_URL}/repositories/0/code-scanning/codeql/variant-analyses`, + ); + expect(init?.method).toBe("POST"); + expect(init?.headers).toMatchObject({ + Authorization: "Bearer the-token", + "Content-Type": "application/json", + }); + expect(JSON.parse(init?.body as string)).toMatchObject({ + action_repo_ref: submission.actionRepoRef, + language: submission.language, + query_pack: submission.pack, + }); + }); + + it("gets a variant analysis and a repo task", async () => { + await getVariantAnalysis(credentials, 0, 5); + await getVariantAnalysisRepo(credentials, 0, 5, 7); + + expect(fetchSpy.mock.calls.map(([url]) => url)).toEqual([ + `${SERVER_URL}/repositories/0/code-scanning/codeql/variant-analyses/5`, + `${SERVER_URL}/repositories/0/code-scanning/codeql/variant-analyses/5/repositories/7`, + ]); + }); + + it("cancels on the server", async () => { + const variantAnalysis = createMockVariantAnalysis({}); + + await cancelVariantAnalysis(credentials, variantAnalysis); + + expect(fetchSpy).toHaveBeenCalledWith( + `${SERVER_URL}/repos/${variantAnalysis.controllerRepo.fullName}/actions/runs/${variantAnalysis.actionsWorkflowRunId}/cancel`, + expect.objectContaining({ method: "POST" }), + ); + }); + + it("does not export to a gist", async () => { + await expect(createGist(credentials, "", {})).rejects.toThrow( + /not supported/, + ); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("fails without a token, without sending a request", async () => { + getVariantAnalysisServerToken.mockResolvedValue(undefined); + + await expect(getVariantAnalysis(credentials, 0, 5)).rejects.toThrow( + /did not return a variant analysis server token/, + ); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("fails if no token provider is configured", async () => { + await setSetting("serverTokenProvider", ""); + + await expect(getVariantAnalysis(credentials, 0, 5)).rejects.toThrow( + /No token provider/, + ); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("fails if the token provider is not installed", async () => { + await setSetting("serverTokenProvider", "other.extension"); + + await expect(getVariantAnalysis(credentials, 0, 5)).rejects.toThrow( + /not installed/, + ); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("reports server errors with the status and message", async () => { + fetchSpy.mockResolvedValue( + new Response(JSON.stringify({ message: "Token expired" }), { + status: 401, + statusText: "Unauthorized", + }), + ); + + const error = await getVariantAnalysis(credentials, 0, 5).catch( + (e: unknown) => e, + ); + + expect(error).toBeInstanceOf(RequestError); + expect((error as RequestError).status).toBe(401); + expect((error as RequestError).message).toContain( + "401 Unauthorized: Token expired", + ); + expect((error as RequestError).message).not.toContain("the-token"); + }); + }); +}); diff --git a/extensions/ql-vscode/test/vscode-tests/test-config.ts b/extensions/ql-vscode/test/vscode-tests/test-config.ts index 04b2517c429..3d1f685af15 100644 --- a/extensions/ql-vscode/test/vscode-tests/test-config.ts +++ b/extensions/ql-vscode/test/vscode-tests/test-config.ts @@ -227,8 +227,18 @@ export const beforeEachAction = async () => { has(key: string) { return configuration.has(section, key); }, - inspect(_key: string) { - throw new Error("inspect is not supported in tests"); + inspect(key: string) { + const valueFor = (target: ConfigurationTarget) => + configuration.getConfiguration(target).get(section, key); + return { + key: section ? `${section}.${key}` : key, + defaultValue: defaultConfiguration.get(section, key), + globalValue: valueFor(ConfigurationTarget.Global), + workspaceValue: valueFor(ConfigurationTarget.Workspace), + workspaceFolderValue: valueFor( + ConfigurationTarget.WorkspaceFolder, + ), + }; }, async update( key: string, From c618efff2a53fad29775fac2042a96fac13c2aff Mon Sep 17 00:00:00 2001 From: Nicolas Will Date: Wed, 28 Oct 2026 19:22:29 +0100 Subject: [PATCH 2/5] Fix custom variant analysis routing and response handling Persist the originating custom server with each run and reject requests when the configured endpoint changes. Reject redirects and endpoint query strings or fragments, and derive export and log availability from the saved run. Share submission mapping across transports, use the existing settings abstraction for global-only reads, and reject malformed successful JSON responses while allowing empty cancellation responses. Consolidate VS Code-dependent regression cases into the no-workspace suite. --- docs/variant-analysis-custom-server.md | 10 ++ extensions/ql-vscode/src/config.ts | 23 +++- .../src/variant-analysis/export-results.ts | 20 +++- .../variant-analysis/gh-api/gh-api-client.ts | 15 +-- .../gh-api/variant-analysis-server-client.ts | 17 ++- .../shared/variant-analysis.ts | 3 + .../variant-analysis/variant-analysis-api.ts | 41 ++++--- .../variant-analysis-manager.ts | 7 +- .../variant-analysis-mapper.ts | 17 +++ .../variant-analysis-monitor.ts | 1 + .../variant-analysis-mapper.test.ts | 21 ++++ .../variant-analysis-api.test.ts | 104 ++++++++++++++---- 12 files changed, 213 insertions(+), 66 deletions(-) diff --git a/docs/variant-analysis-custom-server.md b/docs/variant-analysis-custom-server.md index fb3213c2a2b..41fc39d8ff7 100644 --- a/docs/variant-analysis-custom-server.md +++ b/docs/variant-analysis-custom-server.md @@ -21,6 +21,8 @@ so a repository cannot redirect queries or tokens. | `codeQL.variantAnalysis.serverTokenProvider` | ID (`publisher.name`) of the installed extension that provides the token. | If `serverUrl` is set but invalid, variant analysis fails instead of falling back to GitHub. +Query strings and fragments are rejected; base path prefixes are supported. +The hidden server setting is the opt-in switch; there is no additional canary gate. ## Token provider extension @@ -45,14 +47,22 @@ themselves as providers. - MRVA is enabled regardless of `github-enterprise.uri`. The GitHub sign-in, GitHub database downloads and other extensions that read `github-enterprise.uri` are not affected. - No controller repository is needed. Requests use controller repository ID `0`. +- Runs retain their originating server URL. Polling, task retrieval and cancellation fail if + the current server setting differs. Restore the original setting to resume them. Legacy runs + without a server URL are treated as GitHub runs. - The GitHub token is never sent to the server, and results are never uploaded to GitHub: "Export to gist" and "View logs" (GitHub Actions) are unavailable. - Result artifacts are downloaded from `artifact_url` without authentication, as on GitHub. - If a repository task has no `database_commit_sha`, file links point to `HEAD`. + These links may not match the analyzed revision. +- The canary-only autofix action still downloads repository source through GitHub sign-in. + It requires the analyzed repository and revision to be available on the configured GitHub host. + Custom source retrieval is not supported. ## Server contract All paths are relative to `serverUrl`. Requests send `Authorization: Bearer ` and JSON bodies. +Redirects are rejected rather than forwarding query packs to another endpoint. Request and response shapes match `src/variant-analysis/gh-api/variant-analysis.ts`. | Operation | Request | diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index 0bcec47c18a..12eb4969008 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -54,6 +54,15 @@ export class Setting { .get(this.name)!; } + getGlobalValue(): T | undefined { + if (this.parent === undefined) { + throw new Error("Cannot get the value of a root setting."); + } + return workspace + .getConfiguration(this.parent.qualifiedName) + .inspect(this.name)?.globalValue; + } + updateValue(value: T, target: ConfigurationTarget): Thenable { if (this.parent === undefined) { throw new Error("Cannot update the value of a root setting."); @@ -635,9 +644,7 @@ export interface VariantAnalysisServerConfig { } function getGlobalStringSetting(setting: Setting): string | undefined { - const value = workspace - .getConfiguration(setting.parent!.qualifiedName) - .inspect(setting.name)?.globalValue; + const value = setting.getGlobalValue(); return typeof value === "string" && value.trim() ? value.trim() : undefined; } @@ -661,9 +668,15 @@ export function getVariantAnalysisServerConfig(): `Invalid ${VARIANT_ANALYSIS_SERVER_URL.qualifiedName} setting: ${rawUrl}`, ); } - if (url.protocol !== "https:" || url.username || url.password) { + if ( + url.protocol !== "https:" || + url.username || + url.password || + url.search || + url.hash + ) { throw new Error( - `${VARIANT_ANALYSIS_SERVER_URL.qualifiedName} must be an https URL without credentials.`, + `${VARIANT_ANALYSIS_SERVER_URL.qualifiedName} must be an https URL without credentials, a query string, or a fragment.`, ); } diff --git a/extensions/ql-vscode/src/variant-analysis/export-results.ts b/extensions/ql-vscode/src/variant-analysis/export-results.ts index bfecd36cca4..01ffd04e421 100644 --- a/extensions/ql-vscode/src/variant-analysis/export-results.ts +++ b/extensions/ql-vscode/src/variant-analysis/export-results.ts @@ -11,7 +11,6 @@ import { import { showInformationMessageWithAction } from "../common/vscode/dialog"; import { extLogger } from "../common/logging/vscode"; import { createGist } from "./variant-analysis-api"; -import { isVariantAnalysisServerConfigured } from "../config"; import type { MarkdownFile, RepositorySummary } from "./markdown-generation"; import { generateVariantAnalysisMarkdown } from "./markdown-generation"; import { pluralize } from "../common/word"; @@ -70,7 +69,7 @@ export async function exportVariantAnalysisResults( message: "Determining export format", }); - const exportFormat = await determineExportFormat(); + const exportFormat = await determineExportFormat(variantAnalysis); if (!exportFormat) { return; } @@ -195,13 +194,16 @@ async function exportVariantAnalysisAnalysisResults( credentials, progress, token, + variantAnalysis.serverUrl, ); } /** * Determines the format in which to export the results, from the given export options. */ -async function determineExportFormat(): Promise<"gist" | "local" | undefined> { +async function determineExportFormat( + variantAnalysis: VariantAnalysis, +): Promise<"gist" | "local" | undefined> { const gistOption = { label: "$(ports-open-browser-icon) Create Gist (GitHub)", }; @@ -210,7 +212,7 @@ async function determineExportFormat(): Promise<"gist" | "local" | undefined> { }; // Results must not be sent to GitHub when using a custom server. - const options = isVariantAnalysisServerConfigured() + const options = variantAnalysis.serverUrl ? [localMarkdownOption] : [gistOption, localMarkdownOption]; @@ -242,6 +244,7 @@ async function exportResults( credentials: Credentials, progress?: ProgressCallback, token?: CancellationToken, + serverUrl?: string, ) { if (token?.isCancellationRequested) { throw new UserCancellationException("Cancelled"); @@ -255,6 +258,7 @@ async function exportResults( credentials, progress, token, + serverUrl, ); } else if (exportFormat === "local") { await exportToLocalMarkdown( @@ -274,6 +278,7 @@ async function exportToGist( credentials: Credentials, progress?: ProgressCallback, token?: CancellationToken, + serverUrl?: string, ) { progress?.({ maxStep: MAX_VARIANT_ANALYSIS_EXPORT_PROGRESS_STEPS, @@ -294,7 +299,12 @@ async function exportToGist( {} as { [key: string]: { content: string } }, ); - const gistUrl = await createGist(credentials, description, gistFiles); + const gistUrl = await createGist( + credentials, + description, + gistFiles, + serverUrl, + ); if (gistUrl) { // This needs to use .then to ensure we aren't keeping the progress notification open. We shouldn't await the // "Open gist" button click. diff --git a/extensions/ql-vscode/src/variant-analysis/gh-api/gh-api-client.ts b/extensions/ql-vscode/src/variant-analysis/gh-api/gh-api-client.ts index be8d323c7c4..ea4db34ceb6 100644 --- a/extensions/ql-vscode/src/variant-analysis/gh-api/gh-api-client.ts +++ b/extensions/ql-vscode/src/variant-analysis/gh-api/gh-api-client.ts @@ -3,9 +3,9 @@ import type { VariantAnalysisSubmission } from "../shared/variant-analysis"; import type { VariantAnalysis, VariantAnalysisRepoTask, - VariantAnalysisSubmissionRequest, } from "./variant-analysis"; import type { Repository } from "./repository"; +import { mapVariantAnalysisSubmission } from "../variant-analysis-mapper"; export async function submitVariantAnalysis( credentials: Credentials, @@ -13,17 +13,8 @@ export async function submitVariantAnalysis( ): Promise { const octokit = await credentials.getOctokit(); - const { actionRepoRef, language, pack, databases, controllerRepoId } = - submissionDetails; - - const data: VariantAnalysisSubmissionRequest = { - action_repo_ref: actionRepoRef, - language, - query_pack: pack, - repositories: databases.repositories, - repository_lists: databases.repositoryLists, - repository_owners: databases.repositoryOwners, - }; + const { controllerRepoId } = submissionDetails; + const data = mapVariantAnalysisSubmission(submissionDetails); const response = await octokit.request({ method: "POST", diff --git a/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts b/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts index a8866b4a939..d228970986f 100644 --- a/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts +++ b/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts @@ -6,9 +6,7 @@ import type { VariantAnalysisServerConfig } from "../../config"; * INTERNAL AND UNSUPPORTED: client for a self-hosted variant analysis server * that implements the GitHub variant analysis REST endpoints. See * docs/variant-analysis-custom-server.md. - */ - -/** + * * The export that the configured token provider extension must implement. */ export interface VariantAnalysisServerTokenProvider { @@ -65,11 +63,13 @@ export async function requestVariantAnalysisServer( method: "GET" | "POST", path: string, body?: unknown, + allowEmptyResponse = false, ): Promise { const token = await getToken(server); const url = `${server.url}${path}`; const response = await fetch(url, { + redirect: "error", method, headers: { Accept: "application/json", @@ -84,7 +84,12 @@ export async function requestVariantAnalysisServer( if (text) { try { data = JSON.parse(text); - } catch { + } catch (cause) { + if (response.ok) { + throw new Error("Variant analysis server returned invalid JSON.", { + cause, + }); + } data = text; } } @@ -106,6 +111,10 @@ export async function requestVariantAnalysisServer( ); } + if (!text && !allowEmptyResponse) { + throw new Error("Variant analysis server returned an empty response."); + } + return data as T; } diff --git a/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts b/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts index a83a5f2bbf6..5632591bc62 100644 --- a/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts +++ b/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts @@ -5,6 +5,8 @@ import type { ModelPackDetails } from "../../common/model-pack-details"; export interface VariantAnalysis { id: number; + /** Custom server that created this run; absent for GitHub and legacy runs. */ + serverUrl?: string; controllerRepo: Repository; language: QueryLanguage; query: { @@ -136,6 +138,7 @@ export interface VariantAnalysisScannedRepositoryResult { */ export interface VariantAnalysisSubmission { startTime: number; + serverUrl?: string; controllerRepoId: number; actionRepoRef: string; language: QueryLanguage; diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts index b1f20ed024b..799a83738c0 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts @@ -10,9 +10,9 @@ import { import type { VariantAnalysis as ApiVariantAnalysis, VariantAnalysisRepoTask as ApiVariantAnalysisRepoTask, - VariantAnalysisSubmissionRequest, } from "./gh-api/variant-analysis"; import { requestVariantAnalysisServer } from "./gh-api/variant-analysis-server-client"; +import { mapVariantAnalysisSubmission } from "./variant-analysis-mapper"; import type { VariantAnalysis, VariantAnalysisSubmission, @@ -28,30 +28,30 @@ function variantAnalysesPath(controllerRepoId: number): string { return `/repositories/${controllerRepoId}/code-scanning/codeql/variant-analyses`; } +export function getServerForVariantAnalysis(serverUrl: string | undefined) { + const server = getVariantAnalysisServerConfig(); + if (server?.url !== serverUrl) { + throw new Error( + "This variant analysis belongs to a different server. Restore the server setting used to submit it before retrying.", + ); + } + return server; +} + export async function submitVariantAnalysis( credentials: Credentials, submission: VariantAnalysisSubmission, ): Promise { - const server = getVariantAnalysisServerConfig(); + const server = getServerForVariantAnalysis(submission.serverUrl); if (!server) { return submitGitHubVariantAnalysis(credentials, submission); } - const { actionRepoRef, language, pack, databases, controllerRepoId } = - submission; - const data: VariantAnalysisSubmissionRequest = { - action_repo_ref: actionRepoRef, - language, - query_pack: pack, - repositories: databases.repositories, - repository_lists: databases.repositoryLists, - repository_owners: databases.repositoryOwners, - }; return requestVariantAnalysisServer( server, "POST", - variantAnalysesPath(controllerRepoId), - data, + variantAnalysesPath(submission.controllerRepoId), + mapVariantAnalysisSubmission(submission), ); } @@ -59,8 +59,9 @@ export async function getVariantAnalysis( credentials: Credentials, controllerRepoId: number, variantAnalysisId: number, + serverUrl?: string, ): Promise { - const server = getVariantAnalysisServerConfig(); + const server = getServerForVariantAnalysis(serverUrl); if (!server) { return getGitHubVariantAnalysis( credentials, @@ -81,8 +82,9 @@ export async function getVariantAnalysisRepo( controllerRepoId: number, variantAnalysisId: number, repoId: number, + serverUrl?: string, ): Promise { - const server = getVariantAnalysisServerConfig(); + const server = getServerForVariantAnalysis(serverUrl); if (!server) { return getGitHubVariantAnalysisRepo( credentials, @@ -103,7 +105,7 @@ export async function cancelVariantAnalysis( credentials: Credentials, variantAnalysis: VariantAnalysis, ): Promise { - const server = getVariantAnalysisServerConfig(); + const server = getServerForVariantAnalysis(variantAnalysis.serverUrl); if (!server) { return cancelGitHubVariantAnalysis(credentials, variantAnalysis); } @@ -116,6 +118,8 @@ export async function cancelVariantAnalysis( server, "POST", `/repos/${fullName}/actions/runs/${actionsWorkflowRunId}/cancel`, + undefined, + true, ); } @@ -123,9 +127,10 @@ export async function createGist( credentials: Credentials, description: string, files: { [key: string]: { content: string } }, + serverUrl?: string, ): Promise { // Results from a custom server must not be uploaded to GitHub. - if (getVariantAnalysisServerConfig()) { + if (serverUrl) { throw new Error( "Exporting to a gist is not supported when using a custom variant analysis server.", ); diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts index 68961dd4dbb..df045d762b6 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts @@ -98,7 +98,7 @@ import { resolveCodeScanningQueryPack } from "./code-scanning-pack"; import { isSarifResultsQueryKind } from "../common/query-metadata"; import { isVariantAnalysisEnabledForGitHubHost } from "./ghec-dr"; import type { VariantAnalysisConfig } from "../config"; -import { getEnterpriseUri, isVariantAnalysisServerConfigured } from "../config"; +import { getEnterpriseUri, getVariantAnalysisServerConfig } from "../config"; const maxRetryCount = 3; @@ -337,6 +337,7 @@ export class VariantAnalysisManager } await saveBeforeStart(); + const serverUrl = getVariantAnalysisServerConfig()?.url; progress({ maxStep: 5, @@ -400,6 +401,7 @@ export class VariantAnalysisManager }; const variantAnalysisSubmission: VariantAnalysisSubmission = { + serverUrl, startTime: queryStartTime, actionRepoRef: actionBranch, controllerRepoId: controllerRepo.id, @@ -810,6 +812,7 @@ export class VariantAnalysisManager variantAnalysis.controllerRepo.id, variantAnalysis.id, scannedRepo.repository.id, + variantAnalysis.serverUrl, ); repoTask = mapVariantAnalysisRepositoryTask(repoTaskResponse); @@ -959,7 +962,7 @@ export class VariantAnalysisManager throw new Error(`No variant analysis with id: ${variantAnalysisId}`); } - if (isVariantAnalysisServerConfigured()) { + if (variantAnalysis.serverUrl) { throw new Error( "Viewing logs is not supported when using a custom variant analysis server.", ); diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts index 2b8858035a2..f5d4fedc1fd 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts @@ -9,6 +9,7 @@ import type { VariantAnalysisSkippedRepositoryGroup as ApiVariantAnalysisSkippedRepositoryGroup, VariantAnalysisNotFoundRepositoryGroup as ApiVariantAnalysisNotFoundRepositoryGroup, VariantAnalysisRepoTask as ApiVariantAnalysisRepoTask, + VariantAnalysisSubmissionRequest, } from "./gh-api/variant-analysis"; import type { VariantAnalysis, @@ -24,6 +25,19 @@ import { VariantAnalysisRepoStatus, } from "./shared/variant-analysis"; +export function mapVariantAnalysisSubmission( + submission: VariantAnalysisSubmission, +): VariantAnalysisSubmissionRequest { + return { + action_repo_ref: submission.actionRepoRef, + language: submission.language, + query_pack: submission.pack, + repositories: submission.databases.repositories, + repository_lists: submission.databases.repositoryLists, + repository_owners: submission.databases.repositoryOwners, + }; +} + export function mapVariantAnalysisFromSubmission( submission: VariantAnalysisSubmission, apiVariantAnalysis: ApiVariantAnalysis, @@ -31,6 +45,7 @@ export function mapVariantAnalysisFromSubmission( ): VariantAnalysis { return mapVariantAnalysis( { + serverUrl: submission.serverUrl, language: submission.language, query: { name: submission.query.name, @@ -63,6 +78,7 @@ function mapVariantAnalysis( currentVariantAnalysis: Pick< VariantAnalysis, | "language" + | "serverUrl" | "query" | "queries" | "databases" @@ -92,6 +108,7 @@ function mapVariantAnalysis( const variantAnalysis: VariantAnalysis = { id: response.id, + serverUrl: currentVariantAnalysis.serverUrl, controllerRepo: { id: response.controller_repo.id, fullName: response.controller_repo.full_name, diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts index ed25228c431..739ff6cb42c 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-monitor.ts @@ -101,6 +101,7 @@ export class VariantAnalysisMonitor extends DisposableObject { this.app.credentials, controllerRepoId, variantAnalysisId, + this.getVariantAnalysis(variantAnalysisId).serverUrl, ); } catch (e) { const errorMessage = getErrorMessage(e); diff --git a/extensions/ql-vscode/test/unit-tests/variant-analysis/variant-analysis-mapper.test.ts b/extensions/ql-vscode/test/unit-tests/variant-analysis/variant-analysis-mapper.test.ts index c033d28d8bd..44fce971ad7 100644 --- a/extensions/ql-vscode/test/unit-tests/variant-analysis/variant-analysis-mapper.test.ts +++ b/extensions/ql-vscode/test/unit-tests/variant-analysis/variant-analysis-mapper.test.ts @@ -4,6 +4,7 @@ import type { VariantAnalysisScannedRepository } from "../../../src/variant-anal import { VariantAnalysisRepoStatus } from "../../../src/variant-analysis/shared/variant-analysis"; import { mapScannedRepository, + mapUpdatedVariantAnalysis, mapVariantAnalysisFromSubmission, mapVariantAnalysisRepositoryTask, } from "../../../src/variant-analysis/variant-analysis-mapper"; @@ -27,6 +28,26 @@ describe(mapVariantAnalysisFromSubmission.name, () => { ); const mockSubmission = createMockSubmission(); + it("preserves the server origin through submission and polling updates", () => { + const submission = { + ...mockSubmission, + serverUrl: "https://mrva.example.com", + }; + const run = mapVariantAnalysisFromSubmission( + submission, + mockApiResponse, + [], + ); + expect(run.serverUrl).toBe(submission.serverUrl); + expect(mapUpdatedVariantAnalysis(run, mockApiResponse).serverUrl).toBe( + submission.serverUrl, + ); + expect( + mapVariantAnalysisFromSubmission(mockSubmission, mockApiResponse, []) + .serverUrl, + ).toBeUndefined(); + }); + it("should map an API response and return a variant analysis", () => { const result = mapVariantAnalysisFromSubmission( mockSubmission, diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts index ca7ad016689..9683980ccf5 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts @@ -1,12 +1,12 @@ import { ConfigurationTarget, extensions, workspace } from "vscode"; import type { Extension } from "vscode"; -import { RequestError } from "@octokit/request-error"; import { getVariantAnalysisServerConfig } from "../../../../src/config"; import { cancelVariantAnalysis, createGist, getVariantAnalysis, getVariantAnalysisRepo, + getServerForVariantAnalysis, submitVariantAnalysis, } from "../../../../src/variant-analysis/variant-analysis-api"; import { isVariantAnalysisEnabledForGitHubHost } from "../../../../src/variant-analysis/ghec-dr"; @@ -56,6 +56,18 @@ describe("variant analysis custom server", () => { }); describe("configuration", () => { + it.each([`${SERVER_URL}/?environment=test`, `${SERVER_URL}/#fragment`])( + "rejects endpoint modifiers: %s", + async (url) => { + await setSetting("serverUrl", url); + expect(() => getVariantAnalysisServerConfig()).toThrow(); + }, + ); + + it("preserves a base path", async () => { + await setSetting("serverUrl", `${SERVER_URL}/api/`); + expect(getVariantAnalysisServerConfig()?.url).toBe(`${SERVER_URL}/api`); + }); it("is not configured by default", () => { expect(getVariantAnalysisServerConfig()).toBeUndefined(); }); @@ -122,6 +134,7 @@ describe("variant analysis custom server", () => { it("submits to the server with the provided token", async () => { const submission = createMockSubmission(); submission.controllerRepoId = 0; + submission.serverUrl = SERVER_URL; const result = await submitVariantAnalysis(credentials, submission); @@ -133,6 +146,7 @@ describe("variant analysis custom server", () => { `${SERVER_URL}/repositories/0/code-scanning/codeql/variant-analyses`, ); expect(init?.method).toBe("POST"); + expect(init?.redirect).toBe("error"); expect(init?.headers).toMatchObject({ Authorization: "Bearer the-token", "Content-Type": "application/json", @@ -145,8 +159,8 @@ describe("variant analysis custom server", () => { }); it("gets a variant analysis and a repo task", async () => { - await getVariantAnalysis(credentials, 0, 5); - await getVariantAnalysisRepo(credentials, 0, 5, 7); + await getVariantAnalysis(credentials, 0, 5, SERVER_URL); + await getVariantAnalysisRepo(credentials, 0, 5, 7, SERVER_URL); expect(fetchSpy.mock.calls.map(([url]) => url)).toEqual([ `${SERVER_URL}/repositories/0/code-scanning/codeql/variant-analyses/5`, @@ -155,7 +169,9 @@ describe("variant analysis custom server", () => { }); it("cancels on the server", async () => { + fetchSpy.mockResolvedValue(new Response(null, { status: 202 })); const variantAnalysis = createMockVariantAnalysis({}); + variantAnalysis.serverUrl = SERVER_URL; await cancelVariantAnalysis(credentials, variantAnalysis); @@ -165,8 +181,18 @@ describe("variant analysis custom server", () => { ); }); + it.each(["not JSON", ""])( + "rejects a malformed successful response: %s", + async (body) => { + fetchSpy.mockResolvedValue(new Response(body)); + await expect( + getVariantAnalysis(credentials, 0, 5, SERVER_URL), + ).rejects.toThrow(/invalid JSON|empty response/); + }, + ); + it("does not export to a gist", async () => { - await expect(createGist(credentials, "", {})).rejects.toThrow( + await expect(createGist(credentials, "", {}, SERVER_URL)).rejects.toThrow( /not supported/, ); expect(fetchSpy).not.toHaveBeenCalled(); @@ -175,27 +201,27 @@ describe("variant analysis custom server", () => { it("fails without a token, without sending a request", async () => { getVariantAnalysisServerToken.mockResolvedValue(undefined); - await expect(getVariantAnalysis(credentials, 0, 5)).rejects.toThrow( - /did not return a variant analysis server token/, - ); + await expect( + getVariantAnalysis(credentials, 0, 5, SERVER_URL), + ).rejects.toThrow(/did not return a variant analysis server token/); expect(fetchSpy).not.toHaveBeenCalled(); }); it("fails if no token provider is configured", async () => { await setSetting("serverTokenProvider", ""); - await expect(getVariantAnalysis(credentials, 0, 5)).rejects.toThrow( - /No token provider/, - ); + await expect( + getVariantAnalysis(credentials, 0, 5, SERVER_URL), + ).rejects.toThrow(/No token provider/); expect(fetchSpy).not.toHaveBeenCalled(); }); it("fails if the token provider is not installed", async () => { await setSetting("serverTokenProvider", "other.extension"); - await expect(getVariantAnalysis(credentials, 0, 5)).rejects.toThrow( - /not installed/, - ); + await expect( + getVariantAnalysis(credentials, 0, 5, SERVER_URL), + ).rejects.toThrow(/not installed/); expect(fetchSpy).not.toHaveBeenCalled(); }); @@ -207,16 +233,54 @@ describe("variant analysis custom server", () => { }), ); - const error = await getVariantAnalysis(credentials, 0, 5).catch( - (e: unknown) => e, + await expect( + getVariantAnalysis(credentials, 0, 5, SERVER_URL), + ).rejects.toMatchObject({ + status: 401, + message: expect.stringContaining("401 Unauthorized: Token expired"), + }); + }); + + it("preserves HTTP error status when the response body is empty", async () => { + fetchSpy.mockResolvedValue(new Response(null, { status: 404 })); + await expect( + getVariantAnalysis(credentials, 0, 5, SERVER_URL), + ).rejects.toMatchObject({ status: 404 }); + }); + + it("rejects custom runs after removing the setting", async () => { + await setSetting("serverUrl", ""); + expect(() => getServerForVariantAnalysis(SERVER_URL)).toThrow( + /different server/, + ); + await expect(createGist(credentials, "", {}, SERVER_URL)).rejects.toThrow( + /not supported/, ); + expect(fetchSpy).not.toHaveBeenCalled(); + }); - expect(error).toBeInstanceOf(RequestError); - expect((error as RequestError).status).toBe(401); - expect((error as RequestError).message).toContain( - "401 Unauthorized: Token expired", + it("rejects legacy GitHub runs when a custom server is configured", async () => { + await setSetting("serverUrl", SERVER_URL); + await expect(getVariantAnalysis(credentials, 0, 5)).rejects.toThrow( + /different server/, ); - expect((error as RequestError).message).not.toContain("the-token"); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("rejects status, results, and cancellation after changing servers", async () => { + await setSetting("serverUrl", "https://other.example.com"); + const run = createMockVariantAnalysis({}); + run.serverUrl = SERVER_URL; + await expect( + getVariantAnalysis(credentials, 0, 5, SERVER_URL), + ).rejects.toThrow(/different server/); + await expect( + getVariantAnalysisRepo(credentials, 0, 5, 7, SERVER_URL), + ).rejects.toThrow(/different server/); + await expect(cancelVariantAnalysis(credentials, run)).rejects.toThrow( + /different server/, + ); + expect(fetchSpy).not.toHaveBeenCalled(); }); }); }); From f1a595386917a74be9a61d26d2aa2c15cfdf1b9b Mon Sep 17 00:00:00 2001 From: Nicolas Will Date: Wed, 28 Oct 2026 20:10:24 +0100 Subject: [PATCH 3/5] Fix custom variant analysis persistence and server behavior --- docs/variant-analysis-custom-server.md | 39 ++++-- extensions/ql-vscode/package.json | 2 +- extensions/ql-vscode/src/common/bqrs-utils.ts | 4 +- .../src/common/location-link-utils.ts | 5 +- extensions/ql-vscode/src/config.ts | 2 +- .../ql-vscode/src/databases/ui/db-panel.ts | 10 +- ...-history-variant-analysis-domain-mapper.ts | 3 + ...ery-history-variant-analysis-dto-mapper.ts | 3 + .../query-history-variant-analysis-dto.ts | 3 + .../src/variant-analysis/bqrs-processing.ts | 2 +- .../src/variant-analysis/gh-api/repository.ts | 1 + .../gh-api/variant-analysis-server-client.ts | 29 ++++- .../variant-analysis/markdown-generation.ts | 11 +- .../repo-tasks-domain-mapper.ts | 1 + .../repo-tasks-store/repo-tasks-dto-mapper.ts | 1 + .../repo-tasks-store/repo-tasks-dto.ts | 1 + .../src/variant-analysis/sarif-processing.ts | 16 ++- .../shared/analysis-result.ts | 4 +- .../src/variant-analysis/shared/repository.ts | 1 + .../variant-analysis/variant-analysis-api.ts | 6 +- .../variant-analysis-manager.ts | 6 +- .../variant-analysis-mapper.ts | 3 + .../variant-analysis-results-manager.ts | 56 +++++++-- .../FileCodeSnippet/CodeSnippetMessage.tsx | 3 + .../FileCodeSnippet/FileCodeSnippet.tsx | 19 ++- .../view/variant-analysis/RawResultCell.tsx | 2 +- .../view/variant-analysis/RawResultRow.tsx | 2 +- .../view/variant-analysis/RawResultsTable.tsx | 2 +- .../AnalyzedRepoItemContent.spec.tsx | 36 ++++++ .../test/unit-tests/common/bqrs-utils.test.ts | 22 ++++ .../markdown-generation.test.ts | 36 ++++++ .../variant-analysis-mapper.test.ts | 18 +++ .../variant-analysis-manager.test.ts | 16 +++ .../variant-analysis-results-manager.test.ts | 50 ++++++-- .../store/query-history-store.test.ts | 13 ++ .../variant-analysis-api.test.ts | 117 +++++++++++++++++- 36 files changed, 473 insertions(+), 72 deletions(-) create mode 100644 extensions/ql-vscode/test/unit-tests/common/bqrs-utils.test.ts diff --git a/docs/variant-analysis-custom-server.md b/docs/variant-analysis-custom-server.md index 41fc39d8ff7..771903bc652 100644 --- a/docs/variant-analysis-custom-server.md +++ b/docs/variant-analysis-custom-server.md @@ -12,8 +12,13 @@ extension, owned by the organization, supplies the token. ## Settings -Both settings are only read from **user** or **machine** (remote) settings. Workspace and folder values are ignored, +Both settings are only read from **local or remote user** settings. Workspace and folder values are ignored, so a repository cannot redirect queries or tokens. +`inspect().globalValue` is VS Code's merged user value, including remote user settings; +the remote value takes precedence over the local value. These are not OS-wide policy settings. +Administrators can provision the remote user settings file directly. A companion extension writing +with `ConfigurationTarget.Global` should not assume that the write always targets the remote file: +VS Code's choice depends on the setting's scope and whether a remote value already exists. | Setting | Value | | --- | --- | @@ -38,6 +43,10 @@ CodeQL activates the extension if it isn't already running, and calls `getVarian before every request. The provider is responsible for caching and refreshing the token. It should return `undefined` if no token is available, so that CodeQL can show a clear error instead of sending an unauthenticated request. The token is never logged. +CodeQL waits at most 30 seconds for activation and token acquisition combined. On timeout, no API +request is sent; the provider's own work is not forcibly stopped. Finish sign-in in the provider and retry. +HTTP 401/403 errors identify the provider and ask the user to re-authenticate; there is no automatic retry +or token-refresh argument. Only the extension that the user or administrator names is called. Extensions cannot register themselves as providers. @@ -55,6 +64,11 @@ themselves as providers. - Result artifacts are downloaded from `artifact_url` without authentication, as on GitHub. - If a repository task has no `database_commit_sha`, file links point to `HEAD`. These links may not match the analyzed revision. +- Custom-server file links use `repository.html_url` from the task or analysis response. + If it is absent or not an HTTP(S) URL, results still display, but file locations are plain text. + GitHub run links retain their existing host fallback. +- GitHub's built-in repository lists and the repository tree's "Open on GitHub" action are hidden + while a custom server is configured. - The canary-only autofix action still downloads repository source through GitHub sign-in. It requires the analyzed repository and revision to be available on the configured GitHub host. Custom source retrieval is not supported. @@ -70,14 +84,25 @@ Request and response shapes match `src/variant-analysis/gh-api/variant-analysis. | Submit | `POST /repositories/0/code-scanning/codeql/variant-analyses` | | Get status | `GET /repositories/0/code-scanning/codeql/variant-analyses/{id}` | | Get repository task | `GET /repositories/0/code-scanning/codeql/variant-analyses/{id}/repositories/{repoId}` | -| Cancel | `POST /repos/{controller_repo.full_name}/actions/runs/{actions_workflow_run_id}/cancel` | +| Cancel | `POST /repositories/0/code-scanning/codeql/variant-analyses/{id}/cancel` | -Variant analysis responses must include `controller_repo` and, for cancellation to work, -`actions_workflow_run_id`. Errors should be returned as non-2xx responses with a JSON body +Variant analysis responses must include `controller_repo`; subsequent requests use its `id` as the +repository path parameter (normally `0`). The analysis `id` is the server's run/session identifier. +Custom-server cancellation does not require `actions_workflow_run_id`; GitHub cancellation still uses +the GitHub Actions route and requires that field. Errors should be returned as non-2xx responses with a JSON body `{ "message": "..." }`. CodeQL shows the status and that message to the user. ## Running CodeQL in a remote workspace -To run the CodeQL extension on the remote machine alongside the token provider, set the VS Code -setting `remote.extensionKind` to `{ "github.vscode-codeql": ["workspace"] }`. No change to the -extension is needed. +CodeQL declares a Node `main` entry point, so VS Code normally classifies it as a workspace extension +without an explicit `extensionKind`. Install both CodeQL and its token provider in the same remote +extension host. No override should normally be necessary. + +If a deployment has overridden the placement, inspect `remote.extensionKind` and the editor's +product overrides. The user setting `{ "github.vscode-codeql": ["workspace"] }` under +`remote.extensionKind` can override placement, but should not be installed as a blanket workaround. + +VS Code implementation references: +[configuration inspection](https://github.com/microsoft/vscode/blob/main/src/vs/workbench/api/common/extHostConfiguration.ts), +[local/remote user configuration merging](https://github.com/microsoft/vscode/blob/main/src/vs/platform/configuration/common/configurationModels.ts), +and [extension kind inference](https://github.com/microsoft/vscode/blob/main/src/vs/workbench/services/extensions/common/extensionManifestPropertiesService.ts). diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index cec9ce8fcd3..dddb8d1d4a0 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -1194,7 +1194,7 @@ }, { "command": "codeQLVariantAnalysisRepositories.openOnGitHubContextMenu", - "when": "view == codeQLVariantAnalysisRepositories && viewItem =~ /canBeOpenedOnGitHub/", + "when": "view == codeQLVariantAnalysisRepositories && viewItem =~ /canBeOpenedOnGitHub/ && !config.codeQL.variantAnalysis.serverUrl", "group": "2_qlContextMenu@1" }, { diff --git a/extensions/ql-vscode/src/common/bqrs-utils.ts b/extensions/ql-vscode/src/common/bqrs-utils.ts index 1c0ec8e2834..f598fe97261 100644 --- a/extensions/ql-vscode/src/common/bqrs-utils.ts +++ b/extensions/ql-vscode/src/common/bqrs-utils.ts @@ -12,10 +12,10 @@ export function isEmptyPath(uriStr: string) { export function tryGetRemoteLocation( loc: UrlValue | undefined, - fileLinkPrefix: string, + fileLinkPrefix: string | undefined, sourceLocationPrefix: string | undefined, ): string | undefined { - if (!loc || !isUrlValueResolvable(loc)) { + if (!fileLinkPrefix || !loc || !isUrlValueResolvable(loc)) { return undefined; } diff --git a/extensions/ql-vscode/src/common/location-link-utils.ts b/extensions/ql-vscode/src/common/location-link-utils.ts index 6ed58336e7a..c8ff9a94e10 100644 --- a/extensions/ql-vscode/src/common/location-link-utils.ts +++ b/extensions/ql-vscode/src/common/location-link-utils.ts @@ -6,7 +6,10 @@ export function createRemoteFileRef( endLine?: number, startColumn?: number, endColumn?: number, -): string { +): string | undefined { + if (!fileLink.fileLinkPrefix) { + return undefined; + } if ( startColumn && endColumn && diff --git a/extensions/ql-vscode/src/config.ts b/extensions/ql-vscode/src/config.ts index 12eb4969008..671b8694c2e 100644 --- a/extensions/ql-vscode/src/config.ts +++ b/extensions/ql-vscode/src/config.ts @@ -719,7 +719,7 @@ export class VariantAnalysisConfigListener } public get showSystemDefinedRepositoryLists(): boolean { - return !hasEnterpriseUri(); + return !hasEnterpriseUri() && !isVariantAnalysisServerConfigured(); } public get githubUrl(): URL { diff --git a/extensions/ql-vscode/src/databases/ui/db-panel.ts b/extensions/ql-vscode/src/databases/ui/db-panel.ts index dbc61469d5a..0b39e867f33 100644 --- a/extensions/ql-vscode/src/databases/ui/db-panel.ts +++ b/extensions/ql-vscode/src/databases/ui/db-panel.ts @@ -25,7 +25,10 @@ import type { App } from "../../common/app"; import { QueryLanguage } from "../../common/query-language"; import { getCodeSearchRepositories } from "../code-search-api"; import { showAndLogErrorMessage } from "../../common/logging"; -import { getGitHubInstanceUrl } from "../../config"; +import { + getGitHubInstanceUrl, + isVariantAnalysisServerConfigured, +} from "../../config"; export interface RemoteDatabaseQuickPickItem extends QuickPickItem { remoteDatabaseKind: string; @@ -415,6 +418,11 @@ export class DbPanel extends DisposableObject { } private async openOnGitHub(treeViewItem: DbTreeViewItem): Promise { + if (isVariantAnalysisServerConfigured()) { + throw new Error( + "Opening repositories on GitHub is not supported with a custom variant analysis server.", + ); + } if (treeViewItem.dbItem === undefined) { throw new Error("Unable to open on GitHub. Please select a valid item."); } diff --git a/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-domain-mapper.ts b/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-domain-mapper.ts index 6ca7c63132e..00e4d6b9838 100644 --- a/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-domain-mapper.ts +++ b/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-domain-mapper.ts @@ -48,9 +48,11 @@ function mapVariantAnalysisDtoToDto( ): VariantAnalysisDto { return { id: variantAnalysis.id, + serverUrl: variantAnalysis.serverUrl, controllerRepo: { id: variantAnalysis.controllerRepo.id, fullName: variantAnalysis.controllerRepo.fullName, + htmlUrl: variantAnalysis.controllerRepo.htmlUrl, private: variantAnalysis.controllerRepo.private, }, query: { @@ -96,6 +98,7 @@ function mapVariantAnalysisScannedRepositoryToDto( repository: { id: repo.repository.id, fullName: repo.repository.fullName, + htmlUrl: repo.repository.htmlUrl, private: repo.repository.private, stargazersCount: repo.repository.stargazersCount, updatedAt: repo.repository.updatedAt, diff --git a/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-dto-mapper.ts b/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-dto-mapper.ts index 7f81ed91821..711e4b792f6 100644 --- a/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-dto-mapper.ts +++ b/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-dto-mapper.ts @@ -48,9 +48,11 @@ function mapVariantAnalysisToDomainModel( ): VariantAnalysis { return { id: variantAnalysis.id, + serverUrl: variantAnalysis.serverUrl, controllerRepo: { id: variantAnalysis.controllerRepo.id, fullName: variantAnalysis.controllerRepo.fullName, + htmlUrl: variantAnalysis.controllerRepo.htmlUrl, private: variantAnalysis.controllerRepo.private, }, language: mapQueryLanguageToDomainModel(variantAnalysis.query.language), @@ -102,6 +104,7 @@ function mapVariantAnalysisScannedRepositoryToDomainModel( repository: { id: repo.repository.id, fullName: repo.repository.fullName, + htmlUrl: repo.repository.htmlUrl, private: repo.repository.private, stargazersCount: repo.repository.stargazersCount, updatedAt: repo.repository.updatedAt, diff --git a/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-dto.ts b/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-dto.ts index dfe07af1de3..45c6b47212b 100644 --- a/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-dto.ts +++ b/extensions/ql-vscode/src/query-history/store/query-history-variant-analysis-dto.ts @@ -15,9 +15,11 @@ export interface QueryHistoryVariantAnalysisDto { export interface VariantAnalysisDto { id: number; + serverUrl?: string; controllerRepo: { id: number; fullName: string; + htmlUrl?: string; private: boolean; }; query: { @@ -47,6 +49,7 @@ export interface VariantAnalysisScannedRepositoryDto { repository: { id: number; fullName: string; + htmlUrl?: string; private: boolean; stargazersCount: number; updatedAt: string | null; diff --git a/extensions/ql-vscode/src/variant-analysis/bqrs-processing.ts b/extensions/ql-vscode/src/variant-analysis/bqrs-processing.ts index 0f0506b7784..2ef0da44081 100644 --- a/extensions/ql-vscode/src/variant-analysis/bqrs-processing.ts +++ b/extensions/ql-vscode/src/variant-analysis/bqrs-processing.ts @@ -9,7 +9,7 @@ export async function extractRawResults( cliServer: CodeQLCliServer, logger: Logger, filePath: string, - fileLinkPrefix: string, + fileLinkPrefix: string | undefined, sourceLocationPrefix: string, ): Promise { const bqrsInfo = await cliServer.bqrsInfo(filePath); diff --git a/extensions/ql-vscode/src/variant-analysis/gh-api/repository.ts b/extensions/ql-vscode/src/variant-analysis/gh-api/repository.ts index 08cfefa7546..24794a9a85e 100644 --- a/extensions/ql-vscode/src/variant-analysis/gh-api/repository.ts +++ b/extensions/ql-vscode/src/variant-analysis/gh-api/repository.ts @@ -9,6 +9,7 @@ export interface Repository { id: number; name: string; full_name: string; + html_url?: string; private: boolean; } diff --git a/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts b/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts index d228970986f..2317dd1dce9 100644 --- a/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts +++ b/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts @@ -17,8 +17,31 @@ export interface VariantAnalysisServerTokenProvider { export const SERVER_CONTROLLER_REPO_ID = 0; const MAX_ERROR_BODY_LENGTH = 500; +const TOKEN_PROVIDER_TIMEOUT_MS = 30_000; async function getToken(server: VariantAnalysisServerConfig): Promise { + let timeout: ReturnType | undefined; + try { + return await Promise.race([ + getTokenFromExtension(server), + new Promise((_, reject) => { + timeout = setTimeout(() => { + reject( + new Error( + `The variant analysis token provider '${server.tokenProviderExtensionId}' did not respond within 30 seconds. Complete sign-in in the provider extension and retry.`, + ), + ); + }, TOKEN_PROVIDER_TIMEOUT_MS); + }), + ]); + } finally { + clearTimeout(timeout); + } +} + +async function getTokenFromExtension( + server: VariantAnalysisServerConfig, +): Promise { const extensionId = server.tokenProviderExtensionId; if (!extensionId) { throw new Error( @@ -95,8 +118,12 @@ export async function requestVariantAnalysisServer( } if (!response.ok) { + const authenticationMessage = + response.status === 401 || response.status === 403 + ? ` Re-authenticate using the token provider '${server.tokenProviderExtensionId}' and retry.` + : ""; throw new RequestError( - `Variant analysis server request failed: ${method} ${path}: ${response.status} ${response.statusText}${describeErrorBody(data)}`, + `Variant analysis server request failed: ${method} ${path}: ${response.status} ${response.statusText}${describeErrorBody(data)}${authenticationMessage}`, response.status, { request: { method, url, headers: {} }, diff --git a/extensions/ql-vscode/src/variant-analysis/markdown-generation.ts b/extensions/ql-vscode/src/variant-analysis/markdown-generation.ts index e295282d343..ade753ee4f9 100644 --- a/extensions/ql-vscode/src/variant-analysis/markdown-generation.ts +++ b/extensions/ql-vscode/src/variant-analysis/markdown-generation.ts @@ -323,7 +323,7 @@ function generateMarkdownForRawResults( function generateMarkdownForRawTableCell( value: CellValue, - fileLinkPrefix: string, + fileLinkPrefix: string | undefined, sourceLocationPrefix: string, ) { let cellValue: string; @@ -363,16 +363,15 @@ function createMarkdownRemoteFileRef( region?: HighlightedRegion, linkText?: string, ): string { - const markdownLink = `[${ - linkText || fileLink.filePath - }](${createRemoteFileRef( + const url = createRemoteFileRef( fileLink, region?.startLine, region?.endLine, region?.startColumn, region?.endColumn, - )})`; - return markdownLink; + ); + const text = linkText || fileLink.filePath; + return url ? `[${text}](${url})` : text; } /** diff --git a/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-domain-mapper.ts b/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-domain-mapper.ts index df5f3b09cad..24d53ba8cd7 100644 --- a/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-domain-mapper.ts +++ b/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-domain-mapper.ts @@ -11,6 +11,7 @@ export function mapRepoTaskToDomainModel( repository: { id: repoTask.repository.id, fullName: repoTask.repository.fullName, + htmlUrl: repoTask.repository.htmlUrl, private: repoTask.repository.private, }, analysisStatus: mapRepoTaskAnalysisStatusToDomainModel( diff --git a/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-dto-mapper.ts b/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-dto-mapper.ts index f5b23663352..10165ea078c 100644 --- a/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-dto-mapper.ts +++ b/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-dto-mapper.ts @@ -11,6 +11,7 @@ export function mapRepoTaskToDto( repository: { id: repoTask.repository.id, fullName: repoTask.repository.fullName, + htmlUrl: repoTask.repository.htmlUrl, private: repoTask.repository.private, }, analysisStatus: mapRepoTaskAnalysisStatusToDto(repoTask.analysisStatus), diff --git a/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-dto.ts b/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-dto.ts index 4bfe618a17e..09ced6e2aea 100644 --- a/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-dto.ts +++ b/extensions/ql-vscode/src/variant-analysis/repo-tasks-store/repo-tasks-dto.ts @@ -12,6 +12,7 @@ export interface VariantAnalysisRepositoryTaskDto { interface RepositoryDto { id: number; fullName: string; + htmlUrl?: string; private: boolean; } diff --git a/extensions/ql-vscode/src/variant-analysis/sarif-processing.ts b/extensions/ql-vscode/src/variant-analysis/sarif-processing.ts index f998f228e19..5995d737558 100644 --- a/extensions/ql-vscode/src/variant-analysis/sarif-processing.ts +++ b/extensions/ql-vscode/src/variant-analysis/sarif-processing.ts @@ -35,7 +35,7 @@ const defaultSeverity = "Warning"; export function extractAnalysisAlerts( sarifLog: Log, - fileLinkPrefix: string, + fileLinkPrefix: string | undefined, ): { alerts: AnalysisAlert[]; errors: string[]; @@ -62,7 +62,7 @@ export function extractAnalysisAlerts( function extractResultAlerts( run: Run, result: Result, - fileLinkPrefix: string, + fileLinkPrefix: string | undefined, ): AnalysisAlert[] { const alerts: AnalysisAlert[] = []; @@ -259,7 +259,10 @@ function getHighlightedRegion(region: Region): HighlightedRegion { }; } -function getCodeFlows(result: Result, fileLinkPrefix: string): CodeFlow[] { +function getCodeFlows( + result: Result, + fileLinkPrefix: string | undefined, +): CodeFlow[] { const codeFlows = []; if (result.codeFlows) { @@ -299,7 +302,10 @@ function getCodeFlows(result: Result, fileLinkPrefix: string): CodeFlow[] { return codeFlows; } -function getMessage(result: Result, fileLinkPrefix: string): AnalysisMessage { +function getMessage( + result: Result, + fileLinkPrefix: string | undefined, +): AnalysisMessage { const tokens: AnalysisMessageToken[] = []; const messageText = result.message.text!; @@ -328,7 +334,7 @@ function getMessage(result: Result, fileLinkPrefix: string): AnalysisMessage { function getRelatedLocation( messagePart: SarifLink, result: Result, - fileLinkPrefix: string, + fileLinkPrefix: string | undefined, ): AnalysisMessageLocationTokenLocation | undefined { const relatedLocation = result.relatedLocations!.find( (rl) => rl.id === messagePart.dest, diff --git a/extensions/ql-vscode/src/variant-analysis/shared/analysis-result.ts b/extensions/ql-vscode/src/variant-analysis/shared/analysis-result.ts index 91851c373aa..9cf725b9791 100644 --- a/extensions/ql-vscode/src/variant-analysis/shared/analysis-result.ts +++ b/extensions/ql-vscode/src/variant-analysis/shared/analysis-result.ts @@ -2,7 +2,7 @@ import type { RawResultSet } from "../../common/raw-result-types"; export interface AnalysisRawResults { resultSet: RawResultSet; - fileLinkPrefix: string; + fileLinkPrefix: string | undefined; sourceLocationPrefix: string; capped: boolean; } @@ -18,7 +18,7 @@ export interface AnalysisAlert { } export interface FileLink { - fileLinkPrefix: string; + fileLinkPrefix: string | undefined; filePath: string; } diff --git a/extensions/ql-vscode/src/variant-analysis/shared/repository.ts b/extensions/ql-vscode/src/variant-analysis/shared/repository.ts index cf6b91e45f6..a5fbd5d1528 100644 --- a/extensions/ql-vscode/src/variant-analysis/shared/repository.ts +++ b/extensions/ql-vscode/src/variant-analysis/shared/repository.ts @@ -1,6 +1,7 @@ export interface Repository { id: number; fullName: string; + htmlUrl?: string; private: boolean; } diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts index 799a83738c0..f14c942f97a 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts @@ -110,14 +110,10 @@ export async function cancelVariantAnalysis( return cancelGitHubVariantAnalysis(credentials, variantAnalysis); } - const { - actionsWorkflowRunId, - controllerRepo: { fullName }, - } = variantAnalysis; await requestVariantAnalysisServer( server, "POST", - `/repos/${fullName}/actions/runs/${actionsWorkflowRunId}/cancel`, + `${variantAnalysesPath(variantAnalysis.controllerRepo.id)}/${variantAnalysis.id}/cancel`, undefined, true, ); diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts index df045d762b6..29dce159121 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts @@ -430,6 +430,7 @@ export class VariantAnalysisManager } catch (e) { // If the error is handled by the handleRequestError function, we don't need to throw if ( + !serverUrl && e instanceof RequestError && handleRequestError(e, this.config.githubUrl, this.app.logger) ) { @@ -645,7 +646,7 @@ export class VariantAnalysisManager variantAnalysisId, this.getVariantAnalysisStorageLocation(variantAnalysisId), repositoryFullName, - options, + { ...options, serverUrl: variantAnalysis.serverUrl }, ); } @@ -816,6 +817,7 @@ export class VariantAnalysisManager ); repoTask = mapVariantAnalysisRepositoryTask(repoTaskResponse); + repoTask.repository.htmlUrl ??= scannedRepo.repository.htmlUrl; } catch (e) { repoState.downloadStatus = VariantAnalysisScannedRepositoryDownloadStatus.Failed; @@ -930,7 +932,7 @@ export class VariantAnalysisManager throw new Error(`No variant analysis with id: ${variantAnalysisId}`); } - if (!variantAnalysis.actionsWorkflowRunId) { + if (!variantAnalysis.serverUrl && !variantAnalysis.actionsWorkflowRunId) { throw new Error( `No workflow run id for variant analysis with id: ${variantAnalysis.id}`, ); diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts index f5d4fedc1fd..0f2275fe4c7 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-mapper.ts @@ -112,6 +112,7 @@ function mapVariantAnalysis( controllerRepo: { id: response.controller_repo.id, fullName: response.controller_repo.full_name, + htmlUrl: response.controller_repo.html_url, private: response.controller_repo.private, }, language: currentVariantAnalysis.language, @@ -143,6 +144,7 @@ export function mapVariantAnalysisRepositoryTask( repository: { id: response.repository.id, fullName: response.repository.full_name, + htmlUrl: response.repository.html_url, private: response.repository.private, }, analysisStatus: mapApiRepoStatus(response.analysis_status), @@ -162,6 +164,7 @@ export function mapScannedRepository( repository: { id: scannedRepo.repository.id, fullName: scannedRepo.repository.full_name, + htmlUrl: scannedRepo.repository.html_url, private: scannedRepo.repository.private, stargazersCount: scannedRepo.repository.stargazers_count, updatedAt: scannedRepo.repository.updated_at, diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-results-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-results-manager.ts index b3edf3bb4f7..da71c4f083e 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-results-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-results-manager.ts @@ -37,6 +37,7 @@ type ResultDownloadedEvent = { }; export type LoadResultsOptions = { + serverUrl?: string; // If true, when results are loaded from storage, they will not be stored in the cache. This reduces memory usage if // results are only needed temporarily (e.g. for exporting results to a different format). skipCacheStore?: boolean; @@ -150,6 +151,7 @@ export class VariantAnalysisResultsManager extends DisposableObject { variantAnalysisId, variantAnalysisStoragePath, repositoryFullName, + options.serverUrl, ); } @@ -157,6 +159,7 @@ export class VariantAnalysisResultsManager extends DisposableObject { variantAnalysisId, variantAnalysisStoragePath, repositoryFullName, + options?.serverUrl, ); } @@ -164,11 +167,13 @@ export class VariantAnalysisResultsManager extends DisposableObject { variantAnalysisId: number, variantAnalysisStoragePath: string, repositoryFullName: string, + serverUrl?: string, ): Promise { const result = await this.loadResultsFromStorage( variantAnalysisId, variantAnalysisStoragePath, repositoryFullName, + serverUrl, ); this.cachedResults.set( createCacheKey(variantAnalysisId, repositoryFullName), @@ -182,6 +187,7 @@ export class VariantAnalysisResultsManager extends DisposableObject { variantAnalysisId: number, variantAnalysisStoragePath: string, repositoryFullName: string, + serverUrl?: string, ): Promise { if ( !(await this.isVariantAnalysisRepoDownloaded( @@ -200,12 +206,20 @@ export class VariantAnalysisResultsManager extends DisposableObject { const repoTask: VariantAnalysisRepositoryTask = await readRepoTask(storageDirectory); - // Some servers do not report these, so fall back to linking to the - // default branch and to the legacy source location handling. - const fileLinkPrefix = this.createGitHubFileLinkPrefix( - repoTask.repository.fullName, - repoTask.databaseCommitSha || "HEAD", - ); + const repositoryUrl = + repoTask.repository.htmlUrl ?? + (serverUrl + ? undefined + : new URL( + `/${repoTask.repository.fullName}`, + this.config.githubUrl, + ).toString()); + const fileLinkPrefix = repositoryUrl + ? this.createFileLinkPrefix( + repositoryUrl, + repoTask.databaseCommitSha || "HEAD", + ) + : undefined; const resultsDirectory = join( storageDirectory, @@ -261,7 +275,7 @@ export class VariantAnalysisResultsManager extends DisposableObject { private async readBqrsResults( filePath: string, - fileLinkPrefix: string, + fileLinkPrefix: string | undefined, sourceLocationPrefix: string, ): Promise { return await extractRawResults( @@ -275,7 +289,7 @@ export class VariantAnalysisResultsManager extends DisposableObject { private async readSarifResults( filePath: string, - fileLinkPrefix: string, + fileLinkPrefix: string | undefined, ): Promise { const sarifLog = await sarifParser(filePath); @@ -307,11 +321,27 @@ export class VariantAnalysisResultsManager extends DisposableObject { ); } - private createGitHubFileLinkPrefix(fullName: string, sha: string): string { - return new URL( - `/${fullName}/blob/${sha}`, - this.config.githubUrl, - ).toString(); + private createFileLinkPrefix( + repositoryUrl: string, + sha: string, + ): string | undefined { + let url: URL; + try { + url = new URL(repositoryUrl); + } catch { + return undefined; + } + if ( + !["http:", "https:"].includes(url.protocol) || + url.username || + url.password + ) { + return undefined; + } + url.search = ""; + url.hash = ""; + url.pathname = `${url.pathname.replace(/\/+$/, "")}/blob/${encodeURIComponent(sha)}`; + return url.toString(); } public removeAnalysisResults(variantAnalysis: VariantAnalysis) { diff --git a/extensions/ql-vscode/src/view/common/FileCodeSnippet/CodeSnippetMessage.tsx b/extensions/ql-vscode/src/view/common/FileCodeSnippet/CodeSnippetMessage.tsx index 09ec16d4288..12c06e2f741 100644 --- a/extensions/ql-vscode/src/view/common/FileCodeSnippet/CodeSnippetMessage.tsx +++ b/extensions/ql-vscode/src/view/common/FileCodeSnippet/CodeSnippetMessage.tsx @@ -64,6 +64,9 @@ export const CodeSnippetMessage = ({ case "text": return {token.text}; case "location": + if (!token.location.fileLink.fileLinkPrefix) { + return {token.text}; + } return ( + {fileLink.filePath} + + ) : ( + fileLink.filePath + ); if (!codeSnippet) { return ( - - - {fileLink.filePath} - - + {title} {message && severity && ( {messageChildren} @@ -89,11 +92,7 @@ export const FileCodeSnippet = ({ return ( - - - {fileLink.filePath} - - + {title} {code.map((line, index) => ( ` type RawResultsTableProps = { resultSet: RawResultSet; - fileLinkPrefix: string; + fileLinkPrefix: string | undefined; sourceLocationPrefix: string; }; diff --git a/extensions/ql-vscode/src/view/variant-analysis/__tests__/AnalyzedRepoItemContent.spec.tsx b/extensions/ql-vscode/src/view/variant-analysis/__tests__/AnalyzedRepoItemContent.spec.tsx index 1749fdd5bd4..b8d2da995f4 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/__tests__/AnalyzedRepoItemContent.spec.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/__tests__/AnalyzedRepoItemContent.spec.tsx @@ -19,6 +19,42 @@ describe(AnalyzedRepoItemContent.name, () => { ); }; + it.each([undefined, { startLine: 1, endLine: 1, text: "select 1" }])( + "renders file paths and message locations as text when repository links are unavailable", + (codeSnippet) => { + render({ + interpretedResults: [ + { + message: { + tokens: [ + { + t: "location", + text: "related location", + location: { + fileLink: { + fileLinkPrefix: undefined, + filePath: "src/related.ts", + }, + }, + }, + ], + }, + shortDescription: "A finding", + fileLink: { fileLinkPrefix: undefined, filePath: "src/query.ts" }, + severity: "Warning", + codeSnippet, + codeFlows: [], + }, + ], + }); + expect(screen.getByText("src/query.ts")).toBeInTheDocument(); + if (!codeSnippet) { + expect(screen.getByText("related location")).toBeInTheDocument(); + } + expect(screen.queryAllByRole("link")).toHaveLength(0); + }, + ); + it("renders the succeeded state with interpreted results", () => { render({ status: VariantAnalysisRepoStatus.Succeeded, diff --git a/extensions/ql-vscode/test/unit-tests/common/bqrs-utils.test.ts b/extensions/ql-vscode/test/unit-tests/common/bqrs-utils.test.ts new file mode 100644 index 00000000000..b2041b7e87c --- /dev/null +++ b/extensions/ql-vscode/test/unit-tests/common/bqrs-utils.test.ts @@ -0,0 +1,22 @@ +import { tryGetRemoteLocation } from "../../../src/common/bqrs-utils"; + +describe(tryGetRemoteLocation.name, () => { + it.each([ + [undefined, undefined], + [ + "https://source.example.com/org/repo/blob/abc123", + "https://source.example.com/org/repo/blob/abc123/src/query.ts", + ], + ])( + "only links raw results when a repository URL is available: %s", + (prefix, expected) => { + expect( + tryGetRemoteLocation( + { type: "wholeFileLocation", uri: "file:/sources/src/query.ts" }, + prefix, + "/sources", + ), + ).toBe(expected); + }, + ); +}); diff --git a/extensions/ql-vscode/test/unit-tests/variant-analysis/markdown-generation.test.ts b/extensions/ql-vscode/test/unit-tests/variant-analysis/markdown-generation.test.ts index 4b7601273bb..e22265de820 100644 --- a/extensions/ql-vscode/test/unit-tests/variant-analysis/markdown-generation.test.ts +++ b/extensions/ql-vscode/test/unit-tests/variant-analysis/markdown-generation.test.ts @@ -17,10 +17,46 @@ import type { import pathProblemAnalysesResults from "../data/markdown-generation/interpreted-results/path-problem/analyses-results.json"; import problemAnalysesResults from "../data/markdown-generation/interpreted-results/problem/analyses-results.json"; import rawResultsAnalysesResults from "../data/markdown-generation/raw-results/analyses-results.json"; +import { createMockVariantAnalysis } from "../../factories/variant-analysis/shared/variant-analysis"; +import { createMockScannedRepo } from "../../factories/variant-analysis/shared/scanned-repositories"; const dataPath = resolve(__dirname, "../data/markdown-generation"); describe(generateVariantAnalysisMarkdown.name, () => { + it("exports locations as text when repository links are unavailable", async () => { + const run = createMockVariantAnalysis({}); + const repo = createMockScannedRepo(); + repo.resultCount = 1; + const results: VariantAnalysisScannedRepositoryResult = { + variantAnalysisId: run.id, + repositoryId: repo.repository.id, + interpretedResults: [ + { + message: { tokens: [{ t: "text", text: "A finding" }] }, + shortDescription: "A finding", + severity: "Warning", + fileLink: { fileLinkPrefix: undefined, filePath: "src/query.ts" }, + codeFlows: [], + }, + ], + }; + async function* getUnlinkedResults(): AsyncIterable< + [VariantAnalysisScannedRepository, VariantAnalysisScannedRepositoryResult] + > { + yield [repo, results]; + } + const actual = await generateVariantAnalysisMarkdown( + run, + getUnlinkedResults(), + 1, + "local", + ); + const content = actual.markdownFiles[1].content.join("\n"); + expect(content).toContain("src/query.ts"); + expect(content).not.toContain("[src/query.ts]("); + expect(content).not.toContain("undefined"); + }); + describe("for path-problem query", () => { it("should generate markdown file for each repo with results", async () => { const actualFiles = await generateVariantAnalysisMarkdown( diff --git a/extensions/ql-vscode/test/unit-tests/variant-analysis/variant-analysis-mapper.test.ts b/extensions/ql-vscode/test/unit-tests/variant-analysis/variant-analysis-mapper.test.ts index 44fce971ad7..981e6994644 100644 --- a/extensions/ql-vscode/test/unit-tests/variant-analysis/variant-analysis-mapper.test.ts +++ b/extensions/ql-vscode/test/unit-tests/variant-analysis/variant-analysis-mapper.test.ts @@ -28,6 +28,24 @@ describe(mapVariantAnalysisFromSubmission.name, () => { ); const mockSubmission = createMockSubmission(); + it("preserves repository source URLs supplied by the server", () => { + const response = createMockApiResponse("succeeded"); + response.controller_repo.html_url = + "https://source.example.com/org/controller"; + response.scanned_repositories![0].repository.html_url = + "https://source.example.com/org/repo"; + const run = mapVariantAnalysisFromSubmission(mockSubmission, response, []); + expect(run.controllerRepo.htmlUrl).toBe(response.controller_repo.html_url); + expect(run.scannedRepos?.[0].repository.htmlUrl).toBe( + response.scanned_repositories![0].repository.html_url, + ); + const task = createMockVariantAnalysisRepoTask(); + task.repository.html_url = "https://source.example.com/org/repo"; + expect(mapVariantAnalysisRepositoryTask(task).repository.htmlUrl).toBe( + task.repository.html_url, + ); + }); + it("preserves the server origin through submission and polling updates", () => { const submission = { ...mockSubmission, diff --git a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts index 850e221d84f..98c3e0341e5 100644 --- a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts @@ -3,6 +3,7 @@ import { commands, env, window, workspace } from "vscode"; import { extLogger } from "../../../../src/common/logging/vscode"; import * as ghApiClient from "../../../../src/variant-analysis/gh-api/gh-api-client"; import * as ghActionsApiClient from "../../../../src/variant-analysis/gh-api/gh-actions-api-client"; +import * as variantAnalysisApi from "../../../../src/variant-analysis/variant-analysis-api"; import { ensureDir, outputJson, @@ -654,6 +655,21 @@ describe("Variant Analysis Manager", () => { ); }); + it("allows custom-server cancellation without an Actions workflow run ID", async () => { + const cancel = jest + .spyOn(variantAnalysisApi, "cancelVariantAnalysis") + .mockResolvedValue(undefined); + const serverRun = { + ...variantAnalysis, + serverUrl: "https://mrva.example.com", + actionsWorkflowRunId: undefined, + }; + await variantAnalysisManager.onVariantAnalysisUpdated(serverRun); + await variantAnalysisManager.cancelVariantAnalysis(serverRun.id); + expect(cancel).toHaveBeenCalledWith(app.credentials, serverRun); + expect(mockCancelVariantAnalysis).not.toHaveBeenCalled(); + }); + it("should set the status to canceling", async () => { await variantAnalysisManager.cancelVariantAnalysis(variantAnalysis.id); diff --git a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-results-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-results-manager.test.ts index 842dbaa29e0..8239d8cecd1 100644 --- a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-results-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-results-manager.test.ts @@ -15,6 +15,7 @@ import { mockedObject } from "../../utils/mocking.helpers"; import { createMockVariantAnalysisConfig } from "../../../factories/config"; import { setupServer } from "msw/node"; import { http } from "msw"; +import { writeRepoTask } from "../../../../src/variant-analysis/repo-tasks-store"; jest.setTimeout(10_000); @@ -275,10 +276,7 @@ describe(VariantAnalysisResultsManager.name, () => { describe("when the repo task has been written to disk", () => { beforeEach(async () => { - await outputJson( - join(repoTaskStorageDirectory, "repo_task.json"), - dummyRepoTask, - ); + await writeRepoTask(repoTaskStorageDirectory, dummyRepoTask); }); describe("when the results are not downloaded", () => { @@ -303,14 +301,48 @@ describe(VariantAnalysisResultsManager.name, () => { ); }); - it("should return the results when not cached", async () => { - await expect( - variantAnalysisResultsManager.loadResults( + it.each([ + [ + "https://source.example.com/scm/org/repo", + "https://source.example.com/scm/org/repo/blob/abc123", + ], + [undefined, undefined], + ["javascript:alert(1)", undefined], + ["not a URL", undefined], + ])( + "uses the supplied repository URL or omits links: %s", + async (htmlUrl, expectedPrefix) => { + dummyRepoTask.repository.htmlUrl = htmlUrl; + dummyRepoTask.databaseCommitSha = "abc123"; + await outputJson( + join(repoTaskStorageDirectory, "repo_task.json"), + dummyRepoTask, + ); + const result = await variantAnalysisResultsManager.loadResults( variantAnalysisId, variantAnalysisStoragePath, dummyRepoTask.repository.fullName, - ), - ).resolves.toHaveProperty("interpretedResults"); + { serverUrl: "https://mrva.example.com" }, + ); + expect(result.interpretedResults?.length).toBeGreaterThan(0); + for (const alert of result.interpretedResults ?? []) { + expect(alert.fileLink.fileLinkPrefix).toBe(expectedPrefix); + } + }, + ); + + it("should return the results when not cached", async () => { + const result = await variantAnalysisResultsManager.loadResults( + variantAnalysisId, + variantAnalysisStoragePath, + dummyRepoTask.repository.fullName, + ); + expect(result.interpretedResults?.length).toBeGreaterThan(0); + for (const alert of result.interpretedResults ?? []) { + expect(alert.fileLink.fileLinkPrefix).toBe( + `https://github.com/${dummyRepoTask.repository.fullName}/blob/${dummyRepoTask.databaseCommitSha}`, + ); + } expect(onResultLoadedSpy).toHaveBeenCalledTimes(1); expect(onResultLoadedSpy).toHaveBeenCalledWith( diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/store/query-history-store.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/store/query-history-store.test.ts index e8ae5a82975..2b940dab40f 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/store/query-history-store.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/store/query-history-store.test.ts @@ -95,6 +95,7 @@ describe("write and read", () => { } } }); + expectedHistory.forEach((info) => { if (info.t === "local" && info.completedQuery) { (info.completedQuery as any).dispose = undefined; @@ -108,6 +109,18 @@ describe("write and read", () => { expect(allHistoryActual.length).toEqual(expectedHistory.length); }); + it("preserves server origins and repository URLs without changing legacy history", async () => { + const serverRun = createMockVariantAnalysisHistoryItem({}); + serverRun.variantAnalysis.serverUrl = "https://mrva.example.com"; + serverRun.variantAnalysis.scannedRepos![0].repository.htmlUrl = + "https://source.example.com/org/repo"; + const legacyRun = createMockVariantAnalysisHistoryItem({}); + const historyPath = join(tmpDir.name, "server-and-legacy-history.json"); + await writeQueryHistoryToFile([serverRun, legacyRun], historyPath); + const actual = await readQueryHistoryFromFile(historyPath); + expect(actual).toEqual([serverRun, legacyRun]); + }); + it("should read query output dir from completed query if not present", async () => { const historyPath = join(tmpDir.name, "workspace-query-history.json"); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts index 9683980ccf5..ce1789eefa4 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts @@ -1,6 +1,9 @@ import { ConfigurationTarget, extensions, workspace } from "vscode"; import type { Extension } from "vscode"; -import { getVariantAnalysisServerConfig } from "../../../../src/config"; +import { + getVariantAnalysisServerConfig, + VariantAnalysisConfigListener, +} from "../../../../src/config"; import { cancelVariantAnalysis, createGist, @@ -15,6 +18,14 @@ import { createMockVariantAnalysis } from "../../../factories/variant-analysis/s import { mockedObject } from "../../utils/mocking.helpers"; import { testCredentialsWithStub } from "../../../factories/authentication"; import type { VariantAnalysisServerTokenProvider } from "../../../../src/variant-analysis/gh-api/variant-analysis-server-client"; +import { + writeQueryHistoryToFile, + readQueryHistoryFromFile, +} from "../../../../src/query-history/store/query-history-store"; +import { createMockVariantAnalysisHistoryItem } from "../../../factories/query-history/variant-analysis-history-item"; +import { join } from "path"; +import { tmpDir } from "../../../../src/tmp-dir"; +import { remove } from "fs-extra"; const SERVER_URL = "https://mrva.example.com"; const PROVIDER_ID = "example.token-provider"; @@ -83,6 +94,19 @@ describe("variant analysis custom server", () => { expect(isVariantAnalysisEnabledForGitHubHost()).toBe(true); }); + it("hides system-defined repository lists only in server mode", async () => { + const config = new VariantAnalysisConfigListener(); + try { + expect(config.showSystemDefinedRepositoryLists).toBe(true); + await setSetting("serverUrl", SERVER_URL); + expect(config.showSystemDefinedRepositoryLists).toBe(false); + await setSetting("serverUrl", ""); + expect(config.showSystemDefinedRepositoryLists).toBe(true); + } finally { + config.dispose(); + } + }); + it("ignores workspace settings", async () => { await setSetting("serverUrl", SERVER_URL, ConfigurationTarget.Workspace); await setSetting( @@ -131,6 +155,94 @@ describe("variant analysis custom server", () => { await setSetting("serverTokenProvider", PROVIDER_ID); }); + it("routes a saved and reloaded server run back to its original endpoint", async () => { + const history = createMockVariantAnalysisHistoryItem({}); + history.variantAnalysis.serverUrl = SERVER_URL; + history.variantAnalysis.controllerRepo.id = 0; + history.variantAnalysis.actionsWorkflowRunId = 42; + history.variantAnalysis.controllerRepo.htmlUrl = + "https://source.example.com/org/controller"; + const historyPath = join(tmpDir.name, "custom-server-history.json"); + try { + await writeQueryHistoryToFile([history], historyPath); + const [reloaded] = await readQueryHistoryFromFile(historyPath); + if (reloaded.t !== "variant-analysis") { + throw new Error("Expected a variant analysis history item"); + } + const run = reloaded.variantAnalysis; + expect(run.serverUrl).toBe(SERVER_URL); + expect(run.controllerRepo.htmlUrl).toBe( + history.variantAnalysis.controllerRepo.htmlUrl, + ); + await getVariantAnalysis( + credentials, + run.controllerRepo.id, + run.id, + run.serverUrl, + ); + await getVariantAnalysisRepo( + credentials, + run.controllerRepo.id, + run.id, + 7, + run.serverUrl, + ); + await cancelVariantAnalysis(credentials, run); + expect(fetchSpy.mock.calls.map(([url]) => url)).toEqual([ + `${SERVER_URL}/repositories/0/code-scanning/codeql/variant-analyses/${run.id}`, + `${SERVER_URL}/repositories/0/code-scanning/codeql/variant-analyses/${run.id}/repositories/7`, + `${SERVER_URL}/repositories/0/code-scanning/codeql/variant-analyses/${run.id}/cancel`, + ]); + } finally { + await remove(historyPath); + } + }); + + it.each(["activation", "token"])( + "bounds a stalled provider %s", + async (stage) => { + jest.useFakeTimers(); + try { + if (stage === "activation") { + jest.spyOn(extensions, "getExtension").mockReturnValue( + mockedObject>({ + isActive: false, + activate: () => new Promise(() => {}), + }), + ); + } else { + getVariantAnalysisServerToken.mockReturnValue( + new Promise(() => {}), + ); + } + const request = getVariantAnalysis(credentials, 0, 5, SERVER_URL); + const rejected = expect(request).rejects.toThrow( + /did not respond within 30 seconds/, + ); + await jest.advanceTimersByTimeAsync(30_000); + await rejected; + expect(fetchSpy).not.toHaveBeenCalled(); + expect(jest.getTimerCount()).toBe(0); + } finally { + jest.useRealTimers(); + } + }, + ); + + it.each([401, 403])( + "identifies the provider on HTTP %s without retrying", + async (status) => { + fetchSpy.mockResolvedValue(new Response(null, { status })); + await expect( + getVariantAnalysis(credentials, 0, 5, SERVER_URL), + ).rejects.toThrow( + `Re-authenticate using the token provider '${PROVIDER_ID}'`, + ); + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(getVariantAnalysisServerToken).toHaveBeenCalledTimes(1); + }, + ); + it("submits to the server with the provided token", async () => { const submission = createMockSubmission(); submission.controllerRepoId = 0; @@ -172,11 +284,12 @@ describe("variant analysis custom server", () => { fetchSpy.mockResolvedValue(new Response(null, { status: 202 })); const variantAnalysis = createMockVariantAnalysis({}); variantAnalysis.serverUrl = SERVER_URL; + variantAnalysis.actionsWorkflowRunId = undefined; await cancelVariantAnalysis(credentials, variantAnalysis); expect(fetchSpy).toHaveBeenCalledWith( - `${SERVER_URL}/repos/${variantAnalysis.controllerRepo.fullName}/actions/runs/${variantAnalysis.actionsWorkflowRunId}/cancel`, + `${SERVER_URL}/repositories/${variantAnalysis.controllerRepo.id}/code-scanning/codeql/variant-analyses/${variantAnalysis.id}/cancel`, expect.objectContaining({ method: "POST" }), ); }); From 652b3c188ecdfad6b0ba8328d39247ef42bf0dd5 Mon Sep 17 00:00:00 2001 From: Nicolas Will Date: Thu, 29 Oct 2026 11:16:29 +0100 Subject: [PATCH 4/5] Fix history, logging, and clean up tests --- docs/variant-analysis-custom-server.md | 14 +- extensions/ql-vscode/package.json | 12 +- .../model-alerts/model-alerts-view.ts | 3 +- .../history-tree-data-provider.ts | 19 +- .../query-history/query-history-manager.ts | 26 ++- .../src/variant-analysis/export-results.ts | 66 ++---- .../variant-analysis/gh-api/gh-api-client.ts | 61 ----- .../gh-api/variant-analysis-server-client.ts | 50 ++-- .../shared/variant-analysis.ts | 9 + .../variant-analysis/variant-analysis-api.ts | 122 +++++++--- .../variant-analysis-manager.ts | 6 +- .../variant-analysis/variant-analysis-view.ts | 5 +- .../src/view/model-alerts/ModelAlerts.tsx | 6 +- .../variant-analysis/FailureReasonAlert.tsx | 42 +++- .../view/variant-analysis/VariantAnalysis.tsx | 11 +- .../VariantAnalysisOutcomePanels.tsx | 6 +- .../VariantAnalysisStatusStats.tsx | 4 +- .../__tests__/VariantAnalysis.spec.tsx | 18 ++ .../VariantAnalysisOutcomePanels.spec.tsx | 28 +++ .../__tests__/VariantAnalysisStats.spec.tsx | 2 +- .../VariantAnalysisStatusStats.spec.tsx | 4 +- .../variant-analysis-manager.test.ts | 215 +++++++++++++++++- .../variant-analysis-monitor.test.ts | 8 +- .../variant-analysis-manager.test.ts | 7 +- .../history-tree-data-provider.test.ts | 44 ++++ .../query-history-manager.test.ts | 73 ++++++ .../store/query-history-store.test.ts | 2 + .../variant-analysis}/gh-api-client.test.ts | 4 +- .../variant-analysis-api.test.ts | 136 +++++++---- 29 files changed, 730 insertions(+), 273 deletions(-) rename extensions/ql-vscode/test/{unit-tests/variant-analysis/gh-api => vscode-tests/no-workspace/variant-analysis}/gh-api-client.test.ts (94%) diff --git a/docs/variant-analysis-custom-server.md b/docs/variant-analysis-custom-server.md index 771903bc652..4de91351b66 100644 --- a/docs/variant-analysis-custom-server.md +++ b/docs/variant-analysis-custom-server.md @@ -36,6 +36,7 @@ The extension named in `serverTokenProvider` must return the following from `act ```ts export interface VariantAnalysisServerTokenProvider { getVariantAnalysisServerToken(serverUrl: string): Promise; + openVariantAnalysisLogs?(serverUrl: string, analysisId: number): Promise; } ``` @@ -51,6 +52,15 @@ or token-refresh argument. Only the extension that the user or administrator names is called. Extensions cannot register themselves as providers. +The optional `openVariantAnalysisLogs` hook enables **View logs** in history and result views. +The provider owns log retrieval, authentication, and presentation (for example, opening a browser +or an output channel). CodeQL passes the originating server URL and analysis/session ID, not an +Actions workflow ID. Without the hook the action is hidden. Capability discovery activates the +provider with the same 30-second activation bound; opening logs does not request a token from CodeQL. +If discovery fails, CodeQL logs the error and hides the action without preventing saved results from loading. +The hook should resolve once its viewer is opened and reject on failure. Runs from another configured +server cannot invoke it. No log endpoint or GitHub Actions compatibility is required. + ## Behavior when a server is configured - MRVA is enabled regardless of `github-enterprise.uri`. The GitHub sign-in, GitHub database @@ -60,7 +70,9 @@ themselves as providers. the current server setting differs. Restore the original setting to resume them. Legacy runs without a server URL are treated as GitHub runs. - The GitHub token is never sent to the server, and results are never uploaded to GitHub: - "Export to gist" and "View logs" (GitHub Actions) are unavailable. + "Export to gist" is unavailable; "View logs" uses the companion's optional hook, not GitHub Actions. +- Removing a running server analysis from history offers to cancel it through the server. + Dismissing that offer removes only the local history entry; the server run continues. - Result artifacts are downloaded from `artifact_url` without authentication, as on GitHub. - If a repository task has no `database_commit_sha`, file links point to `HEAD`. These links may not match the analyzed revision. diff --git a/extensions/ql-vscode/package.json b/extensions/ql-vscode/package.json index dddb8d1d4a0..68173c41f09 100644 --- a/extensions/ql-vscode/package.json +++ b/extensions/ql-vscode/package.json @@ -1245,12 +1245,12 @@ { "command": "codeQLQueryHistory.removeHistoryItemContextMenu", "group": "7_queryHistory@0", - "when": "viewItem == interpretedResultsItem || viewItem == rawResultsItem || viewItem == remoteResultsItem || viewItem == cancelledRemoteResultsItemWithoutLogs || viewItem == cancelledResultsItem || viewItem == cancelledRemoteResultsItem" + "when": "viewItem == interpretedResultsItem || viewItem == rawResultsItem || viewItem == remoteResultsItem || viewItem == remoteResultsItemWithoutLogs || viewItem == cancelledRemoteResultsItemWithoutLogs || viewItem == cancelledResultsItem || viewItem == cancelledRemoteResultsItem" }, { "command": "codeQLQueryHistory.removeHistoryItemContextInline", "group": "inline", - "when": "viewItem == interpretedResultsItem || viewItem == rawResultsItem || viewItem == remoteResultsItem || viewItem == cancelledRemoteResultsItemWithoutLogs || viewItem == cancelledResultsItem || viewItem == cancelledRemoteResultsItem" + "when": "viewItem == interpretedResultsItem || viewItem == rawResultsItem || viewItem == remoteResultsItem || viewItem == remoteResultsItemWithoutLogs || viewItem == cancelledRemoteResultsItemWithoutLogs || viewItem == cancelledResultsItem || viewItem == cancelledRemoteResultsItem" }, { "command": "codeQLQueryHistory.renameItem", @@ -1300,7 +1300,7 @@ { "command": "codeQLQueryHistory.exportResults", "group": "1_queryHistory@0", - "when": "view == codeQLQueryHistory && viewItem == remoteResultsItem" + "when": "view == codeQLQueryHistory && (viewItem == remoteResultsItem || viewItem == remoteResultsItemWithoutLogs)" }, { "command": "codeQLQueryHistory.viewCsvResults", @@ -1325,7 +1325,7 @@ { "command": "codeQLQueryHistory.cancel", "group": "9_qlCommands", - "when": "viewItem == inProgressResultsItem || viewItem == inProgressRemoteResultsItem" + "when": "viewItem == inProgressResultsItem || viewItem == inProgressRemoteResultsItem || viewItem == inProgressRemoteResultsItemWithoutLogs" }, { "command": "codeQLQueryHistory.openOnGithub", @@ -1335,12 +1335,12 @@ { "command": "codeQLQueryHistory.copyRepoList", "group": "1_queryHistory@1", - "when": "viewItem == remoteResultsItem" + "when": "viewItem == remoteResultsItem || viewItem == remoteResultsItemWithoutLogs" }, { "command": "codeQLQueryHistory.viewAutofixes", "group": "1_queryHistory@2", - "when": "viewItem == remoteResultsItem && config.codeQL.canary" + "when": "(viewItem == remoteResultsItem || viewItem == remoteResultsItemWithoutLogs) && config.codeQL.canary" }, { "command": "codeQLQueries.runLocalQueryFromQueriesPanel", diff --git a/extensions/ql-vscode/src/model-editor/model-alerts/model-alerts-view.ts b/extensions/ql-vscode/src/model-editor/model-alerts/model-alerts-view.ts index beb08e1e83c..7abf377f47f 100644 --- a/extensions/ql-vscode/src/model-editor/model-alerts/model-alerts-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-alerts/model-alerts-view.ts @@ -22,6 +22,7 @@ import type { import type { AppEvent, AppEventEmitter } from "../../common/events"; import type { ModeledMethod } from "../modeled-method"; import type { MethodSignature } from "../method"; +import { withVariantAnalysisLogCapability } from "../../variant-analysis/variant-analysis-api"; export class ModelAlertsView extends AbstractWebview< ToModelAlertsMessage, @@ -130,7 +131,7 @@ export class ModelAlertsView extends AbstractWebview< await this.postMessage({ t: "setVariantAnalysis", - variantAnalysis, + variantAnalysis: await withVariantAnalysisLogCapability(variantAnalysis), }); } diff --git a/extensions/ql-vscode/src/query-history/history-tree-data-provider.ts b/extensions/ql-vscode/src/query-history/history-tree-data-provider.ts index 8d1f43dd62f..9fe086b1a35 100644 --- a/extensions/ql-vscode/src/query-history/history-tree-data-provider.ts +++ b/extensions/ql-vscode/src/query-history/history-tree-data-provider.ts @@ -7,6 +7,7 @@ import { getLanguage, getQueryId } from "./query-history-info"; import { QueryStatus } from "./query-status"; import type { HistoryItemLabelProvider } from "./history-item-label-provider"; import type { LanguageContextStore } from "../language-context-store"; +import { canViewVariantAnalysisLogs } from "../variant-analysis/variant-analysis-api"; export enum SortOrder { NameAsc = "NameAsc", @@ -122,12 +123,18 @@ export class HistoryTreeDataProvider } private async getContextValue(element: QueryHistoryInfo): Promise { + const hasServerLogs = + element.t === "variant-analysis" && element.variantAnalysis.serverUrl + ? await canViewVariantAnalysisLogs(element.variantAnalysis) + : false; switch (element.status) { case QueryStatus.InProgress: if (element.t === "local") { return "inProgressResultsItem"; + } else if (element.variantAnalysis.serverUrl && !hasServerLogs) { + return "inProgressRemoteResultsItemWithoutLogs"; } else if ( - element.t === "variant-analysis" && + !element.variantAnalysis.serverUrl && element.variantAnalysis.actionsWorkflowRunId === undefined ) { return "pendingRemoteResultsItem"; @@ -140,12 +147,18 @@ export class HistoryTreeDataProvider await element.completedQuery?.query.hasInterpretedResults(); return hasResults ? "interpretedResultsItem" : "rawResultsItem"; } else { - return "remoteResultsItem"; + return element.variantAnalysis.serverUrl && !hasServerLogs + ? "remoteResultsItemWithoutLogs" + : "remoteResultsItem"; } case QueryStatus.Failed: if (element.t === "local") { return "cancelledResultsItem"; - } else if (element.variantAnalysis.actionsWorkflowRunId === undefined) { + } else if ( + element.variantAnalysis.serverUrl + ? !hasServerLogs + : element.variantAnalysis.actionsWorkflowRunId === undefined + ) { return "cancelledRemoteResultsItemWithoutLogs"; } else { return "cancelledRemoteResultsItem"; diff --git a/extensions/ql-vscode/src/query-history/query-history-manager.ts b/extensions/ql-vscode/src/query-history/query-history-manager.ts index 6087816778f..826b3be7f64 100644 --- a/extensions/ql-vscode/src/query-history/query-history-manager.ts +++ b/extensions/ql-vscode/src/query-history/query-history-manager.ts @@ -612,8 +612,6 @@ export class QueryHistoryManager extends DisposableObject { private async removeVariantAnalysis( item: VariantAnalysisHistoryItem, ): Promise { - // We can remove a Variant Analysis locally, but not remotely. - // The user must cancel the query on GitHub Actions explicitly. if (item.status === QueryStatus.InProgress) { const response = await showBinaryChoiceDialog( `You are about to delete this query: ${this.labelProvider.getLabel( @@ -623,12 +621,26 @@ export class QueryHistoryManager extends DisposableObject { if (!response) { return; } + if (item.variantAnalysis.serverUrl) { + const shouldCancel = await showInformationMessageWithAction( + "Removing this query from history will not stop the variant analysis running on the server.", + "Cancel variant analysis", + ); + if (shouldCancel) { + await this.variantAnalysisManager.cancelVariantAnalysis( + item.variantAnalysis.id, + ); + } + } } this.treeDataProvider.remove(item); void this.app.logger.log(`Deleted ${this.labelProvider.getLabel(item)}.`); - if (item.status === QueryStatus.InProgress) { + if ( + item.status === QueryStatus.InProgress && + !item.variantAnalysis.serverUrl + ) { await this.showToastWithWorkflowRunLink(item); } @@ -1043,12 +1055,8 @@ export class QueryHistoryManager extends DisposableObject { if (item.t !== "variant-analysis") { return; } - - const actionsWorkflowRunUrl = getActionsWorkflowRunUrl(item); - - await this.app.commands.execute( - "vscode.open", - Uri.parse(actionsWorkflowRunUrl), + await this.variantAnalysisManager.openVariantAnalysisLogs( + item.variantAnalysis.id, ); } diff --git a/extensions/ql-vscode/src/variant-analysis/export-results.ts b/extensions/ql-vscode/src/variant-analysis/export-results.ts index 01ffd04e421..d4028d70029 100644 --- a/extensions/ql-vscode/src/variant-analysis/export-results.ts +++ b/extensions/ql-vscode/src/variant-analysis/export-results.ts @@ -185,17 +185,25 @@ async function exportVariantAnalysisAnalysisResults( summaries, ); - await exportResults( - exportedResultsPath, - description, - markdownFiles, - exportFormat, - commandManager, - credentials, - progress, - token, - variantAnalysis.serverUrl, - ); + if (exportFormat === "gist") { + await exportToGist( + description, + markdownFiles, + commandManager, + credentials, + progress, + token, + variantAnalysis.serverUrl, + ); + } else { + await exportToLocalMarkdown( + exportedResultsPath, + markdownFiles, + commandManager, + progress, + token, + ); + } } /** @@ -235,42 +243,6 @@ async function determineExportFormat( return undefined; } -async function exportResults( - exportedResultsPath: string, - description: string, - markdownFiles: MarkdownFile[], - exportFormat: "gist" | "local", - commandManager: AppCommandManager, - credentials: Credentials, - progress?: ProgressCallback, - token?: CancellationToken, - serverUrl?: string, -) { - if (token?.isCancellationRequested) { - throw new UserCancellationException("Cancelled"); - } - - if (exportFormat === "gist") { - await exportToGist( - description, - markdownFiles, - commandManager, - credentials, - progress, - token, - serverUrl, - ); - } else if (exportFormat === "local") { - await exportToLocalMarkdown( - exportedResultsPath, - markdownFiles, - commandManager, - progress, - token, - ); - } -} - async function exportToGist( description: string, markdownFiles: MarkdownFile[], diff --git a/extensions/ql-vscode/src/variant-analysis/gh-api/gh-api-client.ts b/extensions/ql-vscode/src/variant-analysis/gh-api/gh-api-client.ts index ea4db34ceb6..84694f7c303 100644 --- a/extensions/ql-vscode/src/variant-analysis/gh-api/gh-api-client.ts +++ b/extensions/ql-vscode/src/variant-analysis/gh-api/gh-api-client.ts @@ -1,66 +1,5 @@ import type { Credentials } from "../../common/authentication"; -import type { VariantAnalysisSubmission } from "../shared/variant-analysis"; -import type { - VariantAnalysis, - VariantAnalysisRepoTask, -} from "./variant-analysis"; import type { Repository } from "./repository"; -import { mapVariantAnalysisSubmission } from "../variant-analysis-mapper"; - -export async function submitVariantAnalysis( - credentials: Credentials, - submissionDetails: VariantAnalysisSubmission, -): Promise { - const octokit = await credentials.getOctokit(); - - const { controllerRepoId } = submissionDetails; - const data = mapVariantAnalysisSubmission(submissionDetails); - - const response = await octokit.request({ - method: "POST", - url: "/repositories/:controllerRepoId/code-scanning/codeql/variant-analyses", - controllerRepoId, - data, - }); - - return response.data; -} - -export async function getVariantAnalysis( - credentials: Credentials, - controllerRepoId: number, - variantAnalysisId: number, -): Promise { - const octokit = await credentials.getOctokit(); - - const response = await octokit.request({ - method: "GET", - url: "/repositories/:controllerRepoId/code-scanning/codeql/variant-analyses/:variantAnalysisId", - controllerRepoId, - variantAnalysisId, - }); - - return response.data; -} - -export async function getVariantAnalysisRepo( - credentials: Credentials, - controllerRepoId: number, - variantAnalysisId: number, - repoId: number, -): Promise { - const octokit = await credentials.getOctokit(); - - const response = await octokit.request({ - method: "GET", - url: "/repositories/:controllerRepoId/code-scanning/codeql/variant-analyses/:variantAnalysisId/repositories/:repoId", - controllerRepoId, - variantAnalysisId, - repoId, - }); - - return response.data; -} export async function getRepositoryFromNwo( credentials: Credentials, diff --git a/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts b/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts index 2317dd1dce9..0374671884c 100644 --- a/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts +++ b/extensions/ql-vscode/src/variant-analysis/gh-api/variant-analysis-server-client.ts @@ -11,6 +11,10 @@ import type { VariantAnalysisServerConfig } from "../../config"; */ export interface VariantAnalysisServerTokenProvider { getVariantAnalysisServerToken(serverUrl: string): Promise; + openVariantAnalysisLogs?( + serverUrl: string, + analysisId: number, + ): Promise; } /** The server does not use controller repositories, so this ID is sent instead. */ @@ -19,11 +23,14 @@ export const SERVER_CONTROLLER_REPO_ID = 0; const MAX_ERROR_BODY_LENGTH = 500; const TOKEN_PROVIDER_TIMEOUT_MS = 30_000; -async function getToken(server: VariantAnalysisServerConfig): Promise { +export async function withVariantAnalysisServerProvider( + server: VariantAnalysisServerConfig, + action: (provider: Partial) => Promise, +): Promise { let timeout: ReturnType | undefined; try { return await Promise.race([ - getTokenFromExtension(server), + activateProvider(server).then(action), new Promise((_, reject) => { timeout = setTimeout(() => { reject( @@ -39,9 +46,27 @@ async function getToken(server: VariantAnalysisServerConfig): Promise { } } -async function getTokenFromExtension( +async function getToken(server: VariantAnalysisServerConfig): Promise { + return withVariantAnalysisServerProvider(server, async (api) => { + if (typeof api?.getVariantAnalysisServerToken !== "function") { + throw new Error( + `The extension '${server.tokenProviderExtensionId}' does not provide a variant analysis server token.`, + ); + } + + const token = await api.getVariantAnalysisServerToken(server.url); + if (typeof token !== "string" || !token) { + throw new Error( + `The extension '${server.tokenProviderExtensionId}' did not return a variant analysis server token. Check that you are signed in.`, + ); + } + return token; + }); +} + +async function activateProvider( server: VariantAnalysisServerConfig, -): Promise { +): Promise> { const extensionId = server.tokenProviderExtensionId; if (!extensionId) { throw new Error( @@ -59,22 +84,7 @@ async function getTokenFromExtension( ); } - const api = extension.isActive - ? extension.exports - : await extension.activate(); - if (typeof api?.getVariantAnalysisServerToken !== "function") { - throw new Error( - `The extension '${extensionId}' does not provide a variant analysis server token.`, - ); - } - - const token = await api.getVariantAnalysisServerToken(server.url); - if (typeof token !== "string" || !token) { - throw new Error( - `The extension '${extensionId}' did not return a variant analysis server token. Check that you are signed in.`, - ); - } - return token; + return extension.isActive ? extension.exports : await extension.activate(); } /** diff --git a/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts b/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts index 5632591bc62..3f7388dfbff 100644 --- a/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts +++ b/extensions/ql-vscode/src/variant-analysis/shared/variant-analysis.ts @@ -7,6 +7,8 @@ export interface VariantAnalysis { id: number; /** Custom server that created this run; absent for GitHub and legacy runs. */ serverUrl?: string; + /** Runtime capability supplied to views, not persisted in query history. */ + canViewLogs?: boolean; controllerRepo: Repository; language: QueryLanguage; query: { @@ -33,6 +35,13 @@ export interface VariantAnalysis { skippedRepos?: VariantAnalysisSkippedRepositories; } +export function hasVariantAnalysisLogs(run: VariantAnalysis): boolean { + return ( + run.canViewLogs ?? + (!run.serverUrl && run.actionsWorkflowRunId !== undefined) + ); +} + export function parseVariantAnalysisQueryLanguage( language: string, ): QueryLanguage | undefined { diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts index f14c942f97a..8d4e48f8cde 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts @@ -1,17 +1,17 @@ import type { Credentials } from "../common/authentication"; +import { getErrorMessage } from "../common/helpers-pure"; +import { extLogger } from "../common/logging/vscode"; import { getVariantAnalysisServerConfig } from "../config"; import { cancelVariantAnalysis as cancelGitHubVariantAnalysis } from "./gh-api/gh-actions-api-client"; -import { - createGist as createGitHubGist, - getVariantAnalysis as getGitHubVariantAnalysis, - getVariantAnalysisRepo as getGitHubVariantAnalysisRepo, - submitVariantAnalysis as submitGitHubVariantAnalysis, -} from "./gh-api/gh-api-client"; +import { createGist as createGitHubGist } from "./gh-api/gh-api-client"; import type { VariantAnalysis as ApiVariantAnalysis, VariantAnalysisRepoTask as ApiVariantAnalysisRepoTask, } from "./gh-api/variant-analysis"; -import { requestVariantAnalysisServer } from "./gh-api/variant-analysis-server-client"; +import { + withVariantAnalysisServerProvider, + requestVariantAnalysisServer, +} from "./gh-api/variant-analysis-server-client"; import { mapVariantAnalysisSubmission } from "./variant-analysis-mapper"; import type { VariantAnalysis, @@ -28,6 +28,21 @@ function variantAnalysesPath(controllerRepoId: number): string { return `/repositories/${controllerRepoId}/code-scanning/codeql/variant-analyses`; } +async function request( + credentials: Credentials, + serverUrl: string | undefined, + method: "GET" | "POST", + path: string, + body?: unknown, +): Promise { + const server = getServerForVariantAnalysis(serverUrl); + if (server) { + return requestVariantAnalysisServer(server, method, path, body); + } + const octokit = await credentials.getOctokit(); + return (await octokit.request({ method, url: path, data: body })).data; +} + export function getServerForVariantAnalysis(serverUrl: string | undefined) { const server = getVariantAnalysisServerConfig(); if (server?.url !== serverUrl) { @@ -38,17 +53,67 @@ export function getServerForVariantAnalysis(serverUrl: string | undefined) { return server; } +export async function canViewVariantAnalysisLogs( + run: VariantAnalysis, +): Promise { + if (!run.serverUrl) { + return run.actionsWorkflowRunId !== undefined; + } + try { + return (await getServerLogOpener(run)) !== undefined; + } catch (error) { + void extLogger.log( + `Unable to discover variant analysis logs: ${getErrorMessage(error)}`, + ); + return false; + } +} + +async function getServerLogOpener( + run: VariantAnalysis, +): Promise<(() => Promise) | undefined> { + const server = getVariantAnalysisServerConfig(); + if ( + !run.serverUrl || + server?.url !== run.serverUrl || + !server.tokenProviderExtensionId + ) { + return undefined; + } + return withVariantAnalysisServerProvider(server, async (provider) => { + const openLogs = provider?.openVariantAnalysisLogs; + return typeof openLogs === "function" + ? () => openLogs.call(provider, server.url, run.id) + : undefined; + }); +} + +export async function withVariantAnalysisLogCapability( + run: VariantAnalysis, +): Promise { + return { ...run, canViewLogs: await canViewVariantAnalysisLogs(run) }; +} + +export async function openServerVariantAnalysisLogs( + run: VariantAnalysis, +): Promise { + getServerForVariantAnalysis(run.serverUrl); + const openLogs = await getServerLogOpener(run); + if (!openLogs) { + throw new Error( + "The configured variant analysis provider does not support viewing logs.", + ); + } + await openLogs(); +} + export async function submitVariantAnalysis( credentials: Credentials, submission: VariantAnalysisSubmission, ): Promise { - const server = getServerForVariantAnalysis(submission.serverUrl); - if (!server) { - return submitGitHubVariantAnalysis(credentials, submission); - } - - return requestVariantAnalysisServer( - server, + return request( + credentials, + submission.serverUrl, "POST", variantAnalysesPath(submission.controllerRepoId), mapVariantAnalysisSubmission(submission), @@ -61,17 +126,9 @@ export async function getVariantAnalysis( variantAnalysisId: number, serverUrl?: string, ): Promise { - const server = getServerForVariantAnalysis(serverUrl); - if (!server) { - return getGitHubVariantAnalysis( - credentials, - controllerRepoId, - variantAnalysisId, - ); - } - - return requestVariantAnalysisServer( - server, + return request( + credentials, + serverUrl, "GET", `${variantAnalysesPath(controllerRepoId)}/${variantAnalysisId}`, ); @@ -84,18 +141,9 @@ export async function getVariantAnalysisRepo( repoId: number, serverUrl?: string, ): Promise { - const server = getServerForVariantAnalysis(serverUrl); - if (!server) { - return getGitHubVariantAnalysisRepo( - credentials, - controllerRepoId, - variantAnalysisId, - repoId, - ); - } - - return requestVariantAnalysisServer( - server, + return request( + credentials, + serverUrl, "GET", `${variantAnalysesPath(controllerRepoId)}/${variantAnalysisId}/repositories/${repoId}`, ); diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts index 29dce159121..75921103a8d 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-manager.ts @@ -4,6 +4,7 @@ import { cancelVariantAnalysis, submitVariantAnalysis, getVariantAnalysisRepo, + openServerVariantAnalysisLogs, } from "./variant-analysis-api"; import type { VariantAnalysis as ApiVariantAnalysis } from "./gh-api/variant-analysis"; import type { @@ -965,9 +966,8 @@ export class VariantAnalysisManager } if (variantAnalysis.serverUrl) { - throw new Error( - "Viewing logs is not supported when using a custom variant analysis server.", - ); + await openServerVariantAnalysisLogs(variantAnalysis); + return; } const actionsWorkflowRunUrl = getActionsWorkflowRunUrl( diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts index 555a3686e68..44ffa488f55 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts @@ -22,6 +22,7 @@ import type { import { telemetryListener } from "../common/vscode/telemetry"; import { redactableError } from "../common/errors"; import { DataFlowPathsView } from "./data-flow-paths-view"; +import { withVariantAnalysisLogCapability } from "./variant-analysis-api"; import type { DataFlowPaths } from "./shared/data-flow-paths"; import type { App } from "../common/app"; import { @@ -70,7 +71,7 @@ export class VariantAnalysisView await this.postMessage({ t: "setVariantAnalysis", - variantAnalysis, + variantAnalysis: await withVariantAnalysisLogCapability(variantAnalysis), }); const panel = await this.getPanel(); @@ -212,7 +213,7 @@ export class VariantAnalysisView await this.postMessage({ t: "setVariantAnalysis", - variantAnalysis, + variantAnalysis: await withVariantAnalysisLogCapability(variantAnalysis), }); await this.postMessage({ diff --git a/extensions/ql-vscode/src/view/model-alerts/ModelAlerts.tsx b/extensions/ql-vscode/src/view/model-alerts/ModelAlerts.tsx index 70169f0a9f4..58c68aef566 100644 --- a/extensions/ql-vscode/src/view/model-alerts/ModelAlerts.tsx +++ b/extensions/ql-vscode/src/view/model-alerts/ModelAlerts.tsx @@ -19,6 +19,7 @@ import { import type { ModelAlertsFilterSortState } from "../../model-editor/shared/model-alerts-filter-sort"; import type { ModeledMethod } from "../../model-editor/modeled-method"; import { useMessageFromExtension } from "../common/useMessageFromExtension"; +import { hasVariantAnalysisLogs } from "../../variant-analysis/shared/variant-analysis"; type Props = { initialViewState?: ModelAlertsViewState; @@ -118,8 +119,9 @@ export function ModelAlerts({ }); }; - const onViewLogsClick = - variantAnalysis.actionsWorkflowRunId === undefined ? undefined : openLogs; + const onViewLogsClick = hasVariantAnalysisLogs(variantAnalysis) + ? openLogs + : undefined; return ( <> diff --git a/extensions/ql-vscode/src/view/variant-analysis/FailureReasonAlert.tsx b/extensions/ql-vscode/src/view/variant-analysis/FailureReasonAlert.tsx index 1b6c631dbb7..9c380e5a054 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/FailureReasonAlert.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/FailureReasonAlert.tsx @@ -6,6 +6,8 @@ import { Link } from "../common/Link"; type Props = { failureReason: VariantAnalysisFailureReason; + serverUrl?: string; + canViewLogs?: boolean; }; const openLogs = () => { @@ -14,27 +16,45 @@ const openLogs = () => { }); }; -const getTitle = (failureReason: VariantAnalysisFailureReason): string => { +const getTitle = ( + failureReason: VariantAnalysisFailureReason, + serverUrl?: string, +): string => { switch (failureReason) { case VariantAnalysisFailureReason.NoReposQueried: return "No repositories to analyze"; case VariantAnalysisFailureReason.ActionsWorkflowRunFailed: - return "GitHub Actions workflow run failed"; + return serverUrl + ? "Variant analysis failed" + : "GitHub Actions workflow run failed"; case VariantAnalysisFailureReason.InternalError: return "Something unexpected happened"; } }; -const getMessage = (failureReason: VariantAnalysisFailureReason): ReactNode => { +const getMessage = ( + failureReason: VariantAnalysisFailureReason, + serverUrl?: string, + canViewLogs?: boolean, +): ReactNode => { switch (failureReason) { case VariantAnalysisFailureReason.NoReposQueried: return "No repositories available after processing. No repositories were analyzed."; case VariantAnalysisFailureReason.ActionsWorkflowRunFailed: return ( <> - The GitHub Actions workflow run has failed.{" "} - View actions logs and try running this - query again. + {serverUrl + ? "The variant analysis run has failed." + : "The GitHub Actions workflow run has failed."}{" "} + {(canViewLogs ?? !serverUrl) && ( + <> + + {serverUrl ? "View logs" : "View actions logs"} + {" "} + and{" "} + + )} + try running this query again. ); case VariantAnalysisFailureReason.InternalError: @@ -42,12 +62,16 @@ const getMessage = (failureReason: VariantAnalysisFailureReason): ReactNode => { } }; -export const FailureReasonAlert = ({ failureReason }: Props) => { +export const FailureReasonAlert = ({ + failureReason, + serverUrl, + canViewLogs, +}: Props) => { return ( ); }; diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx index 3ac0599ae6c..650006acd5c 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysis.tsx @@ -5,7 +5,10 @@ import type { VariantAnalysisScannedRepositoryResult, VariantAnalysisScannedRepositoryState, } from "../../variant-analysis/shared/variant-analysis"; -import { VariantAnalysisStatus } from "../../variant-analysis/shared/variant-analysis"; +import { + VariantAnalysisStatus, + hasVariantAnalysisLogs, +} from "../../variant-analysis/shared/variant-analysis"; import { VariantAnalysisHeader } from "./VariantAnalysisHeader"; import { VariantAnalysisOutcomePanels } from "./VariantAnalysisOutcomePanels"; import { VariantAnalysisLoading } from "./VariantAnalysisLoading"; @@ -151,13 +154,15 @@ export function VariantAnalysis({ if ( variantAnalysis === undefined || (variantAnalysis.status === VariantAnalysisStatus.InProgress && + !variantAnalysis.serverUrl && variantAnalysis.actionsWorkflowRunId === undefined) ) { return ; } - const onViewLogsClick = - variantAnalysis.actionsWorkflowRunId === undefined ? undefined : openLogs; + const onViewLogsClick = hasVariantAnalysisLogs(variantAnalysis) + ? openLogs + : undefined; return ( <> diff --git a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisOutcomePanels.tsx b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisOutcomePanels.tsx index 1f138ffd8cd..8f766aedb27 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisOutcomePanels.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/VariantAnalysisOutcomePanels.tsx @@ -102,7 +102,11 @@ export const VariantAnalysisOutcomePanels = ({ )} {variantAnalysis.status === VariantAnalysisStatus.Failed && variantAnalysis.failureReason && ( - + )} {overLimitRepositoryCount > 0 && ( {completedAt !== undefined ? formatDate(completedAt) : "-"} )} - {onViewLogsClick && ( - View actions logs - )} + {onViewLogsClick && View logs} ); }; diff --git a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysis.spec.tsx b/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysis.spec.tsx index 3279ed446d2..34f4b454f86 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysis.spec.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysis.spec.tsx @@ -34,6 +34,24 @@ describe(VariantAnalysis.name, () => { ).toBeInTheDocument(); }); + it.each([true, false])( + "shows a server run without a workflow ID (logs supported: %s)", + (canViewLogs) => { + render({ + variantAnalysis: { + ...createMockVariantAnalysis({}), + serverUrl: "https://mrva.example.com", + actionsWorkflowRunId: undefined, + canViewLogs, + }, + }); + expect( + screen.queryByText("We are getting everything ready"), + ).not.toBeInTheDocument(); + expect(screen.queryByText("View logs") !== null).toBe(canViewLogs); + }, + ); + it("renders an analysis where there were no repos to analyse", () => { const variantAnalysis = createMockVariantAnalysis({ status: VariantAnalysisStatus.Failed, diff --git a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisOutcomePanels.spec.tsx b/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisOutcomePanels.spec.tsx index 0ac4ab23df3..09347f56fef 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisOutcomePanels.spec.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisOutcomePanels.spec.tsx @@ -95,6 +95,34 @@ describe(VariantAnalysisOutcomePanels.name, () => { expect(screen.getByText("Analyzed")).toBeInTheDocument(); }); + it("offers companion logs for a failed server run when supported", () => { + render({ + serverUrl: "https://mrva.example.com", + canViewLogs: true, + status: VariantAnalysisStatus.Failed, + failureReason: VariantAnalysisFailureReason.ActionsWorkflowRunFailed, + }); + expect(screen.getByText("View logs")).toBeInTheDocument(); + expect(screen.queryByText("View actions logs")).not.toBeInTheDocument(); + }); + + it.each([undefined, "https://mrva.example.com"])( + "only offers Actions logs for GitHub runs (server: %s)", + (serverUrl) => { + render({ + serverUrl, + status: VariantAnalysisStatus.Failed, + failureReason: VariantAnalysisFailureReason.ActionsWorkflowRunFailed, + }); + if (serverUrl) { + expect(screen.queryByText("View actions logs")).not.toBeInTheDocument(); + expect(screen.getByText(/Variant analysis failed/)).toBeInTheDocument(); + } else { + expect(screen.getByText("View actions logs")).toBeInTheDocument(); + } + }, + ); + it("does not render panels without skipped repos", () => { render({ skippedRepos: undefined, diff --git a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStats.spec.tsx b/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStats.spec.tsx index e3a9f3a4abf..0c54d4557c2 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStats.spec.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStats.spec.tsx @@ -144,7 +144,7 @@ describe(VariantAnalysisStats.name, () => { completedAt: new Date(), }); - await userEvent.click(screen.getByText("View actions logs")); + await userEvent.click(screen.getByText("View logs")); expect(onViewLogsClick).toHaveBeenCalledTimes(1); }); diff --git a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStatusStats.spec.tsx b/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStatusStats.spec.tsx index b6e363ed2c7..737b6c60372 100644 --- a/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStatusStats.spec.tsx +++ b/extensions/ql-vscode/src/view/variant-analysis/__tests__/VariantAnalysisStatusStats.spec.tsx @@ -51,7 +51,7 @@ describe(VariantAnalysisStatusStats.name, () => { onViewLogsClick: () => undefined, }); - expect(screen.getByText("View actions logs")).toBeInTheDocument(); + expect(screen.getByText("View logs")).toBeInTheDocument(); }); it("renders when there isn't a viewLogs links", () => { @@ -60,6 +60,6 @@ describe(VariantAnalysisStatusStats.name, () => { onViewLogsClick: undefined, }); - expect(screen.queryByText("View actions logs")).not.toBeInTheDocument(); + expect(screen.queryByText("View logs")).not.toBeInTheDocument(); }); }); diff --git a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts index 98c3e0341e5..2049fb60c27 100644 --- a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-manager.test.ts @@ -1,7 +1,13 @@ -import type { TextDocument, TextEditor, Uri } from "vscode"; -import { commands, env, window, workspace } from "vscode"; +import type { Extension, TextDocument, TextEditor, Uri } from "vscode"; +import { + commands, + ConfigurationTarget, + env, + extensions, + window, + workspace, +} from "vscode"; import { extLogger } from "../../../../src/common/logging/vscode"; -import * as ghApiClient from "../../../../src/variant-analysis/gh-api/gh-api-client"; import * as ghActionsApiClient from "../../../../src/variant-analysis/gh-api/gh-actions-api-client"; import * as variantAnalysisApi from "../../../../src/variant-analysis/variant-analysis-api"; import { @@ -52,6 +58,15 @@ import { setupServer } from "msw/node"; import type { RequestHandler } from "msw"; import { http } from "msw"; import { getErrorMessage } from "../../../../src/common/helpers-pure"; +import { createMockSubmission } from "../../../factories/variant-analysis/shared/variant-analysis-submission"; +import { createMockApiResponse } from "../../../factories/variant-analysis/gh-api/variant-analysis-api-response"; +import { mapVariantAnalysisFromSubmission } from "../../../../src/variant-analysis/variant-analysis-mapper"; +import { VariantAnalysisMonitor } from "../../../../src/variant-analysis/variant-analysis-monitor"; +import { createMockVariantAnalysisHistoryItem } from "../../../factories/query-history/variant-analysis-history-item"; +import { + readQueryHistoryFromFile, + writeQueryHistoryToFile, +} from "../../../../src/query-history/store/query-history-store"; // up to 3 minutes per test jest.setTimeout(3 * 60 * 1000); @@ -118,6 +133,169 @@ describe("Variant Analysis Manager", () => { ); }); + it("runs the custom-server lifecycle through persisted history and downloaded results", async () => { + const serverUrl = "https://mrva.example.com"; + const artifactUrl = "https://artifacts.example.com/results.zip"; + const sourceUrl = "https://source.example.com/example/repo"; + const config = workspace.getConfiguration("codeQL.variantAnalysis"); + await config.update("serverUrl", serverUrl, ConfigurationTarget.Global); + await config.update( + "serverTokenProvider", + "example.provider", + ConfigurationTarget.Global, + ); + const token = jest.fn().mockResolvedValue("server-token"); + const activate = jest.fn().mockResolvedValue({ + getVariantAnalysisServerToken: token, + }); + const extensionLookup = extensions.getExtension.bind(extensions); + jest + .spyOn(extensions, "getExtension") + .mockImplementation((id) => + id === "example.provider" + ? mockedObject>({ isActive: false, activate }) + : extensionLookup(id), + ); + const github = jest + .spyOn(app.credentials, "getOctokit") + .mockRejectedValue( + new Error("Custom-server requests must not use GitHub authentication"), + ); + const response = createMockApiResponse("in_progress", []); + response.controller_repo.id = 0; + delete response.actions_workflow_run_id; + const task = createMockVariantAnalysisRepoTask(); + task.artifact_url = artifactUrl; + task.repository.html_url = sourceUrl; + const apiPath = `${serverUrl}/repositories/0/code-scanning/codeql/variant-analyses`; + const runPath = `${apiPath}/${response.id}`; + let polls = 0; + server.resetHandlers( + http.post(apiPath, () => Response.json(response)), + http.get(runPath, () => + Response.json( + ++polls === 1 + ? response + : { + ...response, + status: "succeeded", + scanned_repositories: [ + { + repository: task.repository, + analysis_status: "succeeded", + result_count: 1, + }, + ], + }, + ), + ), + http.get(`${runPath}/repositories/${task.repository.id}`, () => + Response.json(task), + ), + http.get( + artifactUrl, + async () => + new Response( + await readFile( + join(__dirname, "data/variant-analysis-results.zip"), + ), + ), + ), + ); + const submission = { + ...createMockSubmission(), + serverUrl, + controllerRepoId: 0, + }; + const storage = variantAnalysisManager.getVariantAnalysisStorageLocation( + response.id, + ); + const historyPath = join(storage, "history.json"); + const sleepTime = VariantAnalysisMonitor.sleepTime; + VariantAnalysisMonitor.sleepTime = 1; + const downloads: Array> = []; + jest + .spyOn(app.commands, "execute") + .mockImplementation(async (command, ...args) => { + if (command === "codeQL.autoDownloadVariantAnalysisResult") { + const [repo, run] = args as [ + VariantAnalysisScannedRepository, + VariantAnalysis, + ]; + const download = + variantAnalysisManager.autoDownloadVariantAnalysisResult(repo, run); + downloads.push(download); + await download; + } + }); + try { + const submitted = await variantAnalysisApi.submitVariantAnalysis( + app.credentials, + submission, + ); + const run = mapVariantAnalysisFromSubmission(submission, submitted, []); + await createTimestampFile(storage); + await writeQueryHistoryToFile( + [createMockVariantAnalysisHistoryItem({ variantAnalysis: run })], + historyPath, + ); + const [saved] = await readQueryHistoryFromFile(historyPath); + if (saved.t !== "variant-analysis") { + throw new Error("Expected persisted server run"); + } + await variantAnalysisManager.rehydrateVariantAnalysis( + saved.variantAnalysis, + ); + expect( + variantAnalysisManager.tryGetVariantAnalysis(run.id)?.serverUrl, + ).toBe(serverUrl); + await variantAnalysisManager.monitorVariantAnalysis( + saved.variantAnalysis, + ); + await Promise.all(downloads); + expect(downloads).toHaveLength(1); + expect(variantAnalysisManager.tryGetVariantAnalysis(run.id)?.status).toBe( + VariantAnalysisStatus.Succeeded, + ); + expect(variantAnalysisManager.getRepoStates(run.id)).toEqual([ + expect.objectContaining({ + downloadStatus: + VariantAnalysisScannedRepositoryDownloadStatus.Succeeded, + }), + ]); + const results = await variantAnalysisManager.loadResults( + run.id, + task.repository.full_name, + ); + expect(results.interpretedResults?.length).toBeGreaterThan(0); + expect(results.interpretedResults?.[0].fileLink.fileLinkPrefix).toBe( + `${sourceUrl}/blob/HEAD`, + ); + expect( + requests.map((request) => `${request.method} ${request.url}`), + ).toEqual([ + `POST ${apiPath}`, + `GET ${runPath}`, + `GET ${runPath}`, + `GET ${runPath}/repositories/${task.repository.id}`, + `GET ${artifactUrl}`, + ]); + for (const request of requests) { + expect(request.headers.get("authorization")).toBe( + request.url === artifactUrl ? null : "Bearer server-token", + ); + } + expect(activate).toHaveBeenCalled(); + expect(github).not.toHaveBeenCalled(); + } finally { + VariantAnalysisMonitor.sleepTime = sleepTime; + await Promise.allSettled(downloads); + await remove(storage); + variantAnalysisManager.dispose(); + variantAnalysisResultsManager.dispose(); + } + }); + describe("rehydrateVariantAnalysis", () => { const variantAnalysis = createMockVariantAnalysis({}); @@ -198,14 +376,14 @@ describe("Variant Analysis Manager", () => { describe("autoDownloadVariantAnalysisResult", () => { let getVariantAnalysisRepoStub: jest.SpiedFunction< - typeof ghApiClient.getVariantAnalysisRepo + typeof variantAnalysisApi.getVariantAnalysisRepo >; let repoStatesPath: string; beforeEach(async () => { getVariantAnalysisRepoStub = jest.spyOn( - ghApiClient, + variantAnalysisApi, "getVariantAnalysisRepo", ); @@ -655,6 +833,33 @@ describe("Variant Analysis Manager", () => { ); }); + it("delegates custom-server logs without requiring a workflow ID", async () => { + const openLogs = jest + .spyOn(variantAnalysisApi, "openServerVariantAnalysisLogs") + .mockResolvedValue(undefined); + const serverRun = { + ...variantAnalysis, + serverUrl: "https://mrva.example.com", + actionsWorkflowRunId: undefined, + }; + await variantAnalysisManager.onVariantAnalysisUpdated(serverRun); + await variantAnalysisManager.openVariantAnalysisLogs(serverRun.id); + expect(openLogs).toHaveBeenCalledWith(serverRun); + }); + + it("opens the GitHub workflow URL for GitHub logs", async () => { + const open = jest + .spyOn(commands, "executeCommand") + .mockResolvedValue(undefined); + await variantAnalysisManager.openVariantAnalysisLogs(variantAnalysis.id); + expect(open).toHaveBeenCalledWith( + "vscode.open", + expect.objectContaining({ + path: `/${variantAnalysis.controllerRepo.fullName}/actions/runs/${variantAnalysis.actionsWorkflowRunId}`, + }), + ); + }); + it("allows custom-server cancellation without an Actions workflow run ID", async () => { const cancel = jest .spyOn(variantAnalysisApi, "cancelVariantAnalysis") diff --git a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts index 736df7daf4f..1b8d550700a 100644 --- a/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/activated-extension/variant-analysis/variant-analysis-monitor.test.ts @@ -1,4 +1,4 @@ -import * as ghApiClient from "../../../../src/variant-analysis/gh-api/gh-api-client"; +import * as variantAnalysisApi from "../../../../src/variant-analysis/variant-analysis-api"; import { RequestError } from "@octokit/request-error"; import { VariantAnalysisMonitor } from "../../../../src/variant-analysis/variant-analysis-monitor"; import type { @@ -28,7 +28,7 @@ jest.setTimeout(60_000); describe("Variant Analysis Monitor", () => { let mockGetVariantAnalysisFromApi: jest.SpiedFunction< - typeof ghApiClient.getVariantAnalysis + typeof variantAnalysisApi.getVariantAnalysis >; let variantAnalysisMonitor: VariantAnalysisMonitor; let shouldCancelMonitor: jest.Mock, [number]>; @@ -61,7 +61,7 @@ describe("Variant Analysis Monitor", () => { variantAnalysisMonitor.onVariantAnalysisChange(onVariantAnalysisChangeSpy); mockGetVariantAnalysisFromApi = jest - .spyOn(ghApiClient, "getVariantAnalysis") + .spyOn(variantAnalysisApi, "getVariantAnalysis") .mockRejectedValue(new Error("Not mocked")); mockGetVariantAnalysis.mockReturnValue(variantAnalysis); @@ -344,7 +344,7 @@ describe("Variant Analysis Monitor", () => { describe("cancelation", () => { it("should maintain canceling status", async () => { - mockGetVariantAnalysis.mockReturnValueOnce({ + mockGetVariantAnalysis.mockReturnValue({ ...variantAnalysis, status: VariantAnalysisStatus.Canceling, }); diff --git a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts index 62a8af2b61f..557330e4095 100644 --- a/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/cli-integration/variant-analysis/variant-analysis-manager.test.ts @@ -11,6 +11,7 @@ import { setRemoteControllerRepo, } from "../../../../src/config"; import * as ghApiClient from "../../../../src/variant-analysis/gh-api/gh-api-client"; +import * as variantAnalysisApi from "../../../../src/variant-analysis/variant-analysis-api"; import { isAbsolute, join } from "path"; import { VariantAnalysisManager } from "../../../../src/variant-analysis/variant-analysis-manager"; @@ -77,7 +78,7 @@ describe("Variant Analysis Manager", () => { typeof ghApiClient.getRepositoryFromNwo >; let mockSubmitVariantAnalysis: jest.SpiedFunction< - typeof ghApiClient.submitVariantAnalysis + typeof variantAnalysisApi.submitVariantAnalysis >; let mockApiResponse: VariantAnalysisApiResponse; let executeCommandSpy: jest.SpiedFunction; @@ -106,7 +107,7 @@ describe("Variant Analysis Manager", () => { mockApiResponse = createMockApiResponse("in_progress"); mockSubmitVariantAnalysis = jest - .spyOn(ghApiClient, "submitVariantAnalysis") + .spyOn(variantAnalysisApi, "submitVariantAnalysis") .mockResolvedValue(mockApiResponse); executeCommandSpy = jest.spyOn(commands, "executeCommand"); @@ -255,7 +256,7 @@ describe("Variant Analysis Manager", () => { describe("check variant analysis generated packs", () => { beforeEach(() => { mockSubmitVariantAnalysis = jest - .spyOn(ghApiClient, "submitVariantAnalysis") + .spyOn(variantAnalysisApi, "submitVariantAnalysis") .mockResolvedValue({ id: 1, query_language: QueryLanguage.Javascript, diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/history-tree-data-provider.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/history-tree-data-provider.test.ts index a0305761e91..be0b390bfa0 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/history-tree-data-provider.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/history-tree-data-provider.test.ts @@ -32,6 +32,7 @@ import { createMockApp } from "../../../__mocks__/appMock"; import { LanguageContextStore } from "../../../../src/language-context-store"; import type { App } from "../../../../src/common/app"; import { QueryLanguage } from "../../../../src/common/query-language"; +import * as variantAnalysisApi from "../../../../src/variant-analysis/variant-analysis-api"; describe("HistoryTreeDataProvider", () => { const mockExtensionLocation = join(tmpDir.name, "mock-extension-location"); @@ -144,6 +145,49 @@ describe("HistoryTreeDataProvider", () => { }); describe("getTreeItem", () => { + describe.each([ + [ + QueryStatus.InProgress, + VariantAnalysisStatus.InProgress, + "inProgressRemoteResultsItem", + ], + [ + QueryStatus.Completed, + VariantAnalysisStatus.Succeeded, + "remoteResultsItem", + ], + [ + QueryStatus.Failed, + VariantAnalysisStatus.Failed, + "cancelledRemoteResultsItem", + ], + ])( + "server run with status %s", + (historyItemStatus, variantAnalysisStatus, context) => { + it.each([ + [true, undefined], + [false, undefined], + [false, 123], + ] as const)( + "uses log capability %s rather than workflow ID %s", + async (canViewLogs, workflowId) => { + jest + .spyOn(variantAnalysisApi, "canViewVariantAnalysisLogs") + .mockResolvedValue(canViewLogs); + const item = createMockVariantAnalysisHistoryItem({ + historyItemStatus, + variantAnalysisStatus, + }); + item.variantAnalysis.serverUrl = "https://mrva.example.com"; + item.variantAnalysis.actionsWorkflowRunId = workflowId; + expect( + (await historyTreeDataProvider.getTreeItem(item)).contextValue, + ).toBe(`${context}${canViewLogs ? "" : "WithoutLogs"}`); + }, + ); + }, + ); + it("should get a tree item with raw results", async () => { const mockQueryWithRawResults = createMockLocalQueryInfo({ dbName: "a", diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts index 38eb471c918..93556d61338 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/query-history-manager.test.ts @@ -79,6 +79,7 @@ describe("QueryHistoryManager", () => { onVariantAnalysisRemoved: jest.fn(), removeVariantAnalysis: jest.fn(), cancelVariantAnalysis: jest.fn(), + openVariantAnalysisLogs: jest.fn(), exportResults: jest.fn(), showView: jest.fn(), } as any as VariantAnalysisManager; @@ -148,6 +149,25 @@ describe("QueryHistoryManager", () => { } }); + describe("handleOpenOnGithub", () => { + it.each([undefined, "https://mrva.example.com"])( + "delegates log viewing without opening a workflow URL directly: %s", + async (serverUrl) => { + queryHistoryManager = await createMockQueryHistory(allHistory); + const item = variantAnalysisHistory[0]; + item.variantAnalysis.serverUrl = serverUrl; + await queryHistoryManager.handleOpenOnGithub(item); + expect( + variantAnalysisManagerStub.openVariantAnalysisLogs, + ).toHaveBeenCalledWith(item.variantAnalysis.id); + expect(executeCommand).not.toHaveBeenCalledWith( + "vscode.open", + expect.anything(), + ); + }, + ); + }); + describe("handleItemClicked", () => { describe("single click", () => { describe("local query", () => { @@ -408,6 +428,59 @@ describe("QueryHistoryManager", () => { expect(showInformationMessageWithActionSpy).toHaveBeenCalled(); }); + it.each([true, false])( + "offers server cancellation before removing history (cancel: %s)", + async (shouldCancel) => { + toDelete.variantAnalysis.serverUrl = "https://mrva.example.com"; + toDelete.variantAnalysis.actionsWorkflowRunId = undefined; + showInformationMessageWithActionSpy.mockResolvedValue( + shouldCancel, + ); + + await queryHistoryManager.handleRemoveHistoryItem([toDelete]); + + expect(showInformationMessageWithActionSpy).toHaveBeenCalledWith( + "Removing this query from history will not stop the variant analysis running on the server.", + "Cancel variant analysis", + ); + if (shouldCancel) { + expect(cancelVariantAnalysisSpy).toHaveBeenCalledWith( + toDelete.variantAnalysis.id, + ); + expect( + cancelVariantAnalysisSpy.mock.invocationCallOrder[0], + ).toBeLessThan( + jest.mocked(variantAnalysisManagerStub.removeVariantAnalysis) + .mock.invocationCallOrder[0], + ); + } else { + expect(cancelVariantAnalysisSpy).not.toHaveBeenCalled(); + } + expect( + queryHistoryManager.treeDataProvider.allHistory, + ).not.toContain(toDelete); + }, + ); + + it("keeps a server run in history if cancellation fails", async () => { + toDelete.variantAnalysis.serverUrl = "https://mrva.example.com"; + showInformationMessageWithActionSpy.mockResolvedValue(true); + cancelVariantAnalysisSpy.mockRejectedValue( + new Error("Server unavailable"), + ); + + await expect( + queryHistoryManager.handleRemoveHistoryItem([toDelete]), + ).rejects.toThrow("Server unavailable"); + + expect(queryHistoryManager.treeDataProvider.allHistory).toContain( + toDelete, + ); + expect( + variantAnalysisManagerStub.removeVariantAnalysis, + ).not.toHaveBeenCalled(); + }); + describe("when you choose 'No' in the 'Are you sure?' modal", () => { beforeEach(async () => { showBinaryChoiceDialogSpy.mockResolvedValue(false); diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/store/query-history-store.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/store/query-history-store.test.ts index 2b940dab40f..480de5e04d8 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/store/query-history-store.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/store/query-history-store.test.ts @@ -112,6 +112,8 @@ describe("write and read", () => { it("preserves server origins and repository URLs without changing legacy history", async () => { const serverRun = createMockVariantAnalysisHistoryItem({}); serverRun.variantAnalysis.serverUrl = "https://mrva.example.com"; + serverRun.variantAnalysis.controllerRepo.htmlUrl = + "https://source.example.com/org/controller"; serverRun.variantAnalysis.scannedRepos![0].repository.htmlUrl = "https://source.example.com/org/repo"; const legacyRun = createMockVariantAnalysisHistoryItem({}); diff --git a/extensions/ql-vscode/test/unit-tests/variant-analysis/gh-api/gh-api-client.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/gh-api-client.test.ts similarity index 94% rename from extensions/ql-vscode/test/unit-tests/variant-analysis/gh-api/gh-api-client.test.ts rename to extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/gh-api-client.test.ts index 85a956a5e8c..89aab88a31f 100644 --- a/extensions/ql-vscode/test/unit-tests/variant-analysis/gh-api/gh-api-client.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/gh-api-client.test.ts @@ -1,9 +1,9 @@ import { - getRepositoryFromNwo, getVariantAnalysis, getVariantAnalysisRepo, submitVariantAnalysis, -} from "../../../../src/variant-analysis/gh-api/gh-api-client"; +} from "../../../../src/variant-analysis/variant-analysis-api"; +import { getRepositoryFromNwo } from "../../../../src/variant-analysis/gh-api/gh-api-client"; import { createMockSubmission } from "../../../factories/variant-analysis/shared/variant-analysis-submission"; import { MockGitHubApiServer } from "../../../../src/common/mock-gh-api/mock-gh-api-server"; diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts index ce1789eefa4..e7e020655a0 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts @@ -6,6 +6,9 @@ import { } from "../../../../src/config"; import { cancelVariantAnalysis, + canViewVariantAnalysisLogs, + openServerVariantAnalysisLogs, + withVariantAnalysisLogCapability, createGist, getVariantAnalysis, getVariantAnalysisRepo, @@ -18,14 +21,7 @@ import { createMockVariantAnalysis } from "../../../factories/variant-analysis/s import { mockedObject } from "../../utils/mocking.helpers"; import { testCredentialsWithStub } from "../../../factories/authentication"; import type { VariantAnalysisServerTokenProvider } from "../../../../src/variant-analysis/gh-api/variant-analysis-server-client"; -import { - writeQueryHistoryToFile, - readQueryHistoryFromFile, -} from "../../../../src/query-history/store/query-history-store"; -import { createMockVariantAnalysisHistoryItem } from "../../../factories/query-history/variant-analysis-history-item"; -import { join } from "path"; -import { tmpDir } from "../../../../src/tmp-dir"; -import { remove } from "fs-extra"; +import { extLogger } from "../../../../src/common/logging/vscode"; const SERVER_URL = "https://mrva.example.com"; const PROVIDER_ID = "example.token-provider"; @@ -155,47 +151,91 @@ describe("variant analysis custom server", () => { await setSetting("serverTokenProvider", PROVIDER_ID); }); - it("routes a saved and reloaded server run back to its original endpoint", async () => { - const history = createMockVariantAnalysisHistoryItem({}); - history.variantAnalysis.serverUrl = SERVER_URL; - history.variantAnalysis.controllerRepo.id = 0; - history.variantAnalysis.actionsWorkflowRunId = 42; - history.variantAnalysis.controllerRepo.htmlUrl = - "https://source.example.com/org/controller"; - const historyPath = join(tmpDir.name, "custom-server-history.json"); - try { - await writeQueryHistoryToFile([history], historyPath); - const [reloaded] = await readQueryHistoryFromFile(historyPath); - if (reloaded.t !== "variant-analysis") { - throw new Error("Expected a variant analysis history item"); - } - const run = reloaded.variantAnalysis; - expect(run.serverUrl).toBe(SERVER_URL); - expect(run.controllerRepo.htmlUrl).toBe( - history.variantAnalysis.controllerRepo.htmlUrl, - ); - await getVariantAnalysis( - credentials, - run.controllerRepo.id, - run.id, - run.serverUrl, - ); - await getVariantAnalysisRepo( - credentials, - run.controllerRepo.id, - run.id, - 7, - run.serverUrl, + it("hides server logs when the companion has no log hook", async () => { + const run = { ...createMockVariantAnalysis({}), serverUrl: SERVER_URL }; + expect(await canViewVariantAnalysisLogs(run)).toBe(false); + await expect(openServerVariantAnalysisLogs(run)).rejects.toThrow( + "does not support viewing logs", + ); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it.each(["missing", "failed", "stalled"])( + "preserves saved results when the log provider is %s", + async (state) => { + const run = { ...createMockVariantAnalysis({}), serverUrl: SERVER_URL }; + const log = jest.spyOn(extLogger, "log").mockResolvedValue(undefined); + jest.spyOn(extensions, "getExtension").mockReturnValue( + state === "missing" + ? undefined + : mockedObject>({ + isActive: false, + activate: () => + state === "failed" + ? Promise.reject(new Error("Activation failed")) + : new Promise(() => {}), + }), ); - await cancelVariantAnalysis(credentials, run); - expect(fetchSpy.mock.calls.map(([url]) => url)).toEqual([ - `${SERVER_URL}/repositories/0/code-scanning/codeql/variant-analyses/${run.id}`, - `${SERVER_URL}/repositories/0/code-scanning/codeql/variant-analyses/${run.id}/repositories/7`, - `${SERVER_URL}/repositories/0/code-scanning/codeql/variant-analyses/${run.id}/cancel`, - ]); - } finally { - await remove(historyPath); - } + if (state === "stalled") { + jest.useFakeTimers(); + } + try { + const result = withVariantAnalysisLogCapability(run); + if (state === "stalled") { + await jest.advanceTimersByTimeAsync(30_000); + expect(jest.getTimerCount()).toBe(0); + } + await expect(result).resolves.toEqual({ ...run, canViewLogs: false }); + expect(log).toHaveBeenCalledWith( + expect.stringContaining("Unable to discover variant analysis logs"), + ); + expect(getVariantAnalysisServerToken).not.toHaveBeenCalled(); + expect(fetchSpy).not.toHaveBeenCalled(); + if (state !== "stalled") { + await expect(openServerVariantAnalysisLogs(run)).rejects.toThrow(); + } + } finally { + if (state === "stalled") { + jest.useRealTimers(); + } + } + }, + ); + + it("activates the companion and delegates log viewing without fetching a token", async () => { + const openVariantAnalysisLogs = jest.fn().mockResolvedValue(undefined); + const activate = jest.fn().mockResolvedValue({ + getVariantAnalysisServerToken, + openVariantAnalysisLogs, + }); + jest.spyOn(extensions, "getExtension").mockReturnValue( + mockedObject>({ + isActive: false, + activate, + }), + ); + const run = { + ...createMockVariantAnalysis({}), + serverUrl: SERVER_URL, + actionsWorkflowRunId: undefined, + }; + expect(await canViewVariantAnalysisLogs(run)).toBe(true); + await openServerVariantAnalysisLogs(run); + expect(activate).toHaveBeenCalled(); + expect(openVariantAnalysisLogs).toHaveBeenCalledWith(SERVER_URL, run.id); + expect(getVariantAnalysisServerToken).not.toHaveBeenCalled(); + expect(fetchSpy).not.toHaveBeenCalled(); + openVariantAnalysisLogs.mockRejectedValue(new Error("Cannot open logs")); + await expect(openServerVariantAnalysisLogs(run)).rejects.toThrow( + "Cannot open logs", + ); + await setSetting("serverUrl", "https://other.example.com"); + expect(await canViewVariantAnalysisLogs(run)).toBe(false); + openVariantAnalysisLogs.mockClear(); + await expect(openServerVariantAnalysisLogs(run)).rejects.toThrow( + "different server", + ); + expect(openVariantAnalysisLogs).not.toHaveBeenCalled(); }); it.each(["activation", "token"])( From 7d42d34d3c8bcb5c6c650686e145dd6e066b7c3d Mon Sep 17 00:00:00 2001 From: Nicolas Will Date: Thu, 29 Oct 2026 12:58:02 +0100 Subject: [PATCH 5/5] Discover companion log support without blocking history or result views --- docs/variant-analysis-custom-server.md | 11 +- extensions/ql-vscode/src/extension.ts | 2 + .../model-alerts/model-alerts-view.ts | 16 ++- .../history-tree-data-provider.ts | 12 +- .../variant-analysis/variant-analysis-api.ts | 62 ++++++++-- .../variant-analysis/variant-analysis-view.ts | 17 ++- .../history-tree-data-provider.test.ts | 2 +- .../variant-analysis-api.test.ts | 111 ++++++++++++++++-- 8 files changed, 204 insertions(+), 29 deletions(-) diff --git a/docs/variant-analysis-custom-server.md b/docs/variant-analysis-custom-server.md index 4de91351b66..0620271821e 100644 --- a/docs/variant-analysis-custom-server.md +++ b/docs/variant-analysis-custom-server.md @@ -56,11 +56,18 @@ The optional `openVariantAnalysisLogs` hook enables **View logs** in history and The provider owns log retrieval, authentication, and presentation (for example, opening a browser or an output channel). CodeQL passes the originating server URL and analysis/session ID, not an Actions workflow ID. Without the hook the action is hidden. Capability discovery activates the -provider with the same 30-second activation bound; opening logs does not request a token from CodeQL. -If discovery fails, CodeQL logs the error and hides the action without preventing saved results from loading. +provider in the background with the same 30-second activation bound. History and result views render +immediately without the action, then refresh when discovery finishes. Discovery is shared and cached +per server URL and provider ID, including failures; changing either setting clears the cache. +If discovery fails, CodeQL logs the error and leaves the action hidden without delaying saved results. +Opening logs does not request a token from CodeQL. The hook should resolve once its viewer is opened and reject on failure. Runs from another configured server cannot invoke it. No log endpoint or GitHub Actions compatibility is required. +Providers should publish their exports immediately from `activate()`. Catalog downloads, sign-in +prompts, and other potentially unbounded work must not block activation; perform them lazily in the +operation that needs them. + ## Behavior when a server is configured - MRVA is enabled regardless of `github-enterprise.uri`. The GitHub sign-in, GitHub database diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 925e231b00d..1e76d43f622 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -110,6 +110,7 @@ import { LogScannerService } from "./log-insights/log-scanner-service"; import { VariantAnalysisView } from "./variant-analysis/variant-analysis-view"; import { VariantAnalysisViewSerializer } from "./variant-analysis/variant-analysis-view-serializer"; import { VariantAnalysisManager } from "./variant-analysis/variant-analysis-manager"; +import { watchVariantAnalysisLogCapabilities } from "./variant-analysis/variant-analysis-api"; import { createVariantAnalysisContentProvider } from "./variant-analysis/variant-analysis-content-provider"; import { VSCodeMockGitHubApiServer } from "./common/mock-gh-api/vscode/vscode-mock-gh-api-server"; import { VariantAnalysisResultsManager } from "./variant-analysis/variant-analysis-results-manager"; @@ -920,6 +921,7 @@ async function activateWithInstalledDistribution( ); await ensureDir(variantAnalysisStorageDir); const variantAnalysisConfig = new VariantAnalysisConfigListener(); + ctx.subscriptions.push(watchVariantAnalysisLogCapabilities()); const variantAnalysisResultsManager = new VariantAnalysisResultsManager( cliServer, variantAnalysisConfig, diff --git a/extensions/ql-vscode/src/model-editor/model-alerts/model-alerts-view.ts b/extensions/ql-vscode/src/model-editor/model-alerts/model-alerts-view.ts index 7abf377f47f..25e4540f8e2 100644 --- a/extensions/ql-vscode/src/model-editor/model-alerts/model-alerts-view.ts +++ b/extensions/ql-vscode/src/model-editor/model-alerts/model-alerts-view.ts @@ -22,7 +22,10 @@ import type { import type { AppEvent, AppEventEmitter } from "../../common/events"; import type { ModeledMethod } from "../modeled-method"; import type { MethodSignature } from "../method"; -import { withVariantAnalysisLogCapability } from "../../variant-analysis/variant-analysis-api"; +import { + onDidChangeVariantAnalysisLogCapabilities, + withVariantAnalysisLogCapability, +} from "../../variant-analysis/variant-analysis-api"; export class ModelAlertsView extends AbstractWebview< ToModelAlertsMessage, @@ -32,6 +35,7 @@ export class ModelAlertsView extends AbstractWebview< public readonly onEvaluationRunStopClicked: AppEvent; private readonly onEvaluationRunStopClickedEventEmitter: AppEventEmitter; + private variantAnalysis?: VariantAnalysis; public constructor( app: App, @@ -43,6 +47,13 @@ export class ModelAlertsView extends AbstractWebview< super(app); this.registerToModelingEvents(); + this.push( + onDidChangeVariantAnalysisLogCapabilities(() => { + if (this.variantAnalysis) { + void this.setVariantAnalysis(this.variantAnalysis); + } + }), + ); this.onEvaluationRunStopClickedEventEmitter = this.push( app.createEventEmitter(), @@ -125,13 +136,14 @@ export class ModelAlertsView extends AbstractWebview< public async setVariantAnalysis( variantAnalysis: VariantAnalysis, ): Promise { + this.variantAnalysis = variantAnalysis; if (!this.isShowingPanel) { return; } await this.postMessage({ t: "setVariantAnalysis", - variantAnalysis: await withVariantAnalysisLogCapability(variantAnalysis), + variantAnalysis: withVariantAnalysisLogCapability(variantAnalysis), }); } diff --git a/extensions/ql-vscode/src/query-history/history-tree-data-provider.ts b/extensions/ql-vscode/src/query-history/history-tree-data-provider.ts index 9fe086b1a35..babfc6a3bbd 100644 --- a/extensions/ql-vscode/src/query-history/history-tree-data-provider.ts +++ b/extensions/ql-vscode/src/query-history/history-tree-data-provider.ts @@ -7,7 +7,10 @@ import { getLanguage, getQueryId } from "./query-history-info"; import { QueryStatus } from "./query-status"; import type { HistoryItemLabelProvider } from "./history-item-label-provider"; import type { LanguageContextStore } from "../language-context-store"; -import { canViewVariantAnalysisLogs } from "../variant-analysis/variant-analysis-api"; +import { + canViewVariantAnalysisLogs, + onDidChangeVariantAnalysisLogCapabilities, +} from "../variant-analysis/variant-analysis-api"; export enum SortOrder { NameAsc = "NameAsc", @@ -69,6 +72,11 @@ export class HistoryTreeDataProvider private readonly languageContext: LanguageContextStore, ) { super(); + this.push( + onDidChangeVariantAnalysisLogCapabilities(() => + this._onDidChangeTreeData.fire(undefined), + ), + ); } async getTreeItem(element: QueryHistoryInfo): Promise { @@ -125,7 +133,7 @@ export class HistoryTreeDataProvider private async getContextValue(element: QueryHistoryInfo): Promise { const hasServerLogs = element.t === "variant-analysis" && element.variantAnalysis.serverUrl - ? await canViewVariantAnalysisLogs(element.variantAnalysis) + ? canViewVariantAnalysisLogs(element.variantAnalysis) : false; switch (element.status) { case QueryStatus.InProgress: diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts index 8d4e48f8cde..df18e1fed06 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-api.ts @@ -17,6 +17,7 @@ import type { VariantAnalysis, VariantAnalysisSubmission, } from "./shared/variant-analysis"; +import { Disposable, EventEmitter, workspace } from "vscode"; /** * Variant analysis API calls. These go to GitHub, unless an internal custom @@ -53,20 +54,57 @@ export function getServerForVariantAnalysis(serverUrl: string | undefined) { return server; } -export async function canViewVariantAnalysisLogs( - run: VariantAnalysis, -): Promise { +const logCapabilities = new Map(); +const logCapabilitiesChanged = new EventEmitter(); +export const onDidChangeVariantAnalysisLogCapabilities = + logCapabilitiesChanged.event; + +export function watchVariantAnalysisLogCapabilities(): Disposable { + logCapabilities.clear(); + return Disposable.from( + workspace.onDidChangeConfiguration((event) => { + if ( + event.affectsConfiguration("codeQL.variantAnalysis.serverUrl") || + event.affectsConfiguration("codeQL.variantAnalysis.serverTokenProvider") + ) { + logCapabilities.clear(); + logCapabilitiesChanged.fire(); + } + }), + new Disposable(() => logCapabilities.clear()), + ); +} + +export function canViewVariantAnalysisLogs(run: VariantAnalysis): boolean { if (!run.serverUrl) { return run.actionsWorkflowRunId !== undefined; } - try { - return (await getServerLogOpener(run)) !== undefined; - } catch (error) { - void extLogger.log( - `Unable to discover variant analysis logs: ${getErrorMessage(error)}`, - ); + const server = getVariantAnalysisServerConfig(); + if (server?.url !== run.serverUrl || !server.tokenProviderExtensionId) { return false; } + const key = JSON.stringify([server.url, server.tokenProviderExtensionId]); + const cached = logCapabilities.get(key); + if (cached) { + return cached.available; + } + const capability = { available: false }; + logCapabilities.set(key, capability); + void getServerLogOpener(run) + .then((openLogs) => { + capability.available = openLogs !== undefined; + }) + .catch((error: unknown) => { + void extLogger.log( + `Unable to discover variant analysis logs: ${getErrorMessage(error)}`, + ); + }) + .then(() => { + if (logCapabilities.get(key) === capability) { + logCapabilitiesChanged.fire(); + } + }); + return false; } async function getServerLogOpener( @@ -88,10 +126,10 @@ async function getServerLogOpener( }); } -export async function withVariantAnalysisLogCapability( +export function withVariantAnalysisLogCapability( run: VariantAnalysis, -): Promise { - return { ...run, canViewLogs: await canViewVariantAnalysisLogs(run) }; +): VariantAnalysis { + return { ...run, canViewLogs: canViewVariantAnalysisLogs(run) }; } export async function openServerVariantAnalysisLogs( diff --git a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts index 44ffa488f55..9e55a4cead7 100644 --- a/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts +++ b/extensions/ql-vscode/src/variant-analysis/variant-analysis-view.ts @@ -22,7 +22,10 @@ import type { import { telemetryListener } from "../common/vscode/telemetry"; import { redactableError } from "../common/errors"; import { DataFlowPathsView } from "./data-flow-paths-view"; -import { withVariantAnalysisLogCapability } from "./variant-analysis-api"; +import { + onDidChangeVariantAnalysisLogCapabilities, + withVariantAnalysisLogCapability, +} from "./variant-analysis-api"; import type { DataFlowPaths } from "./shared/data-flow-paths"; import type { App } from "../common/app"; import { @@ -48,6 +51,14 @@ export class VariantAnalysisView manager.registerView(this); this.dataFlowPathsView = new DataFlowPathsView(app); + this.push( + onDidChangeVariantAnalysisLogCapabilities(() => { + const run = this.manager.tryGetVariantAnalysis(this.variantAnalysisId); + if (run) { + void this.updateView(run); + } + }), + ); } public async openView() { @@ -71,7 +82,7 @@ export class VariantAnalysisView await this.postMessage({ t: "setVariantAnalysis", - variantAnalysis: await withVariantAnalysisLogCapability(variantAnalysis), + variantAnalysis: withVariantAnalysisLogCapability(variantAnalysis), }); const panel = await this.getPanel(); @@ -213,7 +224,7 @@ export class VariantAnalysisView await this.postMessage({ t: "setVariantAnalysis", - variantAnalysis: await withVariantAnalysisLogCapability(variantAnalysis), + variantAnalysis: withVariantAnalysisLogCapability(variantAnalysis), }); await this.postMessage({ diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/history-tree-data-provider.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/history-tree-data-provider.test.ts index be0b390bfa0..49ac23e2e72 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/history-tree-data-provider.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/query-history/history-tree-data-provider.test.ts @@ -173,7 +173,7 @@ describe("HistoryTreeDataProvider", () => { async (canViewLogs, workflowId) => { jest .spyOn(variantAnalysisApi, "canViewVariantAnalysisLogs") - .mockResolvedValue(canViewLogs); + .mockReturnValue(canViewLogs); const item = createMockVariantAnalysisHistoryItem({ historyItemStatus, variantAnalysisStatus, diff --git a/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts index e7e020655a0..f19d03827af 100644 --- a/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts +++ b/extensions/ql-vscode/test/vscode-tests/no-workspace/variant-analysis/variant-analysis-api.test.ts @@ -1,5 +1,10 @@ -import { ConfigurationTarget, extensions, workspace } from "vscode"; -import type { Extension } from "vscode"; +import { + ConfigurationTarget, + EventEmitter, + extensions, + workspace, +} from "vscode"; +import type { ConfigurationChangeEvent, Disposable, Extension } from "vscode"; import { getVariantAnalysisServerConfig, VariantAnalysisConfigListener, @@ -9,6 +14,8 @@ import { canViewVariantAnalysisLogs, openServerVariantAnalysisLogs, withVariantAnalysisLogCapability, + onDidChangeVariantAnalysisLogCapabilities, + watchVariantAnalysisLogCapabilities, createGist, getVariantAnalysis, getVariantAnalysisRepo, @@ -22,6 +29,7 @@ import { mockedObject } from "../../utils/mocking.helpers"; import { testCredentialsWithStub } from "../../../factories/authentication"; import type { VariantAnalysisServerTokenProvider } from "../../../../src/variant-analysis/gh-api/variant-analysis-server-client"; import { extLogger } from "../../../../src/common/logging/vscode"; +import { setImmediate } from "timers/promises"; const SERVER_URL = "https://mrva.example.com"; const PROVIDER_ID = "example.token-provider"; @@ -43,8 +51,15 @@ describe("variant analysis custom server", () => { [string] >; let credentials: ReturnType; + let configurationChanged: EventEmitter; + let watcher: Disposable; beforeEach(() => { + configurationChanged = new EventEmitter(); + jest + .spyOn(workspace, "onDidChangeConfiguration") + .mockImplementation(configurationChanged.event); + watcher = watchVariantAnalysisLogCapabilities(); credentials = testCredentialsWithStub(); fetchSpy = jest .spyOn(global, "fetch") @@ -62,6 +77,20 @@ describe("variant analysis custom server", () => { ); }); + afterEach(() => { + watcher.dispose(); + configurationChanged.dispose(); + }); + + function discoveryFinished(): Promise { + return new Promise((resolve) => { + const subscription = onDidChangeVariantAnalysisLogCapabilities(() => { + subscription.dispose(); + resolve(); + }); + }); + } + describe("configuration", () => { it.each([`${SERVER_URL}/?environment=test`, `${SERVER_URL}/#fragment`])( "rejects endpoint modifiers: %s", @@ -153,7 +182,10 @@ describe("variant analysis custom server", () => { it("hides server logs when the companion has no log hook", async () => { const run = { ...createMockVariantAnalysis({}), serverUrl: SERVER_URL }; - expect(await canViewVariantAnalysisLogs(run)).toBe(false); + const finished = discoveryFinished(); + expect(canViewVariantAnalysisLogs(run)).toBe(false); + await finished; + expect(canViewVariantAnalysisLogs(run)).toBe(false); await expect(openServerVariantAnalysisLogs(run)).rejects.toThrow( "does not support viewing logs", ); @@ -165,7 +197,7 @@ describe("variant analysis custom server", () => { async (state) => { const run = { ...createMockVariantAnalysis({}), serverUrl: SERVER_URL }; const log = jest.spyOn(extLogger, "log").mockResolvedValue(undefined); - jest.spyOn(extensions, "getExtension").mockReturnValue( + const lookup = jest.spyOn(extensions, "getExtension").mockReturnValue( state === "missing" ? undefined : mockedObject>({ @@ -180,12 +212,20 @@ describe("variant analysis custom server", () => { jest.useFakeTimers(); } try { + const finished = discoveryFinished(); const result = withVariantAnalysisLogCapability(run); + expect(result).toEqual({ ...run, canViewLogs: false }); + for (let id = 0; id < 10; id++) { + expect(canViewVariantAnalysisLogs({ ...run, id })).toBe(false); + } + expect(lookup).toHaveBeenCalledTimes(1); if (state === "stalled") { await jest.advanceTimersByTimeAsync(30_000); expect(jest.getTimerCount()).toBe(0); } - await expect(result).resolves.toEqual({ ...run, canViewLogs: false }); + await finished; + expect(withVariantAnalysisLogCapability(run)).toEqual(result); + expect(lookup).toHaveBeenCalledTimes(1); expect(log).toHaveBeenCalledWith( expect.stringContaining("Unable to discover variant analysis logs"), ); @@ -202,6 +242,56 @@ describe("variant analysis custom server", () => { }, ); + it.each(["serverUrl", "serverTokenProvider"])( + "invalidates discovery on %s changes and ignores stale completions", + async (setting) => { + let finishActivation!: ( + provider: VariantAnalysisServerTokenProvider, + ) => void; + const activate = jest.fn().mockImplementation( + () => + new Promise((resolve) => { + finishActivation = resolve; + }), + ); + jest.spyOn(extensions, "getExtension").mockReturnValue( + mockedObject>({ + isActive: false, + activate, + }), + ); + const run = { ...createMockVariantAnalysis({}), serverUrl: SERVER_URL }; + const changed = jest.fn(); + const subscription = onDidChangeVariantAnalysisLogCapabilities(changed); + try { + expect(canViewVariantAnalysisLogs(run)).toBe(false); + const oldActivation = finishActivation; + configurationChanged.fire({ + affectsConfiguration: (key) => + key === `codeQL.variantAnalysis.${setting}`, + }); + expect(changed).toHaveBeenCalledTimes(1); + expect(canViewVariantAnalysisLogs(run)).toBe(false); + expect(activate).toHaveBeenCalledTimes(2); + const finished = discoveryFinished(); + finishActivation({ getVariantAnalysisServerToken }); + await finished; + expect(canViewVariantAnalysisLogs(run)).toBe(false); + oldActivation({ + getVariantAnalysisServerToken, + openVariantAnalysisLogs: jest.fn(), + }); + // Drain the obsolete discovery's continuation without a real timer. + await setImmediate(); + expect(canViewVariantAnalysisLogs(run)).toBe(false); + expect(changed).toHaveBeenCalledTimes(2); + expect(activate).toHaveBeenCalledTimes(2); + } finally { + subscription.dispose(); + } + }, + ); + it("activates the companion and delegates log viewing without fetching a token", async () => { const openVariantAnalysisLogs = jest.fn().mockResolvedValue(undefined); const activate = jest.fn().mockResolvedValue({ @@ -219,7 +309,14 @@ describe("variant analysis custom server", () => { serverUrl: SERVER_URL, actionsWorkflowRunId: undefined, }; - expect(await canViewVariantAnalysisLogs(run)).toBe(true); + const finished = discoveryFinished(); + expect(canViewVariantAnalysisLogs(run)).toBe(false); + expect(canViewVariantAnalysisLogs({ ...run, id: run.id + 1 })).toBe( + false, + ); + await finished; + expect(canViewVariantAnalysisLogs(run)).toBe(true); + expect(activate).toHaveBeenCalledTimes(1); await openServerVariantAnalysisLogs(run); expect(activate).toHaveBeenCalled(); expect(openVariantAnalysisLogs).toHaveBeenCalledWith(SERVER_URL, run.id); @@ -230,7 +327,7 @@ describe("variant analysis custom server", () => { "Cannot open logs", ); await setSetting("serverUrl", "https://other.example.com"); - expect(await canViewVariantAnalysisLogs(run)).toBe(false); + expect(canViewVariantAnalysisLogs(run)).toBe(false); openVariantAnalysisLogs.mockClear(); await expect(openServerVariantAnalysisLogs(run)).rejects.toThrow( "different server",