Repository navigation
Fix cross-session system Python cache misses on remotes - #1951
Stella Huang (StellaHuang95) wants to merge 2 commits into
Conversation
Mirror the global system Python selection into workspace state so the same remote workspace can use its persisted fast path when global state is cold. Version mirrored entries and tombstones to preserve workspace-first behavior across partial storage failures, serialize mutations, and invalidate only definitively missing paths. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c45696e-17ec-4b65-9e06-21ea196234d6
|
🔒 Automated review in progress — Bill Schnurr (@bschnurr) is auto-reviewing this PR. |
There was a problem hiding this comment.
🟡 Changes recommended
A partial workspace tombstone failure can resurrect a cleared interpreter after global state is reset or replaced.
1 open issue
What changed in this PR
This PR improves the system Python startup cache for remote workspaces by mirroring global selections into workspace state, allowing reuse when global state becomes cold.
Changes:
- Adds workspace-first lookup, revisioned entries, tombstones, and serialized cache mutations.
- Invalidates definitively missing global paths and makes persistent-state clears recover after failures.
- Adds cache and failure-recovery tests and updates startup-flow documentation.
| File | Description |
|---|---|
src/test/managers/fastPath.get.unit.test.ts |
Tests system-manager stale-path invalidation. |
src/test/managers/common/fastPath.unit.test.ts |
Tests invalidation and transient-failure behavior. |
src/test/managers/builtin/sysPythonManager.pymanager.unit.test.ts |
Tests persisted global-selection reset. |
src/test/managers/builtin/cache.systemEnvGlobal.unit.test.ts |
Covers mirrored persistence, partial failures, and concurrent updates. |
src/test/common/persistentState.unit.test.ts |
Tests recovery and operation blocking during failed clears. |
src/managers/common/fastPath.ts |
Adds optional stale global-path invalidation. |
src/managers/builtin/sysPythonManager.ts |
Connects invalidation and persists selection resets. |
src/managers/builtin/cache.ts |
Implements mirrored, revisioned global-selection persistence. |
src/common/persistentState.ts |
Waits for all deletions and releases blocked operations after failure. |
docs/startup-flow.md |
Documents the mirrored global fast path. |
Review effort: Balanced
| return globalSelection.path; | ||
| } | ||
| if (hasStoredGlobalSelection(workspace.selection)) { | ||
| return getStoredGlobalSelectionPath(workspace.selection); |
There was a problem hiding this comment.
Issue · Please address or respond
📍 src/managers/builtin/cache.ts:190
After caching A, a reset whose workspace payload write fails leaves A behind a newer revision marker; reopening with cold global state then returns A here, resurrecting the cleared selection (runtime-verified by the Skeptic). Treat a known revision mismatch without matching recovery data as a cache miss, and cover reset followed by cold-global reopening.
[unverified]
| await Promise.all([ | ||
| runCacheOperation('update', 'workspaceState', async () => { | ||
| const state = workspaceState ?? (await getWorkspacePersistentState()); | ||
| await state.set(SYSTEM_GLOBAL_KEY, selection); |
There was a problem hiding this comment.
Issue · Please address or respond
📍 src/managers/builtin/cache.ts:234
If workspace storage rejects both the marker and payload writes while global state successfully saves B, the old workspace A and marker still agree, so reads keep returning A even across reloads (runtime-verified by both the Advocate and Skeptic). Make successful global writes reconcilable after failed mirror preparation, including across reloads, and cover a whole-layer workspace write failure.
[unverified]
|
Result: 🔴 Verification detailsVerification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified. Details: Verification could not complete: timeout must be None or at least 2073600 seconds (24 days); shorter completion deadlines are unsupported. Test runs: none recorded. |
Treat a revision marker without a matching completed entry as a cache miss so an older workspace value cannot reappear after a partial clear and cold global state. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0c45696e-17ec-4b65-9e06-21ea196234d6
| await workspaceState.set(SYSTEM_GLOBAL_REVISION_KEY, revision); | ||
| } catch (error) { | ||
| traceError('Failed to prepare workspaceState system Python cache update:', error); | ||
| } |
|
Result: 🔴 Verification detailsVerification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified. Details: Verification could not complete: timeout must be None or at least 2073600 seconds (24 days); shorter completion deadlines are unsupported. Test runs: none recorded. |

Summary
workspaceStateso the same remote workspace survives a coldglobalStateValidation
npm run lintnpm run compile-testsnpm run unittest— 3,417 passing, 9 pendingnpm run compilenpm run smoke-test— 31 passing, 1 pendingnpm run e2e-test— 4 passingnpm run integration-test-multiroot— 3 passing, 1 pendingThe full integration run reached 60 passing and 10 pending; its sole failure was the existing environment-creation fixture timing out because project A selects the system manager and enters the interactive Python installer. The cache-relevant integration suites passed separately.
Fixes #1581