GeezerVison

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

GeezerVison is showing github.com — Remove TRAP cache cleanup by henrymercer · Pull Request #4243 · github/codeql-action

Conversation

henrymercer commented Oct 16, 2026

Copy link

Copy Markdown

Contributor

Follows the deprecation in #3795, which announced that TRAP cache cleanup would be removed in May 2026.

We add a warning when the CODEQL_ACTION_CLEANUP_TRAP_CACHES environment variable is set, since it no longer has any effect.

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.
- 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

henrymercer and others added 2 commits

October 16, 2026 14:00


          Remove TRAP cache cleanup

1a7b87e

Co-authored-by: Copilot App <[email protected]>


          Warn when CODEQL_ACTION_CLEANUP_TRAP_CACHES is set

9b5d335

Co-authored-by: Copilot App <[email protected]>

Copilot AI max review requested due to automatic review settings

October 16, 2026 13:02


          Add change note

ba6f821

Co-authored-by: Copilot App <[email protected]>

github-actions Bot added the size/L May be hard to review label

Oct 16, 2026

Copilot started reviewing on behalf of henrymercer

October 16, 2026 13:02

View session

Copilot AI reviewed

Oct 16, 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.

🟢 Approval recommended

The removal is consistent, the warning has focused coverage, and remaining feedback is non-blocking.

1 open finding

Add a change note for the user-facing removal · New

What changed in this PR

Removes deprecated TRAP cache cleanup from the CodeQL Action while preserving TRAP caching itself.

Changes:

Removes cleanup logic, configuration, API support, and telemetry.
Adds a warning for the obsolete environment variable, with unit coverage.

File	Description
`src/trap-caching.ts`	Replaces cleanup with an obsolete-variable warning.
`src/trap-caching.test.ts`	Replaces cleanup tests with warning tests.
`src/feature-flags.ts`	Removes the cleanup feature flag.
`src/doc-url.ts`	Removes the unused permissions documentation URL.
`src/api-client.ts`	Removes the cache-deletion helper.
`src/analyze-action.ts`	Integrates the warning and removes cleanup telemetry.

Files excluded by content exclusion policy (1)

lib/entry-points.js

🧠 Review effort: Max

💡 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

src/trap-caching.ts Resolved

Uh oh!

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

henrymercer requested a max review from Copilot

October 16, 2026 13:16

Copilot started reviewing on behalf of henrymercer

October 16, 2026 13:17

View session

Copilot AI reviewed

Oct 16, 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.

🟢 Approval recommended

The focused removal consistently updates callers and includes tests for the replacement warning.

0 open findings

1 resolved since last review

Add a change note for the user-facing removal

Files excluded by content exclusion policy (1)

lib/entry-points.js

🧠 Review effort: Max

Sorry, something went wrong.

Uh oh!

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

All reactions

henrymercer marked this pull request as ready for review

October 16, 2026 13:24

henrymercer requested a review from a team as a code owner

October 16, 2026 13:24

mbg previously approved these changes

Oct 16, 2026

View reviewed changes

mbg left a comment

Copy link

Copy Markdown

Member

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.

Makes sense, thanks for cleaning up. A couple of minor comments, but nothing blocking.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

unreleased-change-notes/remove-trap-cache-cleanup.md Resolved

Uh oh!

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

Comment thread

src/trap-caching.ts

		logger,
		}: ActionState<["Logger", "ReadOnlyEnv"]>): void {
		if (env.getOptional("CODEQL_ACTION_CLEANUP_TRAP_CACHES") !== undefined) {
		logger.warning(

mbg Oct 16, 2026

Copy link

Copy Markdown

Member

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.

This is a good example of where those logging changes we discussed on my PR are useful. (Now in a separate PR at #4235)

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

src/trap-caching.test.ts

Comment on lines +212 to 215

		t.deepEqual(
		target.getLogger().messages.map((m) => m.type),
		["warning"],
		);

mbg Oct 16, 2026

Copy link

Copy Markdown

Member

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.

Minor: Perhaps worth modifying the logs/notLogs methods, or adding new ones, to BaseEnvBuilder in testing-utils.ts to allow these assertions to be chained with the target.passes call.

Sorry, something went wrong.

Uh oh!

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

All reactions

henrymercer Oct 16, 2026

Copy link

Copy Markdown

Contributor Author

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.

Good idea, but I'll leave this to a future PR.

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

src/api-client.ts

		*
		* See https://docs.github.com/en/rest/actions/cache#delete-a-github-actions-cache-for-a-repository-using-a-cache-id.
		*/
		export async function deleteActionsCache(id: number) {

mbg Oct 16, 2026

Copy link

Copy Markdown

Member

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'm mildly conscious that we might need this again in the future since we still have other cache usage, but it's easy enough to add back in if needed.

Sorry, something went wrong.

Uh oh!

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

All reactions

henrymercer Oct 16, 2026

Copy link

Copy Markdown

Contributor Author

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.

The annoying thing is this requires actions: write, which we'd rather most of our jobs didn't have. This kind of API is best suited for a dedicated Action to minimize the code that has the actions: write permission.

Sorry, something went wrong.

Uh oh!

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

All reactions


          Improve changelog note

734778a

Co-authored-by: Michael B. Gale <[email protected]>

henrymercer dismissed mbg’s stale review via

734778a

October 16, 2026 15:13

henrymercer requested a review from mbg

October 16, 2026 15:14

mbg approved these changes

Oct 16, 2026

View reviewed changes

henrymercer added this pull request to the merge queue

Oct 21, 2026

Merged via the queue into main with commit 0b9cddc

Oct 21, 2026

234 checks passed

Uh oh!

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

henrymercer deleted the henrymercer/remove-trap-cache-cleanup branch

October 21, 2026 16:17

This was referenced Nov 3, 2026

Merge main into releases/v4 #4286

Merged

Merge releases/v4 into releases/v3 #4291

Merged

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

mbg

mbg approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

size/L

May be 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

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.