CI refactor: cache key collisions, job graph ordering, artifact reuse

[ScrewTSW]

Summary

CI is slow, wasteful and occasionally wrong because of three separable problems: colliding cache keys, an unstructured job graph, and a stale Node version matrix. #4 treated a symptom of the first two (a cold build exceeding timeout-minutes: 10, which GitHub reports as cancelled, not failure) by raising the cap to 25 minutes. That unblocked PRs but fixed nothing underneath.

This issue tracks the actual refactor. It is deliberately not a single PR.

Ordering: the Node v22 cull lands first. Dropping the v18/v20 jobs removes 6 CI jobs per PR before any restructuring begins, so this refactor is measured against an honest baseline. Any before/after timing comparison must be taken after the cull, not across it.

---

1. Cache keys collide and never partially restore

1a. ${{ runner.os }}-node- is reused for four different paths

Seven cache entries share one prefix while pointing at unrelated directories:

File	Line	Path	Key
.github/actions/build-vscode-extension/action.yml	57	extensions/vscode/node_modules	${{ runner.os }}-node-${{ hashFiles('extensions/vscode/package-lock.json') }}
.github/actions/build-vscode-extension/action.yml	63	core/node_modules	${{ runner.os }}-node-${{ hashFiles('core/package-lock.json') }}
.github/actions/build-vscode-extension/action.yml	69	gui/node_modules	${{ runner.os }}-node-${{ hashFiles('gui/package-lock.json') }}

⚠️ Updated after #11: the jetbrains-release.yaml rows were removed from this table — that workflow was deleted along with the rest of the inherited release flow. The remaining build-vscode-extension entries are unaffected.

These only stay distinct because the four lockfiles happen to hash differently. The namespace carries no information about what is cached — nothing but luck keeps core from restoring into gui. Other actions in the repo already do this correctly (-core-node-modules-, -vscode-node-modules-, -gui-node-modules-, -binary-node-modules-); build-vscode-extension and jetbrains-release are the holdouts.

Fix: adopt the ${{ runner.os }}-<component>-node-modules-<hash> convention everywhere. Mechanical, low-risk, do it first.

1b. No restore-keys anywhere except the CLI workflow

Only cli-pr-checks.yml:79 declares restore-keys. Every other cache is all-or-nothing: touch one line in one lockfile and the entire node_modules tree is refetched and rebuilt from zero.

extensions/vscode, gui, core and binary each pay a full npm ci on any lockfile change. This is the single biggest contributor to the cold-build times that made #4 necessary.

Fix: add restore-keys fallbacks to every node_modules and ~/.npm cache. A near-miss restore that npm ci then reconciles is dramatically cheaper than a cold install.

1c. ~/.npm over-keys on **/package-lock.json

build-vscode-extension/action.yml:51:

key: ${{ runner.os }}-npm-cache-build-${{ hashFiles('**/package-lock.json') }}

** spans every lockfile in the monorepo, including ones this action never installs. A change in extensions/cli or any packages/* invalidates the npm download cache for the VSIX build. Other call sites already scope this correctly — pr-checks.yaml:240 uses hashFiles('core/package-lock.json', 'extensions/vscode/package-lock.json').

Fix: narrow the glob to lockfiles this action actually consumes, and add restore-keys.

1d. Mixed actions/cache@v4 and @v5

@v4 in the composite actions, @v5 in the workflows. No reason for the split.

Fix: standardise on @v5.

1e. One literal key: CONSTANT

.github/actions/run-vscode-e2e-test/action.yml:43 caches extensions/vscode/e2e/.test-extensions under the literal string CONSTANT. It can never be invalidated — if the contents ever need to change, the only remedy is manually purging the cache. Intentional or not, it needs a comment explaining why, or a real key.

---

2. Job graph: no ordering, no shared build artifacts

Every job independently reinstalls and rebuilds the same dependency trees. core, gui and packages/* are built from scratch in pr-checks.yaml, again in pr-build-upload-vsix.yaml, and again in run-jetbrains-tests. There is no needs:-based fan-out from a single setup job, and no artifact handoff between them.

A single PR currently fires 40+ check runs across six workflows. From #4's own check list: lint, prettier-check, core-checks, gui-checks, vscode-checks, binary-checks, 7× packages-checks, 12× CLI test, 3× build-and-upload-vsix, jetbrains-tests, smoke-api, plus aggregators.

Worth noting: .github/workflows/pr-checks.yaml contains a job literally named ${{ (matrix.test_file || 'unknown') }} (${{ matrix.command }}) — an unexpanded expression in a job name, i.e. a matrix that isn't wired up correctly.

Fix (design work required, do not rush):

• One setup job producing built packages/*, core, gui as artifacts.
• Downstream jobs needs: setup and download rather than rebuild.
• Collapse the six overlapping PR workflows into one with a proper matrix.
• Add concurrency: cancel-in-progress per workflow — promoted to its own section, see 3. It is independent of the restructure and should not wait for it.

---

3. No PR workflow cancels superseded runs

Every push to a PR branch leaves the previous run going to completion. Three
validation workflows fire on pull_request and each rebuilds the whole
dependency tree, so a rapid review cycle — push, address a comment, push again —
has two or three full matrix builds racing each other, and only the last one's
result is ever looked at.

No PR workflow currently declares a concurrency block. The mechanism
already exists in the repo and is used correctly in
.github/workflows/osv-scanner.yml:42 (the original example,
jetbrains-release.yaml:29, was deleted in
#11):

concurrency:
  group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
  cancel-in-progress: true

This does not serialize parallel PRs

Worth stating plainly, because it is the obvious objection and the answer is not
obvious: the group key includes the PR number, so PR #7 and PR #8 land in
different groups and never contend. Cancellation happens strictly within a
group — that is, "a newer commit on the same PR supersedes the older run of the
same workflow."

Two details in the key matter:

• github.workflow keeps workflows independent, so a push cancels the prior
  PR Checks run and the prior PR Build And Upload VSIX run separately rather
  than one evicting the other.
• github.ref as fallback covers non-PR triggers. pr-checks.yaml also
  fires on push to main, where github.event.pull_request.number is empty;
  without the fallback the key would collapse to the bare workflow name and
  that would serialize main builds — the very failure mode this section is
  ruling out.

A constant group key, like group: "pages" in docs-gh-pages.yml:17, is a
global lock. That is correct for Pages (one deployment to contend over) and is
exactly the anti-pattern to avoid here.

Scope

Apply to the three heavy validation workflows:

File	Trigger	Why
.github/workflows/pr-checks.yaml	pull_request + push to main	Largest job count; needs the github.ref fallback for the push trigger
.github/workflows/pr-build-upload-vsix.yaml	pull_request	3-platform matrix at timeout-minutes: 25 — the most expensive thing a superseded push leaves running
.github/workflows/cli-pr-checks.yml	pull_request	12 test jobs

Deliberately excluded:

File	Trigger	Why not
.github/workflows/cla.yaml	pull_request_target, issue_comment	Cancelling mid-flight risks leaving a PR wrongly marked unsigned
.github/workflows/label-merged-prs.yml	pull_request [closed]	Fires once; nothing to supersede
.github/workflows/similar-issues.yml	pull_request [opened]	Fires once; never re-runs on push

Release workflows must not get cancel-in-progress. stable-release.yml,
reusable-release.yml and the release-* set publish
artifacts; interrupting an upload can leave a partial or corrupt release. If any
of them ever needs a concurrency block it should be cancel-in-progress: false, to queue rather than kill.

⚠️ Updated after #11: auto-release.yml was deleted (notes-only, no build step) and preview.yaml / main.yaml / jetbrains-release.yaml with it. Their replacements nightly.yaml and release.yaml already declare concurrency with cancel-in-progress: false, so they need nothing from this section.

This item is small, independent of the cache work, and pays for itself
immediately. It can ship well ahead of the rest.

---

4. pr-checks.yaml re-runs every job on push to main

.github/workflows/pr-checks.yaml:4-11 triggers on both pull_request and
push to main. No job is gated on github.event_name — 12 of its 13 jobs
carry no if: at all (the exceptions are track-rerun, gated on
github.run_attempt, and the always() aggregator). So merging a PR re-runs
the entire suite that just passed on that PR: core-checks, gui-checks,
binary-checks, vscode-checks, the packages-checks matrix, vscode-e2e-tests,
jetbrains-tests, VSIX packaging — against a tree that, for a fast-forward or
rebase merge, is identical to the head commit already validated.

It is not merely redundant — it produces contradictory verdicts

This is the part that makes it a correctness bug rather than a waste of runners.

core-checks resolves its API-key gate at line 61:

is_fork: ${{ github.event.pull_request.head.repo.fork == true }}

On a push event there is no pull_request object, so the expression evaluates
to empty — never true. The gate concludes "not a fork" and sets
IGNORE_API_KEY_TESTS: false, enabling the live-network tests in
core/llm/llm.test.ts that core/llm/llm.test.ts:205 would otherwise skip.

The result is that the same job runs a different test set depending on the
event, and the push variant depends on live third-party APIs.

Observed on the merge of #7:

Run	Commit	Event	Result
32247887739	539c5931c (merge of #7)	push	failure — core-checks
—	a28b88a46 (head of #7)	pull_request	success

Same code, opposite verdicts. The failing assertions are
LLM › anthropic/claude-sonnet-4-0 › … and LLM › openai/gpt-4o › … — live API
calls, not anything #7 changed.

Four of the five most recent push-to-main runs failed this way. A main
branch that is red by default trains everyone to ignore it, which is worse than
having no check at all.

jetbrains-tests fails on the push runs too (e.g.
32238932311),
which is a separate pre-existing issue but adds to the same noise.

Fix: split the secret-bearing jobs into their own main-only workflow

The underlying problem is that one workflow is serving two audiences with two
different trust levels. A PR gate must run on untrusted contributor code and
therefore must not hold production API keys; a post-merge job runs on reviewed
code on main and legitimately can. Bolting an if: onto shared jobs leaves
both concerns in one file, where the next person to add a job has to know which
category it falls into — and the is_fork bug above is exactly what happens
when someone gets that wrong.

Split them:

• pr-checks.yaml keeps the pull_request trigger and the checks that need no
  secrets — typecheck, lint, prettier, unit tests, packaging. Drop the push
  trigger. These are the merge gate.
• A new main-only workflow (main-checks.yaml) takes the push trigger and
  owns the jobs that require real credentials — the live llm.test.ts suite and
  anything else touching OPENAI_API_KEY / ANTHROPIC_API_KEY. Non-blocking or
  alerting, since it depends on third-party uptime.

This makes the secret boundary a property of the file, so it cannot be got
wrong per-job, and it removes the duplicate full-suite run as a side effect.
It also means IGNORE_API_KEY_TESTS and the is_fork plumbing can disappear
from the PR path entirely rather than being conditionally disarmed.

Smaller alternatives, if the split is deferred:

1. Gate the jobs on event type. Add if: github.event_name == 'pull_request'
   to the jobs that only make sense as PR gates. This is the pattern
   .github/workflows/osv-scanner.yml already uses to keep its PR and
   main/release jobs distinct.
2. Or drop the push trigger entirely. If the PR gate is trusted, re-running
   it post-merge adds nothing for rebase/fast-forward merges. Worth keeping only
   if squash or merge-commit strategies can produce a tree no PR run validated.
3. Fix the fork detection regardless of the above. is_fork silently
   mis-evaluates on any non-pull_request event. Even after gating, that
   expression should not quietly resolve to "not a fork" — it should either
   handle the event explicitly or fail loudly.
4. Reconsider live-API tests in CI at all. They make results depend on third-party
   availability, rate limits and quota. Recording them or moving them to a
   scheduled job would decouple merge velocity from OpenAI/Anthropic uptime.

Item 3 is the actual bug and is worth doing on its own even if the split is
deferred — is_fork should never silently resolve to "not a fork".

---

5. Node version matrix — tracked separately

Stale v18/v20 jobs and the .nvmrc bump to lts/v22 are the cull, tracked in its own issue and landing before any of the above. Not in scope here beyond the timing caveat already noted.

---

6. External actions are not pinned to commit SHAs

A version tag is a mutable pointer. actions/checkout@v6 resolves to
whatever commit the v6 tag points at today; an attacker who compromises an
action repository (or its maintainer's account) can move that tag and every
workflow referencing it silently executes new code on the next run. This is the
mechanism behind the tj-actions/changed-files compromise (CVE-2025-30066),
where a retagged action exfiltrated CI secrets from ~23,000 repositories.

Commit SHAs are immutable, so pinning removes the entire class.

Current state — 122 external references, 12 pinned

Category	Count	Notes
Total uses: referencing an external action	122	excludes local ./.github/actions/*
Pinned to a 40-char SHA	12	10 from #11, 2 from #9
Unpinned	110

⚠️ The 12 pinned references make the tree inconsistent, not safe: 36 other
actions/checkout@v6 call sites still float. Partial pinning provides close to
zero security benefit while costing the appearance of it. This should be all or
nothing.

6a. jlumbroso/free-disk-space@main — a moving branch, not even a tag

.github/workflows/pr-checks.yaml:339 tracks a branch. Every run executes
whatever is on that repository's main at that moment, with no release process
in between. This is the single highest-risk reference in the tree and worth
fixing on its own, ahead of any wider sweep.

6b. Third-party actions are the real exposure

actions/* is maintained by GitHub; a compromise there is a different-magnitude
event. The third-party set is where an individual maintainer account is the
whole trust boundary:

File	Line	Action
pr-checks.yaml	339	jlumbroso/free-disk-space@main ⚠️ branch
pr-checks.yaml	378	re-actors/alls-green@release/v1 ⚠️ branch-like
cli-pr-checks.yml	181	re-actors/alls-green@release/v1 ⚠️ branch-like
cla.yaml	22	contributor-assistant/github-action@v2.6.1
auto-assign-issue.yaml	14	pozil/auto-assign-issue@v2
metrics.yaml	39	github/issue-metrics@v3
metrics.yaml	55	slackapi/slack-github-action@v3.0.1
delete-stale-branches.yaml	17	crs-k/stale-branches@v8.2.2
submit-github-dependency-graph.yml	24	gradle/actions/dependency-submission@v6
run-jetbrains-tests/action.yml	74	AnimMouse/setup-ffmpeg@v1
run-jetbrains-tests/action.yml	80	gradle/actions/setup-gradle@v3

⚠️ re-actors/alls-green@release/v1 is a branch ref, not a tag — same
mutability problem as @main, just less obvious.

⚠️ .github/actions/*/action.yml must be included in the sweep. Composite
actions are invisible to a .github/workflows/-only grep, and
run-jetbrains-tests/action.yml alone carries two unpinned third-party actions.

