Repository navigation
data/process: degrade gracefully when a workspace lacks a stored ID - #670
Closed
andyrewlee wants to merge 1 commit into
Closed
andyrewlee wants to merge 1 commit into
andyrewlee wants to merge 1 commit into
Conversation
A transient store error during LoadMetadataFor/Save — most measurably the darwin openat-under-os.Root ENOENT race in lockRegistryFile — left the snapshot's primary checkout with no storeID, and every subsequent spawn failed behind an unactionable "cannot reserve a port range" dialog for the rest of the session (seen in the wild as a flaky e2e failure under host load). Three layers, ordered by proximity to the cause: - registry_lock_open.go: the spurious-ENOENT retry previously scoped to PortReservationStore.withLock moves into openRegistryLockRootRetry so every store caller (workspace metadata, project registry, port reservations) rides out the same kernel race; a genuinely deleted state home still fails closed after the bounded budget. Both unix and windows lockRegistryFile paths share it, and a test seam proves convergence and fail-closed exhaustion. - workspace_store_discovery.go: mergeDiscoveryLocked now reflects the resolved store key onto the caller's discovered object on every successful path — previously the in-place merge and vanished-record branches saved under storedID but left discovered.storeID empty. - process/ports.go: ReserveWorkspace with no StoredID degrades to the root-keyed in-memory allocator (the pre-registry contract) with a Warn log instead of returning ErrWorkspaceMetadataNotPersisted — matching the adoption guard's degrade-not-block posture; LookupWorkspaceInterval reports the transient interval truthfully. The sentinel is removed. Gates: internal/data + internal/process green incl. -race; full e2e suite passes (the storeID-wedged dialog no longer appears); devcheck unit sweep green; strict lint 0 issues.
Owner
Author
|
Superseded — content landed on main via merge 8580442 (plans/ scaffolding intentionally excluded). |
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
Fixes a real regression in this stack (introduced by 051's fail-closed durable reservations, made visible by 055's added startup concurrency): a transient store error during workspace load — most measurably the darwin openat-under-os.Root ENOENT race in
lockRegistryFile— left the snapshot's workspace with nostoreID, and every subsequent spawn failed behind an unactionable "cannot reserve a port range" dialog for the rest of the session.Three layers, ordered by proximity to the cause:
registry_lock_open.go— the spurious-ENOENT retry previously scoped toPortReservationStore.withLockmoves intoopenRegistryLockRootRetry, so every store caller (workspace metadata, project registry, port reservations) rides out the same kernel race. A genuinely deleted state home still fails closed after the bounded budget; unix + windowslockRegistryFileshare it; a test seam proves convergence and fail-closed exhaustion.workspace_store_discovery.go—mergeDiscoveryLockednow reflects the resolved store key onto the caller'sdiscoveredobject on every successful path (in-place merge, vanished-record, and rename branches). Previously the record was saved but the caller's object keptstoreID="".process/ports.go—ReserveWorkspacewith noStoredIDdegrades to the root-keyed in-memory allocator (the exact pre-registry contract) with a Warn log instead of returningErrWorkspaceMetadataNotPersisted. This matches the adoption guard's degrade-not-block posture;LookupWorkspaceIntervalreports the transient interval truthfully in theistatus. The sentinel error is removed.Test plan
go test ./internal/data ./internal/process -count=1and-race— greenTestLockRegistryFileRetriesTransientENOENT,TestLockRegistryFileENOENTBudgetExhausted,TestWorkspaceStore_UpsertFromDiscovery_ReflectsStoreID,TestPortAllocator_DurableUnsavedWorkspaceDegrades, env/session-env degrade casesTestMouseWheelScrollsNormalScreenChatRedrawHistory(the e2e flake that exposed this) — passes; the storeID-wedged dialog no longer appearsmake lint-strict-new— 0 issuesStacked on #669.