Skip to content

fix(core): make the cache stack survive CI checkouts and the action cache#1060

Merged
rayhanadev merged 1 commit into
mainfrom
ray/perf-ci-action-cache
Jul 4, 2026
Merged

fix(core): make the cache stack survive CI checkouts and the action cache#1060
rayhanadev merged 1 commit into
mainfrom
ray/perf-ci-action-cache

Conversation

@rayhanadev

@rayhanadev rayhanadev commented Jul 4, 2026

Copy link
Copy Markdown
Member

Why

An audit of the cache stack under the GitHub Action's cache found the headline feature broken: the whole-repo scan cache never worked in CI — it resolved its own on-disk location (node_modules/.cacheos.tmpdir(), neither of which is the ${runner.temp}/react-doctor-cache path the action persists) AND three of its key inputs (config / dotenv / toolchain fingerprints, all mtime-based) rotated on every fresh checkout. The two dead-code caches were stat-fingerprinted, so re-cloned trees missed permanently, and the supply-chain purl store grew monotonically across restore→save cycles.

CI simulation (fresh clone at a stable runner-style path each run, cache dir persisted, output equivalence vs no-cache controls on every run):

control (no cache):            2441ms
cold fill:                     2821ms (+15%, once per content state)
re-clone, same sha:             221ms  — whole-repo TURBO hit (11x)
re-clone + 1 committed change: 1565ms  — lint 618/619, sidecar 618/618, summaries 618/1
re-clone, scan entry dropped:   989ms  — dead-code mtime-repair HIT (scan 305ms vs 2313ms)

What changed

  • Path unification: the scan-result cache routes through the shared resolveReactDoctorCacheDir (env override → node_modules/.cache → uid-scoped tmpdir), landing inside the action's cached path; the tmpdir fallback gains the same uid/per-project scoping as the other caches.
  • Checkout-proof keys: config + dotenv fingerprints switched stat → content hash; the toolchain fingerprint keys package versions (matching the lint ruleset-hash precedent) with process.version for node, stat retained only for foreign nvm binaries.
  • Ninja-style mtime repair in both dead-code caches (schema v2): entries carry a content hash beside the stats; on stat mismatch the content is hashed — a match accepts the entry and refreshes the stored stats in place, so a fresh checkout pays the repair once. Stores hash-then-re-stat-verify against the pre-analysis snapshot, so a racing edit can never mint a repairable stale entry. deslop's manifest/bundler-config fingerprints feeding collectHash/resolutionHash also move to content hashes.
  • Growth fixes: supply-chain cache entries expire with their 24h TTL; the PR baseline sub-scan (per-run temp dir identity that can never hit) runs with the per-file/sidecar caches off, ending one orphan cache subdir per PR run.
  • Assessed, no change needed: concurrency is only keyed when explicitly pinned (runner-core variance can't thrash keys); content-keyed caches (per-file lint, sidecar probes) were already CI-proof; dead-code stays off on PR diff scans by design.
  • action.yml: comment-only documentation update — the release guard recommends a patch bump (v2.2.5 → v2.2.6) on merge.

Test plan

  • core 1175 / react-doctor 2154+26skip / deslop-js 508 — all green post-rebase onto the merged temperature PR; root typecheck + lint + vp fmt + smoke:json-report clean.
  • New tests: mtime-bumped-identical-content → hit with persisted repair (second lookup takes the stat fast path), same-size content change → miss, add/delete → miss, REACT_DOCTOR_CACHE_DIR honored by the scan cache, fresh-checkout key stability, supply-chain TTL pruning.
  • The CI simulation table above, all runs output-equivalent to controls.

🤖 Generated with Claude Code


Note

Medium Risk
Touches cache invalidation across scan, dead-code, and deslop paths; behavior is designed fail-open with schema bumps and tests, but wrong repair logic could theoretically serve stale diagnostics until a content change.

Overview
Makes the GitHub Action’s persisted REACT_DOCTOR_CACHE_DIR actually warm the full cache stack after fresh CI checkouts, where stat-only fingerprints and wrong on-disk paths previously forced cold scans.

The whole-repo scan-result cache now lives under the shared resolveReactDoctorCacheDir root (same as lint, sidecar, dead-code, supply-chain) instead of node_modules/.cache or an unscoped temp path. Its cache key no longer rotates on re-clone: config and dotenv inputs use content hashes; the toolchain fingerprint uses package versions (and process.version for the current Node) instead of install mtimes.

Dead-code caching (schema v2) stops folding every file’s mtime into the key. Entries store per-file (mtime, size, content hash); on lookup, stat mismatches with unchanged size trigger a one-time content hash and mtime repair (ninja/restat), persisted so the next run is stat-fast. deslop-js incremental summary cache (schema v2) gets the same repair for summaries and package facts, and manifest/bundler-config layers in collect/resolution hashes move to content hashes.

Supply-chain cache prunes files older than the 24h TTL so restored CI caches don’t grow forever with unused purls. PR baseline sub-scans disable per-file and sidecar lint caches to avoid orphan subdirs in the persisted cache dir. action.yml comments document the expanded restored cache layers.

Reviewed by Cursor Bugbot for commit fcc18b8. Bugbot is set up for automated code reviews on this repo. Configure here.

…ache

The whole-repo scan cache escaped the action's cached path entirely (own
tmpdir resolution) and three key inputs (config/dotenv/toolchain stats)
rotated on every fresh checkout; the dead-code caches were stat-keyed so
re-cloned trees never hit. Route every cache through the shared cache-dir
resolution, content-hash the checkout-sensitive key inputs, add ninja-style
mtime repair to both dead-code caches, TTL-prune the supply-chain store,
and stop the PR baseline sub-scan from minting orphan cache dirs.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 4, 2026 05:27

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