Convention

Keep the version readable — a bare SHA is unreviewable:

uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6

⚠️ Resolve SHAs from the API, and dereference annotated tags.
git/ref/tags/<tag> returns a tag object for annotated tags; its .object.sha
is the tag, not the commit. Pinning that value fails at runtime:

gh api repos/<owner>/<repo>/git/ref/tags/<tag> --jq '.object.sha'   # may be a tag object
gh api repos/<owner>/<repo>/git/tags/<sha>     --jq '.object.sha'   # → commit

⚠️ Do not take SHAs from a review comment or from memory — verify each resolves
with gh api repos/<owner>/<repo>/commits/<sha>.

Keeping pins current

Pinning trades silent compromise for silent staleness: security patches stop
arriving too. Dependabot understands SHA pins and will raise PRs that update the
SHA and the trailing version comment, so add an .github/dependabot.yml
github-actions ecosystem entry as part of this work. Without it, pinning
converts one problem into another.

Suggested split

1. @main and @release/v1 refs (6a) — 3 references, highest risk, do first.
2. Remaining third-party actions (6b) — 8 references.
3. actions/* sweep — ~99 references, mechanical, best scripted.
4. Dependabot config — must land with or before step 3, or the pins rot.

---

Suggested order

Each step should be independently verifiable — the point of sequencing is that when something breaks, the cause is unambiguous.

0. Node v22 cull — separate issue, lands first, re-baselines the timings.
1. Cache key namespacing (1a) — mechanical, no behaviour change.
2. restore-keys + ~/.npm scoping + @v5 standardisation (1b, 1c, 1d) — biggest wall-clock win, low risk.
3. concurrency: cancel-in-progress (3) — small, immediate runner savings; independent of 1–2, can land at any time.
4. Fix is_fork mis-evaluating on push (4) — a real bug, not a cleanup; main is red by default because of it. Independent, do it early.
5. Split secret-bearing jobs into a main-only workflow (4) — makes the secret boundary a property of the file, and removes the duplicate full-suite run per merge.
6. Job graph restructure (2) — the actual refactor; needs a design pass first.
7. Pin @main / @release/v1 refs + Dependabot config (6a, 6.4) — small, high value, independent of everything else.
8. Full SHA-pinning sweep (6b, 6.3) — mechanical but touches ~110 references; best done in one pass so the tree is never half-pinned.
9. Revisit timeout-minutes: 25 — once 1–2 land, cold builds should sit well under the old 10-minute cap and the raised limit can come back down. Keep some headroom.

Reference: the numbers behind #4

Two jobs from separate runs were killed at the 10-minute mark, on different platforms — ruling out anything Windows-specific:

Run	Job	Duration	Result
32234833306	win32/x64	10m41s	cancelled
32231012936	darwin/arm64	10m16s	cancelled

After raising the cap (run 32237470680, all three green):

Job	Duration	Result
linux/x64	5m02s	success
win32/x64	8m52s	success
darwin/arm64	10m10s	success

darwin finished ten seconds past the old limit. That is the entire margin between "green" and "cancelled" — and it is why the cache work above is the real fix, not the timeout bump.

[ScrewTSW]

Blocked on #5 — the Node v22 cull lands first and re-baselines all timings.