Skip to content

[rush-lib] Write build cache entries for operations whose dependencies were retained by a warm graph - #6064

Merged
Sean Larkin (TheLarkInn) merged 3 commits into
mainfrom
thelarkinn-fix-warm-build-cache-writes
Sep 24, 2026
Merged

Sean Larkin (TheLarkInn) merged 3 commits into
mainfrom
thelarkinn-fix-warm-build-cache-writes

Conversation

@TheLarkInn

@TheLarkInn Sean Larkin (TheLarkInn) commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Summary

In a warm Rush daemon iteration, an operation that is dirty while its dependencies are unchanged runs, but it writes no build cache entry. Native builds, CI, other agents, and the daemon itself after a restart or branch switch then run the operation again when they could restore it. This PR lets those operations write cache entries. Dependencies that the user skipped (--only, --to-except, --impacted-by) still block cache writes.

Root cause

  • PhasedOperationPlugin.configureOperations disables an operation when its previous result succeeded at the same state-hash components. OperationExecutionRecord.executeAsync then gives that disabled record the status Skipped (OperationExecutionRecord.ts:563-568).
  • CacheableOperationPlugin's afterExecuteOperationAsync tap treated every Skipped result the same way: "Skipping means cannot guarantee integrity". It set isCacheWriteAllowed = false on every consumer, and the flag was passed down transitively. That rule is correct for a user-requested skip, where the dependency's on-disk state is unknown. It is wrong for a result the graph kept because it is still up to date.

Fix

CacheableOperationPlugin now keeps a per-graph map named trustedStateHashByOperation. It records the state hash at which each operation last completed with a status in SUCCESS_STATUSES (Success, FromCache or NoOp) in an iteration where cache writes were allowed for it, meaning no upstream dependency had an unknown state.

A Skipped dependency now blocks consumer cache writes unless all of these are true:

  1. cache writes were still allowed for the dependency in this iteration (no upstream user skip passed a block down to it);
  2. operation.enabled !== false, so the user did not deselect it;
  3. trustedStateHashByOperation.get(op) === record.getStateHash(), so the graph produced the retained result at exactly the current state hash while every one of its own dependencies was trusted.

In every other case the dependency blocks consumer writes as before, and its trust entry is dropped. Any non-success status also drops the entry. beforeDeleteResults (warm-set eviction) removes the entries for the evicted operations. The map lives in the onGraphCreatedAsync closure, so every graph has its own map and graphs never share entries.

Why this cannot poison the cache

The consumer's cache key comes from record.getStateHash(). That hash includes name=stateHash for every dependency (OperationExecutionRecord.getStateHashComponents). Each dependency's state hash in turn covers its own local inputs, its config hash and, recursively, all of its dependencies' state hashes. A retained dependency counts as trusted only if its current state hash is identical to the hash at which a trusted result was produced. So the consumer's key covers the exact dependency state that its inputs were built from. This is the same guarantee a cold build gets after it restores the dependency from the cache at that key.

These cases are still blocked:

  • User skips (--only, --to-except, --impacted-by): operation.enabled === false.
  • ignore-dependency-changes operations that were kept although their dependency hashes changed: their current hash no longer matches the trusted hash.
  • Results produced while a dependency was unknown: for example, --only b runs first and a plain build follows. b is retained, but it was never trusted, so its consumers still don't write.

Tests

New file libraries/rush-lib/src/logic/operations/test/CacheableOperationPlugin.test.ts. It uses a real OperationGraph with PhasedOperationPlugin and CacheableOperationPlugin, and a mocked OperationBuildCache that records cache writes. It covers:

  • a cold iteration writes every entry;
  • iteration 2 runs only the leaf and keeps its dependencies; the leaf writes a cache entry;
  • several warm iterations in a row keep writing entries;
  • a user skip (enabled = false, i.e. --only) in a warm iteration still blocks the write;
  • a cold --only still blocks the write;
  • a result that was kept but built while its dependency was user-skipped is not trusted;
  • an ignore-dependency-changes operation whose state hash changed is not trusted.

With the fix, all 7 tests pass. On the unfixed plugin, 2 tests fail: the retained-dependency leaf write and the multi-iteration write. The other 5 guard tests pass on both versions, so the existing blocking behavior is unchanged.

Linux validation (WSL Ubuntu-24.04)

  • rush build --to @microsoft/rush-lib (includes lint) passes with no warnings. rush test --only @microsoft/rush-lib passes.
  • Repro based on A06's cw2.sh: a synthetic 12-project layered workspace (mkws-synth --projects 12), one bash, pinned env (env _=fixed SHLVL=1). The steps are:
    1. two warm rush-client build runs;
    2. append unique content to packages/p12/src/index.js;
    3. rush-client build (only p12 executes);
    4. count packages/p12/.rush/temp/*.tar.log;
    5. rm -rf packages/p12/lib && RUSH_DAEMON=0 node <fix>/apps/rush/bin/rush build.
warm edit build p12 cache writes (tar.log) native build afterwards
before (unfixed rush-client) only p12 executed 0 -> 0 "p12 (build)" completed successfully in 0.41 seconds. (cache miss)
after (fixed rush-client) only p12 executed 1 -> 2 "p12 (build)" was restored from the build cache.

A second analysis agent re-ran the tests and repros independently on the current head, with the same results.

Merge ordering

Please merge this after, or together with, the fix for #6073. That fix adds a write guard that skips the cache write if an op's inputs changed while it was running. The cache-poisoning race in #6073 already exists on main and still reproduces with this PR. This PR adds many more cache writes during warm daemon builds, so without the guard more builds could hit that race. A separate fix session is working on #6073.

Optional follow-up (not in this PR): a rushd-wire-e2e test covering "warm edit, then native build restores from the cache".

This came out of the automated rushd Linux performance/behavior analysis (board BUG #41; confirmed in #53, #58 and #105).

Fixes #6056

…s were retained by a warm graph

Fixes #6056

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>

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

Copilot review overview

🔵 Needs a closer look

Cache integrity depends on coordinated merge ordering with the unresolved #6073 write guard.

Review effort: Balanced
Findings: None

What changed in this PR

Enables warm Rush operation graphs to write valid build-cache entries when dependencies retain trusted results.

Changes:

  • Tracks trusted successful state hashes per graph.
  • Preserves blocking for user-skipped or stale dependencies.
  • Adds seven warm-graph cache behavior tests.
File Description
CacheableOperationPlugin.ts Tracks and validates retained dependency state.
CacheableOperationPlugin.test.ts Tests cache writes and trust safeguards.
fix-warm-build-cache-writes_2026-09-24.json Adds the Rush patch changelog.

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

@TheLarkInn
Sean Larkin (TheLarkInn) merged commit 1f51ac8 into main Sep 24, 2026
11 checks passed
@TheLarkInn
Sean Larkin (TheLarkInn) deleted the thelarkinn-fix-warm-build-cache-writes branch September 24, 2026 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Closed

Development

Successfully merging this pull request may close these issues.

[rush] rush-lib: warm daemon incremental builds don't write build-cache entries when upstream operation results were retained

3 participants