refactor(project): Handle unresolved reads in ResourceRequestManager#1466
Open
RandomByte wants to merge 2 commits into
Open
refactor(project): Handle unresolved reads in ResourceRequestManager#1466RandomByte wants to merge 2 commits into
RandomByte wants to merge 2 commits into
Conversation
RandomByte
force-pushed
the
fix/request-manager-unresolved-reads
branch
2 times, most recently
from
July 22, 2026 13:10
f681180 to
ba5ed1a
Compare
MonitoredReader records every byPath/byGlob call regardless of result, so a task probing an optional file or using a glob that matches nothing still contributes those requests. On #addRequestSet, when findExactMatch misses and findBestParent picks a proper-subset parent, the delta can hold only requests that resolve to zero resources. That threw "Unexpected empty added resources for request set ID ...", reproduced by branch-switching in OpenUI5. These reads are not benign: whether a probed file exists can change task output, so collapsing an empty-delta node onto its parent's signature would serve stale results after the file's existence changes. Replace the throw with a marker scheme: - On an empty-resolving delta, derive an empty tree from the parent (deriveTree([])) and record the added requests on metadata.unresolvedRequests. An all-empty root recording keeps the empty ResourceIndex and marks every request unresolved. Partially-resolving deltas mark only the unresolved subset. - Route getIndexSignatures() and #addRequestSet's returned signature through #computeNodeSignature: the tree hash when unresolvedRequests is empty, otherwise sha256(treeHash \0 sorted keys). - In updateIndices, drop entries from unresolvedRequests as their request resolves to a real resource. Once drained, the signature collapses to the tree hash, matching a same-shape recording with the resource present. - Persist unresolvedRequests on root and delta entries through toCacheObject/fromCache. The set holds request keys (Request.toKey() form), mirroring RequestSetNode's own addedRequests field, so the name tracks the domain concept rather than the string representation. SharedHashTree is untouched: deriveTree([]) already produces a distinct ResourceIndex whose tree hashes to the parent's, so the tree-identity map keys stay distinct across nodes. Rewrite the three repro tests to assert no-throw plus signature distinctness; add convergence, cache round-trip, empty-root discrimination, and BuildTaskCache-shape coverage.
…estManager request.type 'path' vs 'patterns', with the path branch scanning the path array via Array.includes. Extract the check into #requestResolvesAgainstPaths and back the path lookup with a Set; the patterns branch still gets the array form micromatch needs.
RandomByte
force-pushed
the
fix/request-manager-unresolved-reads
branch
from
July 22, 2026 19:31
ba5ed1a to
81feb0c
Compare
RandomByte
marked this pull request as ready for review
July 22, 2026 19:38
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
1 participant
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.
Problem
A request set records the reads a task made (a
byPathprobe for a specific file, or abyGlobpattern) and derives a signature from the resources those reads resolved to. Deriving a delta request set assumed at least one resource always resolves, and threw otherwise:That assumption is false. A
byPathprobe for an optional file that doesn't exist, or every request after a branch switch that deleted the probed files, legitimately resolves to zero resources. Those reads still influence task output, so the signature must reflect them and must stay distinct from the signature of an otherwise-identical index that never made the probe.Fix
Track unresolved requests per node in
metadata.unresolvedRequests(aSetofRequest.toKey()values, mirroringRequestSetNode.addedRequestsso the name tracks the domain concept rather than the string representation):#collectUnresolvedRequestsrecords, after a recording, which requests matched none of the fetched paths.#computeNodeSignatureexposessha256(treeSignature \0 sorted keys)when unresolved requests exist, and falls back to the bare tree signature when the set is empty. A probe-for-absent-file signature differs from a clean one, but collapses back to the plain tree hash once the file appears.#drainUnresolvedRequests, called fromupdateIndicesafter newly-appeared resources are upserted, removes requests that now resolve and deletes the set once empty.unresolvedRequestsis serialized into and restored from the cache for both root and delta indices.The shared "does this request resolve against these paths?" predicate is extracted into
#requestResolvesAgainstPaths(Set lookup forpathrequests, micromatch forpatterns), used by both collect and drain.Tests
New cases in
test/lib/build/cache/ResourceRequestManager.jsandtest/lib/build/cache/BuildTaskCache.jscover the unresolved-request lifecycle: collection, signature distinction from a clean index, drain-on-appearance, and cache round-trip.