[rush-lib] Write build cache entries for operations whose dependencies were retained by a warm graph - #6064
Merged
Sean Larkin (TheLarkInn) merged 3 commits intoSep 24, 2026
Conversation
…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>
Contributor
There was a problem hiding this comment.
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.
Mo Jazayeri (mojaza)
approved these changes
Sep 24, 2026
Sean Larkin (TheLarkInn)
deleted the
thelarkinn-fix-warm-build-cache-writes
branch
September 24, 2026 18:27
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
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.configureOperationsdisables an operation when its previous result succeeded at the same state-hash components.OperationExecutionRecord.executeAsyncthen gives that disabled record the statusSkipped(OperationExecutionRecord.ts:563-568).CacheableOperationPlugin'safterExecuteOperationAsynctap treated everySkippedresult the same way: "Skipping means cannot guarantee integrity". It setisCacheWriteAllowed = falseon 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
CacheableOperationPluginnow keeps a per-graph map namedtrustedStateHashByOperation. It records the state hash at which each operation last completed with a status inSUCCESS_STATUSES(Success, FromCache or NoOp) in an iteration where cache writes were allowed for it, meaning no upstream dependency had an unknown state.A
Skippeddependency now blocks consumer cache writes unless all of these are true:operation.enabled !== false, so the user did not deselect it;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 theonGraphCreatedAsyncclosure, 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 includesname=stateHashfor 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:
--only,--to-except,--impacted-by):operation.enabled === false.ignore-dependency-changesoperations that were kept although their dependency hashes changed: their current hash no longer matches the trusted hash.--only bruns first and a plain build follows.bis 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 realOperationGraphwithPhasedOperationPluginandCacheableOperationPlugin, and a mockedOperationBuildCachethat records cache writes. It covers:enabled = false, i.e.--only) in a warm iteration still blocks the write;--onlystill blocks the write;ignore-dependency-changesoperation 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-libpasses.cw2.sh: a synthetic 12-project layered workspace (mkws-synth --projects 12), one bash, pinned env (env _=fixed SHLVL=1). The steps are:rush-client buildruns;packages/p12/src/index.js;rush-client build(only p12 executes);packages/p12/.rush/temp/*.tar.log;rm -rf packages/p12/lib && RUSH_DAEMON=0 node <fix>/apps/rush/bin/rush build.rush-client)"p12 (build)" completed successfully in 0.41 seconds.(cache miss)rush-client)"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
mainand 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