GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Include diagnostics in failed SARIF if they were created before a database was initialised by mbg · Pull Request #4223 · github/codeql-action

Conversation

@mbg

@mbg mbg commented Oct 9, 2026

Copy link
Copy Markdown
Member

Diagnostics are typically written to the database cluster and are then included in the SARIF that is generated by the CLI. However, in the CodeQL Action we may produce diagnostics before the database cluster is initialised. To work around this, we have so far relied on tracking diagnostics in memory that were created before the database cluster is ready. After the cluster is initialised, those diagnostics are then flushed to disk. However, this represents a problem if the database cluster is never successfully initialised.

This PR changes this so that diagnostics are written directly to a temporary directory on disk if the database cluster is not yet ready. They are then moved to the database cluster once it is initialised. If the cluster is never initialised, then we point the CLI at the temporary directory on disk using a --diagnostic-dir argument. The CLI will then pick up any diagnostics that are still in that location and include them in the failed SARIF.

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:

  • Advanced setup - Impacts users who have custom CodeQL workflows.
  • 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.
  • Other first-party - The changes impact other first-party analyses.

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?

  • Unit tests - I am depending on unit test coverage (i.e. tests in .test.ts files).
  • 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mbg
mbg changed the base branch from main to mbg/fs-abstraction October 9, 2026 22:30
@mbg
mbg added this pull request to stack #4224 October 9, 2026 22:30
@github-actions github-actions Bot added the size/L May be hard to review label Oct 9, 2026
@mbg
mbg force-pushed the mbg/diagnostics/failed-sarif branch 2 times, most recently from 520eff7 to 2613b9c Compare October 9, 2026 22:51
@github-actions github-actions Bot added size/XL May be very hard to review and removed size/L May be hard to review labels Oct 12, 2026
@mbg
mbg removed this pull request from stack #4224 October 12, 2026 11:00
@mbg mbg mentioned this pull request Oct 12, 2026
Store and restore "failed" configuration if init step aborts #4225
Merged
@mbg
mbg force-pushed the mbg/diagnostics/failed-sarif branch from b5ce898 to 9312ede Compare October 12, 2026 11:54
@mbg
mbg changed the base branch from mbg/fs-abstraction to mbg/reduce-config-requirements October 12, 2026 11:54
@github-actions github-actions Bot added size/L May be hard to review and removed size/XL May be very hard to review labels Oct 12, 2026
@mbg
mbg force-pushed the mbg/diagnostics/failed-sarif branch from 9312ede to 65d8aa4 Compare October 12, 2026 12:02
@mbg
mbg force-pushed the mbg/diagnostics/failed-sarif branch from 65d8aa4 to 9eeab46 Compare October 12, 2026 13:23
@mbg
mbg marked this pull request as ready for review October 12, 2026 13:24
@mbg
mbg requested a review from a team as a code owner October 12, 2026 13:24
Copilot AI balanced review requested due to automatic review settings October 12, 2026 13:24
Copilot started reviewing on behalf of mbg October 12, 2026 13:25
View session
Copilot AI reviewed Oct 12, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

  • Copilot's review of this pull request may be incomplete because some of the changed files are excluded by your Copilot content exclusion settings. See Excluding content from Copilot for details.

🟡 Changes recommended

Temporary-write failures can abort initialization, cross-filesystem moves can lose diagnostics, and the new check does not validate failed SARIF export.

5 open findings
  • Medium severity Test does not verify failed-SARIF diagnostic export · New
  • Medium severity Best-effort diagnostics can make init fail on filesystem errors · New
  • Medium severity Cross-filesystem diagnostic moves lack an EXDEV fallback · New
  • Low severity Spelling correction · New
  • Low severity Documentation typo · New
What changed in this PR

Persists pre-database diagnostics on disk so they can be included in failed SARIF output.

Changes:

  • Replaces in-memory buffering with temporary diagnostic files.
  • Passes temporary diagnostics to CodeQL export.
  • Adds unit and PR-check coverage.
File Description
src/​diagnostics.ts Implements temporary persistence and database relocation.
src/​init-action.ts Flushes and logs temporary diagnostics.
src/​init-action-post-helper.ts Supplies temporary diagnostics during failed SARIF generation.
src/​codeql.ts Adds --diagnostic-dir support.
src/​diagnostics.test.ts Tests temporary persistence and flushing.
src/​setup-codeql.test.ts Provides temporary-directory test setup.
src/​config/​db-config.test.ts Provides temporary-directory test setup.
pr-checks/​diagnostics-without-db.ts Checks for the expected diagnostic file.
pr-checks/​checks/​diagnostics-without-db.yml Defines the new PR check.
lib/​entry-points.js Generated bundle; excluded from review.
.github/​workflows/​__diagnostics-without-db.yml Generated workflow; excluded from review.
Files excluded by content exclusion policy (2)
  • .github/workflows/__diagnostics-without-db.yml
  • lib/entry-points.js

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread pr-checks/checks/diagnostics-without-db.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/diagnostics.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/diagnostics.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread pr-checks/checks/diagnostics-without-db.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/diagnostics.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
@mbg
mbg added this pull request to stack #4227 October 12, 2026 13:49
@mbg
mbg force-pushed the mbg/diagnostics/failed-sarif branch from 4b32487 to 9104ff3 Compare October 12, 2026 16:51
@github-actions github-actions Bot added size/XL May be very hard to review and removed size/L May be hard to review labels Oct 12, 2026
Base automatically changed from mbg/reduce-config-requirements to main October 13, 2026 09:46
@mbg
mbg force-pushed the mbg/diagnostics/failed-sarif branch 3 times, most recently from 3cf39b0 to b5557b8 Compare October 16, 2026 11:23
Copilot AI reviewed Oct 21, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused diagnostic-persistence changes have unit and integration coverage, with no unresolved blocking findings.

1 open issue
  • Medium severity Keep configuration lookup lazy for single-file SARIF uploads
2 files not reviewed
  • .github/workflows/__diagnostics-without-db.yml: Excluded by your content exclusion settings
  • lib/entry-points.js: Excluded by your content exclusion settings

Review effort: Balanced

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
henrymercer
henrymercer reviewed Oct 21, 2026
View reviewed changes

@henrymercer henrymercer left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing my comments, just a couple of final things before we merge this.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/diagnostics.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/codeql.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/diagnostics.ts Outdated
if (language !== undefined) {
return path.resolve(basePath, language, "diagnostic", "codeql-action");
}
return basePath;

@henrymercer henrymercer Oct 21, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this has introduced a new issue: language-specific diagnostics which are created after the config is initialized, but before the DB is created, now don't get picked up because they live in ${tempDiagnosticsRootDir}/${language}/diagnostic/codeql-action, but init-post passes --diagnostic-dir=${tempDiagnosticsRootDir}/diagnostic/codeql-action.

We could modify init post to pass --diagnostic-dir=${tempDiagnosticsRootDir}. It might also be good to add a regression test.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
mbg added 5 commits October 22, 2026 10:30
@mbg
Improve logging and error handling in logTemporaryDiagnostics
26f5c5e
@mbg
Only warn for missing config if init was successful
308d941
@mbg
Clean up path functions in diagnostics.ts and add some tests
b79966a
@mbg
Merge branch 'main' into mbg/diagnostics/failed-sarif
2d0d1c1
@mbg
Merge branch 'main' into mbg/diagnostics/failed-sarif
ac509b6
@mbg
mbg requested review from henrymercer and a max review from Copilot October 22, 2026 10:52
Copilot started reviewing on behalf of mbg October 22, 2026 10:52
View session
Copilot AI reviewed Oct 22, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Cross-step persistence and CLI export need human validation, particularly the uncovered database-initialization failure path that skips failed-SARIF generation.

2 open issues
  • High severity Save aborted configuration on database initialization failure · New
  • Medium severity Keep configuration lookup lazy for single-file SARIF uploads
3 files not reviewed
  • .github/workflows/__diagnostics-without-db.yml: Excluded by your content exclusion settings
  • .github/workflows/__submit-sarif-failure.yml: Excluded by your content exclusion settings
  • lib/entry-points.js: Excluded by your content exclusion settings

Review effort: Max

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/init-action.ts
return;
} finally {
logUnwrittenDiagnostics();
logTemporaryDiagnostics(actionState);

@henrymercer henrymercer Oct 22, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can leave this for follow up.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
henrymercer
henrymercer previously approved these changes Oct 22, 2026
View reviewed changes
Comment thread src/diagnostics.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/diagnostics.test.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/codeql.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/diagnostics.ts Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/init-action.ts
return;
} finally {
logUnwrittenDiagnostics();
logTemporaryDiagnostics(actionState);

@henrymercer henrymercer Oct 22, 2026

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can leave this for follow up.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mbg
Improve wording to address review feedback
87da5f3
@mbg
mbg dismissed henrymercer’s stale review via 87da5f3 October 22, 2026 12:50
@mbg
mbg requested a review from henrymercer October 22, 2026 12:50
henrymercer
henrymercer previously approved these changes Oct 22, 2026
View reviewed changes
@mbg
mbg enabled stacked auto-merge October 22, 2026 12:53
@mbg
mbg added this pull request to the merge queue Oct 22, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 22, 2026
@mbg
Merge remote-tracking branch 'origin/main' into mbg/diagnostics/faile…
5196bae 
…d-sarif
@mbg
mbg dismissed henrymercer’s stale review via 5196bae October 22, 2026 13:24
@mbg
mbg requested a review from henrymercer October 22, 2026 13:25
henrymercer
henrymercer approved these changes Oct 22, 2026
View reviewed changes
@mbg
mbg enabled stacked auto-merge October 22, 2026 13:33
@mbg
mbg added this pull request to the merge queue Oct 22, 2026
Merged via the queue into main with commit 0acb923 Oct 22, 2026
230 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@mbg
mbg deleted the mbg/diagnostics/failed-sarif branch October 22, 2026 13:53
This was referenced Nov 3, 2026
Merge main into releases/v4 #4286
Merged
Merge releases/v4 into releases/v3 #4291
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@henrymercer henrymercer henrymercer approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

size/XL May be very hard to review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@mbg @henrymercer
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.