Skip to content

Commit 6045ee8

Browse files
henrymercerCopilot
andcommitted
Disable overlay analysis when pull request analyses fail
Once branch selection has settled on an overlay-base build, make a single non-paginating request for the most recent pull request failure marker. In the happy path no markers exist and the list is empty. On detection, save the persistent overlay status cache entry, which is what makes pull requests skip overlay analysis too, and disable overlay analysis for this run. Any failure is treated as if no marker was found, so the check never disables overlay analysis on its own errors. `overlay_analysis_status_check_pr_dry_run` performs the same request and emits the same telemetry, but leaves overlay analysis enabled. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: ff943063-d9be-440f-9cac-5e44c4fcfbaa
1 parent 4ed9f4c commit 6045ee8

7 files changed

Lines changed: 667 additions & 3 deletions

File tree

lib/entry-points.js

Lines changed: 156 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/config-utils.test.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1008,6 +1008,7 @@ interface OverlayDatabaseModeTestSetup {
10081008
diskUsage: DiskUsage | undefined;
10091009
memoryFlagValue: number;
10101010
shouldSkipOverlayAnalysisDueToCachedStatus: boolean;
1011+
shouldSkipOverlayAnalysisDueToPullRequestFailure: boolean;
10111012
repositoryProperties: RepositoryProperties;
10121013
}
10131014

@@ -1029,6 +1030,7 @@ const defaultOverlayDatabaseModeTestSetup: OverlayDatabaseModeTestSetup = {
10291030
},
10301031
memoryFlagValue: 6920,
10311032
shouldSkipOverlayAnalysisDueToCachedStatus: false,
1033+
shouldSkipOverlayAnalysisDueToPullRequestFailure: false,
10321034
repositoryProperties: {},
10331035
};
10341036

