refactor: simplify PR close flow with deterministic workspace state updates (#106)

Author: knightedcodemonkey

.github/instructions/pr-review.md

@@ -24,6 +24,7 @@ You are reviewing changes for @knighted/develop. Be concise, technical, and spec
 ## What to verify
 
 - No generated artifacts are edited (dist/, coverage/, test-results/).
+- Duplicated logic paths are avoided when a shared helper/module already exists; prefer reusing the established implementation.
 - CDN import/fallback behavior is not bypassed with ad hoc URLs in feature modules.
 - Sensitive values (PAT/token) are not logged or exposed in UI/status output.
 - New UI behavior is covered in Playwright where appropriate.

docs/idb-workspace-state.md

@@ -46,6 +46,12 @@ If a value is required to accurately restore PR/workspace behavior after reload,
 
 `localStorage` should only mirror user preferences and lightweight bootstrap values.
 
+PR metadata boundary:
+
+- PR drawer form edits are draft-only input.
+- Persisted PR metadata in workspace records is updated on successful workflow outcomes
+  (Open PR, Push Commit, Close/verified closed updates), not on each form edit.
+
 ## Post-Push Baseline Invariant
 
 After a successful Push Commit action for an active PR workspace:

docs/pr-context-storage-matrix.md

@@ -48,6 +48,9 @@ When the app loads, workspace restore scope depends on whether a repository is s
 - If a repository is selected: use repository-scoped records only (`repo` match).
 - If no repository is selected: evaluate all stored workspace records.
 
+If a repository is selected and no repository-scoped records are found, restore falls back
+to evaluating all stored workspace records.
+
 Selection order:
 
 1. Load candidate records using the scope above.
@@ -59,14 +62,33 @@ Selection order:
 3. If preferred-by-id or preferred-by-key exists and is `active`, select it.
 4. Otherwise select the first `active` record in candidates.
 5. Otherwise select preferred-by-id or preferred-by-key if present.
-6. Otherwise fall back to the first record returned by IDB ordering.
+6. Otherwise fall back to the first candidate by recency (`lastModified` descending).
 
 Notes:
 
 - No `active workspace` pointer is stored in `localStorage`.
 - Restore behavior is intentionally derived from IDB workspace records + in-memory runtime state.
 - This avoids cross-storage drift between `localStorage` and IndexedDB.
 
+## PR Drawer Persistence Boundary
+
+PR drawer field edits are treated as draft input.
+
+- Editing PR base/head/title in the drawer does not persist workspace record metadata by itself.
+- Workspace PR metadata is committed by explicit successful workflow outcomes (for example:
+  successful Open PR, successful Push Commit, or successful Close PR/verified closed updates).
+
+This keeps workspace records aligned to verified workflow outcomes instead of intermediate
+form state.
+
+## Active Record Model
+
+Multiple `active` PR workspaces may exist for the same repository.
+
+- The model does not enforce a single-active-record-per-repository invariant.
+- Restore selection still follows the algorithm above and picks one workspace to load into
+  the current runtime.
+
 ## Why PR Context Lives In IDB Only
 
 PR workflow state is part of workspace state.
@@ -101,14 +123,10 @@ indexedDB.open('knighted-develop-workspaces').onsuccess = event => {
 }
 ```
 
-## End-Of-Session Behavior
-
-`Close` is the end-of-session action for PR-linked workspaces.
-
-When close is confirmed:
+## Close Behavior
 
-1. The current workspace is archived as historical (`closed`).
-2. The app immediately switches to a fresh local workspace (`inactive`) with a single empty entry tab.
-3. Status messaging guides the user to continue locally or reopen a stored workspace from Workspaces.
+`Close` archives PR-linked context in IDB and clears active PR context in runtime.
 
-In the Workspaces drawer, inactive local-only workspace options are prefixed with `local:`.
+- Matching workspace records for that PR context are persisted as `closed`.
+- Close does not force an automatic fresh-local-workspace handoff.
+- Workspace switching remains an explicit Workspaces action.

playwright/github-pr-drawer/active-context-switch.spec.ts

@@ -37,6 +37,12 @@ test('Switching active workspace to closed preserves switched-from record integr
     page,
     targetState: 'closed',
   })
+
+  await expect(page.getByRole('button', { name: 'Open pull request' })).toBeVisible()
+  await expect(
+    page.getByRole('button', { name: 'Close active pull request context' }),
+  ).toBeHidden()
+
   await expect(page.getByRole('status', { name: 'App status' })).toContainText('Rendered')
 })
 
@@ -925,8 +931,7 @@ test('Active PR context updates controls and can be closed from AI controls', as
   ).toBeVisible()
   await expect(
     page.getByRole('list', { name: 'Workspace editor tabs' }).getByRole('listitem'),
-  ).toHaveCount(1)
-  await expect(page.locator('#preview-host iframe')).toHaveCount(0)
+  ).toHaveCount(2)
 
   await expect
     .poll(async () => {
@@ -941,12 +946,15 @@ test('Active PR context updates controls and can be closed from AI controls', as
       return {
         prContextState: closedRecord?.prContextState,
         prNumber: closedRecord?.prNumber,
+        prTitle: closedRecord?.prTitle,
       }
     })
     .toEqual({
       prContextState: 'closed',
       prNumber: 2,
+      prTitle: 'Existing PR context from storage',
     })
+
   expect(closePullRequestRequestCount).toBe(1)
 })
 

playwright/github-pr-drawer/github-pr-drawer.helpers.ts

@@ -926,10 +926,7 @@ export const runActiveWorkspaceSwitchIntegrityScenario = async ({
         JSON.stringify(snapshot.active) === JSON.stringify(originalSnapshot.active)
 
       const target = snapshot.target
-      const targetStateMatches =
-        targetState === 'closed'
-          ? target?.prContextState === 'closed' || target?.prContextState === 'inactive'
-          : target?.prContextState === expectedTargetPrContextState
+      const targetStateMatches = target?.prContextState === expectedTargetPrContextState
 
       const targetMatches =
         target?.repo === originalSnapshot.target.repo &&

src/app.js

@@ -812,15 +812,15 @@ const {
       return
     }
 
+    prDrawerController.clearSelectedRepositoryActivePrContext({ resetForm: false })
+
     const nextWorkspaceRepositoryFullName =
       typeof workspace.repo === 'string' ? workspace.repo.trim() : ''
     if (nextWorkspaceRepositoryFullName) {
       workspaceRepositoryFullName = nextWorkspaceRepositoryFullName
       byotControls.setSelectedRepository(nextWorkspaceRepositoryFullName)
     }
 
-    prDrawerController.clearSelectedRepositoryActivePrContext({ resetForm: false })
-
     const state =
       typeof workspace.prContextState === 'string'
         ? workspace.prContextState.trim().toLowerCase()
@@ -955,6 +955,7 @@ const workspacePrSessionHandoffController = createWorkspacePrSessionHandoffContr
   defaults: {
     defaultComponentTabName,
     defaultComponentTabPath,
+    defaultComponentTabContent: defaultJsx,
   },
   state: {
     getWorkspacePrNumber: () => workspacePrNumber,
@@ -998,17 +999,6 @@ const workspacePrSessionHandoffController = createWorkspacePrSessionHandoffContr
   },
 })
 
-const archivePrSessionAndStartFreshLocal = ({ result, archivedState, statusMessage }) => {
-  hasObservedActivePrContextInSession = false
-  setWorkspacePrNumber(result?.pullRequestNumber)
-  byotControls.clearSelectedRepositoryPreference()
-  workspaceRepositoryFullName = ''
-  workspacePrSessionHandoffController.archivePrWorkspaceAndStartFreshLocal({
-    archivedState,
-    statusMessage,
-  })
-}
-
 const onPrContextStateChange = createPrContextStateChangeHandler({
   toNonEmptyWorkspaceText,
   toPullRequestNumber,
@@ -1120,16 +1110,25 @@ const githubWorkflows = createGitHubWorkflowsSetup({
       if (nextPrNumber !== null) {
         setWorkspacePrNumber(nextPrNumber)
       }
-      persistWorkspacePrContextState('closed')
+      setWorkspacePrContextState('closed')
 
       const persistClosedRecords = async () => {
+        const activeWorkspaceId = toNonEmptyWorkspaceText(activeWorkspaceRecordId)
+        const activeWorkspaceRecord = activeWorkspaceId
+          ? await workspaceStorage.getWorkspaceById(activeWorkspaceId)
+          : null
+        const preservedPrTitle =
+          toNonEmptyWorkspaceText(activeWorkspaceRecord?.prTitle) ||
+          toNonEmptyWorkspaceText(githubPrTitle?.value)
+
         await persistClosedPrContextRecords({
           workspaceStorage,
           selectedRepository: toNonEmptyWorkspaceText(
             getCurrentSelectedRepositoryFullName(),
           ),
           nextPrNumber,
           normalizedHead: toNonEmptyWorkspaceText(githubPrHeadBranch?.value),
+          fallbackPrTitle: preservedPrTitle,
           toNonEmptyWorkspaceText,
           refreshLocalContextOptions,
         })
@@ -1140,11 +1139,40 @@ const githubWorkflows = createGitHubWorkflowsSetup({
       })
     },
     onPrContextClosed: result => {
-      archivePrSessionAndStartFreshLocal({
-        result,
-        archivedState: 'closed',
-        statusMessage:
-          'PR context closed. Open Workspaces to load a saved workspace or continue with this local workspace.',
+      hasObservedActivePrContextInSession = false
+      const nextPrNumber =
+        toPullRequestNumber(result?.pullRequestNumber) ??
+        parsePullRequestNumberFromUrl(result?.pullRequestUrl)
+      if (nextPrNumber !== null) {
+        setWorkspacePrNumber(nextPrNumber)
+      }
+      setWorkspacePrContextState('closed')
+
+      const persistClosedRecords = async () => {
+        const activeWorkspaceId = toNonEmptyWorkspaceText(activeWorkspaceRecordId)
+        const activeWorkspaceRecord = activeWorkspaceId
+          ? await workspaceStorage.getWorkspaceById(activeWorkspaceId)
+          : null
+        const preservedPrTitle =
+          toNonEmptyWorkspaceText(result?.prTitle) ||
+          toNonEmptyWorkspaceText(activeWorkspaceRecord?.prTitle) ||
+          toNonEmptyWorkspaceText(githubPrTitle?.value)
+
+        await persistClosedPrContextRecords({
+          workspaceStorage,
+          selectedRepository: toNonEmptyWorkspaceText(
+            getCurrentSelectedRepositoryFullName(),
+          ),
+          nextPrNumber,
+          normalizedHead: toNonEmptyWorkspaceText(githubPrHeadBranch?.value),
+          fallbackPrTitle: preservedPrTitle,
+          toNonEmptyWorkspaceText,
+          refreshLocalContextOptions,
+        })
+      }
+
+      void persistClosedRecords().catch(() => {
+        /* Save failures are already surfaced through saver onError. */
       })
     },
     getPersistedActivePrContext,

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

@@ -1,3 +1,5 @@
+import { toWorkspaceRecordKey } from '../workspace/workspace-tab-helpers.js'
+
 const initializeGitHubWorkflows = ({
   createGitHubPrEditorSyncController,
   createGitHubChatDrawer,
@@ -228,13 +230,6 @@ const initializeGitHubWorkflows = ({
     setSelectedRepository: setCurrentSelectedRepository,
     getFileCommits: getWorkspacePrFileCommits,
     getEditorSyncTargets,
-    persistWorkspaceMetadataOnSubmit: async () => {
-      if (typeof flushWorkspaceSave !== 'function') {
-        return
-      }
-
-      await flushWorkspaceSave({ preserveRecordId: true })
-    },
     getTopLevelDeclarations,
     getRenderMode,
     getStyleMode,
@@ -249,6 +244,7 @@ const initializeGitHubWorkflows = ({
       fileUpdates,
       repositoryFullName,
       pullRequestNumber,
+      branch,
     }) => {
       if (typeof onPrContextStateChange === 'function') {
         onPrContextStateChange(githubAiContextState.activePrContext)
@@ -285,6 +281,28 @@ const initializeGitHubWorkflows = ({
           activeWorkspaceRecordId,
         )
         if (activeWorkspaceRecord && typeof activeWorkspaceRecord === 'object') {
+          const nextHeadBranch =
+            typeof githubAiContextState.activePrContext?.headBranch === 'string' &&
+            githubAiContextState.activePrContext.headBranch.trim()
+              ? githubAiContextState.activePrContext.headBranch.trim()
+              : typeof branch === 'string' && branch.trim()
+                ? branch.trim()
+                : typeof activeWorkspaceRecord.head === 'string'
+                  ? activeWorkspaceRecord.head
+                  : ''
+          const nextBaseBranch =
+            typeof githubAiContextState.activePrContext?.baseBranch === 'string' &&
+            githubAiContextState.activePrContext.baseBranch.trim()
+              ? githubAiContextState.activePrContext.baseBranch.trim()
+              : typeof activeWorkspaceRecord.base === 'string'
+                ? activeWorkspaceRecord.base
+                : ''
+          const nextRepositoryFullName =
+            toSafeRepositoryFullName(repositoryFullName) ||
+            toSafeRepositoryFullName(
+              githubAiContextState.activePrContext?.repositoryFullName,
+            ) ||
+            toSafeRepositoryFullName(activeWorkspaceRecord.repo)
           const nextPrTitle =
             typeof githubAiContextState.activePrContext?.prTitle === 'string' &&
             githubAiContextState.activePrContext.prTitle.trim()
@@ -303,6 +321,14 @@ const initializeGitHubWorkflows = ({
 
           const savedWorkspaceRecord = await workspaceStorage.upsertWorkspace({
             ...activeWorkspaceRecord,
+            workspaceScope: nextRepositoryFullName ? 'repository' : 'local',
+            workspaceKey: toWorkspaceRecordKey({
+              repositoryFullName: nextRepositoryFullName,
+              headBranch: nextHeadBranch,
+            }),
+            repo: nextRepositoryFullName,
+            base: nextBaseBranch,
+            head: nextHeadBranch,
             prContextState: 'active',
             prNumber: nextPrNumber,
             prTitle: nextPrTitle,
@@ -544,8 +570,10 @@ const initializeGitHubWorkflows = ({
     onPrContextStateChange(prDrawerController.getActivePrContext())
   }
 
+  let isClosingActivePullRequest = false
+
   githubPrContextClose?.addEventListener('click', () => {
-    if (!githubAiContextState.activePrContext) {
+    if (!githubAiContextState.activePrContext || isClosingActivePullRequest) {
       return
     }
 
@@ -559,6 +587,15 @@ const initializeGitHubWorkflows = ({
       copy: `${referenceLine}PR title: ${githubAiContextState.activePrContext.prTitle}\nHead branch: ${githubAiContextState.activePrContext.headBranch}\n\nThis will close the pull request on GitHub and clear the active pull request context for the selected repository.`,
       confirmButtonText: 'Close PR on GitHub',
       onConfirm: () => {
+        if (isClosingActivePullRequest) {
+          return
+        }
+
+        isClosingActivePullRequest = true
+        if (githubPrContextClose instanceof HTMLButtonElement) {
+          githubPrContextClose.disabled = true
+        }
+
         void prDrawerController
           .closeActivePullRequestOnGitHub()
           .then(result => {
@@ -586,6 +623,12 @@ const initializeGitHubWorkflows = ({
             setStatus(`Close context failed: ${message}`, 'error')
             showAppToast(`Close context failed: ${message}`)
           })
+          .finally(() => {
+            isClosingActivePullRequest = false
+            if (githubPrContextClose instanceof HTMLButtonElement) {
+              githubPrContextClose.disabled = false
+            }
+          })
       },
     })
   })