Merge pull request #4219 from github/mbg/ci/improve-pr-checks-triggers

Improve `pr-checks.yml` triggers and conditions

.github/workflows/__diagnostics-export.yml

@@ -48,15 +48,6 @@ jobs:
     steps:
       - name: Check out repository
         uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
-      - name: Install Node.js
-        uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0
-        with:
-          node-version: 20.x
-          cache: npm
-      - name: Install newer npm
-        run: npm install -g npm@11.19.1
-      - name: Install dependencies
-        run: npm ci
       - name: Prepare test
         id: prepare-test
         uses: ./.github/actions/prepare-test
@@ -90,13 +81,56 @@ jobs:
           name: diagnostics-export-${{ matrix.os }}-${{ matrix.version }}.sarif.json
           path: '${{ runner.temp }}/results/javascript.sarif'
           retention-days: 7
-
       - name: Check diagnostics appear in SARIF
+        uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0
         env:
           SARIF_PATH: '${{ runner.temp }}/results/javascript.sarif'
-        run: |
-          cd ../action
-          npx tsx ./pr-checks/check-diagnostics.ts
+        with:
+          script: |
+            const fs = require('fs');
+
+            function checkStatusPageNotification(n) {
+              const expectedMessage = 'Plaintext message';
+              if (n.message.text !== expectedMessage) {
+                core.setFailed(`Expected the status page diagnostic to have the message '${expectedMessage}', but found '${n.message.text}'.`);
+              }
+              if (n.locations.length !== 1) {
+                core.setFailed(`Expected the status page diagnostic to have exactly 1 location, but found ${n.locations.length}.`);
+              }
+            }
+
+            const sarif = JSON.parse(fs.readFileSync(process.env['SARIF_PATH'], 'utf8'));
+            const run = sarif.runs[0];
+
+            const toolExecutionNotifications = run.invocations[0].toolExecutionNotifications;
+            const statusPageNotifications = toolExecutionNotifications.filter(n =>
+              n.descriptor.id === 'lang/diagnostics/example' && n.properties?.visibility?.statusPage
+            );
+            if (statusPageNotifications.length !== 1) {
+              core.setFailed(
+                'Expected exactly one status page reporting descriptor for this diagnostic in the ' +
+                  `'runs[].invocations[].toolExecutionNotifications[]' SARIF property, but found ` +
+                  `${statusPageNotifications.length}. All notification reporting descriptors: ` +
+                  `${JSON.stringify(toolExecutionNotifications)}.`
+              );
+            }
+            checkStatusPageNotification(statusPageNotifications[0]);
+
+            const notifications = run.tool.driver.notifications;
+            const diagnosticNotification = notifications.filter(n =>
+              n.id === 'lang/diagnostics/example' && n.name === 'lang/diagnostics/example' &&
+                n.fullDescription.text === 'Diagnostic name'
+            );
+            if (diagnosticNotification.length !== 1) {
+              core.setFailed(
+                'Expected exactly one notification for this diagnostic in the ' +
+                  `'runs[].tool.driver.notifications[]' SARIF property, but found ` +
+                  `${diagnosticNotification.length}. All notifications: ` +
+                  `${JSON.stringify(notifications)}.`
+              );
+            }
+
+            core.info('Finished diagnostic export test');
     env:
       CODEQL_ACTION_EXPORT_DIAGNOSTICS: true
       CODEQL_ACTION_TEST_MODE: true

.github/workflows/__diagnostics-without-db.yml

@@ -1,138 +0,0 @@
-# Warning: This file is generated automatically, and should not be modified.
-# Instead, please modify the template in the pr-checks directory and run:
-#     pr-checks/sync.sh
-# to regenerate this file.
-
-name: PR Check - Diagnostic export without database
-env:
-  GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-  GO111MODULE: auto
-on:
-  push:
-    branches:
-      - main
-      - releases/v*
-  pull_request: {}
-  merge_group:
-    types:
-      - checks_requested
-  schedule:
-    - cron: '0 5 * * *'
-  workflow_dispatch:
-    inputs: {}
-  workflow_call:
-    inputs: {}
-defaults:
-  run:
-    shell: bash
-concurrency:
-  cancel-in-progress: ${{ github.event_name == 'pull_request' || false }}
-  group: diagnostics-without-db-${{github.ref}}
-jobs:
-  diagnostics-without-db:
-    strategy:
-      fail-fast: false
-      matrix:
-        include:
-          - os: ubuntu-latest
-            version: linked
-    name: Diagnostic export without database
-    if: github.triggering_actor != 'dependabot[bot]'
-    permissions:
-      contents: read
-      security-events: read
-    timeout-minutes: 45
-    runs-on: ${{ matrix.os }}
-    steps:
-      # We deliberately need to make this step fail before the database is initialised, but after
-      # some diagnostic has been produced. The simplest way to do that right now is to force the
-      # use of a nightly release and then fail because we are attempting to analyse Swift on
-      # a non-macOS runner.
-      - name: Check out repository
-        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
-      - name: Install Node.js
-        uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0
-        with:
-          node-version: 20.x
-          cache: npm
-      - name: Install newer npm
-        run: npm install -g npm@11.19.1
-      - name: Install dependencies
-        run: npm ci
-      - name: Prepare test
-        id: prepare-test
-        uses: ./.github/actions/prepare-test
-        with:
-          version: ${{ matrix.version }}
-          use-all-platform-bundle: 'false'
-          setup-kotlin: 'true'
-      - name: Initialise failing analysis
-        id: init
-        uses: ./../action/init
-        continue-on-error: true
-        env:
-          CODEQL_ACTION_FORCE_NIGHTLY: true
-        with:
-          languages: swift
-          tools: ${{ steps.prepare-test.outputs.tools-url }}
-
-      - name: Analyze
-        uses: ./analyze
-        # In a real workflow, this step wouldn't run. Since we used `continue-on-error`
-        # above, we manually disable it with an `if` condition.
-        if: false
-        with:
-          category: '/test-codeql-version:${{ matrix.version }}'
-
-      - name: Check diagnostics appear on disk
-        env:
-          SARIF_PATH: '${{ runner.temp }}/codeql-action-temp-diagnostics/'
-        run: |
-          cd ../action
-          npx tsx ./pr-checks/diagnostics-without-db.ts
-    env:
-      CODEQL_ACTION_SKIP_GENERATE_FAILED_SARIF: false
-      CODEQL_ACTION_SKIP_SARIF_UPLOAD: true
-      CODEQL_ACTION_TESTING_ENVIRONMENT: codeql-action-pr-checks
-      CODEQL_ACTION_TEST_MODE: true
-  diagnostics-without-db-validation:
-    if: github.triggering_actor != 'dependabot[bot]'
-    needs:
-      - diagnostics-without-db
-    permissions:
-      contents: read
-      security-events: read
-    timeout-minutes: 5
-    runs-on: ubuntu-latest
-    steps:
-      - name: Check out repository
-        uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
-      - name: Install Node.js
-        uses: actions/setup-node@949feb2413d6458794dcd2491c4babbbce0c15c1 # v7.1.0
-        with:
-          node-version: 20.x
-          cache: npm
-      - name: Install newer npm
-        run: npm install -g npm@11.19.1
-      - name: Install dependencies
-        run: npm ci
-      - name: Download workflow artifact
-        uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
-        with:
-          pattern: test-mode-sarif-*
-          path: sarif
-
-      - name: List artifact contents
-        run: ls -laR ./sarif
-
-      - name: Check diagnostics appear in SARIF
-        env:
-          SARIF_PATH: "./sarif/codeql-failed-run.sarif"
-          DIAGNOSTIC_ID: "codeql-action/forced-nightly-cli"
-          DIAGNOSTIC_NAME: "A nightly release of CodeQL was used"
-          EXPECTED_LOCATIONS: "0"
-          EXPECTED_MESSAGE: |
-            GitHub configured this analysis to use a nightly release of CodeQL to allow you to preview changes from an upcoming release.
-        run: npx tsx ./pr-checks/check-diagnostics.ts
-    env:
-      CODEQL_ACTION_TEST_MODE: true

.github/workflows/pr-checks.yml

@@ -2,6 +2,9 @@ name: PR Checks
 
 on:
   push:
+    branches:
+      - "main"
+      - "releases/v*"
   pull_request:
   merge_group:
     types: [checks_requested]
@@ -145,12 +148,11 @@ jobs:
 
       - name: Fetch base commit
         id: fetch-base
-        # Forks and Dependabot PRs don't have permission to write comments, so skip the repo size
-        # check in those cases.
+        # Skip this (and subsequent steps) if the PR originates from a fork, since this workflow
+        # won't have the necessary permissions to post comments on the PR.
         if: >-
           github.event_name == 'pull_request' &&
-          github.event.pull_request.head.repo.full_name == github.repository &&
-          github.event.pull_request.user.login != 'dependabot[bot]'
+          github.event.pull_request.head.repo.full_name == github.repository
         env:
           BASE_SHA: ${{ github.event.pull_request.base.sha }}
           HEAD_SHA: ${{ github.event.pull_request.head.sha }}
@@ -209,7 +211,7 @@ jobs:
     if: >-
       github.event_name == 'pull_request' &&
       github.event.pull_request.head.repo.full_name == github.repository &&
-      github.event.pull_request.user.login != 'dependabot[bot]' &&
+      github.actor != 'dependabot[bot]' &&
       needs.other-checks.result == 'success'
     permissions:
       contents: read

pr-checks/check-diagnostics.ts

@@ -1,92 +0,0 @@
-import fs from "fs";
-
-import * as core from "@actions/core";
-
-import * as sarif from "../src/sarif";
-
-const DIAGNOSTIC_ID =
-  process.env["DIAGNOSTIC_ID"] ?? "lang/diagnostics/example";
-const DIAGNOSTIC_NAME = process.env["DIAGNOSTIC_NAME"] ?? "Diagnostic name";
-const EXPECTED_MESSAGE = process.env["EXPECTED_MESSAGE"] ?? "Plaintext message";
-const EXPECTED_LOCATIONS = Number.parseInt(
-  process.env["EXPECTED_LOCATIONS"] ?? "1",
-);
-
-function checkStatusPageNotification(n: sarif.Notification) {
-  const expectedMessage = EXPECTED_MESSAGE.trim();
-  if (!n.message.text?.includes(expectedMessage)) {
-    if (n.message.text !== undefined) {
-      core.setFailed(
-        `Expected the status page diagnostic to contain the message '${expectedMessage}', but found '${n.message.text.replaceAll("\n", "\\n")}'.`,
-      );
-    } else {
-      core.setFailed(
-        `Expected the status page diagnostic to contain the message '${expectedMessage}', but found nothing.`,
-      );
-    }
-  }
-
-  const locationCount = n.locations?.length ?? 0;
-  if (locationCount !== EXPECTED_LOCATIONS) {
-    core.setFailed(
-      `Expected the status page diagnostic to have exactly ${EXPECTED_LOCATIONS} location(s), but found ${locationCount}.`,
-    );
-  }
-}
-
-const sarifPath = process.env["SARIF_PATH"];
-
-if (sarifPath === undefined) {
-  throw new Error("The path to a SARIF file must be set in 'SARIF_PATH'.");
-}
-
-const log = JSON.parse(fs.readFileSync(sarifPath, "utf8")) as sarif.Log;
-const run = log.runs[0];
-
-if (run.invocations === undefined) {
-  throw new Error("run.invocations is undefined");
-}
-
-const toolExecutionNotifications =
-  run.invocations[0].toolExecutionNotifications;
-
-if (toolExecutionNotifications === undefined) {
-  throw new Error("toolExecutionNotifications is undefined");
-}
-
-const statusPageNotifications = toolExecutionNotifications.filter(
-  (n) =>
-    n.descriptor?.id === DIAGNOSTIC_ID && n.properties?.visibility?.statusPage,
-);
-if (statusPageNotifications.length !== 1) {
-  core.setFailed(
-    "Expected exactly one status page reporting descriptor for this diagnostic in the " +
-      `'runs[].invocations[].toolExecutionNotifications[]' SARIF property, but found ` +
-      `${statusPageNotifications.length}. All notification reporting descriptors: ` +
-      `${JSON.stringify(toolExecutionNotifications)}.`,
-  );
-}
-checkStatusPageNotification(statusPageNotifications[0]);
-
-const notifications = run.tool.driver.notifications;
-
-if (notifications === undefined) {
-  throw new Error("notifications is undefined");
-}
-
-const diagnosticNotification = notifications.filter(
-  (n) =>
-    n.id === DIAGNOSTIC_ID &&
-    n.name === DIAGNOSTIC_ID &&
-    n.fullDescription?.text === DIAGNOSTIC_NAME,
-);
-if (diagnosticNotification.length !== 1) {
-  core.setFailed(
-    "Expected exactly one notification for this diagnostic in the " +
-      `'runs[].tool.driver.notifications[]' SARIF property, but found ` +
-      `${diagnosticNotification.length}. All notifications: ` +
-      `${JSON.stringify(notifications)}.`,
-  );
-}
-
-core.info("Finished diagnostic export test");

pr-checks/checks/diagnostics-export.yml

@@ -5,7 +5,6 @@ versions:
   - nightly-latest
 env:
   CODEQL_ACTION_EXPORT_DIAGNOSTICS: true
-installNode: true
 steps:
   - uses: ./../action/init
     id: init
@@ -33,10 +32,53 @@ steps:
       name: diagnostics-export-${{ matrix.os }}-${{ matrix.version }}.sarif.json
       path: "${{ runner.temp }}/results/javascript.sarif"
       retention-days: 7
-
   - name: Check diagnostics appear in SARIF
+    uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0
     env:
       SARIF_PATH: "${{ runner.temp }}/results/javascript.sarif"
-    run: |
-      cd ../action
-      npx tsx ./pr-checks/check-diagnostics.ts
+    with:
+      script: |
+        const fs = require('fs');
+
+        function checkStatusPageNotification(n) {
+          const expectedMessage = 'Plaintext message';
+          if (n.message.text !== expectedMessage) {
+            core.setFailed(`Expected the status page diagnostic to have the message '${expectedMessage}', but found '${n.message.text}'.`);
+          }
+          if (n.locations.length !== 1) {
+            core.setFailed(`Expected the status page diagnostic to have exactly 1 location, but found ${n.locations.length}.`);
+          }
+        }
+
+        const sarif = JSON.parse(fs.readFileSync(process.env['SARIF_PATH'], 'utf8'));
+        const run = sarif.runs[0];
+
+        const toolExecutionNotifications = run.invocations[0].toolExecutionNotifications;
+        const statusPageNotifications = toolExecutionNotifications.filter(n =>
+          n.descriptor.id === 'lang/diagnostics/example' && n.properties?.visibility?.statusPage
+        );
+        if (statusPageNotifications.length !== 1) {
+          core.setFailed(
+            'Expected exactly one status page reporting descriptor for this diagnostic in the ' +
+              `'runs[].invocations[].toolExecutionNotifications[]' SARIF property, but found ` +
+              `${statusPageNotifications.length}. All notification reporting descriptors: ` +
+              `${JSON.stringify(toolExecutionNotifications)}.`
+          );
+        }
+        checkStatusPageNotification(statusPageNotifications[0]);
+
+        const notifications = run.tool.driver.notifications;
+        const diagnosticNotification = notifications.filter(n =>
+          n.id === 'lang/diagnostics/example' && n.name === 'lang/diagnostics/example' &&
+            n.fullDescription.text === 'Diagnostic name'
+        );
+        if (diagnosticNotification.length !== 1) {
+          core.setFailed(
+            'Expected exactly one notification for this diagnostic in the ' +
+              `'runs[].tool.driver.notifications[]' SARIF property, but found ` +
+              `${diagnosticNotification.length}. All notifications: ` +
+              `${JSON.stringify(notifications)}.`
+          );
+        }
+
+        core.info('Finished diagnostic export test');

pr-checks/checks/diagnostics-without-db.yml

@@ -1,67 +0,0 @@
-name: "Diagnostic export without database"
-description: "Tests that temporary diagnostics are correctly included in the failed SARIF."
-versions:
-  # Overridden by the FF below.
-  - linked
-installNode: true
-
-env:
-  # Allow the failed SARIF file to be generated.
-  CODEQL_ACTION_SKIP_GENERATE_FAILED_SARIF: false
-  # But don't actually upload it.
-  CODEQL_ACTION_SKIP_SARIF_UPLOAD: true
-  # Mark telemetry for this workflow so it can be treated separately.
-  CODEQL_ACTION_TESTING_ENVIRONMENT: codeql-action-pr-checks
-
-steps:
-  # We deliberately need to make this step fail before the database is initialised, but after
-  # some diagnostic has been produced. The simplest way to do that right now is to force the
-  # use of a nightly release and then fail because we are attempting to analyse Swift on
-  # a non-macOS runner.
-  - name: Initialise failing analysis
-    id: init
-    uses: ./../action/init
-    continue-on-error: true
-    env:
-      CODEQL_ACTION_FORCE_NIGHTLY: true
-    with:
-      languages: swift
-      tools: ${{ steps.prepare-test.outputs.tools-url }}
-
-  - name: Analyze
-    uses: ./analyze
-    # In a real workflow, this step wouldn't run. Since we used `continue-on-error`
-    # above, we manually disable it with an `if` condition.
-    if: false
-    with:
-      category: "/test-codeql-version:${{ matrix.version }}"
-
-  - name: Check diagnostics appear on disk
-    env:
-      SARIF_PATH: "${{ runner.temp }}/codeql-action-temp-diagnostics/"
-    run: |
-      cd ../action
-      npx tsx ./pr-checks/diagnostics-without-db.ts
-validationJobs:
-  diagnostics-without-db-validation:
-    checkout: true
-    installNode: true
-    steps:
-      - name: Download workflow artifact
-        uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1
-        with:
-          pattern: test-mode-sarif-*
-          path: sarif
-
-      - name: List artifact contents
-        run: ls -laR ./sarif
-
-      - name: Check diagnostics appear in SARIF
-        env:
-          SARIF_PATH: "./sarif/codeql-failed-run.sarif"
-          DIAGNOSTIC_ID: "codeql-action/forced-nightly-cli"
-          DIAGNOSTIC_NAME: "A nightly release of CodeQL was used"
-          EXPECTED_LOCATIONS: "0"
-          EXPECTED_MESSAGE: |
-            GitHub configured this analysis to use a nightly release of CodeQL to allow you to preview changes from an upcoming release.
-        run: npx tsx ./pr-checks/check-diagnostics.ts

pr-checks/diagnostics-without-db.ts

@@ -1,48 +0,0 @@
-import * as fs from "fs";
-import * as path from "path";
-
-import * as core from "@actions/core";
-
-import type * as diagnostics from "../src/diagnostics";
-
-const DIAGNOSTIC_ID = "codeql-action/forced-nightly-cli";
-const sarifDirectory = process.env["SARIF_PATH"];
-
-if (sarifDirectory === undefined) {
-  throw new Error("SARIF_PATH is undefined.");
-}
-
-const paths = fs
-  .readdirSync(sarifDirectory, {
-    recursive: true,
-    encoding: "utf-8",
-  })
-  .filter((p) => p.endsWith(".json"));
-
-core.info(`Found ${paths.length} paths in '${sarifDirectory}'.`);
-core.info(`Searching for a diagnostic with id '${DIAGNOSTIC_ID}'...`);
-
-let found = false;
-for (const diagnosticPath of paths) {
-  if (path.extname(diagnosticPath) !== ".json") {
-    core.info(`Skipping ${diagnosticPath}.`);
-    continue;
-  }
-  core.info(`Found ${diagnosticPath}.`);
-
-  const contents = JSON.parse(
-    fs.readFileSync(path.resolve(sarifDirectory, diagnosticPath), "utf-8"),
-  ) as diagnostics.DiagnosticMessage;
-
-  if (contents.source.id === DIAGNOSTIC_ID) {
-    core.info(
-      `Found '${DIAGNOSTIC_ID}' in '${diagnosticPath}'. Skipping remaining files.`,
-    );
-    found = true;
-    break;
-  }
-}
-
-if (!found) {
-  throw new Error(`Didn't find '${DIAGNOSTIC_ID}' diagnostic.`);
-}

pr-checks/sync.ts

@@ -90,7 +90,7 @@ interface Specification extends JobSpecification {
   services?: any;
 
   /** Additional jobs to run after the main PR check job. */
-  validationJobs?: Record<string, ValidationJobSpecification>;
+  validationJobs?: Record<string, JobSpecification>;
 
   /** If set, this check is part of a named collection that gets its own caller workflow. */
   collection?: string;
@@ -122,10 +122,6 @@ interface JobSpecification {
   installYq?: boolean;
 }
 
-type ValidationJobSpecification = JobSpecification & {
-  checkout?: boolean;
-};
-
 /** Describes language/framework-specific steps and inputs. */
 interface LanguageSetup {
   specProperty: keyof JobSpecification;
@@ -466,20 +462,6 @@ function generateJobMatrix(
   return matrix;
 }
 
-/**
- * Gets the `actions/checkout` step.
- */
-function getCheckoutStep(): Step {
-  return {
-    name: "Check out repository",
-    uses: pinnedUses(
-      "actions/checkout",
-      "3d3c42e5aac5ba805825da76410c181273ba90b1",
-      "v7.0.1",
-    ),
-  };
-}
-
 /**
  * Retrieves setup steps and additional input definitions based on specific languages or frameworks
  * that are requested by the `checkSpecification`.
@@ -549,7 +531,14 @@ function generateJob(
 
   // Construct the workflow steps needed for this check.
   const steps: Step[] = [
-    getCheckoutStep(),
+    {
+      name: "Check out repository",
+      uses: pinnedUses(
+        "actions/checkout",
+        "3d3c42e5aac5ba805825da76410c181273ba90b1",
+        "v7.0.1",
+      ),
+    },
     ...setupInfo.steps,
     {
       name: "Prepare test",
@@ -620,16 +609,13 @@ function generateJob(
 /** Generates a validation job. */
 function generateValidationJob(
   specDocument: yaml.Document,
-  jobSpecification: ValidationJobSpecification,
+  jobSpecification: JobSpecification,
   checkName: string,
   name: string,
 ) {
   // Determine which languages or frameworks have to be installed.
   const { inputs, steps } = getSetupSteps(jobSpecification);
 
-  // Construct a checkout step if requested.
-  const checkoutStep = jobSpecification.checkout ? [getCheckoutStep()] : [];
-
   // Extract the sequence of steps from the YAML document to persist as much formatting as possible.
   const specSteps = specDocument.getIn([
     "validationJobs",
@@ -638,7 +624,7 @@ function generateValidationJob(
   ]) as yaml.YAMLSeq;
 
   // Add the generated steps in front of the ones from the specification.
-  specSteps.items.unshift(...checkoutStep, ...steps);
+  specSteps.items.unshift(...steps);
 
   const validationJob: Record<string, any> = {
     name: jobSpecification.name,
@@ -649,7 +635,7 @@ function generateValidationJob(
       "security-events": "read",
     },
     "timeout-minutes": 5,
-    "runs-on": "ubuntu-latest",
+    "runs-on": "ubuntu-slim",
     steps: specSteps,
   };
 

src/analyze-action.ts

@@ -361,7 +361,6 @@ async function run(action: ActionState<["Base", "Logger", "Env", "Actions"]>) {
       uploadResults = await postProcessAndUploadSarif(
         logger,
         features,
-        config,
         uploadKind,
         checkoutPath,
         outputDir,

src/codeql.ts

@@ -199,7 +199,6 @@ export interface CodeQL {
     sarifFile: string,
     automationDetailsId: string | undefined,
     config: ActionConfig<"tempDir">,
-    diagnosticDir: string | undefined,
   ): Promise<void>;
   /** Get the location of an extractor for the specified language. */
   resolveExtractor(language: Language): Promise<string>;
@@ -916,32 +915,15 @@ async function getCodeQLForCmd(
       sarifFile: string,
       automationDetailsId: string | undefined,
       config: ActionConfig<"tempDir">,
-      diagnosticDir: string | undefined,
     ): Promise<void> {
-      // We may be exporting diagnostics for a failed SARIF, in which case the configuration
-      // file may not have been written to disk and there may not be a database. The CLI
-      // will produce a SARIF for diagnostics from `diagnosticDir` without it. Therefore,
-      // if `diagnosticDir` is set, we check whether `configPath` exists and only provide
-      // it to the CLI if it does. Although not needed, the CLI will produce an error if it
-      // is provided with an argument to `--sarif-codescanning-config` that does not exist.
-      let configPath: string | undefined =
-        getGeneratedCodeScanningConfigPath(config);
-
-      if (diagnosticDir !== undefined && !fs.existsSync(configPath)) {
-        configPath = undefined;
-      }
-
       const args = [
         "diagnostics",
         "export",
         "--format=sarif-latest",
         `--output=${sarifFile}`,
-        ...(configPath !== undefined
-          ? [`--sarif-codescanning-config=${configPath}`]
-          : []),
-        ...(diagnosticDir !== undefined
-          ? [`--diagnostic-dir=${diagnosticDir}`]
-          : []),
+        `--sarif-codescanning-config=${getGeneratedCodeScanningConfigPath(
+          config,
+        )}`,
         ...getExtraOptionsFromEnv(["diagnostics", "export"]),
       ];
       if (automationDetailsId !== undefined) {

src/config/db-config.test.ts

@@ -9,9 +9,8 @@ import {
   LoggedMessage,
   makeMacro,
   RecordingLogger,
-  setupActionsVars,
 } from "../testing-utils";
-import { ConfigurationError, prettyPrintPack, withTmpDir } from "../util";
+import { ConfigurationError, prettyPrintPack } from "../util";
 
 import * as dbConfig from "./db-config";
 
@@ -563,84 +562,69 @@ test("mergeDefaultSetupAndUserConfigs - keeps other properties from user-supplie
   t.deepEqual(result, configFile);
 });
 
-test.serial(
-  "mergeDefaultSetupAndUserConfigs - ignores, but warns about, unknown keys from Default Setup",
-  async (t) => {
-    const logger = new RecordingLogger();
-    const configFile: dbConfig.UserConfig = {
-      "query-filters": [{ exclude: { a: "b" } }],
-      "paths-ignore": ["path"],
-    };
+test("mergeDefaultSetupAndUserConfigs - ignores, but warns about, unknown keys from Default Setup", async (t) => {
+  const logger = new RecordingLogger();
+  const configFile: dbConfig.UserConfig = {
+    "query-filters": [{ exclude: { a: "b" } }],
+    "paths-ignore": ["path"],
+  };
 
-    await withTmpDir(async (tmpDir) => {
-      setupActionsVars(tmpDir, tmpDir);
-      const result = dbConfig.mergeDefaultSetupAndUserConfigs(
-        logger,
-        {
-          "default-setup": {
-            borg: [],
-            org: {
-              unknown: "foo",
-              "model-packs": [],
-            },
-          } as unknown as dbConfig.DefaultSetupConfig,
-          "paths-ignore": ["other-path"],
+  const result = dbConfig.mergeDefaultSetupAndUserConfigs(
+    logger,
+    {
+      "default-setup": {
+        borg: [],
+        org: {
+          unknown: "foo",
+          "model-packs": [],
         },
-        configFile,
-      );
+      } as unknown as dbConfig.DefaultSetupConfig,
+      "paths-ignore": ["other-path"],
+    },
+    configFile,
+  );
 
-      t.deepEqual(result, {
-        ...configFile,
-        "default-setup": { org: { "model-packs": [] } },
-      });
+  t.deepEqual(result, {
+    ...configFile,
+    "default-setup": { org: { "model-packs": [] } },
+  });
 
-      const expectedUnrecognisedKeys = [
-        ".default-setup.org.unknown",
-        ".default-setup.borg",
-        ".paths-ignore",
-      ].join(", ");
-      checkExpectedLogMessages(t, logger.messages, [
-        `Unrecognised keys in Default Setup configuration: ${expectedUnrecognisedKeys}`,
-      ]);
-    });
-  },
-);
+  const expectedUnrecognisedKeys = [
+    ".default-setup.org.unknown",
+    ".default-setup.borg",
+    ".paths-ignore",
+  ].join(", ");
+  checkExpectedLogMessages(t, logger.messages, [
+    `Unrecognised keys in Default Setup configuration: ${expectedUnrecognisedKeys}`,
+  ]);
+});
 
-test.serial(
-  "mergeDefaultSetupAndUserConfigs - warns about invalid keys from Default Setup",
-  async (t) => {
-    const logger = new RecordingLogger();
-    const configFile: dbConfig.UserConfig = {};
+test("mergeDefaultSetupAndUserConfigs - warns about invalid keys from Default Setup", async (t) => {
+  const logger = new RecordingLogger();
+  const configFile: dbConfig.UserConfig = {};
 
-    await withTmpDir(async (tmpDir) => {
-      setupActionsVars(tmpDir, tmpDir);
-
-      const result = dbConfig.mergeDefaultSetupAndUserConfigs(
-        logger,
-        {
-          "default-setup": {
-            org: {
-              "model-packs": [123],
-            },
-          } as unknown as dbConfig.DefaultSetupConfig,
+  const result = dbConfig.mergeDefaultSetupAndUserConfigs(
+    logger,
+    {
+      "default-setup": {
+        org: {
+          "model-packs": [123],
         },
-        configFile,
-      );
+      } as unknown as dbConfig.DefaultSetupConfig,
+    },
+    configFile,
+  );
 
-      t.deepEqual(result, {
-        ...configFile,
-        "default-setup": { org: { "model-packs": [123] } },
-      });
+  t.deepEqual(result, {
+    ...configFile,
+    "default-setup": { org: { "model-packs": [123] } },
+  });
 
-      const expectedInvalidKeys = [".default-setup.org.model-packs[0]"].join(
-        ", ",
-      );
-      checkExpectedLogMessages(t, logger.messages, [
-        `Invalid keys in Default Setup configuration: ${expectedInvalidKeys}`,
-      ]);
-    });
-  },
-);
+  const expectedInvalidKeys = [".default-setup.org.model-packs[0]"].join(", ");
+  checkExpectedLogMessages(t, logger.messages, [
+    `Invalid keys in Default Setup configuration: ${expectedInvalidKeys}`,
+  ]);
+});
 
 /** Parses `contents` as a configuration without validating it. */
 function parseUnvalidatedConfig(contents: string): dbConfig.UserConfig {

src/diagnostics.test.ts

@@ -1,111 +0,0 @@
-import * as fs from "fs";
-import path from "path";
-
-import test, { type ExecutionContext } from "ava";
-
-import * as diagnostics from "./diagnostics";
-import { BuiltInLanguage } from "./languages";
-import {
-  checkExpectedLogMessages,
-  createTestConfig,
-  initAllState,
-  RecordingLogger,
-  setupTests,
-} from "./testing-utils";
-import { withTmpDir } from "./util";
-
-setupTests(test);
-
-const id = "codeql-action/test-diagnostic";
-const name = "Test title";
-
-test("makeDiagnostic - adds expected properties", (t) => {
-  const diagnostic = diagnostics.makeDiagnostic(id, name, {});
-
-  t.truthy(diagnostic.timestamp);
-  t.is(diagnostic.source.id, id);
-  t.is(diagnostic.source.name, name);
-});
-
-/**
- * Diagnostic files that are written to disk don't have deterministic names.
- * This function recursively finds all `.json` files at {@link rootPath},
- * asserts that there is exactly one, and that it matches {@link expected}.
- *
- * @param rootPath The path to start searching from.
- */
-function assertSingleDiagnosticIn(
-  t: ExecutionContext<unknown>,
-  rootPath: string,
-  expected: diagnostics.DiagnosticMessage,
-) {
-  // Find all `.json` files in `rootPath` and check that there is exactly one.
-  const filePaths = fs
-    .readdirSync(rootPath, {
-      recursive: true,
-      encoding: "utf-8",
-    })
-    .filter((p) => p.endsWith(".json"));
-
-  t.is(filePaths.length, 1);
-
-  // Check that the written diagnostic matches the one we added.
-  const contents = fs.readFileSync(
-    path.resolve(rootPath, filePaths[0]),
-    "utf-8",
-  );
-
-  const readDiagnostic = JSON.parse(contents) as diagnostics.DiagnosticMessage;
-  t.deepEqual(readDiagnostic, expected);
-}
-
-test("addDiagnostic writes temporary diagnostics and flushDiagnostics moves them to the database", async (t) =>
-  withTmpDir(async (tmpDir) => {
-    const logger = new RecordingLogger();
-
-    const tempDiagDir = path.resolve(tmpDir, "temp");
-    const testDbDir = path.resolve(tmpDir, "test", "database");
-    const diagnostic = diagnostics.makeDiagnostic(id, name, {});
-
-    // Write the diagnostic. The path provided for `dbLocation` doesn't exist,
-    // so the diagnostic will be written to a temporary location.
-    t.notThrows(() => {
-      diagnostics.addDiagnostic(
-        createTestConfig({ tempDir: tempDiagDir, dbLocation: testDbDir }),
-        BuiltInLanguage.actions,
-        diagnostic,
-        logger,
-      );
-    });
-
-    // Check that `tempDiagDir` was created.
-    t.true(fs.existsSync(tempDiagDir));
-
-    // Check that the diagnostic exists under `tempDiagDir`.
-    assertSingleDiagnosticIn(t, tempDiagDir, diagnostic);
-
-    // Flush the diagnostics. This will cause the diagnostic we wrote previously to
-    // be moved to a location under `dbLocation`.
-    t.notThrows(() => {
-      diagnostics.flushDiagnostics(
-        initAllState({ logger }),
-        createTestConfig({
-          tempDir: tempDiagDir,
-          dbLocation: testDbDir,
-        }),
-      );
-    });
-
-    // Check that `testDbDir` was created.
-    t.true(fs.existsSync(testDbDir));
-
-    // Check that the diagnostic exists under `testDbDir`.
-    assertSingleDiagnosticIn(t, testDbDir, diagnostic);
-
-    // Check that we logged that the database directory doesn't exist yet
-    // and that we moved one diagnostic.
-    checkExpectedLogMessages(t, logger.messages, [
-      `but the database at ${path.join(testDbDir, "actions")} does not exist yet`,
-      "Moving 1 diagnostic(s) to their respective databases.",
-    ]);
-  }));

src/diagnostics.ts

@@ -1,16 +1,10 @@
-import * as fs from "fs";
+import { existsSync, mkdirSync, writeFileSync } from "fs";
 import path from "path";
 
-import type { ActionState } from "./action-common";
-import { getTemporaryDirectory } from "./actions-util";
 import type { Config } from "./config-utils";
 import { Language } from "./languages";
-import { getActionsLogger, type Logger } from "./logging";
-import {
-  getCodeQLDatabasePath,
-  getErrorMessage,
-  isErrnoException,
-} from "./util";
+import { getActionsLogger } from "./logging";
+import { getCodeQLDatabasePath } from "./util";
 
 /**
  * Known tags for diagnostics. There is currently only "internal-error",
@@ -90,16 +84,22 @@ export type DiagnosticMessage = DiagnosticMessageOptions & {
   source: DiagnosticSource;
 };
 
-/** Represents a diagnostic message that has not yet been saved to the database. */
-interface TemporaryDiagnostic {
-  /** The path to which the diagnostic has temporarily been written to. */
-  path: string;
-  /** The language the diagnostic is for, if any. */
-  language?: Language;
+/** Represents a diagnostic message that has not yet been written to the database. */
+interface UnwrittenDiagnostic {
+  /** The diagnostic message that has not yet been written. */
+  diagnostic: DiagnosticMessage;
+  /** The language the diagnostic is for. */
+  language: Language;
 }
 
-/** A list of diagnostics which have not yet been saved to the database. */
-let temporaryDiagnostics: TemporaryDiagnostic[] = [];
+/** A list of diagnostics which have not yet been written to disk. */
+let unwrittenDiagnostics: UnwrittenDiagnostic[] = [];
+
+/**
+ * A list of diagnostics which have not yet been written to disk,
+ * and where the language does not matter.
+ */
+let unwrittenDefaultLanguageDiagnostics: DiagnosticMessage[] = [];
 
 /**
  * Counter used to generate a unique suffix for each diagnostic filename, so that
@@ -128,29 +128,6 @@ export function makeDiagnostic(
   };
 }
 
-/**
- * Gets a path relative to {@link tmpDir} where diagnostics for {@link language}
- * can temporarily be stored at before the database is initialised. If {@link language}
- * is `undefined`, then the common base path for all languages relative to {@link tmpDir}
- * is returned.
- */
-export function getTempDiagnosticPath(tmpDir: string, language?: Language) {
-  const root = path.join(tmpDir, "codeql-action-temp-diagnostics");
-  return getDiagnosticsPath(root, language);
-}
-
-/**
- * Gets a path relative to {@link basePath} where diagnostics for {@link language}
- * should be stored at. If {@link language} is `undefined`, then {@link basePath}
- * is returned instead.
- */
-export function getDiagnosticsPath(basePath: string, language?: Language) {
-  if (language !== undefined) {
-    return path.resolve(basePath, language, "diagnostic", "codeql-action");
-  }
-  return basePath;
-}
-
 /**
  * Adds the given diagnostic to the database. If the database does not yet exist,
  * the diagnostic will be written to it once it has been created.
@@ -163,23 +140,22 @@ export function addDiagnostic(
   config: Config,
   language: Language,
   diagnostic: DiagnosticMessage,
-  logger: Logger = getActionsLogger(),
 ) {
+  const logger = getActionsLogger();
   const databasePath = language
     ? getCodeQLDatabasePath(config, language)
     : config.dbLocation;
 
   // Check that the database exists before writing to it. If the database does not yet exist,
   // store the diagnostic in memory and write it later.
-  if (fs.existsSync(databasePath)) {
-    writeDiagnostic({ logger }, config, language, diagnostic);
+  if (existsSync(databasePath)) {
+    writeDiagnostic(config, language, diagnostic);
   } else {
     logger.debug(
       `Writing a diagnostic for ${language}, but the database at ${databasePath} does not exist yet.`,
     );
 
-    // Write the diagnostic to a temporary location.
-    writeTemporaryDiagnosticFile(logger, config.tempDir, language, diagnostic);
+    unwrittenDiagnostics.push({ diagnostic, language });
   }
 }
 
@@ -187,7 +163,6 @@ export function addDiagnostic(
 export function addNoLanguageDiagnostic(
   config: Config | undefined,
   diagnostic: DiagnosticMessage,
-  logger: Logger = getActionsLogger(),
 ) {
   if (config !== undefined) {
     addDiagnostic(
@@ -198,82 +173,10 @@ export function addNoLanguageDiagnostic(
       diagnostic,
     );
   } else {
-    logger.debug(
-      `Writing a diagnostic not associated with any language, but we have no configuration to tell us where the database is yet.`,
-    );
-
-    // Write the diagnostic to a temporary location.
-    writeTemporaryDiagnosticFile(
-      logger,
-      getTemporaryDirectory(),
-      undefined,
-      diagnostic,
-    );
+    unwrittenDefaultLanguageDiagnostics.push(diagnostic);
   }
 }
 
-/**
- * Writes {@link diagnostic} for an optional {@link language} to a subdirectory of {@link tempDir}
- * and tracks it as a diagnostic that was written to a temporary location.
- */
-function writeTemporaryDiagnosticFile(
-  logger: Logger,
-  tempDir: string,
-  language: Language | undefined,
-  diagnostic: DiagnosticMessage,
-) {
-  const tempDiagnosticsPath = getTempDiagnosticPath(tempDir, language);
-
-  try {
-    // Write the diagnostic to a temporary location.
-    const diagnosticPath = writeDiagnosticFile(tempDiagnosticsPath, diagnostic);
-
-    // Track the temporary file.
-    temporaryDiagnostics.push({ path: diagnosticPath, language });
-  } catch (err) {
-    logger.warning(
-      `Unable to write temporary diagnostic to '${tempDiagnosticsPath}': ${getErrorMessage(err)}\n\n${JSON.stringify(diagnostic)}`,
-    );
-  }
-}
-
-/**
- * Writes {@link diagnostic} to a file in {@link diagnosticsPath}.
- */
-function writeDiagnosticFile(
-  diagnosticsPath: string,
-  diagnostic: DiagnosticMessage,
-): string {
-  // Create the directory if it doesn't exist yet.
-  fs.mkdirSync(diagnosticsPath, { recursive: true });
-
-  // Include a monotonically increasing suffix to avoid filename collisions
-  // between diagnostics produced within the same millisecond.
-  const uniqueSuffix = (diagnosticCounter++).toString();
-  // We should only need to remove colons, but to be defensive, only allow a restricted set of
-  // characters.
-  const sanitizedTimestamp = diagnostic.timestamp.replace(
-    /[^a-zA-Z0-9.-]/g,
-    "",
-  );
-  const jsonPath = path.resolve(
-    diagnosticsPath,
-    `codeql-action-${sanitizedTimestamp}-${uniqueSuffix}.json`,
-  );
-
-  fs.writeFileSync(jsonPath, JSON.stringify(diagnostic));
-
-  return jsonPath;
-}
-
-/** Gets the path where diagnostics for {@link language} should be stored in the database. */
-export function getDatabaseDiagnosticsPath(
-  config: Config,
-  language: Language | undefined,
-) {
-  return getDiagnosticsPath(config.dbLocation, language);
-}
-
 /**
  * Writes the given diagnostic to the database.
  *
@@ -282,85 +185,78 @@ export function getDatabaseDiagnosticsPath(
  * @param diagnostic The diagnostic message to add to the database.
  */
 function writeDiagnostic(
-  action: ActionState<["Logger"]>,
   config: Config,
   language: Language | undefined,
   diagnostic: DiagnosticMessage,
 ) {
-  const diagnosticsPath = getDatabaseDiagnosticsPath(config, language);
+  const logger = getActionsLogger();
+  const databasePath = language
+    ? getCodeQLDatabasePath(config, language)
+    : config.dbLocation;
+  const diagnosticsPath = path.resolve(
+    databasePath,
+    "diagnostic",
+    "codeql-action",
+  );
 
   try {
-    writeDiagnosticFile(diagnosticsPath, diagnostic);
-  } catch (err) {
-    action.logger.warning(
-      `Unable to write diagnostic message to database: ${err}`,
+    // Create the directory if it doesn't exist yet.
+    mkdirSync(diagnosticsPath, { recursive: true });
+
+    // Include a monotonically increasing suffix to avoid filename collisions
+    // between diagnostics produced within the same millisecond.
+    const uniqueSuffix = (diagnosticCounter++).toString();
+    // We should only need to remove colons, but to be defensive, only allow a restricted set of
+    // characters.
+    const sanitizedTimestamp = diagnostic.timestamp.replace(
+      /[^a-zA-Z0-9.-]/g,
+      "",
     );
-    action.logger.debug(JSON.stringify(diagnostic));
+    const jsonPath = path.resolve(
+      diagnosticsPath,
+      `codeql-action-${sanitizedTimestamp}-${uniqueSuffix}.json`,
+    );
+
+    writeFileSync(jsonPath, JSON.stringify(diagnostic));
+  } catch (err) {
+    logger.warning(`Unable to write diagnostic message to database: ${err}`);
+    logger.debug(JSON.stringify(diagnostic));
   }
 }
 
-/** Report if there are temporary diagnostics and write them to the log. */
-export function logTemporaryDiagnostics(action: ActionState<["Logger"]>) {
-  const num = temporaryDiagnostics.length;
-
+/** Report if there are unwritten diagnostics and write them to the log. */
+export function logUnwrittenDiagnostics() {
+  const logger = getActionsLogger();
+  const num = unwrittenDiagnostics.length;
   if (num > 0) {
-    action.logger.warning(
+    logger.warning(
       `${num} diagnostic(s) could not be written to the database and will not appear on the Tool Status Page.`,
     );
 
-    for (const temporary of temporaryDiagnostics) {
-      action.logger.debug(fs.readFileSync(temporary.path, "utf-8"));
+    for (const unwritten of unwrittenDiagnostics) {
+      logger.debug(JSON.stringify(unwritten.diagnostic));
     }
   }
 }
 
-/** Relocates all temporary diagnostics to the respective databases. */
-export function flushDiagnostics(
-  action: ActionState<["Logger"]>,
-  config: Config,
-) {
-  const diagnosticsCount = temporaryDiagnostics.length;
-  action.logger.debug(
-    `Moving ${diagnosticsCount} diagnostic(s) to their respective databases.`,
-  );
+/** Writes all unwritten diagnostics to disk. */
+export function flushDiagnostics(config: Config) {
+  const logger = getActionsLogger();
 
-  const failedFlushDiagnostics: TemporaryDiagnostic[] = [];
-  for (const temporary of temporaryDiagnostics) {
-    // If `temporary.language` is `undefined`, then we didn't have a `config` at the time that
-    // `addNoLanguageDiagnostic` was called. In that case, we arbitrarily choose the first
-    // configured language here like we would have done in `addNoLanguageDiagnostic`.
-    const directory = getDatabaseDiagnosticsPath(
-      config,
-      temporary.language ?? config.languages[0],
-    );
-    const filename = path.basename(temporary.path);
-    const destination = path.join(directory, filename);
+  const diagnosticsCount =
+    unwrittenDiagnostics.length + unwrittenDefaultLanguageDiagnostics.length;
+  logger.debug(`Writing ${diagnosticsCount} diagnostic(s) to database.`);
 
-    try {
-      fs.mkdirSync(directory, { recursive: true });
-      try {
-        fs.renameSync(temporary.path, destination);
-      } catch (renameErr: unknown) {
-        // Rethrow the error if it isn't an EXDEV one.
-        if (!isErrnoException(renameErr) || renameErr.code !== "EXDEV") {
-          throw renameErr;
-        }
-
-        // The error was an EXDEV one: try to copy the file and delete the temporary one.
-        fs.copyFileSync(temporary.path, destination);
-        fs.unlinkSync(temporary.path);
-      }
-    } catch (err) {
-      action.logger.warning(
-        `Failed to relocate '${temporary.path}' to '${destination}': ${getErrorMessage(err)}`,
-      );
-
-      failedFlushDiagnostics.push(temporary);
-    }
+  for (const unwritten of unwrittenDiagnostics) {
+    writeDiagnostic(config, unwritten.language, unwritten.diagnostic);
+  }
+  for (const unwritten of unwrittenDefaultLanguageDiagnostics) {
+    addNoLanguageDiagnostic(config, unwritten);
   }
 
-  // Reset the temporary diagnostics arrays.
-  temporaryDiagnostics = failedFlushDiagnostics;
+  // Reset the unwritten diagnostics arrays.
+  unwrittenDiagnostics = [];
+  unwrittenDefaultLanguageDiagnostics = [];
 }
 
 /**

src/environment.ts

@@ -165,13 +165,6 @@ export enum EnvVar {
    */
   SARIF_DUMP_DIR = "CODEQL_ACTION_SARIF_DUMP_DIR",
 
-  /**
-   * Whether to skip generating a failed SARIF file. Intended for testing purposes.
-   * This setting is more specific than `CODEQL_ACTION_TEST_MODE` and
-   * `CODEQL_ACTION_SKIP_SARIF_UPLOAD`, which imply this option.
-   */
-  SKIP_GENERATE_FAILED_SARIF = "CODEQL_ACTION_SKIP_GENERATE_FAILED_SARIF",
-
   /**
    * Whether to skip uploading SARIF results to GitHub. Intended for testing purposes.
    * This setting is more specific than `CODEQL_ACTION_TEST_MODE`, which implies this option.

src/init-action-post-helper.test.ts

@@ -1,6 +1,3 @@
-import * as fs from "fs";
-import * as path from "path";
-
 import * as core from "@actions/core";
 import test, { ExecutionContext } from "ava";
 import * as sinon from "sinon";
@@ -11,7 +8,6 @@ import * as apiClient from "./api-client";
 import * as codeql from "./codeql";
 import * as configUtils from "./config-utils";
 import * as debugArtifacts from "./debug-artifacts";
-import { getTempDiagnosticPath } from "./diagnostics";
 import { EnvVar } from "./environment";
 import { Feature } from "./feature-flags";
 import * as initActionPostHelper from "./init-action-post-helper";
@@ -39,47 +35,6 @@ const NUM_BYTES_PER_GIB = 1024 * 1024 * 1024;
 
 setupTests(test);
 
-test.serial("generateFailedSarif picks up unwritten diagnostics", async (t) =>
-  util.withTmpDir(async (tmpDir) => {
-    const logger = new RecordingLogger();
-
-    const tempDiagDir = getTempDiagnosticPath(tmpDir);
-    fs.mkdirSync(tempDiagDir);
-
-    const codeqlObject = await codeql.getCodeQLForTesting();
-    const diagnosticsExport = sinon
-      .stub(codeqlObject, "diagnosticsExport")
-      .resolves();
-
-    sinon.stub(codeql, "getCodeQL").resolves(codeqlObject);
-
-    const sarifFile = path.join(tmpDir, "failed.sarif");
-    const result = await initActionPostHelper.generateFailedSarif(
-      logger,
-      createFeatures([]),
-      createTestConfig({
-        tempDir: tmpDir,
-        // Explicitly undefined to ensure we get the "no database" path.
-        dbLocation: undefined,
-      }),
-      "category",
-      "",
-      sarifFile,
-    );
-
-    t.truthy(result);
-    t.is(diagnosticsExport.callCount, 1);
-    t.true(
-      diagnosticsExport.calledOnceWith(
-        sarifFile,
-        "category",
-        sinon.match.any,
-        tempDiagDir,
-      ),
-    );
-  }),
-);
-
 test.serial("init-post action with debug mode off", async (t) => {
   return await util.withTmpDir(async (tmpDir) => {
     setupActionsVars(tmpDir, tmpDir);
@@ -838,7 +793,6 @@ async function testFailedSarifUpload(
     }
     t.true(
       uploadFiles.calledOnceWith(
-        config,
         sinon.match.string,
         sinon.match.string,
         category,

src/init-action-post-helper.ts

@@ -24,7 +24,6 @@ import {
   type UploadDebugArtifactsConfig,
 } from "./debug-artifacts";
 import * as dependencyCaching from "./dependency-caching";
-import { getTempDiagnosticPath } from "./diagnostics";
 import { EnvVar, ReadOnlyEnv } from "./environment";
 import { Feature, FeatureEnablement } from "./feature-flags";
 import { Logger } from "./logging";
@@ -45,7 +44,6 @@ import {
   getRequiredEnvParam,
   parseMatrixInput,
   Result,
-  shouldSkipGenerateFailedSarif,
   shouldSkipSarifUpload,
   Success,
   wrapError,
@@ -117,7 +115,7 @@ async function prepareFailedSarif(
   const jobName = getRequiredEnvParam("GITHUB_JOB");
   const matrix = parseMatrixInput(actionsUtil.getRequiredInput("matrix"));
 
-  if (shouldSkipGenerateFailedSarif()) {
+  if (shouldSkipSarifUpload()) {
     return new Failure({
       upload_failed_run_skipped_because: "SARIF upload is disabled",
     });
@@ -178,16 +176,7 @@ async function prepareFailedSarif(
   }
 }
 
-/**
- * Generates a SARIF file for a failed run, i.e. an analysis where no SARIF was produced as a result of the analysis.
- *
- * Not intended to be called directly, other than by `prepareFailedSarif`.
- *
- * @param category The analysis category to include in the SARIF.
- * @param checkoutPath The path where the repository was checked out at.
- * @param sarifFile The SARIF file to produce.
- */
-export async function generateFailedSarif(
+async function generateFailedSarif(
   logger: Logger,
   features: FeatureEnablement,
   config: ActionConfig<"codeQLCmd" | "tempDir">,
@@ -208,19 +197,7 @@ export async function generateFailedSarif(
     databasePath === undefined ||
     !(await features.getValue(Feature.ExportDiagnosticsEnabled, codeql))
   ) {
-    // If we don't have a database path, then we may have diagnostics that were written to a
-    // temporary location. Get the path that they would have been written to and check whether
-    // it exists. If so, pass it on to the CLI. The CLI will not complain if we give it a
-    // path that doesn't exist, so we are just being extra defensive here.
-    let diagnosticDir: string | undefined = getTempDiagnosticPath(
-      config.tempDir,
-    );
-
-    if (!fs.existsSync(diagnosticDir)) {
-      diagnosticDir = undefined;
-    }
-
-    await codeql.diagnosticsExport(sarifFile, category, config, diagnosticDir);
+    await codeql.diagnosticsExport(sarifFile, category, config);
   } else {
     // We call 'database export-diagnostics' to find any per-database diagnostics.
     await codeql.databaseExportDiagnostics(databasePath, sarifFile, category);
@@ -249,7 +226,6 @@ async function maybeUploadFailedSarif(
 
   logger.info(`Uploading failed SARIF file ${failedSarif.sarifFile}`);
   const uploadResult = await uploadLib.uploadFiles(
-    config,
     failedSarif.sarifFile,
     failedSarif.checkoutPath,
     failedSarif.category,
@@ -263,14 +239,6 @@ async function maybeUploadFailedSarif(
     logger,
     { isUnsuccessfulExecution: true },
   );
-
-  // If SARIF upload is disabled for testing, `uploadFiles` will not have uploaded the failed
-  // SARIF file and instead will have written it to disk. Upload it as a workflow artifact so that
-  // we can inspect it in validation jobs.
-  if (uploadResult.sarifID && shouldSkipSarifUpload()) {
-    await uploadSarifArtifact(logger, "test-mode-sarif", failedSarif);
-  }
-
   return uploadResult
     ? { ...uploadResult.statusReport, sarifID: uploadResult.sarifID }
     : {};
@@ -289,34 +257,23 @@ async function maybeUploadFailedSarifArtifact(
   }
 
   const failedSarif = failedSarifResult.value;
-  return uploadSarifArtifact(logger, "sarif-artifact", failedSarif);
-}
 
-/** Uploads a failed SARIF file as workflow artifact. */
-async function uploadSarifArtifact(
-  logger: Logger,
-  prefix: string,
-  failedSarif: FailedSarifInfo,
-): Promise<UploadFailedSarifResult> {
-  logger.startGroup(`Uploading failed SARIF file as artifact`);
-  logger.info(`Uploading '${failedSarif.sarifFile}' as artifact`);
+  logger.info(
+    `Uploading failed SARIF file ${failedSarif.sarifFile} as artifact`,
+  );
 
-  try {
-    const gitHubVersion = await getGitHubVersion();
-    const client = await getArtifactUploaderClient(logger, gitHubVersion.type);
+  const gitHubVersion = await getGitHubVersion();
+  const client = await getArtifactUploaderClient(logger, gitHubVersion.type);
 
-    const suffix = getArtifactSuffix(actionsUtil.getOptionalInput("matrix"));
-    const name = sanitizeArtifactName(`${prefix}-${suffix}`);
-    await client.uploadArtifact(
-      name,
-      [path.normalize(failedSarif.sarifFile)],
-      path.normalize(path.dirname(failedSarif.sarifFile)),
-    );
+  const suffix = getArtifactSuffix(actionsUtil.getOptionalInput("matrix"));
+  const name = sanitizeArtifactName(`sarif-artifact-${suffix}`);
+  await client.uploadArtifact(
+    name,
+    [path.normalize(failedSarif.sarifFile)],
+    path.normalize(".."),
+  );
 
-    return { sarifID: name };
-  } finally {
-    logger.endGroup();
-  }
+  return { sarifID: name };
 }
 
 /**

src/init-action-post.ts

@@ -76,9 +76,6 @@ async function run(action: ActionState<["Base", "Logger", "Env"]>) {
     // Try to load the "failed" configuration file instead which may have been written by the `init` step
     // with a minimal configuration for this post-action.
     if (config === undefined) {
-      logger.debug(
-        "Will try to load the configuration file from an aborted 'init' step, since there is no regular one.",
-      );
       config = getAbortedInitConfig(action, tmpDir);
     }
 

src/init-action.ts

@@ -40,7 +40,7 @@ import {
   addDiagnostic,
   addNoLanguageDiagnostic,
   flushDiagnostics,
-  logTemporaryDiagnostics,
+  logUnwrittenDiagnostics,
   makeDiagnostic,
   makeTelemetryDiagnostic,
 } from "./diagnostics";
@@ -484,7 +484,6 @@ async function run(
     if (statusReportBase !== undefined) {
       await sendStatusReport(statusReportBase);
     }
-    logTemporaryDiagnostics(actionState);
     return;
   }
 
@@ -767,9 +766,9 @@ async function run(
       );
     }
 
-    // Move diagnostics to the database that we previously kept in a temporary location
-    // because the database did not exist until now
-    flushDiagnostics(actionState, config);
+    // Write diagnostics to the database that we previously stored in memory because the database
+    // did not exist until now.
+    flushDiagnostics(config);
 
     // We save the config here instead of at the end of `initConfig` because we
     // may have updated the config returned from `initConfig`, e.g. to revert to
@@ -798,7 +797,7 @@ async function run(
     );
     return;
   } finally {
-    logTemporaryDiagnostics(actionState);
+    logUnwrittenDiagnostics();
   }
   await sendCompletedStatusReport(
     startedAt,

src/setup-codeql.test.ts

@@ -1626,20 +1626,16 @@ test.serial(
       },
     ]);
 
-    await withTmpDir(async (tmpDir) => {
-      setupActionsVars(tmpDir, tmpDir);
-      const result =
-        await setupCodeql.getEnabledVersionsWithOverlayBaseDatabases(
-          overlayMatchEnabledVersions,
-          ["javascript"],
-          createFeatures([Feature.OverlayAnalysisMatchCodeqlVersion]),
-          getRunnerLogger(true),
-        );
-      t.deepEqual(result, [
-        { cliVersion: "2.20.1", tagName: "codeql-bundle-v2.20.1" },
-        { cliVersion: "2.20.0", tagName: "codeql-bundle-v2.20.0" },
-      ]);
-    });
+    const result = await setupCodeql.getEnabledVersionsWithOverlayBaseDatabases(
+      overlayMatchEnabledVersions,
+      ["javascript"],
+      createFeatures([Feature.OverlayAnalysisMatchCodeqlVersion]),
+      getRunnerLogger(true),
+    );
+    t.deepEqual(result, [
+      { cliVersion: "2.20.1", tagName: "codeql-bundle-v2.20.1" },
+      { cliVersion: "2.20.0", tagName: "codeql-bundle-v2.20.0" },
+    ]);
   },
 );
 
@@ -1756,25 +1752,21 @@ test.serial(
       },
     ]);
 
-    await withTmpDir(async (tmpDir) => {
-      setupActionsVars(tmpDir, tmpDir);
-      const result =
-        await setupCodeql.getEnabledVersionsWithOverlayBaseDatabases(
-          overlayMatchEnabledVersions,
-          ["javascript"],
-          createFeatures([Feature.OverlayAnalysisMatchCodeqlVersionDryRun]),
-          getRunnerLogger(true),
-        );
-      t.deepEqual(
-        result,
-        [],
-        "Dry-run should return an empty list so the caller falls back.",
-      );
-      t.assert(
-        listStub.calledOnce,
-        "Dry-run should still list Actions caches to populate the diagnostic.",
-      );
-    });
+    const result = await setupCodeql.getEnabledVersionsWithOverlayBaseDatabases(
+      overlayMatchEnabledVersions,
+      ["javascript"],
+      createFeatures([Feature.OverlayAnalysisMatchCodeqlVersionDryRun]),
+      getRunnerLogger(true),
+    );
+    t.deepEqual(
+      result,
+      [],
+      "Dry-run should return an empty list so the caller falls back.",
+    );
+    t.assert(
+      listStub.calledOnce,
+      "Dry-run should still list Actions caches to populate the diagnostic.",
+    );
   },
 );
 
@@ -1788,22 +1780,18 @@ test.serial(
       },
     ]);
 
-    await withTmpDir(async (tmpDir) => {
-      setupActionsVars(tmpDir, tmpDir);
-      const result =
-        await setupCodeql.getEnabledVersionsWithOverlayBaseDatabases(
-          overlayMatchEnabledVersions,
-          ["javascript"],
-          createFeatures([
-            Feature.OverlayAnalysisMatchCodeqlVersion,
-            Feature.OverlayAnalysisMatchCodeqlVersionDryRun,
-          ]),
-          getRunnerLogger(true),
-        );
-      t.deepEqual(result, [
-        { cliVersion: "2.20.1", tagName: "codeql-bundle-v2.20.1" },
-      ]);
-    });
+    const result = await setupCodeql.getEnabledVersionsWithOverlayBaseDatabases(
+      overlayMatchEnabledVersions,
+      ["javascript"],
+      createFeatures([
+        Feature.OverlayAnalysisMatchCodeqlVersion,
+        Feature.OverlayAnalysisMatchCodeqlVersionDryRun,
+      ]),
+      getRunnerLogger(true),
+    );
+    t.deepEqual(result, [
+      { cliVersion: "2.20.1", tagName: "codeql-bundle-v2.20.1" },
+    ]);
   },
 );
 

src/upload-lib.ts

@@ -12,7 +12,7 @@ import * as analyses from "./analyses";
 import * as api from "./api-client";
 import { getGitHubVersion, wrapApiConfigurationError } from "./api-client";
 import { CodeQL, getCodeQL } from "./codeql";
-import type { ActionConfig } from "./config/action-config";
+import { getConfig } from "./config-utils";
 import { readDiffRangesJsonFile } from "./diff-informed-analysis-utils";
 import { ActionsEnvVars, EnvVar, exportEnvVar } from "./environment";
 import { FeatureEnablement } from "./feature-flags";
@@ -94,19 +94,15 @@ async function shouldDisableCombineSarifFiles(
   return true;
 }
 
-/**
- * Takes a list of paths to sarif files and combines them together using the
- * CLI `github merge-results` command when all SARIF files are produced by
- * CodeQL. Otherwise, it will fall back to combining the files in the action.
- *
- * @returns The contents of the combined sarif file.
- */
+// Takes a list of paths to sarif files and combines them together using the
+// CLI `github merge-results` command when all SARIF files are produced by
+// CodeQL. Otherwise, it will fall back to combining the files in the action.
+// Returns the contents of the combined sarif file.
 async function combineSarifFilesUsingCLI(
   sarifFiles: string[],
   gitHubVersion: GitHubVersion,
   features: FeatureEnablement,
   logger: Logger,
-  config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined,
 ): Promise<Partial<sarif.Log>> {
   const env = util.getEnv();
 
@@ -144,6 +140,7 @@ async function combineSarifFilesUsingCLI(
   let codeQL: CodeQL;
   let tempDir: string = actionsUtil.getTemporaryDirectory();
 
+  const config = await getConfig(tempDir, logger);
   if (config !== undefined) {
     codeQL = await getCodeQL(logger, config.codeQLCmd);
     tempDir = config.tempDir;
@@ -231,14 +228,6 @@ function getAutomationID(
   return api.computeAutomationID(analysis_key, environment);
 }
 
-/** Gets the path to a saved payload file for {@link analysisKind}. */
-export function getSavedPayloadPath(analysisKind: analyses.AnalysisKind) {
-  return path.join(
-    actionsUtil.getTemporaryDirectory(),
-    `payload-${analysisKind}.json`,
-  );
-}
-
 /**
  * Upload the given payload.
  * If the request fails then this will retry a small number of times.
@@ -253,7 +242,10 @@ export async function uploadPayload(
   logger.info("Uploading results");
 
   if (util.shouldSkipSarifUpload()) {
-    const payloadSaveFile = getSavedPayloadPath(analysis.kind);
+    const payloadSaveFile = path.join(
+      actionsUtil.getTemporaryDirectory(),
+      `payload-${analysis.kind}.json`,
+    );
     logger.info(
       `SARIF upload disabled by an environment variable. Saving to ${payloadSaveFile}`,
     );
@@ -607,7 +599,6 @@ export interface PostProcessingResults {
 export async function postProcessSarifFiles(
   logger: Logger,
   features: FeatureEnablement,
-  config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined,
   checkoutPath: string,
   sarifPaths: string[],
   category: string | undefined,
@@ -632,7 +623,6 @@ export async function postProcessSarifFiles(
       gitHubVersion,
       features,
       logger,
-      config,
     );
   } else {
     const sarifPath = sarifPaths[0];
@@ -693,7 +683,6 @@ export async function writePostProcessedFiles(
  * to.
  */
 export async function uploadFiles(
-  config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined,
   inputSarifPath: string,
   checkoutPath: string,
   category: string | undefined,
@@ -708,7 +697,6 @@ export async function uploadFiles(
 
   return uploadSpecifiedFiles(
     sarifPaths,
-    config,
     checkoutPath,
     category,
     features,
@@ -722,7 +710,6 @@ export async function uploadFiles(
  */
 async function uploadSpecifiedFiles(
   sarifPaths: string[],
-  config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined,
   checkoutPath: string,
   category: string | undefined,
   features: FeatureEnablement,
@@ -732,7 +719,6 @@ async function uploadSpecifiedFiles(
   const processingResults: PostProcessingResults = await postProcessSarifFiles(
     logger,
     features,
-    config,
     checkoutPath,
     sarifPaths,
     category,
@@ -871,16 +857,6 @@ export async function waitForProcessing(
 ): Promise<void> {
   logger.startGroup("Waiting for processing to finish");
   try {
-    // If SARIF uplaod is disabled and we reach this point, then the file was written to disk and
-    // there is nothing to wait for. We still allow ourselves to get to this point so that we can
-    // see in the log that we would have waited for processing to finish if the upload had happened.
-    if (util.shouldSkipSarifUpload()) {
-      logger.info(
-        "Nothing to wait for, because SARIF upload is disabled by an environment variable and the SARIF was saved to disk.",
-      );
-      return;
-    }
-
     const client = api.getApiClient();
 
     // Do an initial wait because processing will always take a minimum of 2-3 seconds

src/upload-sarif-action.ts

@@ -5,7 +5,6 @@ import * as actionsUtil from "./actions-util";
 import { getActionVersion, getTemporaryDirectory } from "./actions-util";
 import * as analyses from "./analyses";
 import { getGitHubVersion } from "./api-client";
-import { getConfig } from "./config-utils";
 import { initFeatures } from "./feature-flags";
 import { Logger } from "./logging";
 import { getRepositoryNwo } from "./repository";
@@ -67,12 +66,11 @@ async function run({ startedAt, logger }: ActionState<["Base", "Logger"]>) {
     // Make inputs accessible in the `post` step.
     actionsUtil.persistInputs();
 
-    const tempDir = getTemporaryDirectory();
     const repositoryNwo = getRepositoryNwo();
     const features = initFeatures(
       gitHubVersion,
       repositoryNwo,
-      tempDir,
+      getTemporaryDirectory(),
       logger,
     );
 
@@ -93,15 +91,9 @@ async function run({ startedAt, logger }: ActionState<["Base", "Logger"]>) {
     const checkoutPath = actionsUtil.getRequiredInput("checkout_path");
     const category = actionsUtil.getOptionalInput("category");
 
-    // Although we wouldn't typically expect there to be a configuration since we don't expect another
-    // CodeQL Action step to have come before `upload-sarif`, this has previously been the behaviour
-    // when `combineSarifFilesUsingCLI` called `getConfig` directly. Since the only effect of this would
-    // be that an existing CLI that was installed gets re-used, we maintain this behaviour.
-    const maybeConfig = await getConfig(tempDir, logger);
     const uploadResults = await postProcessAndUploadSarif(
       logger,
       features,
-      maybeConfig,
       "always",
       checkoutPath,
       sarifPath,

src/upload-sarif.test.ts

@@ -26,7 +26,6 @@ function mockPostProcessSarifFiles() {
     const analysisConfig = getAnalysisConfig(analysisKind);
     postProcessSarifFiles
       .withArgs(
-        sinon.match.object,
         sinon.match.any,
         sinon.match.any,
         sinon.match.any,
@@ -76,11 +75,9 @@ const postProcessAndUploadSarifMacro = makeMacro({
         fs.writeFileSync(sarifFile, "");
       }
 
-      const config = undefined;
       const actual = await postProcessAndUploadSarif(
         logger,
         features,
-        config,
         "always",
         "",
         testPath,
@@ -97,7 +94,6 @@ const postProcessAndUploadSarifMacro = makeMacro({
             postProcessSarifFiles.calledWith(
               logger,
               features,
-              config,
               sinon.match.any,
               analysisKindResult.expectedFiles?.map(toFullPath) ??
                 fullSarifPaths,
@@ -227,7 +223,6 @@ test.serial(
       const actual = await postProcessAndUploadSarif(
         logger,
         features,
-        undefined,
         "never",
         "",
         tempDir,
@@ -258,7 +253,6 @@ test.serial(
       const actual = await postProcessAndUploadSarif(
         logger,
         features,
-        undefined,
         "never",
         "",
         tempDir,

src/upload-sarif.ts

@@ -1,6 +1,5 @@
 import { UploadKind } from "./actions-util";
 import * as analyses from "./analyses";
-import type { ActionConfig } from "./config/action-config";
 import { FeatureEnablement } from "./feature-flags";
 import { Logger } from "./logging";
 import * as upload_lib from "./upload-lib";
@@ -27,7 +26,6 @@ export type UploadSarifResults = Partial<
 export async function postProcessAndUploadSarif(
   logger: Logger,
   features: FeatureEnablement,
-  config: ActionConfig<"codeQLCmd" | "tempDir"> | undefined,
   uploadKind: UploadKind,
   checkoutPath: string,
   sarifPath: string,
@@ -47,7 +45,6 @@ export async function postProcessAndUploadSarif(
     const postProcessingResults = await upload_lib.postProcessSarifFiles(
       logger,
       features,
-      config,
       checkoutPath,
       sarifFiles,
       category,

src/util.test.ts

@@ -11,7 +11,7 @@ import * as sinon from "sinon";
 import * as api from "./api-client";
 import { EnvVar } from "./environment";
 import { getRunnerLogger } from "./logging";
-import { getTestEnv, setupTests } from "./testing-utils";
+import { setupTests } from "./testing-utils";
 import * as util from "./util";
 
 setupTests(test);
@@ -493,70 +493,3 @@ test("Failure.orElse returns the default value for a failure result", (t) => {
   const result = new util.Failure(new Error("test error"));
   t.is(result.orElse("default value"), "default value");
 });
-
-test("shouldSkipGenerateFailedSarif", (t) => {
-  const env = getTestEnv();
-
-  // Don't skip by default
-  t.is(util.shouldSkipGenerateFailedSarif(env), false);
-
-  // Skip in test mode
-  env.setAll({
-    [EnvVar.TEST_MODE]: "true",
-    [EnvVar.SKIP_SARIF_UPLOAD]: undefined,
-    [EnvVar.SKIP_GENERATE_FAILED_SARIF]: undefined,
-  });
-  t.is(util.shouldSkipGenerateFailedSarif(env), true);
-
-  // Skip if explicitly requested
-  env.setAll({
-    [EnvVar.TEST_MODE]: undefined,
-    [EnvVar.SKIP_SARIF_UPLOAD]: undefined,
-    [EnvVar.SKIP_GENERATE_FAILED_SARIF]: "true",
-  });
-  t.is(util.shouldSkipGenerateFailedSarif(env), true);
-
-  // Skip if not uploading SARIF
-  env.setAll({
-    [EnvVar.TEST_MODE]: undefined,
-    [EnvVar.SKIP_SARIF_UPLOAD]: "true",
-    [EnvVar.SKIP_GENERATE_FAILED_SARIF]: undefined,
-  });
-  t.is(util.shouldSkipGenerateFailedSarif(env), true);
-
-  // Don't skip if explicitly requested
-  env.setAll({
-    [EnvVar.TEST_MODE]: "true",
-    [EnvVar.SKIP_SARIF_UPLOAD]: "true",
-    [EnvVar.SKIP_GENERATE_FAILED_SARIF]: "false",
-  });
-  t.is(util.shouldSkipGenerateFailedSarif(env), false);
-});
-
-test("shouldSkipSarifUpload", (t) => {
-  const env = getTestEnv();
-
-  // Don't skip by default
-  t.is(util.shouldSkipSarifUpload(env), false);
-
-  // Skip in test mode
-  env.setAll({
-    [EnvVar.TEST_MODE]: "true",
-    [EnvVar.SKIP_SARIF_UPLOAD]: undefined,
-  });
-  t.is(util.shouldSkipSarifUpload(env), true);
-
-  // Skip if explicitly requested
-  env.setAll({
-    [EnvVar.TEST_MODE]: undefined,
-    [EnvVar.SKIP_SARIF_UPLOAD]: "true",
-  });
-  t.is(util.shouldSkipSarifUpload(env), true);
-
-  // Don't skip if explicitly requested
-  env.setAll({
-    [EnvVar.TEST_MODE]: "true",
-    [EnvVar.SKIP_SARIF_UPLOAD]: "false",
-  });
-  t.is(util.shouldSkipSarifUpload(env), false);
-});

src/util.ts

@@ -18,10 +18,10 @@ import type { Config } from "./config-utils";
 import {
   EnvVar,
   getRequiredEnvParam,
+  isInTestMode,
   exportEnvVar,
   getEnv,
   type Env,
-  type ReadOnlyEnv,
 } from "./environment";
 import * as json from "./json";
 import { Language } from "./languages";
@@ -684,30 +684,11 @@ export function isGoodVersion(versionSpec: string) {
   return !BROKEN_VERSIONS.includes(versionSpec);
 }
 
-/**
- * Returns whether we specifically want to skip generating a failed SARIF.
- */
-export function shouldSkipGenerateFailedSarif(
-  env: ReadOnlyEnv = getEnv(),
-): boolean {
-  const skipGenerateFailedSarif = env.getOptional(
-    EnvVar.SKIP_GENERATE_FAILED_SARIF,
-  );
-  return (
-    skipGenerateFailedSarif !== "false" &&
-    (shouldSkipSarifUpload(env) || skipGenerateFailedSarif === "true")
-  );
-}
-
 /**
  * Returns whether we specifically want to skip uploading SARIF files.
  */
-export function shouldSkipSarifUpload(env: ReadOnlyEnv = getEnv()): boolean {
-  const skipSarifUpload = env.getOptional(EnvVar.SKIP_SARIF_UPLOAD);
-  return (
-    skipSarifUpload !== "false" &&
-    (env.isSkippingUploadsInTests() || skipSarifUpload === "true")
-  );
+export function shouldSkipSarifUpload(): boolean {
+  return isInTestMode() || process.env[EnvVar.SKIP_SARIF_UPLOAD] === "true";
 }
 
 /**
@@ -900,21 +881,6 @@ export function getErrorMessage(error: unknown): string {
   return error instanceof Error ? error.message : String(error);
 }
 
-/**
- * Approximates whether {@link error} is like a {@link NodeJS.ErrnoException}.
- * We only typically care about the `code` property, so that's what we check for here.
- */
-export function isErrnoException(
-  error: unknown,
-): error is NodeJS.ErrnoException {
-  return (
-    typeof error === "object" &&
-    error !== null &&
-    "code" in error &&
-    typeof error.code === "string"
-  );
-}
-
 export function prettyPrintPack(pack: Pack) {
   return `${pack.name}${pack.version ? `@${pack.version}` : ""}${
     pack.path ? `:${pack.path}` : ""