refactor: report missing PR diff ranges via OverlayDisabledReason and disable overlay

This commit is contained in:
Sam Robson
2026-04-30 16:12:30 +01:00
parent 5ded561dcd
commit 3cc8dd3e59
6 changed files with 112 additions and 101 deletions
+28 -21
View File
@@ -106378,25 +106378,21 @@ async function prepareDiffInformedAnalysis(codeql, features, logger) {
logger.warning(
`Failed to determine branch information for diff-informed analysis: ${getErrorMessage(e)}`
);
return { shouldRun: false, hasDiffRanges: false };
return false;
}
if (!branches) {
return { shouldRun: false, hasDiffRanges: false };
return false;
}
const hasDiffRanges = await withGroupAsync(
"Computing PR diff ranges",
async () => {
try {
return await computeAndPersistDiffRanges(branches, logger);
} catch (e) {
logger.warning(
`Failed to compute diff-informed analysis ranges: ${getErrorMessage(e)}`
);
return false;
}
return await withGroupAsync("Computing PR diff ranges", async () => {
try {
return await computeAndPersistDiffRanges(branches, logger);
} catch (e) {
logger.warning(
`Failed to compute diff-informed analysis ranges: ${getErrorMessage(e)}`
);
return false;
}
);
return { shouldRun: true, hasDiffRanges };
});
}
function writeDiffRangesJsonFile(logger, ranges) {
const jsonContents = JSON.stringify(ranges, null, 2);
@@ -107283,14 +107279,20 @@ function userConfigFromActionPath(tempDir) {
function hasQueryCustomisation(userConfig) {
return isDefined2(userConfig["disable-default-queries"]) || isDefined2(userConfig.queries) || isDefined2(userConfig["query-filters"]);
}
function applyIncrementalAnalysisSettings(config, diffInformedAnalysis, logger) {
if (config.overlayDatabaseMode === "overlay" /* Overlay */ && diffInformedAnalysis.shouldRun && !diffInformedAnalysis.hasDiffRanges) {
async function applyIncrementalAnalysisSettings(config, hasDiffRanges, codeql, logger) {
if (config.overlayDatabaseMode === "overlay" /* Overlay */ && !hasDiffRanges) {
logger.info(
`Diff-informed analysis is not available for this pull request. Reverting overlay database mode to ${"none" /* None */}.`
`Reverting overlay database mode to ${"none" /* None */} because the PR diff ranges could not be computed.`
);
config.overlayDatabaseMode = "none" /* None */;
config.useOverlayDatabaseCaching = false;
await addOverlayDisablementDiagnostics(
config,
codeql,
"pr-diff-ranges-not-computed" /* PrDiffRangesNotComputed */
);
}
if (config.overlayDatabaseMode === "overlay" /* Overlay */ || diffInformedAnalysis.hasDiffRanges) {
if (hasDiffRanges) {
config.extraQueryExclusions.push({
exclude: { tags: "exclude-from-incremental" }
});
@@ -107405,12 +107407,17 @@ async function initConfig(features, inputs) {
overlayDisabledReason
);
}
const diffInformedAnalysis = await prepareDiffInformedAnalysis(
const hasDiffRanges = await prepareDiffInformedAnalysis(
inputs.codeql,
inputs.features,
logger
);
applyIncrementalAnalysisSettings(config, diffInformedAnalysis, logger);
await applyIncrementalAnalysisSettings(
config,
hasDiffRanges,
inputs.codeql,
logger
);
if (await isTrapCachingEnabled(features, config.overlayDatabaseMode)) {
const { trapCaches, trapCacheDownloadTime } = await downloadCacheWithTime(
inputs.codeql,
+25 -14
View File
@@ -2201,14 +2201,16 @@ test.serial(
},
);
test("applyIncrementalAnalysisSettings: no-op when mode is not Overlay and diff-informed is unavailable", (t) => {
test("applyIncrementalAnalysisSettings: no-op when mode is not Overlay and diff ranges are unavailable", async (t) => {
const config = createTestConfig({});
config.overlayDatabaseMode = OverlayDatabaseMode.None;
const codeql = createStubCodeQL({});
const logger = getRunnerLogger(true);
configUtils.applyIncrementalAnalysisSettings(
await configUtils.applyIncrementalAnalysisSettings(
config,
{ shouldRun: false, hasDiffRanges: false },
false,
codeql,
logger,
);
@@ -2216,14 +2218,17 @@ test("applyIncrementalAnalysisSettings: no-op when mode is not Overlay and diff-
t.deepEqual(config.extraQueryExclusions, []);
});
test("applyIncrementalAnalysisSettings: keeps overlay mode and adds exclusions when diff-informed analysis shouldn't run", (t) => {
const config = createTestConfig({});
config.overlayDatabaseMode = OverlayDatabaseMode.Overlay;
test("applyIncrementalAnalysisSettings: keeps overlay mode and adds exclusions when diff ranges are available", async (t) => {
const config = createTestConfig({
overlayDatabaseMode: OverlayDatabaseMode.Overlay,
});
const codeql = createStubCodeQL({});
const logger = getRunnerLogger(true);
configUtils.applyIncrementalAnalysisSettings(
await configUtils.applyIncrementalAnalysisSettings(
config,
{ shouldRun: false, hasDiffRanges: false },
true,
codeql,
logger,
);
@@ -2233,30 +2238,36 @@ test("applyIncrementalAnalysisSettings: keeps overlay mode and adds exclusions w
]);
});
test("applyIncrementalAnalysisSettings: disables overlay analysis when diff-informed analysis is unavailable", (t) => {
test("applyIncrementalAnalysisSettings: disables overlay analysis when diff ranges are unavailable", async (t) => {
const config = createTestConfig({
overlayDatabaseMode: OverlayDatabaseMode.Overlay,
});
config.useOverlayDatabaseCaching = true;
const codeql = createStubCodeQL({});
const logger = getRunnerLogger(true);
configUtils.applyIncrementalAnalysisSettings(
await configUtils.applyIncrementalAnalysisSettings(
config,
{ shouldRun: true, hasDiffRanges: false },
false,
codeql,
logger,
);
t.is(config.overlayDatabaseMode, OverlayDatabaseMode.None);
t.is(config.useOverlayDatabaseCaching, false);
t.deepEqual(config.extraQueryExclusions, []);
});
test("applyIncrementalAnalysisSettings: adds exclusions for diff-informed-only runs", (t) => {
test("applyIncrementalAnalysisSettings: adds exclusions for diff-informed-only runs", async (t) => {
const config = createTestConfig({});
config.overlayDatabaseMode = OverlayDatabaseMode.None;
const codeql = createStubCodeQL({});
const logger = getRunnerLogger(true);
configUtils.applyIncrementalAnalysisSettings(
await configUtils.applyIncrementalAnalysisSettings(
config,
{ shouldRun: true, hasDiffRanges: true },
true,
codeql,
logger,
);
+29 -27
View File
@@ -31,10 +31,7 @@ import {
addNoLanguageDiagnostic,
makeTelemetryDiagnostic,
} from "./diagnostics";
import {
type DiffInformedAnalysisPreparation,
prepareDiffInformedAnalysis,
} from "./diff-informed-analysis-utils";
import { prepareDiffInformedAnalysis } from "./diff-informed-analysis-utils";
import { EnvVar } from "./environment";
import * as errorMessages from "./error-messages";
import { Feature, FeatureEnablement } from "./feature-flags";
@@ -1082,39 +1079,39 @@ function hasQueryCustomisation(userConfig: UserConfig): boolean {
/**
* Finalize the incremental-analysis configuration for this run.
*
* If overlay mode was selected for a PR but diff-informed analysis should have
* run and could not be prepared, fall back to a full non-overlay analysis.
* Query exclusions for incremental-only queries are applied only when the final
* configuration still uses overlay analysis or the diff ranges are available.
* Overlay analysis has only been validated in combination with diff-informed
* analysis, so if `Overlay` mode was selected for a pull request but the diff
* ranges could not be computed, fall back to a full non-overlay analysis.
*
* Note that `overlayDatabaseMode === Overlay` does not imply
* `diffInformedAnalysis.shouldRun`. Overlay mode is selected based on language
* and feature-flag state and can apply outside of pull-request contexts (e.g.
* on branch pushes that build up the overlay cache), whereas diff-informed
* analysis only runs for pull requests where we can compute a diff. Each
* combination is therefore handled explicitly.
* Query exclusions for incremental-only queries are then applied whenever the
* diff ranges are available — which, after the fallback above, is exactly the
* set of runs where any kind of incremental analysis (overlay or
* diff-informed) is in effect.
*/
export function applyIncrementalAnalysisSettings(
export async function applyIncrementalAnalysisSettings(
config: Config,
diffInformedAnalysis: DiffInformedAnalysisPreparation,
hasDiffRanges: boolean,
codeql: CodeQL,
logger: Logger,
): void {
): Promise<void> {
if (
config.overlayDatabaseMode === OverlayDatabaseMode.Overlay &&
diffInformedAnalysis.shouldRun &&
!diffInformedAnalysis.hasDiffRanges
!hasDiffRanges
) {
logger.info(
"Diff-informed analysis is not available for this pull request. " +
`Reverting overlay database mode to ${OverlayDatabaseMode.None}.`,
`Reverting overlay database mode to ${OverlayDatabaseMode.None} ` +
"because the PR diff ranges could not be computed.",
);
config.overlayDatabaseMode = OverlayDatabaseMode.None;
config.useOverlayDatabaseCaching = false;
await addOverlayDisablementDiagnostics(
config,
codeql,
OverlayDisabledReason.PrDiffRangesNotComputed,
);
}
if (
config.overlayDatabaseMode === OverlayDatabaseMode.Overlay ||
diffInformedAnalysis.hasDiffRanges
) {
if (hasDiffRanges) {
config.extraQueryExclusions.push({
exclude: { tags: "exclude-from-incremental" },
});
@@ -1275,13 +1272,18 @@ export async function initConfig(
);
}
const diffInformedAnalysis = await prepareDiffInformedAnalysis(
const hasDiffRanges = await prepareDiffInformedAnalysis(
inputs.codeql,
inputs.features,
logger,
);
applyIncrementalAnalysisSettings(config, diffInformedAnalysis, logger);
await applyIncrementalAnalysisSettings(
config,
hasDiffRanges,
inputs.codeql,
logger,
);
if (await isTrapCachingEnabled(features, config.overlayDatabaseMode)) {
const { trapCaches, trapCacheDownloadTime } = await downloadCacheWithTime(
+8 -8
View File
@@ -190,7 +190,7 @@ test.serial(
);
test.serial(
"prepareDiffInformedAnalysis: returns shouldRun=false when not a pull request",
"prepareDiffInformedAnalysis: returns false when not a pull request",
async (t) => {
await withTmpDir(async (tmpDir) => {
setupActionsVars(tmpDir, tmpDir);
@@ -209,13 +209,13 @@ test.serial(
logger,
);
t.deepEqual(result, { shouldRun: false, hasDiffRanges: false });
t.false(result);
});
},
);
test.serial(
"prepareDiffInformedAnalysis: returns shouldRun=false when applicability check throws",
"prepareDiffInformedAnalysis: returns false when applicability check throws",
async (t) => {
await withTmpDir(async (tmpDir) => {
setupActionsVars(tmpDir, tmpDir);
@@ -239,13 +239,13 @@ test.serial(
logger,
);
t.deepEqual(result, { shouldRun: false, hasDiffRanges: false });
t.false(result);
});
},
);
test.serial(
"prepareDiffInformedAnalysis: returns hasDiffRanges=true when the diff is fetched successfully",
"prepareDiffInformedAnalysis: returns true when the diff is fetched successfully",
async (t) => {
await withTmpDir(async (tmpDir) => {
setupActionsVars(tmpDir, tmpDir);
@@ -276,13 +276,13 @@ test.serial(
logger,
);
t.deepEqual(result, { shouldRun: true, hasDiffRanges: true });
t.true(result);
});
},
);
test.serial(
"prepareDiffInformedAnalysis: returns hasDiffRanges=false when the diff API call fails",
"prepareDiffInformedAnalysis: returns false when the diff API call fails",
async (t) => {
await withTmpDir(async (tmpDir) => {
setupActionsVars(tmpDir, tmpDir);
@@ -313,7 +313,7 @@ test.serial(
logger,
);
t.deepEqual(result, { shouldRun: true, hasDiffRanges: false });
t.false(result);
});
},
);
+15 -31
View File
@@ -55,30 +55,18 @@ export async function getDiffInformedAnalysisBranches(
return branches;
}
export interface DiffInformedAnalysisPreparation {
/**
* Whether diff-informed analysis applies to this workflow run.
*/
shouldRun: boolean;
/**
* Whether the diff ranges were successfully computed and persisted, and are
* therefore available for use.
*/
hasDiffRanges: boolean;
}
/**
* Prepares the diff ranges needed for diff-informed analysis for the current
* run.
*
* @returns Whether diff-informed analysis applies to this run, and whether the
* diff ranges were successfully computed and persisted.
* @returns `true` if the diff ranges were successfully computed and persisted
* and are therefore available for use, `false` otherwise.
*/
export async function prepareDiffInformedAnalysis(
codeql: CodeQL,
features: FeatureEnablement,
logger: Logger,
): Promise<DiffInformedAnalysisPreparation> {
): Promise<boolean> {
let branches: PullRequestBranches | undefined;
try {
branches = await getDiffInformedAnalysisBranches(codeql, features, logger);
@@ -89,26 +77,22 @@ export async function prepareDiffInformedAnalysis(
logger.warning(
`Failed to determine branch information for diff-informed analysis: ${getErrorMessage(e)}`,
);
return { shouldRun: false, hasDiffRanges: false };
return false;
}
if (!branches) {
return { shouldRun: false, hasDiffRanges: false };
return false;
}
const hasDiffRanges = await withGroupAsync(
"Computing PR diff ranges",
async () => {
try {
return await computeAndPersistDiffRanges(branches, logger);
} catch (e) {
logger.warning(
`Failed to compute diff-informed analysis ranges: ${getErrorMessage(e)}`,
);
return false;
}
},
);
return { shouldRun: true, hasDiffRanges };
return await withGroupAsync("Computing PR diff ranges", async () => {
try {
return await computeAndPersistDiffRanges(branches, logger);
} catch (e) {
logger.warning(
`Failed to compute diff-informed analysis ranges: ${getErrorMessage(e)}`,
);
return false;
}
});
}
export interface DiffThunkRange {
+7
View File
@@ -39,6 +39,13 @@ export enum OverlayDisabledReason {
NotPullRequestOrDefaultBranch = "not-pull-request-or-default-branch",
/** The top-level overlay analysis feature flag is not enabled. */
OverallFeatureNotEnabled = "overall-feature-not-enabled",
/**
* Overlay analysis was selected for a pull request, but the PR diff ranges
* needed for diff-informed analysis could not be computed. Overlay analysis
* has only been validated in combination with diff-informed analysis, so we
* fall back to a non-overlay analysis in this case.
*/
PrDiffRangesNotComputed = "pr-diff-ranges-not-computed",
/** Overlay analysis was skipped because it previously failed with similar hardware resources. */
SkippedDueToCachedStatus = "skipped-due-to-cached-status",
/** Disk usage could not be determined during the overlay status check. */