fix(core): retry failed location initialization - #46957
Open
kitlangton wants to merge 2 commits into
Open
Conversation
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.
Why
A Location can fail to boot because its config references a missing file, or on Unix because its directory is a regular file or is unreadable. Repairing the filesystem still leaves subsequent loads failing: the cache gives any existing path an infinite idle lifetime, including failed builds. Explicit invalidation recovers the same repaired directory.
What Changes
Cache retention now follows the graph's build result instead of local path existence.
{file:username.txt}config reference created000to755(non-root Unix)Failed build ownership
Eviction runs once at the shared
LayerMapbuild boundary, not once per failed caller. A short-lived build identity prevents an old, explicitly invalidated boot from evicting its replacement. Successful builds retain infinite idle TTL; workspace liveness never consults the host filesystem.Failure eviction starts immediately in the map owner's scope. Removing the cache entry permits retry, while owner-scoped cleanup can wait for the failed lookup to finish. This avoids a self-join when all acquisition scopes close before the boot fails.
Scope
Only Location boot-failure retention and its regression tests. This is complementary to #46074, which fixes cleanup of invalidated borrowed entries in Effect; it neither duplicates that dependency patch nor changes the public API.
Verification
d9c85d8d95, both repaired-path cases failed; explicit-invalidation controls and the existing Location tests passed (14 pass, 2 fail).d9c85d8d95cache implementation, verified withgit diff --exit-code. The missing config-reference fixture produced two failed retry cases and one passing explicit-invalidation control. Restored the production implementation unchanged afterward.LocationWatcherfor deterministic ordering. Covers shared failure throughget,contextEffect, andcontextEffectOption; eight failed borrowers released after repair; late failure after replacement; failed-build finalization; and acquisition scopes closing before failure. The late-failure test rejected naive eviction, and the closed-scope test rejected synchronous self-joining cleanup.Platform coverage
The initial Windows run failed three new tests at their first-load assertion: the file-path fixture boots there instead of failing. On the unchanged base, macOS fails in
Config.discoverwhen accessing<file>/.opencode(ENOTDIR/BadResource); theInstance,Config, andFSUtilimplementations are unchanged by this PR. This was a nonportable failure fixture, not a failed repair or retry.The fixture-only correction retains the file-path regressions on Unix and skips permission cases on Windows/root. It adds a real missing-config-reference failure and repair on every platform, including concurrent failed borrowers. The shared-build, cancellation, replacement-identity, and finalization tests still run on every platform. Production code is unchanged by the correction.
CI green on
8301288a4c: Windows unit tests, Linux unit tests, and typecheck all passed. The remaining reported PR checks also passed.