📦 GitHub Action release recommended

This PR changes the React Doctor GitHub Action's release surface:

  • action.yml

The composite action is versioned independently from the npm packages, so it
needs its own git tag once this merges. Based on the PR title, this looks like
a patch bump (fix / refactor / chore / docs): v2.2.5v2.2.6.

After merging, cut the tag from the merge commit on main (tags are GPG-signed
annotated tags, so run this locally where your signing key is configured):

git checkout main && git pull --ff-only
merge_commit=$(git rev-parse HEAD)
git tag -a v2.2.6 "$merge_commit" -m "react-doctor action v2.2.6"
git tag -fa v2 "$merge_commit" -m "react-doctor action v2 (floating major -> v2.2.6)"
git push origin v2.2.6
git push --force origin v2   # moves only the floating major pointer

This bump can also be performed automatically on merge — set the repo
variable AUTO_BUMP_ACTION_TAG=true. Recommendation by the Action Version Bump workflow.

@pkg-pr-new

pkg-pr-new Bot commented Jul 4, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/eslint-plugin-react-doctor@1060
npm i https://pkg.pr.new/oxlint-plugin-react-doctor@1060
npm i https://pkg.pr.new/react-doctor@1060

commit: fcc18b8

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

Open in Devin Review

Comment on lines 12 to 21
export const computeConfigFingerprint = (projectDirectory: string, version: string): string => {
const parts: string[] = [`v=${version}`];
let directory = projectDirectory;
for (;;) {
for (const filename of CONFIG_FINGERPRINT_FILENAMES) {
try {
const stat = fs.statSync(path.join(directory, filename));
parts.push(`${directory}/${filename}=${stat.mtimeMs}:${stat.size}`);
} catch {
continue;
}
const contentHash = hashFileContents(path.join(directory, filename));
if (contentHash !== null) parts.push(`${directory}/${filename}=${contentHash}`);
}
const parent = path.dirname(directory);
if (parent === directory) break;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🚩 Config fingerprint now hashes lockfile contents instead of stat-fingerprinting them

The computeConfigFingerprint at packages/core/src/utils/compute-config-fingerprint.ts:12-24 switched from fs.statSync (mtime + size) to hashFileContents (SHA-1 of file bytes) for every file in CONFIG_FINGERPRINT_FILENAMES. This list includes lockfiles (pnpm-lock.yaml, package-lock.json, yarn.lock, bun.lock, bun.lockb) which can be tens of megabytes in large monorepos. The function also walks UP the directory tree to the filesystem root, hashing these files at every ancestor. The comment at lines 6-11 acknowledges this tradeoff ('lockfiles are the largest, and hashing is far cheaper than the scan they key'). In practice the cost is likely single-digit milliseconds for most projects, but for a monorepo with a 50 MB lockfile and deep nesting, the repeated hashing at each ancestor level could add up. Worth confirming this doesn't regress scan startup time on large monorepos.

(Refers to lines 12-24)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@rayhanadev rayhanadev merged commit ced746f into main Jul 4, 2026
27 of 29 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants