Repository navigation
Remove TRAP cache cleanup - #4243
henrymercer wants to merge 4 commits into
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
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
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.
There was a problem hiding this comment.
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
Files excluded by content exclusion policy (1)
- lib/entry-points.js
🧠 Review effort: Max
mbg
left a comment
mbg
left a comment
There was a problem hiding this comment.
Makes sense, thanks for cleaning up. A couple of minor comments, but nothing blocking.
| logger, | ||
| }: ActionState<["Logger", "ReadOnlyEnv"]>): void { | ||
| if (env.getOptional("CODEQL_ACTION_CLEANUP_TRAP_CACHES") !== undefined) { | ||
| logger.warning( |
There was a problem hiding this comment.
This is a good example of where those logging changes we discussed on my PR are useful. (Now in a separate PR at #4235)
| t.deepEqual( | ||
| target.getLogger().messages.map((m) => m.type), | ||
| ["warning"], | ||
| ); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Good idea, but I'll leave this to a future PR.
| * | ||
| * 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) { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Co-authored-by: Michael B. Gale <mbg@github.com>
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_CACHESenvironment variable is set, since it no longer has any effect.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
dynamicworkflows (Default Setup, Code Quality, ...).Products:
analysis-kinds: code-scanning.analysis-kinds: code-quality.Environments:
github.comand/or GitHub Enterprise Cloud with Data Residency.How did/will you validate this change?
.test.tsfiles).pr-checks).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