Fall back to non-overlay analysis when diff-informed analysis is unavailable

[sam-robson]

When overlay analysis is enabled for a PR but diff-informed analysis fails (e.g. deleted branch, API error, too many changed files), the action previously continued with overlay-only analysis. This is an untested combination that can produce inaccurate results. Now we fall back to a full non-overlay analysis instead.

Risk assessment

For internal use only. Please select the risk level of this change:

• Low risk: Changes are fully under feature flags, or have been fully tested and validated in pre-production environments and are highly observable, or are documentation or test only.

Which use cases does this change impact?

Workflow types:

• Managed - Impacts users with dynamic workflows (Default Setup, Code Quality, ...).

Products:

• Code Scanning - The changes impact analyses when analysis-kinds: code-scanning.
• Code Quality - The changes impact analyses when analysis-kinds: code-quality.

Environments:

• Dotcom - Impacts CodeQL workflows on github.com and/or GitHub Enterprise Cloud with Data Residency.
• GHES - Impacts CodeQL workflows on GitHub Enterprise Server.

How did/will you validate this change?

• Other - Manual code review. The change is a single conditional guard following established patterns in the same file. No unit tests due to the init-action entry point side-effect constraint, but the logic is minimal (3 conditions + mode revert).

If something goes wrong after this change is released, what are the mitigation and rollback strategies?

• Rollback - Change can only be disabled by rolling back the release or releasing a new version with a fix.

How will you know if something goes wrong after this change is released?

• Telemetry - I rely on existing telemetry or have made changes to the telemetry.
  • Dashboards - I will watch relevant dashboards for issues after the release. Consider whether this requires this change to be released at a particular time rather than as part of a regular release.

Are there any special considerations for merging or releasing this change?

• No special considerations - This change can be merged at any time.

Merge / deployment checklist

• Confirm this change is backwards compatible with existing workflows.
• Consider adding a changelog entry for this change.
• Confirm the readme and docs have been updated if necessary.

[Copilot]

Pull request overview

Ensures the CodeQL init step avoids the unsupported “overlay-only” mode by falling back to a full non-overlay analysis when diff-informed analysis was expected but didn’t produce the pr-diff-range.json output.

Changes:

• Add a hasDiffRangesJsonFile() helper to detect whether pr-diff-range.json was produced.
• In init-action, revert overlayDatabaseMode to None when overlay is enabled, diff-informed analysis should have run, but the diff ranges file is missing.
• Update the generated lib/init-action.js to reflect the TypeScript changes.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/init-action.ts	Adds a guard to revert overlay mode to non-overlay when diff-informed output is missing.
src/diff-informed-analysis-utils.ts	Introduces hasDiffRangesJsonFile() helper for detecting persisted diff ranges output.
lib/init-action.js	Generated build output reflecting the TypeScript changes (not reviewed).

[mbg]

Thanks for refactoring some of the logic out of init-action and into revertOverlayModeIfDiffInformedUnavailable, as well as adding tests!

I am still not convinced by the high-level approach here, though. If I see the flow correctly, the key events are:

• In checkOverlayEnablement (in config-utils.ts), we perform most checks to determine whether overlay analysis should be enabled.
• Afterwards, in initConfig (also in config-utils.ts), we then call shouldPerformDiffInformedAnalysis if overlay analysis is enabled.
  • shouldPerformDiffInformedAnalysis is primarily a check to see if we are analysing a pull request.
  • If diff-informed analysis should be enabled, then we add a query exclusion for exclude-from-incremental.
• Back in init-action.ts, we call computeAndPersistDiffRanges after initConfig has returned. This is what produces the file pointed at by getDiffRangesJsonFilePath.
• Your change then happens afterwards (outside the normal try/catch block), since you depend on checking whether the getDiffRangesJsonFilePath file exists or not.
  • You also again call shouldPerformDiffInformedAnalysis to determine what should have happened. We could probably persist this information in the CodeQL Action state instead of recomputing it.

This seems a bit (unnecessarily) convoluted to me. I am wondering if:

1. We can call computeAndPersistDiffRanges as part of the diff-informed analysis check in initConfig in config-utils.ts? Then the logic isn't spread across two locations and we can potentially also re-use the result of getDiffInformedAnalysisBranches. This would require us to move computeAndPersistDiffRanges from init-action.ts to somewhere more suitable. I don't see a reason for it to have to be in init-action.ts, but we should check that calling it from initConfig doesn't cause any problems.
2. Then, we could perform the new check that is introduced here immediately afterwards in initConfig.
3. We could also investigate whether both the initial diff-informed query enablement check and the new logic here could then be added to checkOverlayEnablement

What do you think?

[sam-robson]

1. We can call computeAndPersistDiffRanges as part of the diff-informed analysis check in initConfig in config-utils.ts? Then the logic isn't spread across two locations and we can potentially also re-use the result of getDiffInformedAnalysisBranches. This would require us to move computeAndPersistDiffRanges from init-action.ts to somewhere more suitable. I don't see a reason for it to have to be in init-action.ts, but we should check that calling it from initConfig doesn't cause any problems.
2. Then, we could perform the new check that is introduced here immediately afterwards in initConfig.
3. We could also investigate whether both the initial diff-informed query enablement check and the new logic here could then be added to checkOverlayEnablement

What do you think?

Thanks! Pushed a refactor addressing these.

• computeAndPersistDiffRanges moved to diff-informed-analysis-utils.ts. New prepareDiffInformedAnalysis combines the enablement check with computing/persisting ranges, reusing the PullRequestBranches result.
• initConfig calls it once and passes the result to a new applyIncrementalAnalysisSettings helper, the single place deciding both the overlay-mode revert and the exclude-from-incremental exclusion.