Ensure proper paths are used for retrieving artifacts

This change builds on the previous change to ensure that sarif results
can be displayed properly. Here is what it does:

- Move prepareDownloadDirectory to the RemoteQueryManager
- Store the queryResult to disk
- Use the `artifactStorageDir` as the location where artifacts are kept
- Add `artifactStorageDir` to DownloadLink
- Ensure the webview passes around the right links.

extensions/ql-vscode/src/remote-queries/analyses-results-manager.ts

@@ -8,8 +8,6 @@ import { AnalysisResults, QueryResult } from './shared/analysis-result';
 import { UserCancellationException } from '../commandRunner';
 import * as os from 'os';
 import { sarifParser } from '../sarif-parser';
-import { createTimestampFile } from '../helpers';
-import { nanoid } from 'nanoid';
 
 export class AnalysesResultsManager {
   // Store for the results of various analyses for a single remote query.
@@ -17,7 +15,7 @@ export class AnalysesResultsManager {
 
   constructor(
     private readonly ctx: ExtensionContext,
-    private readonly storagePath: string,
+    readonly storagePath: string,
     private readonly logger: Logger,
   ) {
     this.analysesResults = [];
@@ -81,20 +79,6 @@ export class AnalysesResultsManager {
     return [...this.analysesResults];
   }
 
-  /**
-   * Prepares a directory for storing analysis results for a single query run.
-   * This directory initially contains only a timestamp file, which will be
-   * used by the query history manager to determine when the directory
-   * should be deleted.
-   *
-   * @param queryName The name of the query that was run.
-   */
-  public async prepareDownloadDirectory(queryName: string): Promise<void> {
-    // Prepare the storage directory.
-    const artifactStorageDir = path.join(this.storagePath, `${queryName}-${nanoid()}`);
-    await createTimestampFile(artifactStorageDir);
-  }
-
   private async downloadSingleAnalysisResults(
     analysis: AnalysisSummary,
     credentials: Credentials,
@@ -111,7 +95,7 @@ export class AnalysesResultsManager {
 
     let artifactPath;
     try {
-      artifactPath = await downloadArtifactFromLink(credentials, analysis.downloadLink, this.storagePath);
+      artifactPath = await downloadArtifactFromLink(credentials, analysis.downloadLink);
     }
     catch (e) {
       throw new Error(`Could not download the analysis results for ${analysis.nwo}: ${e.message}`);

extensions/ql-vscode/src/remote-queries/download-link.ts

@@ -1,15 +1,15 @@
 /**
- * Represents a link to an artifact to be downloaded. 
+ * Represents a link to an artifact to be downloaded.
  */
 export interface DownloadLink {
   /**
-   *  A unique id of the artifact being downloaded. 
+   *  A unique id of the artifact being downloaded.
    */
   id: string;
 
   /**
    * The URL path to use against the GitHub API to download the
-   * linked artifact. 
+   * linked artifact.
    */
   urlPath: string;
 
@@ -17,4 +17,9 @@ export interface DownloadLink {
    * An optional path to follow inside the downloaded archive containing the artifact.
    */
   innerFilePath?: string;
+
+  /**
+   * The full path to the directory where the artifacts for this link is stored.
+   */
+  artifactStorageDir: string;
 }

extensions/ql-vscode/src/remote-queries/gh-actions-api-client.ts

@@ -54,8 +54,7 @@ export async function getRemoteQueryIndex(
 
 export async function downloadArtifactFromLink(
   credentials: Credentials,
-  downloadLink: DownloadLink,
-  storagePath: string
+  downloadLink: DownloadLink
 ): Promise<string> {
 
   const octokit = await credentials.getOctokit();
@@ -63,16 +62,14 @@ export async function downloadArtifactFromLink(
   // Download the zipped artifact.
   const response = await octokit.request(`GET ${downloadLink.urlPath}/zip`, {});
 
-  const zipFilePath = path.join(storagePath, `${downloadLink.id}.zip`);
+  const zipFilePath = path.join(downloadLink.artifactStorageDir, `${downloadLink.id}.zip`);
   await saveFile(`${zipFilePath}`, response.data as ArrayBuffer);
 
   // Extract the zipped artifact.
-  const extractedPath = path.join(storagePath, downloadLink.id);
+  const extractedPath = path.join(downloadLink.artifactStorageDir, downloadLink.id);
   await unzipFile(zipFilePath, extractedPath);
 
-  return downloadLink.innerFilePath
-    ? path.join(extractedPath, downloadLink.innerFilePath)
-    : extractedPath;
+  return path.join(extractedPath, downloadLink.innerFilePath || '');
 }
 
 /**

extensions/ql-vscode/src/remote-queries/remote-queries-interface.ts

@@ -24,7 +24,7 @@ import { AnalysisSummary, RemoteQueryResult } from './remote-query-result';
 import { RemoteQuery } from './remote-query';
 import { RemoteQueryResult as RemoteQueryResultViewModel } from './shared/remote-query-result';
 import { AnalysisSummary as AnalysisResultViewModel } from './shared/remote-query-result';
-import { showAndLogWarningMessage, tmpDir } from '../helpers';
+import { showAndLogWarningMessage } from '../helpers';
 import { URLSearchParams } from 'url';
 import { SHOW_QUERY_TEXT_MSG } from '../query-history';
 import { AnalysesResultsManager } from './analyses-results-manager';
@@ -98,7 +98,7 @@ export class RemoteQueriesInterfaceManager {
           enableFindWidget: true,
           retainContextWhenHidden: true,
           localResourceRoots: [
-            Uri.file(tmpDir.name),
+            Uri.file(this.analysesResultsManager.storagePath),
             Uri.file(path.join(this.ctx.extensionPath, 'out')),
           ],
         }
@@ -224,7 +224,7 @@ export class RemoteQueriesInterfaceManager {
 
   private async viewAnalysisResults(msg: RemoteQueryViewAnalysisResultsMessage): Promise<void> {
     const downloadLink = msg.analysisSummary.downloadLink;
-    const filePath = path.join(tmpDir.name, downloadLink.id, downloadLink.innerFilePath || '');
+    const filePath = path.join(downloadLink.artifactStorageDir, downloadLink.id, downloadLink.innerFilePath || '');
 
     const sarifViewerExtensionId = 'MS-SarifVSCode.sarif-viewer';
 

extensions/ql-vscode/src/remote-queries/remote-queries-manager.ts

@@ -1,8 +1,12 @@
 import { CancellationToken, commands, ExtensionContext, Uri, window } from 'vscode';
+import { nanoid } from 'nanoid';
+import * as path from 'path';
+import * as fs from 'fs-extra';
+
 import { Credentials } from '../authentication';
 import { CodeQLCliServer } from '../cli';
 import { ProgressCallback } from '../commandRunner';
-import { showAndLogErrorMessage, showInformationMessageWithAction } from '../helpers';
+import { createTimestampFile, showAndLogErrorMessage, showInformationMessageWithAction } from '../helpers';
 import { Logger } from '../logging';
 import { runRemoteQuery } from './run-remote-query';
 import { RemoteQueriesInterfaceManager } from './remote-queries-interface';
@@ -13,6 +17,7 @@ import { RemoteQueryResultIndex } from './remote-query-result-index';
 import { RemoteQueryResult } from './remote-query-result';
 import { DownloadLink } from './download-link';
 import { AnalysesResultsManager } from './analyses-results-manager';
+import { assertNever } from '../pure/helpers-pure';
 
 const autoDownloadMaxSize = 300 * 1024;
 const autoDownloadMaxCount = 100;
@@ -25,7 +30,7 @@ export class RemoteQueriesManager {
   constructor(
     private readonly ctx: ExtensionContext,
     private readonly cliServer: CodeQLCliServer,
-    readonly storagePath: string,
+    private readonly storagePath: string,
     logger: Logger,
   ) {
     this.analysesResultsManager = new AnalysesResultsManager(ctx, storagePath, logger);
@@ -58,19 +63,23 @@ export class RemoteQueriesManager {
   ): Promise<void> {
     const credentials = await Credentials.initialize(this.ctx);
 
-    const queryResult = await this.remoteQueriesMonitor.monitorQuery(query, cancellationToken);
+    const queryWorkflowResult = await this.remoteQueriesMonitor.monitorQuery(query, cancellationToken);
 
     const executionEndTime = new Date();
 
-    if (queryResult.status === 'CompletedSuccessfully') {
+    if (queryWorkflowResult.status === 'CompletedSuccessfully') {
       const resultIndex = await getRemoteQueryIndex(credentials, query);
       if (!resultIndex) {
         void showAndLogErrorMessage(`There was an issue retrieving the result for the query ${query.queryName}`);
         return;
       }
 
-      const queryResult = this.mapQueryResult(executionEndTime, resultIndex);
-      await this.analysesResultsManager.prepareDownloadDirectory(query.queryName);
+      const artifactStorageDir = await this.prepareStorageDirectory(query.queryName);
+      const queryResult = this.mapQueryResult(executionEndTime, resultIndex, artifactStorageDir);
+
+      // Write the query result to the storage directory.
+      const queryResultFilePath = path.join(artifactStorageDir, 'query-result.json');
+      await fs.writeFile(queryResultFilePath, JSON.stringify(queryResult, null, 2), 'utf8');
 
       // Kick off auto-download of results.
       void commands.executeCommand('codeQL.autoDownloadRemoteQueryResults', queryResult);
@@ -81,13 +90,19 @@ export class RemoteQueriesManager {
       const shouldOpenView = await showInformationMessageWithAction(message, 'View');
       if (shouldOpenView) {
         await this.interfaceManager.showResults(query, queryResult);
-
       }
-    } else if (queryResult.status === 'CompletedUnsuccessfully') {
-      await showAndLogErrorMessage(`Remote query execution failed. Error: ${queryResult.error}`);
-      return;
-    } else if (queryResult.status === 'Cancelled') {
+    } else if (queryWorkflowResult.status === 'CompletedUnsuccessfully') {
+      await showAndLogErrorMessage(`Remote query execution failed. Error: ${queryWorkflowResult.error}`);
+
+    } else if (queryWorkflowResult.status === 'Cancelled') {
       await showAndLogErrorMessage('Remote query monitoring was cancelled');
+
+    } else if (queryWorkflowResult.status === 'InProgress') {
+      // Should not get here
+      await showAndLogErrorMessage(`Unexpected status: ${queryWorkflowResult.status}`);
+    } else {
+      // Ensure all cases are covered
+      assertNever(queryWorkflowResult.status);
     }
   }
 
@@ -111,7 +126,7 @@ export class RemoteQueriesManager {
       results => this.interfaceManager.setAnalysisResults(results));
   }
 
-  private mapQueryResult(executionEndTime: Date, resultIndex: RemoteQueryResultIndex): RemoteQueryResult {
+  private mapQueryResult(executionEndTime: Date, resultIndex: RemoteQueryResultIndex, artifactStorageDir: string): RemoteQueryResult {
     const analysisSummaries = resultIndex.items.map(item => ({
       nwo: item.nwo,
       resultCount: item.resultCount,
@@ -119,13 +134,32 @@ export class RemoteQueriesManager {
       downloadLink: {
         id: item.artifactId.toString(),
         urlPath: `${resultIndex.artifactsUrlPath}/${item.artifactId}`,
-        innerFilePath: item.sarifFileSize ? 'results.sarif' : 'results.bqrs'
+        innerFilePath: item.sarifFileSize ? 'results.sarif' : 'results.bqrs',
+        artifactStorageDir
       } as DownloadLink
     }));
 
     return {
       executionEndTime,
-      analysisSummaries
+      analysisSummaries,
+      artifactStorageDir
     };
   }
+
+  /**
+   * Prepares a directory for storing analysis results for a single query run.
+   * This directory contains a timestamp file, which will be
+   * used by the query history manager to determine when the directory
+   * should be deleted.
+   *
+   * @param queryName The name of the query that was run.
+   *
+   * @returns A promise resolving to the directory where all artifacts for this remote query are stored.
+   */
+  private async prepareStorageDirectory(queryName: string): Promise<string> {
+    const artifactStorageDir = path.join(this.storagePath, `${queryName}-${nanoid()}`);
+    await createTimestampFile(artifactStorageDir);
+
+    return artifactStorageDir;
+  }
 }

extensions/ql-vscode/src/remote-queries/remote-query-result.ts

@@ -3,6 +3,7 @@ import { DownloadLink } from './download-link';
 export interface RemoteQueryResult {
   executionEndTime: Date;
   analysisSummaries: AnalysisSummary[];
+  artifactStorageDir: string;
 }
 
 export interface AnalysisSummary {

extensions/ql-vscode/src/remote-queries/sample-data.ts

@@ -1,3 +1,5 @@
+import * as path from 'path';
+import { tmpDir } from '../helpers';
 import { RemoteQuery } from './remote-query';
 import { RemoteQueryResult } from './remote-query-result';
 import { AnalysisResults } from './shared/analysis-result';
@@ -38,6 +40,7 @@ export const sampleRemoteQuery: RemoteQuery = {
 
 export const sampleRemoteQueryResult: RemoteQueryResult = {
   executionEndTime: new Date('2022-01-06T17:04:37.026Z'),
+  artifactStorageDir: path.join(tmpDir.name, 'query.ql-123-xyz'),
   analysisSummaries: [
     {
       nwo: 'big-corp/repo1',
@@ -46,7 +49,8 @@ export const sampleRemoteQueryResult: RemoteQueryResult = {
       downloadLink: {
         id: '137697017',
         urlPath: '/repos/big-corp/controller-repo/actions/artifacts/137697017',
-        innerFilePath: 'results.sarif'
+        innerFilePath: 'results.sarif',
+        artifactStorageDir: path.join(tmpDir.name, 'query.ql-123-xyz')
       }
     },
     {
@@ -56,7 +60,8 @@ export const sampleRemoteQueryResult: RemoteQueryResult = {
       downloadLink: {
         id: '137697018',
         urlPath: '/repos/big-corp/controller-repo/actions/artifacts/137697018',
-        innerFilePath: 'results.sarif'
+        innerFilePath: 'results.sarif',
+        artifactStorageDir: path.join(tmpDir.name, 'query.ql-123-xyz')
       }
     },
     {
@@ -66,7 +71,8 @@ export const sampleRemoteQueryResult: RemoteQueryResult = {
       downloadLink: {
         id: '137697019',
         urlPath: '/repos/big-corp/controller-repo/actions/artifacts/137697019',
-        innerFilePath: 'results.sarif'
+        innerFilePath: 'results.sarif',
+        artifactStorageDir: path.join(tmpDir.name, 'query.ql-123-xyz')
       }
     },
     {
@@ -76,7 +82,8 @@ export const sampleRemoteQueryResult: RemoteQueryResult = {
       downloadLink: {
         id: '137697020',
         urlPath: '/repos/big-corp/controller-repo/actions/artifacts/137697020',
-        innerFilePath: 'results.sarif'
+        innerFilePath: 'results.sarif',
+        artifactStorageDir: path.join(tmpDir.name, 'query.ql-123-xyz')
       }
     }
   ]