From e740c9d1c5cf1bb1373016eba393b7300c30cda3 Mon Sep 17 00:00:00 2001 From: KCM Date: Fri, 1 May 2026 12:00:16 -0500 Subject: [PATCH 1/5] refactor: close pr flow. --- .../active-context-switch.spec.ts | 32 ++++++++- .../github-pr-drawer.helpers.ts | 5 +- src/app.js | 66 +++++++++++++------ src/modules/app-core/github-workflows.js | 65 +++++++++++++++++- src/modules/app-core/pr-context-records.js | 21 ++++-- ...workspace-pr-session-handoff-controller.js | 7 +- .../pr/drawer/controller/public-actions.js | 12 +++- 7 files changed, 170 insertions(+), 38 deletions(-) diff --git a/playwright/github-pr-drawer/active-context-switch.spec.ts b/playwright/github-pr-drawer/active-context-switch.spec.ts index 333ff38..f8896d7 100644 --- a/playwright/github-pr-drawer/active-context-switch.spec.ts +++ b/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') }) @@ -274,6 +280,26 @@ test('Switching active workspaces with different module sync paths keeps remote await expect .poll(async () => { const records = await getAllWorkspaceRecords(page) + + await expect + .poll(async () => { + const records = await getAllWorkspaceRecords(page) + const closedRecord = records.find( + record => + record?.repo === 'knightedcodemonkey/develop' && + record?.prContextState === 'closed' && + record?.prNumber === 2, + ) + + return { + prContextState: closedRecord?.prContextState, + prNumber: closedRecord?.prNumber, + } + }) + .toEqual({ + prContextState: 'closed', + prNumber: 2, + }) const alphaRecord = records.find(record => { const recordId = typeof record?.id === 'string' ? record.id.trim() : '' const recordHead = typeof record?.head === 'string' ? record.head.trim() : '' @@ -925,8 +951,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 +966,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) }) diff --git a/playwright/github-pr-drawer/github-pr-drawer.helpers.ts b/playwright/github-pr-drawer/github-pr-drawer.helpers.ts index 721a4fa..5c3d1e3 100644 --- a/playwright/github-pr-drawer/github-pr-drawer.helpers.ts +++ b/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 && diff --git a/src/app.js b/src/app.js index f5a69f8..4361cd9 100644 --- a/src/app.js +++ b/src/app.js @@ -812,6 +812,8 @@ const { return } + prDrawerController.clearSelectedRepositoryActivePrContext({ resetForm: false }) + const nextWorkspaceRepositoryFullName = typeof workspace.repo === 'string' ? workspace.repo.trim() : '' if (nextWorkspaceRepositoryFullName) { @@ -819,8 +821,6 @@ const { 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,9 +1110,17 @@ 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( @@ -1130,6 +1128,7 @@ const githubWorkflows = createGitHubWorkflowsSetup({ ), 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, diff --git a/src/modules/app-core/github-workflows.js b/src/modules/app-core/github-workflows.js index b9ad301..c5fd2f4 100644 --- a/src/modules/app-core/github-workflows.js +++ b/src/modules/app-core/github-workflows.js @@ -120,6 +120,22 @@ const initializeGitHubWorkflows = ({ const toSafeRepositoryFullName = value => typeof value === 'string' ? value.trim() : '' + const toWorkspaceIdentitySegment = value => { + const normalized = typeof value === 'string' ? value.trim().toLowerCase() : '' + + if (!normalized) { + return '' + } + + return normalized.replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '') + } + + const toWorkspaceRecordKey = ({ repositoryFullName, headBranch } = {}) => { + const repoSegment = toWorkspaceIdentitySegment(repositoryFullName) || 'local' + const headSegment = toWorkspaceIdentitySegment(headBranch) || 'draft' + return `${repoSegment}::${headSegment}` + } + const shouldApplyActivePrEditorSync = ({ repository, activeContext }) => { const syncedContextKey = getActivePrContextSyncKey(activeContext) const currentSyncKey = getActivePrEditorSyncKey() @@ -285,6 +301,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 +341,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 +590,10 @@ const initializeGitHubWorkflows = ({ onPrContextStateChange(prDrawerController.getActivePrContext()) } + let isClosingActivePullRequest = false + githubPrContextClose?.addEventListener('click', () => { - if (!githubAiContextState.activePrContext) { + if (!githubAiContextState.activePrContext || isClosingActivePullRequest) { return } @@ -559,6 +607,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 +643,12 @@ const initializeGitHubWorkflows = ({ setStatus(`Close context failed: ${message}`, 'error') showAppToast(`Close context failed: ${message}`) }) + .finally(() => { + isClosingActivePullRequest = false + if (githubPrContextClose instanceof HTMLButtonElement) { + githubPrContextClose.disabled = false + } + }) }, }) }) diff --git a/src/modules/app-core/pr-context-records.js b/src/modules/app-core/pr-context-records.js index 6c1b655..40ce7fd 100644 --- a/src/modules/app-core/pr-context-records.js +++ b/src/modules/app-core/pr-context-records.js @@ -3,6 +3,7 @@ const persistClosedPrContextRecords = async ({ selectedRepository, nextPrNumber, normalizedHead, + fallbackPrTitle, toNonEmptyWorkspaceText, refreshLocalContextOptions, }) => { @@ -10,12 +11,13 @@ const persistClosedPrContextRecords = async ({ ? await workspaceStorage.listWorkspaces({ repo: selectedRepository }) : await workspaceStorage.listWorkspaces() - const activeRecordsForContext = siblingRecords.filter(record => { + const recordsForContext = siblingRecords.filter(record => { if (!record || typeof record !== 'object') { return false } - if (toNonEmptyWorkspaceText(record.prContextState).toLowerCase() !== 'active') { + const normalizedState = toNonEmptyWorkspaceText(record.prContextState).toLowerCase() + if (normalizedState !== 'active' && normalizedState !== 'closed') { return false } @@ -32,20 +34,25 @@ const persistClosedPrContextRecords = async ({ return hasMatchingPrNumber || hasMatchingHead }) - if (activeRecordsForContext.length === 0) { + if (recordsForContext.length === 0) { return } + const normalizedFallbackTitle = toNonEmptyWorkspaceText(fallbackPrTitle) const now = Date.now() await Promise.all( - activeRecordsForContext.map(record => - workspaceStorage.upsertWorkspace({ + recordsForContext.map(record => { + const preservedTitle = + toNonEmptyWorkspaceText(record.prTitle) || normalizedFallbackTitle + + return workspaceStorage.upsertWorkspace({ ...record, prContextState: 'closed', prNumber: nextPrNumber, + prTitle: preservedTitle, lastModified: now, - }), - ), + }) + }), ) await refreshLocalContextOptions() diff --git a/src/modules/app-core/workspace-pr-session-handoff-controller.js b/src/modules/app-core/workspace-pr-session-handoff-controller.js index a977933..4fabc49 100644 --- a/src/modules/app-core/workspace-pr-session-handoff-controller.js +++ b/src/modules/app-core/workspace-pr-session-handoff-controller.js @@ -7,7 +7,8 @@ export const createWorkspacePrSessionHandoffController = ({ selectors, utils, }) => { - const { defaultComponentTabName, defaultComponentTabPath } = defaults + const { defaultComponentTabName, defaultComponentTabPath, defaultComponentTabContent } = + defaults const { getWorkspacePrNumber, setWorkspacePrContextState, @@ -50,6 +51,8 @@ export const createWorkspacePrSessionHandoffController = ({ const createFreshLocalEntryTab = () => { const now = Date.now() + const normalizedDefaultComponentTabContent = + typeof defaultComponentTabContent === 'string' ? defaultComponentTabContent : '' return { id: 'entry', @@ -59,7 +62,7 @@ export const createWorkspacePrSessionHandoffController = ({ role: 'entry', isActive: true, scroll: 0, - content: '', + content: normalizedDefaultComponentTabContent, targetPrFilePath: null, isDirty: false, syncedAt: null, diff --git a/src/modules/github/pr/drawer/controller/public-actions.js b/src/modules/github/pr/drawer/controller/public-actions.js index acca1b7..2fa3f92 100644 --- a/src/modules/github/pr/drawer/controller/public-actions.js +++ b/src/modules/github/pr/drawer/controller/public-actions.js @@ -32,6 +32,7 @@ export const createPublicActions = ({ clearRepositoryActivePrContext(repositoryFullName) state.lastActiveContentSyncKey = '' + abortPendingContextVerifyRequest() abortPendingActiveContentSyncRequest() if (resetForm) { @@ -45,7 +46,7 @@ export const createPublicActions = ({ return { clearActivePrContext: () => { - clearSelectedRepositoryActivePrContext({ resetForm: true }) + clearSelectedRepositoryActivePrContext({ resetForm: false }) }, clearSelectedRepositoryActivePrContext, closeActivePullRequestOnGitHub: async () => { @@ -69,7 +70,7 @@ export const createPublicActions = ({ setStatus('Closing pull request on GitHub...', 'pending') - await closeRepositoryPullRequest({ + const closedPullRequest = await closeRepositoryPullRequest({ token, owner: repository.owner, repo: repository.name, @@ -89,7 +90,12 @@ export const createPublicActions = ({ 'ok', ) - return { pullRequestNumber, reference: closedReference } + return { + pullRequestNumber, + reference: closedReference, + prTitle: + toSafeText(closedPullRequest?.title) || toSafeText(activeContext?.prTitle), + } }, setToken: token => { const hasToken = typeof token === 'string' && token.trim().length > 0 From 68f20ce1ab5162974cb1a75f0ec06a7cc045e7e7 Mon Sep 17 00:00:00 2001 From: KCM Date: Fri, 1 May 2026 12:36:43 -0500 Subject: [PATCH 2/5] fix: more workspace change determinism. --- src/modules/app-core/github-workflows.js | 7 -- .../pr-context-state-change-handler.js | 11 -- src/modules/app-core/pr-context-transition.js | 14 --- ...workspace-pr-session-handoff-controller.js | 14 +-- .../app-core/workspace-save-controller.js | 110 ++++++++++++------ .../app-core/workspace-scope-fork-actions.js | 5 +- 6 files changed, 79 insertions(+), 82 deletions(-) diff --git a/src/modules/app-core/github-workflows.js b/src/modules/app-core/github-workflows.js index c5fd2f4..f2bfa8b 100644 --- a/src/modules/app-core/github-workflows.js +++ b/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, diff --git a/src/modules/app-core/pr-context-state-change-handler.js b/src/modules/app-core/pr-context-state-change-handler.js index ffd407d..84692cc 100644 --- a/src/modules/app-core/pr-context-state-change-handler.js +++ b/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() diff --git a/src/modules/app-core/pr-context-transition.js b/src/modules/app-core/pr-context-transition.js index 903eef4..7944b0f 100644 --- a/src/modules/app-core/pr-context-transition.js +++ b/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' } } diff --git a/src/modules/app-core/workspace-pr-session-handoff-controller.js b/src/modules/app-core/workspace-pr-session-handoff-controller.js index 4fabc49..22ab927 100644 --- a/src/modules/app-core/workspace-pr-session-handoff-controller.js +++ b/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) { diff --git a/src/modules/app-core/workspace-save-controller.js b/src/modules/app-core/workspace-save-controller.js index 14eb99d..b6c7e0d 100644 --- a/src/modules/app-core/workspace-save-controller.js +++ b/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,12 +264,19 @@ 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) } @@ -265,12 +284,19 @@ const createWorkspaceSaveController = ({ 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,6 +307,9 @@ const createWorkspaceSaveController = ({ preserveRecordIdOnInput = false, preserveRecordIdOnChange = false, rekeyOnBlur = true, + allowIdentityMutationOnInput = false, + allowIdentityMutationOnChange = false, + allowIdentityMutationOnBlur = rekeyOnBlur, } = {}, ) => { if (!(element instanceof HTMLInputElement || element instanceof HTMLSelectElement)) { @@ -288,15 +317,24 @@ const createWorkspaceSaveController = ({ } 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. */ }) } diff --git a/src/modules/app-core/workspace-scope-fork-actions.js b/src/modules/app-core/workspace-scope-fork-actions.js index 78b69bb..0a9d409 100644 --- a/src/modules/app-core/workspace-scope-fork-actions.js +++ b/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 => { From 437097b460e898e2d79f0e09dbee883947eba3ca Mon Sep 17 00:00:00 2001 From: KCM Date: Fri, 1 May 2026 12:41:16 -0500 Subject: [PATCH 3/5] docs: align with current implementation. --- docs/idb-workspace-state.md | 6 +++++ docs/pr-context-storage-matrix.md | 38 +++++++++++++++++++++++-------- 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/docs/idb-workspace-state.md b/docs/idb-workspace-state.md index 2124523..af9438b 100644 --- a/docs/idb-workspace-state.md +++ b/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: diff --git a/docs/pr-context-storage-matrix.md b/docs/pr-context-storage-matrix.md index 14d9979..b5ddaef 100644 --- a/docs/pr-context-storage-matrix.md +++ b/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,7 +62,7 @@ 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: @@ -67,6 +70,25 @@ Notes: - 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. From 7dff17bebb451d9273eaf5f67717e21a2892f1a3 Mon Sep 17 00:00:00 2001 From: KCM Date: Fri, 1 May 2026 12:52:21 -0500 Subject: [PATCH 4/5] test: update. --- .../active-context-switch.spec.ts | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/playwright/github-pr-drawer/active-context-switch.spec.ts b/playwright/github-pr-drawer/active-context-switch.spec.ts index f8896d7..808b814 100644 --- a/playwright/github-pr-drawer/active-context-switch.spec.ts +++ b/playwright/github-pr-drawer/active-context-switch.spec.ts @@ -280,26 +280,6 @@ test('Switching active workspaces with different module sync paths keeps remote await expect .poll(async () => { const records = await getAllWorkspaceRecords(page) - - await expect - .poll(async () => { - const records = await getAllWorkspaceRecords(page) - const closedRecord = records.find( - record => - record?.repo === 'knightedcodemonkey/develop' && - record?.prContextState === 'closed' && - record?.prNumber === 2, - ) - - return { - prContextState: closedRecord?.prContextState, - prNumber: closedRecord?.prNumber, - } - }) - .toEqual({ - prContextState: 'closed', - prNumber: 2, - }) const alphaRecord = records.find(record => { const recordId = typeof record?.id === 'string' ? record.id.trim() : '' const recordHead = typeof record?.head === 'string' ? record.head.trim() : '' From e1fd88ba52385591731061910d8da8791d841db0 Mon Sep 17 00:00:00 2001 From: KCM Date: Fri, 1 May 2026 13:13:40 -0500 Subject: [PATCH 5/5] refactor: address pr comments. --- .github/instructions/pr-review.md | 1 + src/modules/app-core/github-workflows.js | 19 +++---------------- src/modules/app-core/pr-context-records.js | 10 +++++++++- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/.github/instructions/pr-review.md b/.github/instructions/pr-review.md index d8ff7af..4abca1d 100644 --- a/.github/instructions/pr-review.md +++ b/.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. diff --git a/src/modules/app-core/github-workflows.js b/src/modules/app-core/github-workflows.js index f2bfa8b..d9e5a7b 100644 --- a/src/modules/app-core/github-workflows.js +++ b/src/modules/app-core/github-workflows.js @@ -1,3 +1,5 @@ +import { toWorkspaceRecordKey } from '../workspace/workspace-tab-helpers.js' + const initializeGitHubWorkflows = ({ createGitHubPrEditorSyncController, createGitHubChatDrawer, @@ -120,22 +122,6 @@ const initializeGitHubWorkflows = ({ const toSafeRepositoryFullName = value => typeof value === 'string' ? value.trim() : '' - const toWorkspaceIdentitySegment = value => { - const normalized = typeof value === 'string' ? value.trim().toLowerCase() : '' - - if (!normalized) { - return '' - } - - return normalized.replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '') - } - - const toWorkspaceRecordKey = ({ repositoryFullName, headBranch } = {}) => { - const repoSegment = toWorkspaceIdentitySegment(repositoryFullName) || 'local' - const headSegment = toWorkspaceIdentitySegment(headBranch) || 'draft' - return `${repoSegment}::${headSegment}` - } - const shouldApplyActivePrEditorSync = ({ repository, activeContext }) => { const syncedContextKey = getActivePrContextSyncKey(activeContext) const currentSyncKey = getActivePrEditorSyncKey() @@ -258,6 +244,7 @@ const initializeGitHubWorkflows = ({ fileUpdates, repositoryFullName, pullRequestNumber, + branch, }) => { if (typeof onPrContextStateChange === 'function') { onPrContextStateChange(githubAiContextState.activePrContext) diff --git a/src/modules/app-core/pr-context-records.js b/src/modules/app-core/pr-context-records.js index 40ce7fd..558c761 100644 --- a/src/modules/app-core/pr-context-records.js +++ b/src/modules/app-core/pr-context-records.js @@ -44,11 +44,19 @@ const persistClosedPrContextRecords = async ({ recordsForContext.map(record => { const preservedTitle = toNonEmptyWorkspaceText(record.prTitle) || normalizedFallbackTitle + const preservedPrNumber = + typeof record.prNumber === 'number' && Number.isFinite(record.prNumber) + ? record.prNumber + : null + const nextPersistedPrNumber = + typeof nextPrNumber === 'number' && Number.isFinite(nextPrNumber) + ? nextPrNumber + : preservedPrNumber return workspaceStorage.upsertWorkspace({ ...record, prContextState: 'closed', - prNumber: nextPrNumber, + prNumber: nextPersistedPrNumber, prTitle: preservedTitle, lastModified: now, })