Simplify computation of job status #3420

henrymercer · 2026-01-23T17:04:07Z

Refactor how we compute job status so this happens in a single place, and whether or not we have a CodeQL config. Also add comments to clarify what happens in the case that the CodeQL config is absent.

Risk assessment

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

High risk: Changes are not fully under feature flags, have limited visibility and/or cannot be tested outside of production.

Which use cases does this change impact?

Workflow types:

Advanced setup - Impacts users who have custom CodeQL workflows.
Managed - Impacts users with dynamic workflows (Default Setup, CCR, ...).

Products:

Code Scanning - The changes impact analyses when analysis-kinds: code-scanning.
Code Quality - The changes impact analyses when analysis-kinds: code-quality.
CCR - The changes impact analyses for Copilot Code Reviews.

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?

End-to-end tests - I am depending on PR checks (i.e. tests in pr-checks).

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.
- Alerts - New or existing monitors will trip if something goes wrong with this change.

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.

- Move it out of the failed SARIF reporting so we compute the job status whether or not we have a CodeQL config. - Add comments to clarify what happens in the case that the CodeQL config is absent.

Copilot

Pull request overview

Summary:
This PR refactors job status computation to centralize the logic in a single function (getFinalJobStatus) in init-action-post.ts, moving it from init-action-post-helper.ts where it was previously split across multiple functions. The refactoring adds clarifying comments about behavior when CodeQL config is absent.

Changes:

Moved and enhanced getFinalJobStatus function from init-action-post-helper.ts to init-action-post.ts, accepting a Config parameter to handle the case where config is undefined
Simplified tryUploadSarifIfRunFailed in init-action-post-helper.ts by removing job status setting logic
Fixed spelling from "analysing" to "analyzing" for consistency

Reviewed changes

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

File	Description
src/init-action-post.ts	Added centralized `getFinalJobStatus` function with improved logic and comments; fixed spelling consistency
src/init-action-post-helper.ts	Removed job status setting logic from `tryUploadSarifIfRunFailed`; removed `getFinalJobStatus` export
lib/init-action-post.js	Generated JavaScript (not reviewed per custom guidelines)

mbg

Thanks for working on simplifying this! I think that the changes here are a good step towards accomplishing that.

I agree with the "High risk" assessment for this PR, since there are changes to code that always runs. Whenever we have a "High risk" PR, it's worth assessing whether this is unavoidable as a result of the nature of the change, due to a lack of FF (and whether one can reasonably be added), or because of a lack of test coverage. I note that all of this refactoring was able to pass CI without any adjustments to any tests.

The job status computation spans an entire CodeQL workflow as follows:

Whenever we send a status report for a failure, aborted, or user-error status, we set EnvVar.JOB_STATUS accordingly, unless it is already set to some value.
Consequently, by the time we get to the init-post action, we are in one of the following scenarios:
1. An earlier step failed, we sent a status report, and EnvVar.JOB_STATUS is set accordingly.
2. An earlier step failed catastrophically, and we did not set EnvVar.JOB_STATUS.
3. Everything succeeded, EnvVar.JOB_STATUS is not set, but EnvVar.ANALYZE_DID_COMPLETE_SUCCESSFULLY is set to true.

In general, we do not change the value of EnvVar.JOB_STATUS if it is already set.

Previously, we had the following logic in init-post:

If an init step successfully initialised a Config, then we run initActionPostHelper.run which calls tryUploadSarifIfRunFailed:
- If EnvVar.ANALYZE_DID_COMPLETE_SUCCESSFULLY isn't true:
  - If EnvVar.JOB_STATUS is not already set, it sets EnvVar.JOB_STATUS to JobStatus.ConfigErrorStatus.
  - Otherwise, EnvVar.JOB_STATUS keeps its existing value.
- Else: EnvVar.JOB_STATUS is set to JobStatus.SuccessStatus, if not already set to some value.
If no Config was initialised by an init step, we do not call initActionPostHelper.run and none of the above logic that might set EnvVar.JOB_STATUS runs. I.e. EnvVar.JOB_STATUS remains the same as when init-post started.
Then, initActionPostHelper.getFinalJobStatus() is used to retrieve the value of EnvVar.JOB_STATUS and convert it to a corresponding JobStatus value. If EnvVar.JOB_STATUS is unset or invalid, then JobStatus.UnknownStatus is returned.