@@ -1072,6 +1074,13 @@ const checkOverlayEnablementMacro = makeMacro({
10721074
.stub(overlayStatus, "shouldSkipOverlayAnalysis")
10731075
.resolves(setup.shouldSkipOverlayAnalysisDueToCachedStatus);
10741076

1077+
sinon
1078+
.stub(
1079+
overlayStatus,
1080+
"shouldSkipOverlayAnalysisAfterPullRequestFailure",
1081+
)
1082+
.resolves(setup.shouldSkipOverlayAnalysisDueToPullRequestFailure);
1083+
10751084
// Mock feature flags
10761085
const features = createFeatures(setup.features);
10771086

@@ -1206,6 +1215,59 @@ checkOverlayEnablementMacro.serial(
12061215
},
12071216
);
12081217

1218+
checkOverlayEnablementMacro.serial(
1219+
"No overlay-base database on default branch if a pull request analysis failed",
1220+
{
1221+
languages: [BuiltInLanguage.javascript],
1222+
features: [
1223+
Feature.OverlayAnalysis,
1224+
Feature.OverlayAnalysisJavascript,
1225+
Feature.OverlayAnalysisStatusCheckPr,
1226+
],
1227+
isDefaultBranch: true,
1228+
shouldSkipOverlayAnalysisDueToPullRequestFailure: true,
1229+
},
1230+
{
1231+
disabledReason: OverlayDisabledReason.PullRequestAnalysisFailed,
1232+
},
1233+
);
1234+
1235+
checkOverlayEnablementMacro.serial(
1236+
"Overlay-base database on default branch if no pull request analysis failed",
1237+
{
1238+
languages: [BuiltInLanguage.javascript],
1239+
features: [
1240+
Feature.OverlayAnalysis,
1241+
Feature.OverlayAnalysisJavascript,
1242+
Feature.OverlayAnalysisStatusCheckPr,
1243+
],
1244+
isDefaultBranch: true,
1245+
shouldSkipOverlayAnalysisDueToPullRequestFailure: false,
1246+
},
1247+
{
1248+
overlayDatabaseMode: OverlayDatabaseMode.OverlayBase,
1249+
useOverlayDatabaseCaching: true,
1250+
},
1251+
);
1252+
1253+
checkOverlayEnablementMacro.serial(
1254+
"Pull request analyses do not consult the pull request failure status",
1255+
{
1256+
languages: [BuiltInLanguage.javascript],
1257+
features: [
1258+
Feature.OverlayAnalysis,
1259+
Feature.OverlayAnalysisJavascript,
1260+
Feature.OverlayAnalysisStatusCheckPr,
1261+
],
1262+
isPullRequest: true,
1263+
shouldSkipOverlayAnalysisDueToPullRequestFailure: true,
1264+
},
1265+
{
1266+
overlayDatabaseMode: OverlayDatabaseMode.Overlay,
1267+
useOverlayDatabaseCaching: true,
1268+
},
1269+
);
1270+
12091271
checkOverlayEnablementMacro.serial(
12101272
"Overlay-base database on default branch when feature enabled with custom analysis",
12111273
{

src/config-utils.ts

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,12 @@ import {
7070
OverlayDisabledReason,
7171
} from "./overlay/diagnostics";
7272
import { OverlayDatabaseMode } from "./overlay/overlay-database-mode";
73-
import { shouldSkipOverlayAnalysis } from "./overlay/status";
73+
import {
74+
getPullRequestFailureCheck,
75+
PullRequestFailureCheck,
76+
shouldSkipOverlayAnalysis,
77+
shouldSkipOverlayAnalysisAfterPullRequestFailure,
78+
} from "./overlay/status";
7479
import { RepositoryNwo } from "./repository";
7580
import { ToolsFeature } from "./tools-features";
7681
import { downloadTrapCaches } from "./trap-caching";
@@ -780,8 +785,16 @@ export async function checkOverlayEnablement(
780785
const checkOverlayStatus = await features.getValue(
781786
Feature.OverlayAnalysisStatusCheck,
782787
);
788+
const pullRequestFailureCheck = await getPullRequestFailureCheck(features);
789+
// The pull request failure check needs the disk usage to compute its cache key prefix, but it
790+
// only applies when analyzing the default branch, and it fails open if the disk usage is
791+
// unavailable rather than disabling overlay analysis.
783792
const needDiskUsage = performResourceChecks || checkOverlayStatus;
784-
const diskUsage = needDiskUsage ? await checkDiskUsage(logger) : undefined;
793+
const wantDiskUsage =
794+
needDiskUsage ||
795+
(pullRequestFailureCheck !== PullRequestFailureCheck.None &&
796+
!isAnalyzingPullRequest());
797+
const diskUsage = wantDiskUsage ? await checkDiskUsage(logger) : undefined;
785798
if (needDiskUsage && diskUsage === undefined) {
786799
logger.warning(
787800
`Unable to determine disk usage, therefore setting overlay database mode to ${OverlayDatabaseMode.None}.`,
@@ -822,6 +835,22 @@ export async function checkOverlayEnablement(
822835
"with caching because we are analyzing a pull request.",
823836
);
824837
} else if (await isAnalyzingDefaultBranch()) {
838+
if (
839+
diskUsage !== undefined &&
840+
(await shouldSkipOverlayAnalysisAfterPullRequestFailure(
841+
codeql,
842+
languages,
843+
diskUsage,
844+
pullRequestFailureCheck,
845+
logger,
846+
))
847+
) {
848+
logger.info(
849+
`Setting overlay database mode to ${OverlayDatabaseMode.None} ` +
850+
"because a pull request analysis using overlay analysis did not complete successfully.",
851+
);
852+
return new Failure(OverlayDisabledReason.PullRequestAnalysisFailed);
853+
}
825854
overlayDatabaseMode = OverlayDatabaseMode.OverlayBase;
826855
logger.info(
827856
`Setting overlay database mode to ${overlayDatabaseMode} ` +

src/feature-flags.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,19 @@ export enum Feature {
157157
OverlayAnalysisSkipResourceChecks = "overlay_analysis_skip_resource_checks",
158158
/** Controls whether the Actions cache is checked for overlay build outcomes. */
159159
OverlayAnalysisStatusCheck = "overlay_analysis_status_check",
160+
/**
161+
* Controls whether the Actions cache is checked for pull request analyses that failed while
162+
* using overlay analysis, and overlay analysis disabled if any are found.
163+
*
164+
* Requires `OverlayAnalysisStatusCheck` to be enabled as well: disabling overlay analysis for
165+
* subsequent runs works by writing the cache entry that flag controls the reading of.
166+
*/
167+
OverlayAnalysisStatusCheckPr = "overlay_analysis_status_check_pr",
168+
/**
169+
* Like `OverlayAnalysisStatusCheckPr`, but only logs a diagnostic instead of disabling overlay
170+
* analysis. `OverlayAnalysisStatusCheckPr` overrides this flag.
171+
*/
172+
OverlayAnalysisStatusCheckPrDryRun = "overlay_analysis_status_check_pr_dry_run",
160173
/** Controls whether overlay build failures on the default branch are stored in the Actions cache. */
161174
OverlayAnalysisStatusSave = "overlay_analysis_status_save",
162175
QaTelemetryEnabled = "qa_telemetry_enabled",
@@ -414,6 +427,16 @@ export const featureConfig = {
414427
envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS_STATUS_CHECK",
415428
minimumVersion: undefined,
416429
},
430+
[Feature.OverlayAnalysisStatusCheckPr]: {
431+
defaultValue: false,
432+
envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS_STATUS_CHECK_PR",
433+
minimumVersion: undefined,
434+
},
435+
[Feature.OverlayAnalysisStatusCheckPrDryRun]: {
436+
defaultValue: false,
437+
envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS_STATUS_CHECK_PR_DRY_RUN",
438+
minimumVersion: undefined,
439+
},
417440
[Feature.OverlayAnalysisStatusSave]: {
418441
defaultValue: false,
419442
envVar: "CODEQL_ACTION_OVERLAY_ANALYSIS_STATUS_SAVE",

0 commit comments

Comments
 (0)