fix: more workspace change determinism.

Author: knightedcodemonkey

src/modules/app-core/github-workflows.js

@@ -244,13 +244,6 @@ const initializeGitHubWorkflows = ({
     setSelectedRepository: setCurrentSelectedRepository,
     getFileCommits: getWorkspacePrFileCommits,
     getEditorSyncTargets,
-    persistWorkspaceMetadataOnSubmit: async () => {
-      if (typeof flushWorkspaceSave !== 'function') {
-        return
-      }
-
-      await flushWorkspaceSave({ preserveRecordId: true })
-    },
     getTopLevelDeclarations,
     getRenderMode,
     getStyleMode,

src/modules/app-core/pr-context-state-change-handler.js

@@ -9,11 +9,8 @@ const createPrContextStateChangeHandler = ({
   getWorkspaceRepositoryFullName,
   setWorkspaceRepositoryFullName,
   getWorkspacePrContextState,
-  getHasObservedActivePrContextInSession,
   setHasObservedActivePrContextInSession,
   githubPrStatus,
-  githubPrHeadBranch,
-  githubPrTitle,
   workspacePrSessionHandoffController,
   setWorkspacePrNumber,
   persistWorkspacePrContextState,
@@ -38,12 +35,8 @@ const createPrContextStateChangeHandler = ({
       ),
       workspaceRepositoryFullName: getWorkspaceRepositoryFullName(),
       workspacePrContextState: getWorkspacePrContextState(),
-      hasObservedActivePrContextInSession: getHasObservedActivePrContextInSession(),
       statusText:
         typeof githubPrStatus?.textContent === 'string' ? githubPrStatus.textContent : '',
-      headBranchValue:
-        typeof githubPrHeadBranch?.value === 'string' ? githubPrHeadBranch.value : '',
-      prTitleValue: typeof githubPrTitle?.value === 'string' ? githubPrTitle.value : '',
     })
 
     if (transition.kind === 'ignore') {
@@ -72,10 +65,6 @@ const createPrContextStateChangeHandler = ({
     } else if (transition.kind === 'mark-closed') {
       setHasObservedActivePrContextInSession(false)
       persistWorkspacePrContextState('closed')
-    } else if (transition.kind === 'mark-inactive') {
-      setHasObservedActivePrContextInSession(false)
-      setWorkspacePrNumber(null)
-      persistWorkspacePrContextState('inactive')
     }
 
     editedIndicatorVisibilityController.refreshIndicators()

src/modules/app-core/pr-context-transition.js

@@ -1,6 +1,5 @@
 import {
   hasClosedPrVerificationStatus,
-  hasIncompletePrMetadataInputs,
   hasSelectedRepositoryMismatch,
   hasWorkspaceRepositoryMismatch,
 } from './pr-context-state.js'
@@ -11,10 +10,7 @@ const resolvePrContextTransition = ({
   selectedRepositoryFullName,
   workspaceRepositoryFullName,
   workspacePrContextState,
-  hasObservedActivePrContextInSession,
   statusText,
-  headBranchValue,
-  prTitleValue,
 }) => {
   if (hasActiveContextPayload) {
     if (
@@ -49,16 +45,6 @@ const resolvePrContextTransition = ({
     return { kind: 'mark-closed' }
   }
 
-  if (
-    hasObservedActivePrContextInSession &&
-    hasIncompletePrMetadataInputs({
-      headBranchValue,
-      prTitleValue,
-    })
-  ) {
-    return { kind: 'mark-inactive' }
-  }
-
   return { kind: 'noop' }
 }
 

src/modules/app-core/workspace-pr-session-handoff-controller.js

@@ -303,19 +303,7 @@ export const createWorkspacePrSessionHandoffController = ({
           headBranch: archiveSnapshot.head,
         })
 
-        const saved = await workspaceStorage.upsertWorkspace(archiveSnapshot)
-
-        const staleActiveRecordIds = activeRecordsForContext
-          .map(record => toNonEmptyWorkspaceText(record.id))
-          .filter(recordId => recordId && recordId !== toNonEmptyWorkspaceText(saved?.id))
-
-        if (staleActiveRecordIds.length > 0) {
-          await Promise.all(
-            staleActiveRecordIds.map(recordId =>
-              workspaceStorage.removeWorkspace(recordId),
-            ),
-          )
-        }
+        await workspaceStorage.upsertWorkspace(archiveSnapshot)
 
         await refreshLocalContextOptions()
       } catch (error) {

src/modules/app-core/workspace-save-controller.js

@@ -72,6 +72,8 @@ const createWorkspaceSaveController = ({
   const buildSaveSnapshot = ({
     preserveRecordId = false,
     allowDuplicateWorkspaceKey = false,
+    allowIdentityMutation = false,
+    allowWorkspacePrune = false,
   } = {}) => {
     const activeRecordId =
       preserveRecordId && typeof getActiveWorkspaceRecordId === 'function'
@@ -85,6 +87,14 @@ const createWorkspaceSaveController = ({
       snapshot.allowDuplicateWorkspaceKey = true
     }
 
+    if (allowIdentityMutation) {
+      snapshot.allowIdentityMutation = true
+    }
+
+    if (allowWorkspacePrune) {
+      snapshot.allowWorkspacePrune = true
+    }
+
     snapshot.loadTransactionId = getCurrentWorkspaceLoadTransactionId()
     return snapshot
   }
@@ -96,40 +106,41 @@ const createWorkspaceSaveController = ({
       }
 
       const payloadRecordId = toNonEmptyWorkspaceText(payload?.id)
+      let existingRecord = null
       if (payloadRecordId) {
-        const existingRecord = await workspaceStorage.getWorkspaceById(payloadRecordId)
-        if (existingRecord && typeof existingRecord === 'object') {
-          const existingWorkspaceKey = toNonEmptyWorkspaceText(
-            existingRecord.workspaceKey,
-          )
-          const payloadWorkspaceKey = toNonEmptyWorkspaceText(payload?.workspaceKey)
-          if (
-            existingWorkspaceKey &&
-            payloadWorkspaceKey &&
-            existingWorkspaceKey !== payloadWorkspaceKey
-          ) {
-            const existingWorkspaceScope =
-              toNonEmptyWorkspaceText(existingRecord.workspaceScope).toLowerCase() ||
-              'local'
-            const payloadWorkspaceScope =
-              toNonEmptyWorkspaceText(payload?.workspaceScope).toLowerCase() || 'local'
-            const existingRepository = toNonEmptyWorkspaceText(existingRecord.repo)
-            const payloadRepository = toNonEmptyWorkspaceText(payload?.repo)
-
-            const isLocalToRepositoryRekey =
-              existingWorkspaceScope === 'local' &&
-              payloadWorkspaceScope === 'repository' &&
-              !existingRepository &&
-              Boolean(payloadRepository)
-
-            if (!isLocalToRepositoryRekey) {
-              return null
-            }
-          }
-        }
+        existingRecord = await workspaceStorage.getWorkspaceById(payloadRecordId)
       }
 
-      const { loadTransactionId: _loadTransactionId, ...persistablePayload } = payload
+      const {
+        loadTransactionId: _loadTransactionId,
+        allowIdentityMutation: _allowIdentityMutation,
+        allowWorkspacePrune: _allowWorkspacePrune,
+        ...persistablePayload
+      } = payload
+
+      const allowIdentityMutation =
+        payload && typeof payload === 'object'
+          ? payload.allowIdentityMutation === true
+          : false
+      const allowWorkspacePrune =
+        payload && typeof payload === 'object'
+          ? payload.allowWorkspacePrune === true
+          : false
+
+      if (
+        !allowIdentityMutation &&
+        existingRecord &&
+        typeof existingRecord === 'object'
+      ) {
+        persistablePayload.workspaceScope = toNonEmptyWorkspaceText(
+          existingRecord.workspaceScope,
+        )
+        persistablePayload.workspaceKey = toNonEmptyWorkspaceText(
+          existingRecord.workspaceKey,
+        )
+        persistablePayload.repo = toNonEmptyWorkspaceText(existingRecord.repo)
+        persistablePayload.head = toNonEmptyWorkspaceText(existingRecord.head)
+      }
 
       const allowDuplicateWorkspaceKey =
         persistablePayload && typeof persistablePayload === 'object'
@@ -148,6 +159,7 @@ const createWorkspaceSaveController = ({
 
       if (
         normalizedSavedWorkspaceKey &&
+        allowWorkspacePrune &&
         !allowDuplicateWorkspaceKey &&
         !isSavedInactiveWithoutPrNumber
       ) {
@@ -252,25 +264,39 @@ const createWorkspaceSaveController = ({
   const queueWorkspaceSave = ({
     preserveRecordId = false,
     allowDuplicateWorkspaceKey = false,
+    allowIdentityMutation = false,
+    allowWorkspacePrune = false,
   } = {}) => {
     if (!canPersistWorkspaceState()) {
       return
     }
 
-    const snapshot = buildSaveSnapshot({ preserveRecordId, allowDuplicateWorkspaceKey })
+    const snapshot = buildSaveSnapshot({
+      preserveRecordId,
+      allowDuplicateWorkspaceKey,
+      allowIdentityMutation,
+      allowWorkspacePrune,
+    })
     setActiveWorkspaceRecordId(snapshot.id)
     workspaceSaver.queue(snapshot)
   }
 
   const flushWorkspaceSave = async ({
     preserveRecordId = false,
     allowDuplicateWorkspaceKey = false,
+    allowIdentityMutation = false,
+    allowWorkspacePrune = false,
   } = {}) => {
     if (!canPersistWorkspaceState()) {
       return
     }
 
-    const snapshot = buildSaveSnapshot({ preserveRecordId, allowDuplicateWorkspaceKey })
+    const snapshot = buildSaveSnapshot({
+      preserveRecordId,
+      allowDuplicateWorkspaceKey,
+      allowIdentityMutation,
+      allowWorkspacePrune,
+    })
     setActiveWorkspaceRecordId(snapshot.id)
     await workspaceSaver.flushNow(snapshot)
   }
@@ -281,22 +307,34 @@ const createWorkspaceSaveController = ({
       preserveRecordIdOnInput = false,
       preserveRecordIdOnChange = false,
       rekeyOnBlur = true,
+      allowIdentityMutationOnInput = false,
+      allowIdentityMutationOnChange = false,
+      allowIdentityMutationOnBlur = rekeyOnBlur,
     } = {},
   ) => {
     if (!(element instanceof HTMLInputElement || element instanceof HTMLSelectElement)) {
       return
     }
 
     const queue = () => {
-      queueWorkspaceSave({ preserveRecordId: preserveRecordIdOnInput })
+      queueWorkspaceSave({
+        preserveRecordId: preserveRecordIdOnInput,
+        allowIdentityMutation: allowIdentityMutationOnInput,
+      })
     }
 
     const queueFromChange = () => {
-      queueWorkspaceSave({ preserveRecordId: preserveRecordIdOnChange })
+      queueWorkspaceSave({
+        preserveRecordId: preserveRecordIdOnChange,
+        allowIdentityMutation: allowIdentityMutationOnChange,
+      })
     }
 
     const flush = () => {
-      void flushWorkspaceSave({ preserveRecordId: !rekeyOnBlur }).catch(() => {
+      void flushWorkspaceSave({
+        preserveRecordId: !rekeyOnBlur,
+        allowIdentityMutation: allowIdentityMutationOnBlur,
+      }).catch(() => {
         /* Save failures are already surfaced through saver onError. */
       })
     }

src/modules/app-core/workspace-scope-fork-actions.js

@@ -53,7 +53,10 @@ export const createWorkspaceScopeForkActions = ({
     setWorkspaceScopeMarker(
       toWorkspaceScopeMarker(normalizedRepositoryFullName ? 'repository' : 'local'),
     )
-    await flushWorkspaceSave({ preserveRecordId: !rekeyRecord })
+    await flushWorkspaceSave({
+      preserveRecordId: !rekeyRecord,
+      allowIdentityMutation: true,
+    })
   }
 
   const setActiveWorkspaceScopeMarker = async nextScope => {