The new logic in this PR removes all EnvVar.JOB_STATUS-related code from tryUploadSarifIfRunFailed and moves it into a new getFinalJobStatus() function which combines the EnvVar.JOB_STATUS-related parts from tryUploadSarifIfRunFailed with the old behaviour of initActionPostHelper.getFinalJobStatus().

The change here is limited to the init-post action and we don't change the order in which the EnvVar.JOB_STATUS-related checks/computations are performed. However, we may be computing a final value for EnvVar.JOB_STATUS later than before. Therefore, I would expect these changes to at most impact the job status that is reported in status reports, if e.g. an exception is thrown after we previously would have computed the final job status, but before we do it with these changes.

If the change only affects status reports, then I think that no FF and the associated complexity of maintaining two code paths is required.

I don't see any blockers here. It would be good to use this opportunity to improve our unit test coverage, though. Aside from that, I only have some minor comments (including a couple unrelated to the changes in this PR).

src/init-action-post.ts

mbg · 2026-01-27T11:57:58Z

src/init-action-post.ts

+    return JobStatus.UnknownStatus;
+  }
+
+  let jobStatus: JobStatus;


It would be nice to have a unit test for the part of this function from here onwards.

I'll leave this as is for now.

mbg · 2026-01-27T12:04:04Z

src/init-action-post-helper.ts

+  }
 }

 export async function run(


Not part of this PR, but why do we have a run function here? It's a bit confusing that we typically have run functions in the -action[-post].ts entry points, and here we have one in a module that's not that. We also have a run function call another run function.

I agree this is suboptimal, but will leave this as is for now to avoid scope creep.

src/init-action-post-helper.ts

mbg · 2026-01-27T12:20:53Z

src/init-action-post.ts

      );

-      // If we are analysing the default branch and some kind of caching is enabled,
+      // If we are analyzing the default branch and some kind of caching is enabled,


Minor: This seems like an unnecessary and unrelated change 😅

I have a spell checker in my IDE that was complaining, nothing personal 😆

henrymercer requested a review from a team as a code owner January 23, 2026 17:04

Copilot AI review requested due to automatic review settings January 23, 2026 17:04

github-actions bot added the size/S Should be easy to review label Jan 23, 2026

Copilot started reviewing on behalf of henrymercer January 23, 2026 17:04 View session

Simplify computation of job status

dcd1b12

- Move it out of the failed SARIF reporting so we compute the job status whether or not we have a CodeQL config. - Add comments to clarify what happens in the case that the CodeQL config is absent.

henrymercer force-pushed the henrymercer/compute-job-status-if-no-config branch from c1f6643 to dcd1b12 Compare January 23, 2026 17:07

Copilot AI reviewed Jan 23, 2026

View reviewed changes

mbg previously approved these changes Jan 27, 2026

View reviewed changes

henrymercer added Rebuild Re-transpile JS & re-generate workflows and removed Rebuild Re-transpile JS & re-generate workflows labels Jan 27, 2026

henrymercer added 2 commits January 27, 2026 14:26

Update comment

60b658e

Split up getFinalJobStatus

b333fc6

henrymercer dismissed mbg’s stale review via b333fc6 January 27, 2026 14:30

Merge branch 'main' into henrymercer/compute-job-status-if-no-config

6095dc4

henrymercer requested a review from mbg January 27, 2026 14:43

mbg approved these changes Jan 27, 2026

View reviewed changes

henrymercer enabled auto-merge January 27, 2026 14:47

henrymercer merged commit 835dade into main Jan 27, 2026
251 checks passed

henrymercer deleted the henrymercer/compute-job-status-if-no-config branch January 27, 2026 14:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify computation of job status #3420

Simplify computation of job status #3420

henrymercer commented Jan 23, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mbg left a comment

Uh oh!

Uh oh!

mbg Jan 27, 2026

Uh oh!

henrymercer Jan 27, 2026

Uh oh!

mbg Jan 27, 2026

Uh oh!

henrymercer Jan 27, 2026

Uh oh!

Uh oh!

mbg Jan 27, 2026

Uh oh!

henrymercer Jan 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Simplify computation of job status #3420

Simplify computation of job status #3420

Conversation

henrymercer commented Jan 23, 2026

Risk assessment

Which use cases does this change impact?

How did/will you validate this change?

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

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

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

Merge / deployment checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

mbg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mbg Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

henrymercer Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

mbg Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

henrymercer Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mbg Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

henrymercer Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants