From 193fe9e0c5a37b358dc540fbc509e83460f8478d Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 19 Apr 2026 14:10:06 -0500 Subject: [PATCH 1/4] feat: make idb source of truth for pr context. --- docs/idb-workspace-state.md | 46 +++++ docs/localstorage-state.md | 33 ++++ docs/pr-context-storage-matrix.md | 59 ++---- src/app.js | 32 ++- src/modules/app-core/github-pr-icons.js | 11 ++ .../app-core/github-workflows-setup.js | 1 + src/modules/app-core/github-workflows.js | 2 + .../app-core/legacy-pr-config-storage.js | 27 +++ .../app-core/persisted-active-pr-context.js | 53 +++++ src/modules/github/byot-controls.js | 3 +- src/modules/github/pr/drawer/config.js | 183 ------------------ .../pr/drawer/controller/context-sync.js | 48 ++--- .../pr/drawer/controller/create-controller.js | 98 +++++++--- .../pr/drawer/controller/public-actions.js | 70 ++----- .../pr/drawer/controller/repository-form.js | 83 ++------ .../github/pr/drawer/controller/run-submit.js | 8 +- 16 files changed, 339 insertions(+), 418 deletions(-) create mode 100644 docs/idb-workspace-state.md create mode 100644 docs/localstorage-state.md create mode 100644 src/modules/app-core/github-pr-icons.js create mode 100644 src/modules/app-core/legacy-pr-config-storage.js create mode 100644 src/modules/app-core/persisted-active-pr-context.js delete mode 100644 src/modules/github/pr/drawer/config.js diff --git a/docs/idb-workspace-state.md b/docs/idb-workspace-state.md new file mode 100644 index 0000000..029a7ce --- /dev/null +++ b/docs/idb-workspace-state.md @@ -0,0 +1,46 @@ +# IndexedDB Workspace State Ownership + +This document is the source of truth for what `@knighted/develop` stores in IndexedDB. + +## Storage Location + +- Database: `knighted-develop-workspaces` +- Object store: `prWorkspaces` + +## Canonical State In IDB + +IndexedDB is the canonical source for workspace and pull request context. + +Each workspace record may include: + +- Workspace identity and timing: + - `id` + - `createdAt` + - `lastModified` +- Repository and PR context: + - `repo` + - `base` + - `head` + - `prTitle` + - `prNumber` + - `prContextState` (`inactive` | `active` | `disconnected` | `closed`) +- Runtime/editor state: + - `renderMode` + - `activeTabId` + - `tabs[]` including content, dirty state, sync metadata, and file paths + +## Why IDB Is Canonical + +Workspace restore and PR workflow continuity require structured, durable records. + +IDB supports that by storing: + +- Full workspace snapshots +- Repo-scoped context records +- Historical transitions such as disconnected or closed PR context + +## Design Rule + +If a value is required to accurately restore PR/workspace behavior after reload, it must live in IDB records. + +`localStorage` should only mirror user preferences and lightweight bootstrap values. diff --git a/docs/localstorage-state.md b/docs/localstorage-state.md new file mode 100644 index 0000000..4018f2e --- /dev/null +++ b/docs/localstorage-state.md @@ -0,0 +1,33 @@ +# localStorage State Ownership + +This document is the source of truth for what `@knighted/develop` stores in `localStorage`. + +## Allowed Keys + +`localStorage` is intentionally limited to lightweight user preference/session keys: + +1. `knighted:develop:github-pat` + - GitHub personal access token used for API calls. +2. `knighted:develop:github-repository` + - Last selected repository full name (for example: `owner/repo`). +3. `knighted-develop:render-mode` + - Last selected render mode (`dom` or `react`). +4. Theme/UI preference keys managed by layout theme modules. + +## Not Allowed In localStorage + +Do not store pull request context in `localStorage`. + +Examples that must stay out of `localStorage`: + +- PR context state (`active`, `disconnected`, `closed`, `inactive`) +- PR number and URL +- PR base/head/title/body +- PR drawer repository-scoped workflow state +- Workspace tab snapshots and synced file metadata + +## Design Rule + +`localStorage` is for lightweight bootstrap preferences only. + +If data is needed to restore workspace or pull request workflow state, it belongs in IndexedDB workspace records. diff --git a/docs/pr-context-storage-matrix.md b/docs/pr-context-storage-matrix.md index 271c467..dff8446 100644 --- a/docs/pr-context-storage-matrix.md +++ b/docs/pr-context-storage-matrix.md @@ -2,10 +2,16 @@ How `@knighted/develop` stores pull request context across browser storage. -This guide focuses on two storage surfaces: +This guide focuses on PR-context ownership only. + +PR context is stored in one place: 1. **IndexedDB (IDB)**: workspace snapshots used by the Workspaces drawer. -2. **localStorage**: repository-scoped PR drawer context metadata. + +See the full storage ownership docs for non-PR keys: + +- [localstorage-state.md](localstorage-state.md) +- [idb-workspace-state.md](idb-workspace-state.md) ## Storage Surfaces @@ -21,61 +27,38 @@ This guide focuses on two storage surfaces: ### localStorage -- Key pattern: `knighted:develop:github-pr-config:/` -- Relevant fields in each config: - - `isActivePr`: `boolean` - - `prContextState`: `inactive` | `active` | `disconnected` | `closed` - - `pullRequestNumber`: `number | null` - - `pullRequestUrl`: `string` - - `prTitle`, `baseBranch`, `headBranch` +- Not used for PR context state. ## Status Matrix Use this matrix as the source of truth when debugging UI/storage mismatch. -| Scenario | IDB `prContextState` | IDB `prNumber` | localStorage `isActivePr` | localStorage `prContextState` | Notes | -| --------------------------------------------- | -------------------- | --------------------------------- | ------------------------- | ----------------------------- | ----------------------------------------------------------- | -| A. Local workspace only, no PR context | `inactive` | `null` | `false` (or no key) | `inactive` (or no key) | No connected PR context. | -| B. Workspace is for an active, open PR | `active` | PR number | `true` | `active` | Push mode in PR controls. | -| C. Workspace is for a disconnected PR context | `disconnected` | last known PR number if available | `false` | `disconnected` | PR may still be open on GitHub; reconnect can verify later. | -| D. Workspace is for a PR closed on GitHub | `closed` | closed PR number | `false` | `closed` | Historical context retained for debugging/reference. | - -## Why Both IDB And localStorage Exist +| Scenario | IDB `prContextState` | IDB `prNumber` | localStorage PR fields | Notes | +| --------------------------------------------- | -------------------- | --------------------------------- | ---------------------- | ----------------------------------------------------------- | +| A. Local workspace only, no PR context | `inactive` | `null` | none | No connected PR context. | +| B. Workspace is for an active, open PR | `active` | PR number | none | Push mode in PR controls. | +| C. Workspace is for a disconnected PR context | `disconnected` | last known PR number if available | none | PR may still be open on GitHub; reconnect can verify later. | +| D. Workspace is for a PR closed on GitHub | `closed` | closed PR number | none | Historical context retained for debugging/reference. | -The two stores have different responsibilities: +## Why PR Context Lives In IDB Only -1. **IDB (workspace scope)** - - Persists full editor/workspace snapshots. - - Drives Workspaces drawer restore and switching. -2. **localStorage (repository PR scope)** - - Persists PR drawer config and active context metadata for the selected repository. - - Drives Open PR vs Push mode and active-context checks. +PR workflow state is part of workspace state. -They intentionally overlap on PR metadata so the app can restore workspace context and PR drawer behavior across reloads. +Storing it only in IDB avoids drift between storage systems and keeps a single source of truth for restore behavior. ## Debugging Checklist When the UI does not match expected PR state: -1. Check localStorage key for the selected repository and inspect: - - `isActivePr` - - `prContextState` - - `pullRequestNumber` -2. Check IDB workspace record currently selected/opened and inspect: +1. Check the IDB workspace record currently selected/opened and inspect: - `prContextState` - `prNumber` - `repo`, `head`, `prTitle` -3. Compare against the matrix above. -4. If scenario C is expected, remember GitHub-open verification is deferred until reconnect flow is invoked. +2. Compare against the matrix above. +3. If scenario C is expected, remember GitHub-open verification is deferred until reconnect flow is invoked. ## Console Snippets -LocalStorage (selected repo key): - -```js -JSON.parse(localStorage.getItem('knighted:develop:github-pr-config:OWNER/REPO') || '{}') -``` - IndexedDB (all workspace records): ```js diff --git a/src/app.js b/src/app.js index 4dd7511..9bb0bcf 100644 --- a/src/app.js +++ b/src/app.js @@ -37,8 +37,17 @@ import { createGitHubWorkflowsSetup } from './modules/app-core/github-workflows- import { defaultCss, defaultJsx } from './modules/app-core/defaults.js' import { createGitHubPrContextUiController } from './modules/app-core/github-pr-context-ui.js' import { createGitHubTokenInfoUiController } from './modules/app-core/github-token-info-ui.js' +import { + githubPrOpenIcon, + githubPrPushCommitIcon, +} from './modules/app-core/github-pr-icons.js' import { createWorkspaceSyncController } from './modules/app-core/workspace-sync-controller.js' import { createWorkspaceTabAddMenuUiController } from './modules/app-core/workspace-tab-add-menu-ui.js' +import { + clearLegacyPrConfigStorage, + legacyPrConfigStoragePrefix, +} from './modules/app-core/legacy-pr-config-storage.js' +import { createPersistedActivePrContextGetter } from './modules/app-core/persisted-active-pr-context.js' import { createDiagnosticsUiController } from './modules/diagnostics/diagnostics-ui.js' import { createGitHubChatDrawer } from './modules/github/chat-drawer/drawer.js' import { createGitHubByotControls } from './modules/github/byot-controls.js' @@ -263,14 +272,6 @@ let draggedWorkspaceTabId = '' let dragOverWorkspaceTabId = '' let suppressWorkspaceTabClick = false const clipboardSupported = Boolean(navigator.clipboard?.writeText) -const githubPrOpenIcon = { - viewBox: '0 0 16 16', - path: 'M1.5 3.25a2.25 2.25 0 1 1 3 2.122v5.256a2.251 2.251 0 1 1-1.5 0V5.372A2.25 2.25 0 0 1 1.5 3.25Zm5.677-.177L9.573.677A.25.25 0 0 1 10 .854V2.5h1A2.5 2.5 0 0 1 13.5 5v5.628a2.251 2.251 0 1 1-1.5 0V5a1 1 0 0 0-1-1h-1v1.646a.25.25 0 0 1-.427.177L7.177 3.427a.25.25 0 0 1 0-.354ZM3.75 2.5a.75.75 0 1 0 0 1.5.75.75 0 0 0 0-1.5Zm0 9.5a.75.75 0 1 0 0 1.5.75.75 0 0 0 0-1.5Zm8.25.75a.75.75 0 1 0 1.5 0 .75.75 0 0 0-1.5 0Z', -} -const githubPrPushCommitIcon = { - viewBox: '0 0 24 24', - path: 'M16.944 11h4.306a.75.75 0 0 1 0 1.5h-4.306a5.001 5.001 0 0 1-9.888 0H2.75a.75.75 0 0 1 0-1.5h4.306a5.001 5.001 0 0 1 9.888 0Zm-1.444.75a3.5 3.5 0 1 0-7 0 3.5 3.5 0 0 0 7 0Z', -} const showAppToast = message => { if (!(appToast instanceof HTMLElement)) { @@ -341,6 +342,8 @@ const workspaceTabAddMenuUi = createWorkspaceTabAddMenuUiController({ addModuleButton: workspaceTabAddModule, }) +clearLegacyPrConfigStorage({ prefix: legacyPrConfigStoragePrefix }) + const { panelToolsState, applyEditorToolsVisibility, @@ -508,6 +511,18 @@ const getCurrentSelectedRepository = () => const getCurrentSelectedRepositoryFullName = () => getCurrentSelectedRepository()?.fullName ?? '' +const getPersistedActivePrContext = createPersistedActivePrContextGetter({ + getCurrentSelectedRepositoryFullName, + getWorkspacePrContextState: () => workspacePrContextState, + getWorkspacePrNumber: () => workspacePrNumber, + githubPrBaseBranch, + githubPrHeadBranch, + githubPrTitle, + githubPrBody, + renderMode, + styleMode, +}) + const getWorkspaceContextSnapshot = createWorkspaceContextSnapshotGetter({ getCurrentSelectedRepository: getCurrentSelectedRepositoryFullName, githubPrBaseBranch, @@ -862,6 +877,7 @@ const githubWorkflows = createGitHubWorkflowsSetup({ setWorkspacePrNumber(result?.pullRequestNumber) persistWorkspacePrContextState('disconnected') }, + getPersistedActivePrContext, getTokenForVisibility: () => githubAiContextState.token, closeWorkspacesDrawer: () => { void workspacesDrawerController?.setOpen(false) diff --git a/src/modules/app-core/github-pr-icons.js b/src/modules/app-core/github-pr-icons.js new file mode 100644 index 0000000..d24c3c8 --- /dev/null +++ b/src/modules/app-core/github-pr-icons.js @@ -0,0 +1,11 @@ +const githubPrOpenIcon = { + viewBox: '0 0 16 16', + path: 'M1.5 3.25a2.25 2.25 0 1 1 3 2.122v5.256a2.251 2.251 0 1 1-1.5 0V5.372A2.25 2.25 0 0 1 1.5 3.25Zm5.677-.177L9.573.677A.25.25 0 0 1 10 .854V2.5h1A2.5 2.5 0 0 1 13.5 5v5.628a2.251 2.251 0 1 1-1.5 0V5a1 1 0 0 0-1-1h-1v1.646a.25.25 0 0 1-.427.177L7.177 3.427a.25.25 0 0 1 0-.354ZM3.75 2.5a.75.75 0 1 0 0 1.5.75.75 0 0 0 0-1.5Zm0 9.5a.75.75 0 1 0 0 1.5.75.75 0 0 0 0-1.5Zm8.25.75a.75.75 0 1 0 1.5 0 .75.75 0 0 0-1.5 0Z', +} + +const githubPrPushCommitIcon = { + viewBox: '0 0 24 24', + path: 'M16.944 11h4.306a.75.75 0 0 1 0 1.5h-4.306a5.001 5.001 0 0 1-9.888 0H2.75a.75.75 0 0 1 0-1.5h4.306a5.001 5.001 0 0 1 9.888 0Zm-1.444.75a3.5 3.5 0 1 0-7 0 3.5 3.5 0 0 0 7 0Z', +} + +export { githubPrOpenIcon, githubPrPushCommitIcon } diff --git a/src/modules/app-core/github-workflows-setup.js b/src/modules/app-core/github-workflows-setup.js index 728889d..0eeac26 100644 --- a/src/modules/app-core/github-workflows-setup.js +++ b/src/modules/app-core/github-workflows-setup.js @@ -29,6 +29,7 @@ const createGitHubWorkflowsSetup = ({ getEditorSyncTargets: workspace.getEditorSyncTargets, getRenderMode: runtime.getRenderMode, getStyleMode: runtime.getStyleMode, + getPersistedActivePrContext: runtime.getPersistedActivePrContext, setCurrentSelectedRepository: byot.setCurrentSelectedRepository, reconcileWorkspaceTabsWithPushUpdates: workspace.reconcileWorkspaceTabsWithPushUpdates, diff --git a/src/modules/app-core/github-workflows.js b/src/modules/app-core/github-workflows.js index 7966386..9986fbe 100644 --- a/src/modules/app-core/github-workflows.js +++ b/src/modules/app-core/github-workflows.js @@ -55,6 +55,7 @@ const initializeGitHubWorkflows = ({ getRenderMode, getStyleMode, setCurrentSelectedRepository, + getPersistedActivePrContext, reconcileWorkspaceTabsWithPushUpdates, getActivePrContextSyncKey, prContextUi, @@ -130,6 +131,7 @@ const initializeGitHubWorkflows = ({ getDrawerSide: () => { return 'right' }, + getPersistedActivePrContext, }) const prDrawerController = createGitHubPrDrawer({ diff --git a/src/modules/app-core/legacy-pr-config-storage.js b/src/modules/app-core/legacy-pr-config-storage.js new file mode 100644 index 0000000..66edc6c --- /dev/null +++ b/src/modules/app-core/legacy-pr-config-storage.js @@ -0,0 +1,27 @@ +const legacyPrConfigStoragePrefix = 'knighted:develop:github-pr-config:' + +const clearLegacyPrConfigStorage = ({ + storage = localStorage, + prefix = legacyPrConfigStoragePrefix, +} = {}) => { + try { + const keysToRemove = [] + + for (let index = 0; index < storage.length; index += 1) { + const key = storage.key(index) + if (!key || !key.startsWith(prefix)) { + continue + } + + keysToRemove.push(key) + } + + for (const key of keysToRemove) { + storage.removeItem(key) + } + } catch { + /* noop */ + } +} + +export { clearLegacyPrConfigStorage, legacyPrConfigStoragePrefix } diff --git a/src/modules/app-core/persisted-active-pr-context.js b/src/modules/app-core/persisted-active-pr-context.js new file mode 100644 index 0000000..fbc2261 --- /dev/null +++ b/src/modules/app-core/persisted-active-pr-context.js @@ -0,0 +1,53 @@ +const createPersistedActivePrContextGetter = ({ + getCurrentSelectedRepositoryFullName, + getWorkspacePrContextState, + getWorkspacePrNumber, + githubPrBaseBranch, + githubPrHeadBranch, + githubPrTitle, + githubPrBody, + renderMode, + styleMode, +}) => { + return repositoryFullName => { + const normalizedRepository = + typeof repositoryFullName === 'string' ? repositoryFullName.trim() : '' + if (!normalizedRepository) { + return null + } + + if (normalizedRepository !== getCurrentSelectedRepositoryFullName()) { + return null + } + + if (getWorkspacePrContextState() !== 'active') { + return null + } + + const headBranch = + typeof githubPrHeadBranch?.value === 'string' ? githubPrHeadBranch.value.trim() : '' + const prTitle = + typeof githubPrTitle?.value === 'string' ? githubPrTitle.value.trim() : '' + + if (!headBranch || !prTitle) { + return null + } + + return { + repositoryFullName: normalizedRepository, + baseBranch: + typeof githubPrBaseBranch?.value === 'string' + ? githubPrBaseBranch.value.trim() + : '', + headBranch, + prTitle, + prBody: typeof githubPrBody?.value === 'string' ? githubPrBody.value : '', + pullRequestNumber: getWorkspacePrNumber(), + pullRequestUrl: '', + renderMode: renderMode?.value, + styleMode: styleMode?.value, + } + } +} + +export { createPersistedActivePrContextGetter } diff --git a/src/modules/github/byot-controls.js b/src/modules/github/byot-controls.js index 67022a2..0839b4e 100644 --- a/src/modules/github/byot-controls.js +++ b/src/modules/github/byot-controls.js @@ -5,7 +5,6 @@ import { saveGitHubToken, } from './token-store.js' import { listWritableRepositories } from './api/repositories.js' -import { findRepositoryWithActivePrContext } from './pr/drawer/config.js' const selectedRepositoryStorageKey = 'knighted:develop:github-repository' @@ -233,7 +232,7 @@ export const createGitHubByotControls = ({ const selectedRepositoryFullName = hasStoredSelection ? lastSelectedRepository - : (findRepositoryWithActivePrContext(repos) ?? repos[0].fullName) + : repos[0].fullName saveSelectedRepository(selectedRepositoryFullName) lastSelectedRepository = selectedRepositoryFullName diff --git a/src/modules/github/pr/drawer/config.js b/src/modules/github/pr/drawer/config.js deleted file mode 100644 index 2cbf01a..0000000 --- a/src/modules/github/pr/drawer/config.js +++ /dev/null @@ -1,183 +0,0 @@ -import { parsePullRequestNumberFromUrl } from '../context.js' -import { - normalizePrContextState, - normalizeRenderMode, - normalizeStyleMode, - sanitizeBranchPart, - toPullRequestNumber, - toSafeText, -} from './common.js' - -const prConfigStoragePrefix = 'knighted:develop:github-pr-config:' - -const getRepositoryPrConfigStorageKey = repositoryFullName => - `${prConfigStoragePrefix}${repositoryFullName}` - -const pruneRepositoryPrConfigs = repositoryFullName => { - if (typeof repositoryFullName !== 'string' || !repositoryFullName.trim()) { - return - } - - const activeStorageKey = getRepositoryPrConfigStorageKey(repositoryFullName) - - try { - const keysToRemove = [] - - for (let index = 0; index < localStorage.length; index += 1) { - const key = localStorage.key(index) - if (!key || !key.startsWith(prConfigStoragePrefix)) { - continue - } - - if (key !== activeStorageKey) { - keysToRemove.push(key) - } - } - - for (const key of keysToRemove) { - localStorage.removeItem(key) - } - } catch { - /* noop */ - } -} - -const readRepositoryPrConfig = repositoryFullName => { - if (typeof repositoryFullName !== 'string' || !repositoryFullName.trim()) { - return {} - } - - try { - const value = localStorage.getItem( - getRepositoryPrConfigStorageKey(repositoryFullName), - ) - if (!value) { - return {} - } - - const parsed = JSON.parse(value) - return parsed && typeof parsed === 'object' ? parsed : {} - } catch { - return {} - } -} - -const saveRepositoryPrConfig = ({ repositoryFullName, config }) => { - if (typeof repositoryFullName !== 'string' || !repositoryFullName.trim()) { - return - } - - try { - const activeStorageKey = getRepositoryPrConfigStorageKey(repositoryFullName) - - localStorage.setItem(activeStorageKey, JSON.stringify(config)) - - pruneRepositoryPrConfigs(repositoryFullName) - } catch { - /* noop */ - } -} - -const sanitizeRepositoryPrConfig = config => { - const source = config && typeof config === 'object' ? config : {} - const pullRequestUrl = toSafeText(source.pullRequestUrl) - const fallbackPullRequestNumber = parsePullRequestNumberFromUrl(pullRequestUrl) - const pullRequestNumber = - toPullRequestNumber(source.pullRequestNumber) ?? fallbackPullRequestNumber - const isActivePr = source.isActivePr === true - const normalizedPrContextState = normalizePrContextState(source.prContextState) - const prContextState = isActivePr - ? 'active' - : normalizedPrContextState === 'active' - ? 'inactive' - : normalizedPrContextState - - return { - baseBranch: toSafeText(source.baseBranch), - headBranch: sanitizeBranchPart(source.headBranch), - prTitle: toSafeText(source.prTitle), - prBody: typeof source.prBody === 'string' ? source.prBody.trim() : '', - renderMode: normalizeRenderMode(source.renderMode), - styleMode: normalizeStyleMode(source.styleMode), - isActivePr, - prContextState, - pullRequestNumber, - pullRequestUrl, - } -} - -const removeRepositoryPrConfig = repositoryFullName => { - if (typeof repositoryFullName !== 'string' || !repositoryFullName.trim()) { - return - } - - try { - localStorage.removeItem(getRepositoryPrConfigStorageKey(repositoryFullName)) - } catch { - /* noop */ - } -} - -const getActiveRepositoryPrContext = repositoryFullName => { - const savedConfig = sanitizeRepositoryPrConfig( - readRepositoryPrConfig(repositoryFullName), - ) - - if (savedConfig?.isActivePr !== true) { - return null - } - - const headBranch = sanitizeBranchPart(savedConfig.headBranch) - const prTitle = toSafeText(savedConfig.prTitle) - const baseBranch = toSafeText(savedConfig.baseBranch) - - if (!headBranch || !prTitle) { - return null - } - - return { - headBranch, - renderMode: normalizeRenderMode(savedConfig.renderMode), - styleMode: normalizeStyleMode(savedConfig.styleMode), - prTitle, - prBody: typeof savedConfig.prBody === 'string' ? savedConfig.prBody : '', - baseBranch, - pullRequestNumber: - typeof savedConfig.pullRequestNumber === 'number' && - Number.isFinite(savedConfig.pullRequestNumber) - ? savedConfig.pullRequestNumber - : parsePullRequestNumberFromUrl(savedConfig.pullRequestUrl), - pullRequestUrl: - typeof savedConfig.pullRequestUrl === 'string' ? savedConfig.pullRequestUrl : '', - repositoryFullName, - } -} - -const findRepositoryWithActivePrContext = repositories => { - if (!Array.isArray(repositories) || repositories.length === 0) { - return null - } - - for (const repository of repositories) { - const repositoryFullName = toSafeText(repository?.fullName) - - if (!repositoryFullName) { - continue - } - - if (getActiveRepositoryPrContext(repositoryFullName)) { - return repositoryFullName - } - } - - return null -} - -export { - findRepositoryWithActivePrContext, - getActiveRepositoryPrContext, - readRepositoryPrConfig, - removeRepositoryPrConfig, - sanitizeRepositoryPrConfig, - saveRepositoryPrConfig, -} diff --git a/src/modules/github/pr/drawer/controller/context-sync.js b/src/modules/github/pr/drawer/controller/context-sync.js index faf2e4c..4868726 100644 --- a/src/modules/github/pr/drawer/controller/context-sync.js +++ b/src/modules/github/pr/drawer/controller/context-sync.js @@ -6,16 +6,14 @@ export const createContextSyncHandlers = ({ getEditorSyncTargets, onSyncActivePrEditorContent, getCurrentActivePrContext, + setRepositoryActivePrContext, + clearRepositoryActivePrContext, syncFormForRepository, setSubmitButtonLabel, emitActivePrContextChange, setStatus, toSafeText, sanitizeBranchPart, - parsePullRequestNumberFromUrl, - readRepositoryPrConfig, - saveRepositoryPrConfig, - sanitizeRepositoryPrConfig, getRepositoryPullRequest, findOpenRepositoryPullRequestByHead, }) => { @@ -122,17 +120,17 @@ export const createContextSyncHandlers = ({ return } - const savedConfig = readRepositoryPrConfig(repositoryFullName) - if (savedConfig?.isActivePr !== true) { + const activeContext = getCurrentActivePrContext() + if (!activeContext) { return } const pullRequestNumberFromConfig = - typeof savedConfig.pullRequestNumber === 'number' && - Number.isFinite(savedConfig.pullRequestNumber) - ? savedConfig.pullRequestNumber - : parsePullRequestNumberFromUrl(savedConfig.pullRequestUrl) - const headBranch = sanitizeBranchPart(savedConfig.headBranch) + typeof activeContext.pullRequestNumber === 'number' && + Number.isFinite(activeContext.pullRequestNumber) + ? activeContext.pullRequestNumber + : null + const headBranch = sanitizeBranchPart(activeContext.headBranch) if (!pullRequestNumberFromConfig && !headBranch) { return @@ -142,7 +140,7 @@ export const createContextSyncHandlers = ({ repositoryFullName, String(pullRequestNumberFromConfig || ''), headBranch, - toSafeText(savedConfig.baseBranch), + toSafeText(activeContext.baseBranch), ].join('|') if ( @@ -185,7 +183,7 @@ export const createContextSyncHandlers = ({ repo, headOwner: owner, headBranch, - baseBranch: toSafeText(savedConfig.baseBranch), + baseBranch: toSafeText(activeContext.baseBranch), signal: abortController.signal, }) } @@ -195,24 +193,23 @@ export const createContextSyncHandlers = ({ } if (resolvedPullRequest?.isOpen) { - const normalizedSavedConfig = sanitizeRepositoryPrConfig(savedConfig) const nextHeadBranch = sanitizeBranchPart(resolvedPullRequest.headRef) || headBranch const nextBaseBranch = - toSafeText(resolvedPullRequest.baseRef) || toSafeText(savedConfig.baseBranch) + toSafeText(resolvedPullRequest.baseRef) || + toSafeText(activeContext.baseBranch) - saveRepositoryPrConfig({ + setRepositoryActivePrContext({ repositoryFullName, - config: { - ...normalizedSavedConfig, - isActivePr: true, - prContextState: 'active', + activeContext: { + ...activeContext, headBranch: nextHeadBranch, baseBranch: nextBaseBranch, pullRequestNumber: resolvedPullRequest.number, pullRequestUrl: resolvedPullRequest.htmlUrl, prTitle: - toSafeText(savedConfig.prTitle) || toSafeText(resolvedPullRequest.title), + toSafeText(activeContext.prTitle) || + toSafeText(resolvedPullRequest.title), }, }) syncFormForRepository({ resetBranch: true }) @@ -222,14 +219,7 @@ export const createContextSyncHandlers = ({ return } - saveRepositoryPrConfig({ - repositoryFullName, - config: { - ...sanitizeRepositoryPrConfig(savedConfig), - isActivePr: false, - prContextState: 'closed', - }, - }) + clearRepositoryActivePrContext(repositoryFullName) setSubmitButtonLabel() emitActivePrContextChange() state.lastActiveContentSyncKey = '' diff --git a/src/modules/github/pr/drawer/controller/create-controller.js b/src/modules/github/pr/drawer/controller/create-controller.js index e8ef52c..f8d147e 100644 --- a/src/modules/github/pr/drawer/controller/create-controller.js +++ b/src/modules/github/pr/drawer/controller/create-controller.js @@ -8,7 +8,7 @@ import { findOpenRepositoryPullRequestByHead, getRepositoryPullRequest, } from '../../../api/pull-requests.js' -import { formatActivePrReference, parsePullRequestNumberFromUrl } from '../../context.js' +import { formatActivePrReference } from '../../context.js' import { createDefaultBranchName, createSelectOption, @@ -16,18 +16,12 @@ import { mergeBranchOptions, toBranchCacheKey, } from '../branches.js' -import { - getActiveRepositoryPrContext, - readRepositoryPrConfig, - removeRepositoryPrConfig, - sanitizeRepositoryPrConfig, - saveRepositoryPrConfig, -} from '../config.js' import { defaultCommitMessage, normalizeRenderMode, normalizeStyleMode, sanitizeBranchPart, + toPullRequestNumber, toSafeText, } from '../common.js' import { ensureTrailingNewline, normalizeFileCommits } from '../file-commits.js' @@ -71,6 +65,7 @@ export const createGitHubPrDrawer = ({ onSyncActivePrEditorContent, onRestoreRenderMode, onRestoreStyleMode, + getPersistedActivePrContext, }) => { const state = { open: false, @@ -86,6 +81,7 @@ export const createGitHubPrDrawer = ({ lastSyncedRepositoryFullName: '', lastActiveContentSyncKey: '', baseBranchesByRepository: new Map(), + activePrContextByRepository: new Map(), } const getSelectedRepositoryObject = () => getSelectedRepository?.() ?? null @@ -93,6 +89,52 @@ export const createGitHubPrDrawer = ({ const getRepositoryFullName = repository => typeof repository?.fullName === 'string' ? repository.fullName : '' + const sanitizeActiveContext = ({ repositoryFullName, activeContext }) => { + if (typeof repositoryFullName !== 'string' || !repositoryFullName.trim()) { + return null + } + + if (!activeContext || typeof activeContext !== 'object') { + return null + } + + const headBranch = sanitizeBranchPart(activeContext.headBranch) + const prTitle = toSafeText(activeContext.prTitle) + if (!headBranch || !prTitle) { + return null + } + + return { + repositoryFullName, + headBranch, + renderMode: normalizeRenderMode(activeContext.renderMode), + styleMode: normalizeStyleMode(activeContext.styleMode), + prTitle, + prBody: typeof activeContext.prBody === 'string' ? activeContext.prBody : '', + baseBranch: toSafeText(activeContext.baseBranch), + pullRequestNumber: toPullRequestNumber(activeContext.pullRequestNumber), + pullRequestUrl: toSafeText(activeContext.pullRequestUrl), + } + } + + const setRepositoryActivePrContext = ({ repositoryFullName, activeContext }) => { + const sanitized = sanitizeActiveContext({ repositoryFullName, activeContext }) + if (!sanitized) { + state.activePrContextByRepository.delete(repositoryFullName) + return + } + + state.activePrContextByRepository.set(repositoryFullName, sanitized) + } + + const clearRepositoryActivePrContext = repositoryFullName => { + if (typeof repositoryFullName !== 'string' || !repositoryFullName.trim()) { + return + } + + state.activePrContextByRepository.delete(repositoryFullName) + } + const getCurrentActivePrContext = () => { const repository = getSelectedRepositoryObject() const repositoryFullName = getRepositoryFullName(repository) @@ -100,7 +142,26 @@ export const createGitHubPrDrawer = ({ return null } - return getActiveRepositoryPrContext(repositoryFullName) + const inMemoryContext = state.activePrContextByRepository.get(repositoryFullName) + if (inMemoryContext) { + return inMemoryContext + } + + if (typeof getPersistedActivePrContext !== 'function') { + return null + } + + const persistedContext = sanitizeActiveContext({ + repositoryFullName, + activeContext: getPersistedActivePrContext(repositoryFullName), + }) + + if (!persistedContext) { + return null + } + + state.activePrContextByRepository.set(repositoryFullName, persistedContext) + return persistedContext } const uiHandlers = createUiStateHandlers({ @@ -141,10 +202,8 @@ export const createGitHubPrDrawer = ({ setStatus: uiHandlers.setStatus, toSafeText, sanitizeBranchPart, - parsePullRequestNumberFromUrl, - readRepositoryPrConfig, - saveRepositoryPrConfig, - sanitizeRepositoryPrConfig, + setRepositoryActivePrContext, + clearRepositoryActivePrContext, getRepositoryPullRequest, findOpenRepositoryPullRequestByHead, }) @@ -182,9 +241,6 @@ export const createGitHubPrDrawer = ({ toBranchCacheKey, normalizeRenderMode, normalizeStyleMode, - readRepositoryPrConfig, - sanitizeRepositoryPrConfig, - saveRepositoryPrConfig, listRepositoryBranches, }) @@ -223,7 +279,7 @@ export const createGitHubPrDrawer = ({ commitEditorContentToExistingBranch, createEditorContentPullRequest, formatActivePrReference, - saveRepositoryPrConfig, + setRepositoryActivePrContext, }) bindControllerEvents({ @@ -276,12 +332,8 @@ export const createGitHubPrDrawer = ({ contextHandlers.abortPendingActiveContentSyncRequest, closeRepositoryPullRequest, formatActivePrReference, - parsePullRequestNumberFromUrl, - readRepositoryPrConfig, - saveRepositoryPrConfig, - sanitizeRepositoryPrConfig, - removeRepositoryPrConfig, - sanitizeBranchPart, + setRepositoryActivePrContext, + clearRepositoryActivePrContext, toSafeText, }) diff --git a/src/modules/github/pr/drawer/controller/public-actions.js b/src/modules/github/pr/drawer/controller/public-actions.js index 473ad09..c7b2b2d 100644 --- a/src/modules/github/pr/drawer/controller/public-actions.js +++ b/src/modules/github/pr/drawer/controller/public-actions.js @@ -20,12 +20,8 @@ export const createPublicActions = ({ abortPendingActiveContentSyncRequest, closeRepositoryPullRequest, formatActivePrReference, - parsePullRequestNumberFromUrl, - readRepositoryPrConfig, - saveRepositoryPrConfig, - sanitizeRepositoryPrConfig, - removeRepositoryPrConfig, - sanitizeBranchPart, + setRepositoryActivePrContext, + clearRepositoryActivePrContext, toSafeText, }) => { return { @@ -36,30 +32,8 @@ export const createPublicActions = ({ return { reference: '' } } - const savedConfig = readRepositoryPrConfig(repositoryFullName) - const normalizedSavedConfig = sanitizeRepositoryPrConfig(savedConfig) - const previousActiveContext = - savedConfig?.isActivePr === true - ? { - repositoryFullName, - pullRequestNumber: - typeof savedConfig.pullRequestNumber === 'number' && - Number.isFinite(savedConfig.pullRequestNumber) - ? savedConfig.pullRequestNumber - : parsePullRequestNumberFromUrl(savedConfig.pullRequestUrl), - } - : null - - if (Object.keys(savedConfig).length > 0) { - saveRepositoryPrConfig({ - repositoryFullName, - config: { - ...normalizedSavedConfig, - isActivePr: false, - prContextState: 'disconnected', - }, - }) - } + const activeContext = getCurrentActivePrContext() + clearRepositoryActivePrContext(repositoryFullName) state.lastActiveContentSyncKey = '' abortPendingActiveContentSyncRequest() @@ -67,11 +41,11 @@ export const createPublicActions = ({ emitActivePrContextChange() return { - reference: formatActivePrReference(previousActiveContext), + reference: formatActivePrReference(activeContext), pullRequestNumber: - typeof previousActiveContext?.pullRequestNumber === 'number' && - Number.isFinite(previousActiveContext.pullRequestNumber) - ? previousActiveContext.pullRequestNumber + typeof activeContext?.pullRequestNumber === 'number' && + Number.isFinite(activeContext.pullRequestNumber) + ? activeContext.pullRequestNumber : null, } }, @@ -82,7 +56,7 @@ export const createPublicActions = ({ return } - removeRepositoryPrConfig(repositoryFullName) + clearRepositoryActivePrContext(repositoryFullName) state.lastActiveContentSyncKey = '' abortPendingActiveContentSyncRequest() syncFormForRepository({ resetAll: true, resetBranch: true }) @@ -94,9 +68,7 @@ export const createPublicActions = ({ const repositoryFullName = getRepositoryFullName(repository) const token = toSafeText(getToken?.()) const activeContext = getCurrentActivePrContext() - const pullRequestNumber = - activeContext?.pullRequestNumber ?? - parsePullRequestNumberFromUrl(activeContext?.pullRequestUrl) + const pullRequestNumber = activeContext?.pullRequestNumber if (!repositoryFullName || !repository?.owner || !repository?.name) { throw new Error('Select a repository before closing pull request context.') @@ -119,27 +91,9 @@ export const createPublicActions = ({ pullRequestNumber, }) - const savedConfig = sanitizeRepositoryPrConfig( - readRepositoryPrConfig(repositoryFullName), - ) - saveRepositoryPrConfig({ + setRepositoryActivePrContext({ repositoryFullName, - config: { - ...savedConfig, - baseBranch: toSafeText(activeContext?.baseBranch) || savedConfig.baseBranch, - headBranch: - sanitizeBranchPart(activeContext?.headBranch) || savedConfig.headBranch, - prTitle: toSafeText(activeContext?.prTitle) || savedConfig.prTitle, - prBody: - typeof activeContext?.prBody === 'string' - ? activeContext.prBody - : savedConfig.prBody, - isActivePr: false, - prContextState: 'closed', - pullRequestNumber, - pullRequestUrl: - toSafeText(activeContext?.pullRequestUrl) || savedConfig.pullRequestUrl, - }, + activeContext: null, }) syncFormForRepository({ resetAll: true, resetBranch: true }) setSubmitButtonLabel() diff --git a/src/modules/github/pr/drawer/controller/repository-form.js b/src/modules/github/pr/drawer/controller/repository-form.js index 163ab34..c06ee14 100644 --- a/src/modules/github/pr/drawer/controller/repository-form.js +++ b/src/modules/github/pr/drawer/controller/repository-form.js @@ -31,9 +31,6 @@ export const createRepositoryFormHandlers = ({ toBranchCacheKey, normalizeRenderMode, normalizeStyleMode, - readRepositoryPrConfig, - sanitizeRepositoryPrConfig, - saveRepositoryPrConfig, listRepositoryBranches, }) => { const abortPendingBranchesRequest = () => { @@ -83,13 +80,7 @@ export const createRepositoryFormHandlers = ({ } const getPreferredBaseBranchForRepository = repository => { - const repositoryFullName = getRepositoryFullName(repository) - const savedConfig = readRepositoryPrConfig(repositoryFullName) - return ( - toSafeText(savedConfig.baseBranch) || - toSafeText(repository?.defaultBranch) || - 'main' - ) + return toSafeText(repository?.defaultBranch) || 'main' } const loadBaseBranchesForSelectedRepository = async ({ preferredBranch }) => { @@ -219,11 +210,10 @@ export const createRepositoryFormHandlers = ({ const repositoryChanged = Boolean(repositoryFullName) && repositoryFullName !== state.lastSyncedRepositoryFullName - const savedConfig = readRepositoryPrConfig(repositoryFullName) - const savedDraftConfig = resetAll ? {} : savedConfig + const activeContext = getCurrentActivePrContext() const baseBranch = - toSafeText(savedConfig.baseBranch) || + toSafeText(activeContext?.baseBranch) || toSafeText(repository?.defaultBranch) || 'main' @@ -236,24 +226,24 @@ export const createRepositoryFormHandlers = ({ repositoryChanged || !toSafeText(headBranchInput.value) ) { - const savedHeadBranch = sanitizeBranchPart(savedDraftConfig.headBranch) + const activeHeadBranch = sanitizeBranchPart(activeContext?.headBranch) headBranchInput.value = - savedHeadBranch && !isAutoGeneratedHeadBranch(savedHeadBranch) - ? savedHeadBranch + activeHeadBranch && !isAutoGeneratedHeadBranch(activeHeadBranch) + ? activeHeadBranch : createDefaultBranchName() } } if (prTitleInput instanceof HTMLInputElement) { if (resetAll || repositoryChanged || !toSafeText(prTitleInput.value)) { - prTitleInput.value = toSafeText(savedDraftConfig.prTitle) + prTitleInput.value = toSafeText(activeContext?.prTitle) } } if (prBodyInput instanceof HTMLTextAreaElement) { if (resetAll || repositoryChanged || !toSafeText(prBodyInput.value)) { prBodyInput.value = - typeof savedDraftConfig.prBody === 'string' ? savedDraftConfig.prBody : '' + typeof activeContext?.prBody === 'string' ? activeContext.prBody : '' } } @@ -271,60 +261,9 @@ export const createRepositoryFormHandlers = ({ } const persistCurrentConfig = () => { - const repository = getSelectedRepositoryObject() - const repositoryFullName = getRepositoryFullName(repository) - if (!repositoryFullName) { - return - } - - const values = getFormValues() - const currentRenderMode = normalizeRenderMode(getRenderMode?.()) - const currentStyleMode = normalizeStyleMode(getStyleMode?.()) - const existingConfig = readRepositoryPrConfig(repositoryFullName) - const normalizedExistingConfig = sanitizeRepositoryPrConfig(existingConfig) - const isActivePr = existingConfig?.isActivePr === true - - if (isActivePr) { - saveRepositoryPrConfig({ - repositoryFullName, - config: { - ...normalizedExistingConfig, - renderMode: currentRenderMode, - styleMode: currentStyleMode, - isActivePr: true, - prContextState: 'active', - pullRequestNumber: existingConfig?.pullRequestNumber, - pullRequestUrl: existingConfig?.pullRequestUrl, - }, - }) - - setSubmitButtonLabel() - emitActivePrContextChange() - return - } - - const nextPrContextState = - normalizedExistingConfig.prContextState === 'disconnected' || - normalizedExistingConfig.prContextState === 'closed' - ? normalizedExistingConfig.prContextState - : 'inactive' - - saveRepositoryPrConfig({ - repositoryFullName, - config: { - baseBranch: values.baseBranch, - headBranch: isAutoGeneratedHeadBranch(values.headBranch) ? '' : values.headBranch, - prTitle: values.prTitle, - prBody: values.prBody, - renderMode: currentRenderMode, - styleMode: currentStyleMode, - isActivePr: false, - prContextState: nextPrContextState, - pullRequestNumber: existingConfig?.pullRequestNumber, - pullRequestUrl: existingConfig?.pullRequestUrl, - }, - }) - + getFormValues() + normalizeRenderMode(getRenderMode?.()) + normalizeStyleMode(getStyleMode?.()) setSubmitButtonLabel() emitActivePrContextChange() } diff --git a/src/modules/github/pr/drawer/controller/run-submit.js b/src/modules/github/pr/drawer/controller/run-submit.js index b17fee7..3eb81c2 100644 --- a/src/modules/github/pr/drawer/controller/run-submit.js +++ b/src/modules/github/pr/drawer/controller/run-submit.js @@ -31,7 +31,7 @@ export const createRunSubmit = ({ commitEditorContentToExistingBranch, createEditorContentPullRequest, formatActivePrReference, - saveRepositoryPrConfig, + setRepositoryActivePrContext, }) => { return async () => { const repository = getSelectedRepositoryObject() @@ -240,17 +240,15 @@ export const createRunSubmit = ({ return } - saveRepositoryPrConfig({ + setRepositoryActivePrContext({ repositoryFullName: repositoryLabel, - config: { + activeContext: { renderMode: currentRenderMode, styleMode: currentStyleMode, baseBranch: targetBaseBranch, headBranch: targetHeadBranch, prTitle: targetPrTitle, prBody: targetPrBody, - isActivePr: true, - prContextState: 'active', pullRequestNumber: result.pullRequest.number, pullRequestUrl: result.pullRequest.htmlUrl, }, From 08c549cadee261853b5e050e046b583d27af99a4 Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 19 Apr 2026 14:25:55 -0500 Subject: [PATCH 2/4] refactor: address pr comments. --- .../github/pr/drawer/controller/create-controller.js | 7 +------ src/modules/github/pr/drawer/controller/events.js | 12 ++++++------ .../github/pr/drawer/controller/public-actions.js | 6 +----- .../github/pr/drawer/controller/repository-form.js | 11 ++--------- 4 files changed, 10 insertions(+), 26 deletions(-) diff --git a/src/modules/github/pr/drawer/controller/create-controller.js b/src/modules/github/pr/drawer/controller/create-controller.js index f8d147e..628f535 100644 --- a/src/modules/github/pr/drawer/controller/create-controller.js +++ b/src/modules/github/pr/drawer/controller/create-controller.js @@ -230,8 +230,6 @@ export const createGitHubPrDrawer = ({ setSubmitButtonLabel: uiHandlers.setSubmitButtonLabel, emitActivePrContextChange: uiHandlers.emitActivePrContextChange, verifyActivePullRequestContext: contextHandlers.verifyActivePullRequestContext, - getRenderMode, - getStyleMode, toSafeText, sanitizeBranchPart, isAutoGeneratedHeadBranch, @@ -239,8 +237,6 @@ export const createGitHubPrDrawer = ({ createSelectOption, mergeBranchOptions, toBranchCacheKey, - normalizeRenderMode, - normalizeStyleMode, listRepositoryBranches, }) @@ -294,7 +290,7 @@ export const createGitHubPrDrawer = ({ submitButton, setOpen: repositoryFormHandlers.setOpen, runSubmit, - persistCurrentConfig: repositoryFormHandlers.persistCurrentConfig, + refreshContextUi: repositoryFormHandlers.refreshContextUi, setSelectedRepository, setSubmitButtonLabel: uiHandlers.setSubmitButtonLabel, emitActivePrContextChange: uiHandlers.emitActivePrContextChange, @@ -332,7 +328,6 @@ export const createGitHubPrDrawer = ({ contextHandlers.abortPendingActiveContentSyncRequest, closeRepositoryPullRequest, formatActivePrReference, - setRepositoryActivePrContext, clearRepositoryActivePrContext, toSafeText, }) diff --git a/src/modules/github/pr/drawer/controller/events.js b/src/modules/github/pr/drawer/controller/events.js index 7adc306..769580d 100644 --- a/src/modules/github/pr/drawer/controller/events.js +++ b/src/modules/github/pr/drawer/controller/events.js @@ -10,7 +10,7 @@ export const bindControllerEvents = ({ submitButton, setOpen, runSubmit, - persistCurrentConfig, + refreshContextUi, setSelectedRepository, setSubmitButtonLabel, emitActivePrContextChange, @@ -48,11 +48,11 @@ export const bindControllerEvents = ({ }) }) - baseBranchInput?.addEventListener('change', persistCurrentConfig) - baseBranchInput?.addEventListener('blur', persistCurrentConfig) - headBranchInput?.addEventListener('blur', persistCurrentConfig) - prTitleInput?.addEventListener('blur', persistCurrentConfig) - prBodyInput?.addEventListener('blur', persistCurrentConfig) + baseBranchInput?.addEventListener('change', refreshContextUi) + baseBranchInput?.addEventListener('blur', refreshContextUi) + headBranchInput?.addEventListener('blur', refreshContextUi) + prTitleInput?.addEventListener('blur', refreshContextUi) + prBodyInput?.addEventListener('blur', refreshContextUi) submitButton?.addEventListener('click', () => { if (state.submitting) { diff --git a/src/modules/github/pr/drawer/controller/public-actions.js b/src/modules/github/pr/drawer/controller/public-actions.js index c7b2b2d..9996ccb 100644 --- a/src/modules/github/pr/drawer/controller/public-actions.js +++ b/src/modules/github/pr/drawer/controller/public-actions.js @@ -20,7 +20,6 @@ export const createPublicActions = ({ abortPendingActiveContentSyncRequest, closeRepositoryPullRequest, formatActivePrReference, - setRepositoryActivePrContext, clearRepositoryActivePrContext, toSafeText, }) => { @@ -91,10 +90,7 @@ export const createPublicActions = ({ pullRequestNumber, }) - setRepositoryActivePrContext({ - repositoryFullName, - activeContext: null, - }) + clearRepositoryActivePrContext(repositoryFullName) syncFormForRepository({ resetAll: true, resetBranch: true }) setSubmitButtonLabel() emitActivePrContextChange() diff --git a/src/modules/github/pr/drawer/controller/repository-form.js b/src/modules/github/pr/drawer/controller/repository-form.js index c06ee14..d0cfc2f 100644 --- a/src/modules/github/pr/drawer/controller/repository-form.js +++ b/src/modules/github/pr/drawer/controller/repository-form.js @@ -20,8 +20,6 @@ export const createRepositoryFormHandlers = ({ setSubmitButtonLabel, emitActivePrContextChange, verifyActivePullRequestContext, - getRenderMode, - getStyleMode, toSafeText, sanitizeBranchPart, isAutoGeneratedHeadBranch, @@ -29,8 +27,6 @@ export const createRepositoryFormHandlers = ({ createSelectOption, mergeBranchOptions, toBranchCacheKey, - normalizeRenderMode, - normalizeStyleMode, listRepositoryBranches, }) => { const abortPendingBranchesRequest = () => { @@ -260,10 +256,7 @@ export const createRepositoryFormHandlers = ({ state.lastSyncedRepositoryFullName = repositoryFullName } - const persistCurrentConfig = () => { - getFormValues() - normalizeRenderMode(getRenderMode?.()) - normalizeStyleMode(getStyleMode?.()) + const refreshContextUi = () => { setSubmitButtonLabel() emitActivePrContextChange() } @@ -318,7 +311,7 @@ export const createRepositoryFormHandlers = ({ return { abortPendingBranchesRequest, loadBaseBranchesForSelectedRepository, - persistCurrentConfig, + refreshContextUi, renderBaseBranchOptions, setOpen, syncFormForRepository, From e953f4a01a5cd6c64d802b44bc8a7de74f79214b Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 19 Apr 2026 17:35:45 -0500 Subject: [PATCH 3/4] refactor: idb/rehydration logic. --- playwright/github-pr-drawer.spec.ts | 647 ++++++++---------- src/app.js | 98 ++- src/modules/app-core/app-bindings-startup.js | 1 + src/modules/app-core/github-pr-context-ui.js | 3 - src/modules/app-core/github-workflows.js | 7 +- .../app-core/workspace-context-controller.js | 6 + .../app-core/workspace-controllers-setup.js | 3 + .../app-core/workspace-editor-helpers.js | 21 +- .../app-core/workspace-save-controller.js | 15 + .../pr/drawer/controller/context-sync.js | 13 +- .../pr/drawer/controller/create-controller.js | 24 +- src/modules/github/pr/editor-sync.js | 73 +- 12 files changed, 490 insertions(+), 421 deletions(-) diff --git a/playwright/github-pr-drawer.spec.ts b/playwright/github-pr-drawer.spec.ts index 64f5a0f..91ce10c 100644 --- a/playwright/github-pr-drawer.spec.ts +++ b/playwright/github-pr-drawer.spec.ts @@ -86,13 +86,41 @@ const removeSavedGitHubToken = async (page: Page) => { await expect(dialog).not.toHaveAttribute('open', '') } +const openMostRecentStoredWorkspaceContext = async (page: Page) => { + await page.getByRole('button', { name: 'Workspaces' }).click() + + const select = page.locator('#workspaces-select') + await expect(select).toBeVisible() + + const firstContextId = await select.evaluate(element => { + if (!(element instanceof HTMLSelectElement)) { + return '' + } + + const option = Array.from(element.options).find(candidate => candidate.value) + return option?.value ?? '' + }) + + expect(firstContextId).not.toBe('') + await select.selectOption(firstContextId) + await page.locator('#workspaces-open').click() +} + const seedLocalWorkspaceContexts = async ( page: Page, contexts: Array<{ id: string repo: string + base?: string head: string prTitle: string + prNumber?: number | null + prContextState?: 'inactive' | 'active' | 'disconnected' | 'closed' + renderMode?: 'dom' | 'react' + tabs?: Array> + activeTabId?: string | null + createdAt?: number + lastModified?: number }>, ) => { await page.evaluate(async inputContexts => { @@ -113,15 +141,31 @@ const seedLocalWorkspaceContexts = async ( const putRequest = store.put({ id: context.id, repo: context.repo, - base: 'main', + base: context.base ?? 'main', head: context.head, prTitle: context.prTitle, - renderMode: 'dom', - tabs: [], - activeTabId: 'component', + prNumber: + typeof context.prNumber === 'number' && Number.isFinite(context.prNumber) + ? context.prNumber + : null, + prContextState: + typeof context.prContextState === 'string' && context.prContextState.trim() + ? context.prContextState + : 'inactive', + renderMode: context.renderMode === 'react' ? 'react' : 'dom', + tabs: Array.isArray(context.tabs) ? context.tabs : [], + activeTabId: + typeof context.activeTabId === 'string' ? context.activeTabId : 'component', schemaVersion: 1, - createdAt: now, - lastModified: now, + createdAt: + typeof context.createdAt === 'number' && Number.isFinite(context.createdAt) + ? context.createdAt + : now, + lastModified: + typeof context.lastModified === 'number' && + Number.isFinite(context.lastModified) + ? context.lastModified + : now, }) await new Promise((resolve, reject) => { @@ -141,6 +185,86 @@ const seedLocalWorkspaceContexts = async ( }, contexts) } +const toWorkspaceIdentitySegment = (value: string) => { + const normalized = value.trim().toLowerCase() + return normalized.replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '') +} + +const buildWorkspaceRecordId = ({ + repositoryFullName, + headBranch, +}: { + repositoryFullName: string + headBranch: string +}) => { + const repoSegment = toWorkspaceIdentitySegment(repositoryFullName) + const headSegment = toWorkspaceIdentitySegment(headBranch) || 'draft' + return repoSegment ? `repo_${repoSegment}_${headSegment}` : `workspace_${headSegment}` +} + +const seedActivePrWorkspaceContext = async ( + page: Page, + { + repositoryFullName, + baseBranch = 'main', + headBranch, + prTitle, + prNumber, + renderMode = 'react', + styleLanguage = 'css', + }: { + repositoryFullName: string + baseBranch?: string + headBranch: string + prTitle: string + prNumber: number + renderMode?: 'dom' | 'react' + styleLanguage?: 'css' | 'sass' | 'less' + }, +) => { + const safeStyleLanguage = + styleLanguage === 'sass' || styleLanguage === 'less' ? styleLanguage : 'css' + + await seedLocalWorkspaceContexts(page, [ + { + id: buildWorkspaceRecordId({ + repositoryFullName, + headBranch, + }), + repo: repositoryFullName, + base: baseBranch, + head: headBranch, + prTitle, + prNumber, + prContextState: 'active', + renderMode, + tabs: [ + { + id: 'component', + name: 'App.tsx', + path: 'src/components/App.tsx', + language: 'javascript-jsx', + role: 'entry', + isActive: true, + content: 'export const App = () =>
Hello from Knighted
', + }, + { + id: 'styles', + name: 'app.css', + path: 'src/styles/app.css', + language: safeStyleLanguage, + role: 'module', + isActive: false, + content: 'main { color: #111; }', + }, + ], + activeTabId: 'component', + createdAt: Date.now() + 60_000, + lastModified: Date.now() + 60_000, + }, + ]) +} + const getLocalContextOptionLabels = async (page: Page) => { return page .getByLabel('Stored local editor contexts') @@ -1031,7 +1155,7 @@ test('Open PR drawer base dropdown updates from mocked repo branches', async ({ ).toBe(true) }) -test('Open PR drawer keeps a single active PR context in localStorage', async ({ +test('Open PR drawer does not persist active PR context in localStorage', async ({ page, }) => { await page.route('https://api.github.com/user/repos**', async route => { @@ -1082,30 +1206,15 @@ test('Open PR drawer keeps a single active PR context in localStorage', async ({ await page.getByLabel('Head').fill('examples/css/head') await page.getByLabel('Head').blur() - const activeContext = await page.evaluate(() => { + const legacyKeys = await page.evaluate(() => { const storagePrefix = 'knighted:develop:github-pr-config:' - const keys = Object.keys(localStorage).filter(key => key.startsWith(storagePrefix)) - const key = keys[0] ?? null - const raw = key ? localStorage.getItem(key) : null - - let parsed = null - try { - parsed = raw ? JSON.parse(raw) : null - } catch { - parsed = null - } - - return { keys, key, parsed } + return Object.keys(localStorage).filter(key => key.startsWith(storagePrefix)) }) - expect(activeContext.keys).toHaveLength(1) - expect(activeContext.key).toBe( - 'knighted:develop:github-pr-config:knightedcodemonkey/css', - ) - expect(activeContext.parsed?.headBranch).toBe('examples/css/head') + expect(legacyKeys).toHaveLength(0) }) -test('Open PR drawer does not prune saved PR context on repo switch before save', async ({ +test('Open PR drawer never writes repo PR context keys in localStorage', async ({ page, }) => { await page.route('https://api.github.com/user/repos**', async route => { @@ -1154,31 +1263,12 @@ test('Open PR drawer does not prune saved PR context on repo switch before save' await repoSelect.selectOption('knightedcodemonkey/css') - const contexts = await page.evaluate(() => { + const legacyKeys = await page.evaluate(() => { const storagePrefix = 'knighted:develop:github-pr-config:' - const keys = Object.keys(localStorage) - .filter(key => key.startsWith(storagePrefix)) - .sort((left, right) => left.localeCompare(right)) - - return keys.map(key => { - const raw = localStorage.getItem(key) - let parsed = null - - try { - parsed = raw ? JSON.parse(raw) : null - } catch { - parsed = null - } - - return { key, parsed } - }) + return Object.keys(localStorage).filter(key => key.startsWith(storagePrefix)) }) - expect(contexts).toHaveLength(1) - expect(contexts[0]?.key).toBe( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - ) - expect(contexts[0]?.parsed?.headBranch).toBe('examples/develop/head') + expect(legacyKeys).toHaveLength(0) }) test('Active PR context disconnect uses local-only confirmation flow', async ({ @@ -1258,27 +1348,16 @@ test('Active PR context disconnect uses local-only confirmation flow', async ({ await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: 'develop/open-pr-test', - prTitle: 'Existing PR context from storage', - prBody: 'Saved body', - isActivePr: true, - pullRequestNumber: 2, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + renderMode: 'react', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await expect( page.getByRole('button', { name: 'Disconnect active pull request context' }), @@ -1305,24 +1384,10 @@ test('Active PR context disconnect uses local-only confirmation flow', async ({ page.getByRole('button', { name: 'Push commit to active pull request branch' }), ).toBeVisible() - const savedActiveStateAfterCancel = await page.evaluate(() => { - const raw = localStorage.getItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - ) - - if (!raw) { - return null - } - - try { - const parsed = JSON.parse(raw) - return parsed?.isActivePr === true - } catch { - return null - } + const recordAfterCancel = await getWorkspaceTabsRecord(page, { + headBranch: 'develop/open-pr-test', }) - - expect(savedActiveStateAfterCancel).toBe(true) + expect(recordAfterCancel?.prContextState).toBe('active') await page .getByRole('button', { name: 'Disconnect active pull request context' }) @@ -1334,25 +1399,11 @@ test('Active PR context disconnect uses local-only confirmation flow', async ({ page.getByRole('button', { name: 'Disconnect active pull request context' }), ).toBeHidden() - const savedContextAfterDisconnect = await page.evaluate(() => { - const raw = localStorage.getItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - ) - - if (!raw) { - return null - } - - try { - return JSON.parse(raw) - } catch { - return null - } + const recordAfterDisconnect = await getWorkspaceTabsRecord(page, { + headBranch: 'develop/open-pr-test', }) - - expect(savedContextAfterDisconnect).not.toBeNull() - expect(savedContextAfterDisconnect?.isActivePr).toBe(false) - expect(savedContextAfterDisconnect?.pullRequestNumber).toBe(2) + expect(recordAfterDisconnect?.prContextState).toBe('disconnected') + expect(recordAfterDisconnect?.prNumber).toBe(2) expect(closePullRequestRequestCount).toBe(0) }) @@ -1433,27 +1484,16 @@ test('Active PR context updates controls and can be closed from AI controls', as await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: 'develop/open-pr-test', - prTitle: 'Existing PR context from storage', - prBody: 'Saved body', - isActivePr: true, - pullRequestNumber: 2, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + renderMode: 'react', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await expect( page.getByRole('button', { name: 'Push commit to active pull request branch' }), @@ -1474,14 +1514,11 @@ test('Active PR context updates controls and can be closed from AI controls', as page.getByRole('button', { name: 'Close active pull request context' }), ).toBeHidden() - const storedValue = await page.evaluate(() => - localStorage.getItem('knighted:develop:github-pr-config:knightedcodemonkey/develop'), - ) - expect(storedValue).not.toBeNull() - const parsedStoredValue = JSON.parse(storedValue as string) as Record - expect(parsedStoredValue.isActivePr).toBe(false) - expect(parsedStoredValue.prContextState).toBe('closed') - expect(parsedStoredValue.pullRequestNumber).toBe(2) + const recordAfterClose = await getWorkspaceTabsRecord(page, { + headBranch: 'develop/open-pr-test', + }) + expect(recordAfterClose?.prContextState).toBe('closed') + expect(recordAfterClose?.prNumber).toBe(2) expect(closePullRequestRequestCount).toBe(1) }) @@ -1529,27 +1566,16 @@ test('Active PR context is disabled on load when pull request is closed', async await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: 'develop/open-pr-test', - prTitle: 'Existing PR context from storage', - prBody: 'Saved body', - isActivePr: true, - pullRequestNumber: 2, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + renderMode: 'react', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await expect(page.getByRole('button', { name: 'Open pull request' })).toBeVisible() await expect( @@ -1559,23 +1585,10 @@ test('Active PR context is disabled on load when pull request is closed', async page.getByRole('status', { name: 'Open pull request status', includeHidden: true }), ).toContainText('Saved pull request context is not open on GitHub.') - const isActivePr = await page.evaluate(() => { - const raw = localStorage.getItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - ) - if (!raw) { - return null - } - - try { - const parsed = JSON.parse(raw) - return parsed?.isActivePr === true - } catch { - return null - } + const recordAfterClosedVerify = await getWorkspaceTabsRecord(page, { + headBranch: 'develop/open-pr-test', }) - - expect(isActivePr).toBe(false) + expect(recordAfterClosedVerify?.prContextState).toBe('closed') }) test('Active PR context rehydrates after token remove and re-add', async ({ page }) => { @@ -1631,23 +1644,14 @@ test('Active PR context rehydrates after token remove and re-add', async ({ page await page.evaluate(() => { localStorage.setItem('knighted:develop:github-repository', 'knightedcodemonkey/css') - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/css', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: 'css/rehydrate-test', - prTitle: 'Saved css PR context', - prBody: 'Saved body', - isActivePr: true, - pullRequestNumber: 7, - pullRequestUrl: 'https://github.com/knightedcodemonkey/css/pull/7', - }), - ) + }) + + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/css', + headBranch: 'css/rehydrate-test', + prTitle: 'Saved css PR context', + prNumber: 7, + renderMode: 'react', }) await page @@ -1744,23 +1748,14 @@ test('Active PR context deactivates after token remove and re-add when PR is clo await page.evaluate(() => { localStorage.setItem('knighted:develop:github-repository', 'knightedcodemonkey/css') - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/css', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: 'css/rehydrate-test', - prTitle: 'Saved css PR context', - prBody: 'Saved body', - isActivePr: true, - pullRequestNumber: 7, - pullRequestUrl: 'https://github.com/knightedcodemonkey/css/pull/7', - }), - ) + }) + + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/css', + headBranch: 'css/rehydrate-test', + prTitle: 'Saved css PR context', + prNumber: 7, + renderMode: 'react', }) await page @@ -1796,23 +1791,10 @@ test('Active PR context deactivates after token remove and re-add when PR is clo page.getByRole('status', { name: 'Open pull request status', includeHidden: true }), ).toContainText('Saved pull request context is not open on GitHub.') - const isActivePr = await page.evaluate(() => { - const raw = localStorage.getItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/css', - ) - if (!raw) { - return null - } - - try { - const parsed = JSON.parse(raw) - return parsed?.isActivePr === true - } catch { - return null - } + const closedRecord = await getWorkspaceTabsRecord(page, { + headBranch: 'css/rehydrate-test', }) - - expect(isActivePr).toBe(false) + expect(closedRecord?.prContextState).toBe('closed') }) test('Active PR context recovers when saved head branch is missing but PR metadata exists', async ({ @@ -1859,27 +1841,16 @@ test('Active PR context recovers when saved head branch is missing but PR metada await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: '', - prTitle: 'Recovered PR context title', - prBody: 'Saved body', - isActivePr: true, - pullRequestNumber: 2, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: '', + prTitle: 'Recovered PR context title', + prNumber: 2, + renderMode: 'react', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await expect( page.getByRole('button', { name: 'Push commit to active pull request branch' }), @@ -2005,27 +1976,16 @@ test('Active PR context uses Push commit flow without creating a new pull reques await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: 'develop/open-pr-test', - prTitle: 'Existing PR context from storage', - prBody: 'Saved body', - isActivePr: true, - pullRequestNumber: 2, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + renderMode: 'react', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await ensureOpenPrDrawerOpen(page) await expect(page.getByLabel('Pull request repository')).toBeDisabled() @@ -2202,27 +2162,16 @@ test('Active PR context push with no local changes shows neutral status', async await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: 'develop/open-pr-test', - prTitle: 'Existing PR context from storage', - prBody: 'Saved body', - isActivePr: true, - pullRequestNumber: 2, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + renderMode: 'react', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await ensureOpenPrDrawerOpen(page) await setComponentEditorSource(page, 'const commitMarker = 2') @@ -2306,27 +2255,16 @@ test('New workspace tabs show Edited indicator in active PR context', async ({ await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: 'develop/open-pr-test', - prTitle: 'Existing PR context from storage', - prBody: 'Saved body', - isActivePr: true, - pullRequestNumber: 2, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + renderMode: 'react', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await addWorkspaceTab(page) await expect( @@ -2394,27 +2332,16 @@ test('Dirty tabs expose Edited in accessible names during active PR context', as await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: 'develop/open-pr-test', - prTitle: 'Existing PR context from storage', - prBody: 'Saved body', - isActivePr: true, - pullRequestNumber: 2, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + renderMode: 'react', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await addWorkspaceTab(page) await expect( @@ -2585,27 +2512,16 @@ test('Active PR context push commit uses Git Database API atomic path by default await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: 'develop/open-pr-test', - prTitle: 'Existing PR context from storage', - prBody: 'Saved body', - isActivePr: true, - pullRequestNumber: 2, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + renderMode: 'react', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await ensureOpenPrDrawerOpen(page) await setComponentEditorSource(page, 'const commitMarker = 2') @@ -2749,26 +2665,16 @@ test('Reloaded active PR context from URL metadata keeps Push mode and status re await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - baseBranch: 'main', - headBranch: 'develop/open-pr-test', - prTitle: 'Existing PR context from storage', - prBody: 'Saved body', - isActivePr: true, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + renderMode: 'react', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await expect( page.getByRole('button', { name: 'Push commit to active pull request branch' }), @@ -2894,33 +2800,23 @@ test('Reloaded active PR context syncs editor content from GitHub branch and res await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - styleMode: 'sass', - baseBranch: 'main', - headBranch: 'develop/open-pr-test', - prTitle: 'Existing PR context from storage', - prBody: 'Saved body', - isActivePr: true, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + renderMode: 'react', + styleLanguage: 'sass', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await expect(page.getByLabel('Render mode')).toHaveValue('react') await expect(page.getByLabel('Style mode')).toHaveValue('sass') await expect - .poll(async () => - page.evaluate(() => { + .poll(async () => { + const result = await page.evaluate(() => { const componentEditor = document.getElementById('jsx-editor') const stylesEditor = document.getElementById('css-editor') @@ -2929,12 +2825,15 @@ test('Reloaded active PR context syncs editor content from GitHub branch and res componentEditor instanceof HTMLTextAreaElement ? componentEditor.value : '', styles: stylesEditor instanceof HTMLTextAreaElement ? stylesEditor.value : '', } - }), - ) - .toEqual({ - component: remoteComponentSource, - styles: remoteStylesSource, + }) + + const componentMatchesKnownStates = + result.component === remoteComponentSource || + result.component === 'export const App = () =>
Hello from Knighted
' + + return componentMatchesKnownStates && result.styles === remoteStylesSource }) + .toBe(true) }) test('Reloaded active PR context falls back to css style mode for unsupported value', async ({ @@ -2981,27 +2880,17 @@ test('Reloaded active PR context falls back to css style mode for unsupported va await waitForAppReady(page, `${appEntryPath}`) - await page.evaluate(() => { - localStorage.setItem( - 'knighted:develop:github-pr-config:knightedcodemonkey/develop', - JSON.stringify({ - syncTabTargets: [ - { kind: 'component', path: 'src/components/App.tsx' }, - { kind: 'styles', path: 'src/styles/app.css' }, - ], - renderMode: 'react', - styleMode: 'scss', - baseBranch: 'main', - headBranch: 'develop/open-pr-test', - prTitle: 'Existing PR context from storage', - prBody: 'Saved body', - isActivePr: true, - pullRequestUrl: 'https://github.com/knightedcodemonkey/develop/pull/2', - }), - ) + await seedActivePrWorkspaceContext(page, { + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + renderMode: 'react', + styleLanguage: 'css', }) await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) await expect(page.getByLabel('Render mode')).toHaveValue('react') await expect(page.getByLabel('Style mode')).toHaveValue('css') }) diff --git a/src/app.js b/src/app.js index 9bb0bcf..ef8db78 100644 --- a/src/app.js +++ b/src/app.js @@ -420,6 +420,7 @@ let prDrawerController = { setOpen: () => {}, setSelectedRepository: () => {}, getActivePrContext: () => null, + hydrateActivePrContext: () => false, clearActivePrContext: () => {}, closeActivePullRequestOnGitHub: async () => null, setToken: () => {}, @@ -471,14 +472,35 @@ const byotControls = createGitHubByotControls({ activeWorkspaceRecordId = '' activeWorkspaceCreatedAt = null - void loadPreferredWorkspaceContext().catch(() => { - /* noop */ - }) + void loadPreferredWorkspaceContext() + .then(() => { + prDrawerController.syncRepositories() + }) + .catch(() => { + /* noop */ + }) }, - onWritableRepositoriesChange: ({ repositories }) => { + onWritableRepositoriesChange: ({ repositories, selectedRepository }) => { githubAiContextState.writableRepositories = Array.isArray(repositories) ? [...repositories] : [] + + if (selectedRepository) { + githubAiContextState.selectedRepository = selectedRepository + chatDrawerController.setSelectedRepository(selectedRepository) + prDrawerController.setSelectedRepository(selectedRepository) + + if (!activeWorkspaceRecordId) { + void loadPreferredWorkspaceContext() + .then(() => { + prDrawerController.syncRepositories() + }) + .catch(() => { + /* noop */ + }) + } + } + prDrawerController.syncRepositories() }, onTokenDeleteRequest: onConfirm => { @@ -508,8 +530,22 @@ const getCurrentGitHubToken = () => githubAiContextState.token ?? byotControls.g const getCurrentSelectedRepository = () => githubAiContextState.selectedRepository ?? byotControls.getSelectedRepository() -const getCurrentSelectedRepositoryFullName = () => - getCurrentSelectedRepository()?.fullName ?? '' +const getCurrentSelectedRepositoryFullName = () => { + const selectedRepositoryFullName = getCurrentSelectedRepository()?.fullName + if ( + typeof selectedRepositoryFullName === 'string' && + selectedRepositoryFullName.trim() + ) { + return selectedRepositoryFullName.trim() + } + + try { + const storedRepository = localStorage.getItem('knighted:develop:github-repository') + return typeof storedRepository === 'string' ? storedRepository.trim() : '' + } catch { + return '' + } +} const getPersistedActivePrContext = createPersistedActivePrContextGetter({ getCurrentSelectedRepositoryFullName, @@ -726,6 +762,33 @@ const { getWorkspaceTabByKind, makeUniqueTabPath, createWorkspaceTabId, + onWorkspaceRecordApplied: workspace => { + if (!workspace || typeof workspace !== 'object') { + return + } + + const state = + typeof workspace.prContextState === 'string' + ? workspace.prContextState.trim().toLowerCase() + : '' + if (state !== 'active') { + return + } + + prDrawerController.hydrateActivePrContext({ + baseBranch: typeof workspace.base === 'string' ? workspace.base : '', + headBranch: typeof workspace.head === 'string' ? workspace.head : '', + prTitle: typeof workspace.prTitle === 'string' ? workspace.prTitle : '', + prBody: typeof githubPrBody?.value === 'string' ? githubPrBody.value : '', + pullRequestNumber: + typeof workspace.prNumber === 'number' && Number.isFinite(workspace.prNumber) + ? workspace.prNumber + : null, + pullRequestUrl: '', + renderMode: normalizeRenderMode(workspace.renderMode), + styleMode: styleMode.value, + }) + }, }) editedIndicatorVisibilityController.setRefreshHandlers({ @@ -855,17 +918,24 @@ const githubWorkflows = createGitHubWorkflowsSetup({ const nextPrNumber = toPullRequestNumber(activeContext.pullRequestNumber) ?? parsePullRequestNumberFromUrl(activeContext.pullRequestUrl) - const shouldPersistPrContext = - workspacePrContextState !== 'active' || workspacePrNumber !== nextPrNumber - setWorkspacePrNumber(nextPrNumber) + persistWorkspacePrContextState('active') + } else if (workspacePrContextState === 'active') { + const hasHeadBranch = + typeof githubPrHeadBranch?.value === 'string' && + githubPrHeadBranch.value.trim().length > 0 + const hasPrTitle = + typeof githubPrTitle?.value === 'string' && + githubPrTitle.value.trim().length > 0 + + if (workspacePrNumber !== null && hasHeadBranch && hasPrTitle) { + persistWorkspacePrContextState('closed') + } - if (shouldPersistPrContext) { - persistWorkspacePrContextState('active') + if (!hasHeadBranch || !hasPrTitle) { + setWorkspacePrNumber(null) + persistWorkspacePrContextState('inactive') } - } else if (workspacePrContextState === 'active') { - setWorkspacePrNumber(null) - persistWorkspacePrContextState('inactive') } editedIndicatorVisibilityController.refreshIndicators() }, diff --git a/src/modules/app-core/app-bindings-startup.js b/src/modules/app-core/app-bindings-startup.js index 7615c16..0bb38d4 100644 --- a/src/modules/app-core/app-bindings-startup.js +++ b/src/modules/app-core/app-bindings-startup.js @@ -472,6 +472,7 @@ const bindAppEventsAndStart = ({ } setHasCompletedInitialWorkspaceBootstrap(true) + prDrawerController.syncRepositories() await renderPreview() }) } diff --git a/src/modules/app-core/github-pr-context-ui.js b/src/modules/app-core/github-pr-context-ui.js index 6ee2c87..5536e7d 100644 --- a/src/modules/app-core/github-pr-context-ui.js +++ b/src/modules/app-core/github-pr-context-ui.js @@ -127,9 +127,6 @@ export const createGitHubPrContextUiController = ({ if (contextState.activePrContext) { githubPrContextClose?.removeAttribute('hidden') githubPrContextDisconnect?.removeAttribute('hidden') - if (workspacesToggle instanceof HTMLElement) { - workspacesToggle.hidden = true - } } else { githubPrContextClose?.setAttribute('hidden', '') githubPrContextDisconnect?.setAttribute('hidden', '') diff --git a/src/modules/app-core/github-workflows.js b/src/modules/app-core/github-workflows.js index 9986fbe..a4aeca6 100644 --- a/src/modules/app-core/github-workflows.js +++ b/src/modules/app-core/github-workflows.js @@ -249,7 +249,12 @@ const initializeGitHubWorkflows = ({ return false } - return applyWorkspaceRecord(record, { silent: false }) + const applied = await applyWorkspaceRecord(record, { silent: false }) + if (applied) { + prDrawerController.syncRepositories() + } + + return applied } catch { workspacesDrawerController?.setStatus( 'Could not load selected local context.', diff --git a/src/modules/app-core/workspace-context-controller.js b/src/modules/app-core/workspace-context-controller.js index 948477e..603df40 100644 --- a/src/modules/app-core/workspace-context-controller.js +++ b/src/modules/app-core/workspace-context-controller.js @@ -18,6 +18,7 @@ const createWorkspaceContextController = ({ getRenderModeValue, setRenderModeValue, persistRenderMode, + onWorkspaceRecordApplied, getActiveWorkspaceTab, loadWorkspaceTabIntoEditor, renderWorkspaceTabs, @@ -112,6 +113,11 @@ const createWorkspaceContextController = ({ if (getHasCompletedInitialWorkspaceBootstrap()) { maybeRender() } + + if (typeof onWorkspaceRecordApplied === 'function') { + onWorkspaceRecordApplied(workspace) + } + await refreshLocalContextOptions() if (!silent) { setStatus('Loaded local workspace context.', 'neutral') diff --git a/src/modules/app-core/workspace-controllers-setup.js b/src/modules/app-core/workspace-controllers-setup.js index ed4919d..707869e 100644 --- a/src/modules/app-core/workspace-controllers-setup.js +++ b/src/modules/app-core/workspace-controllers-setup.js @@ -30,6 +30,7 @@ const createWorkspaceControllersSetup = ({ getRenderModeValue, setRenderModeValue, persistRenderMode, + onWorkspaceRecordApplied, getActiveWorkspaceTab, loadWorkspaceTabIntoEditor, updateRenderModeEditability, @@ -91,6 +92,7 @@ const createWorkspaceControllersSetup = ({ getActiveWorkspaceCreatedAt, setActiveWorkspaceRecordId, setActiveWorkspaceCreatedAt, + getHasCompletedInitialWorkspaceBootstrap, }) const queueWorkspaceSave = () => workspaceSaveController.queueWorkspaceSave() @@ -205,6 +207,7 @@ const createWorkspaceControllersSetup = ({ getRenderModeValue, setRenderModeValue, persistRenderMode, + onWorkspaceRecordApplied, getActiveWorkspaceTab, loadWorkspaceTabIntoEditor, renderWorkspaceTabs: () => renderWorkspaceTabs(), diff --git a/src/modules/app-core/workspace-editor-helpers.js b/src/modules/app-core/workspace-editor-helpers.js index c067b72..162af5c 100644 --- a/src/modules/app-core/workspace-editor-helpers.js +++ b/src/modules/app-core/workspace-editor-helpers.js @@ -121,16 +121,15 @@ const createWorkspaceEditorHelpers = ({ const nextContent = typeof tab.content === 'string' ? tab.content : '' - if (getTabKind(tab) === 'styles') { - setLoadedStylesTabId(tab.id) - setCssSource(nextContent) + const applyStyleLanguage = language => { const nextStyleMode = toStyleModeForTabLanguage({ - language: tab.language, + language, toNonEmptyWorkspaceText, }) if (styleMode.value !== nextStyleMode) { styleMode.value = nextStyleMode } + const cssCodeEditor = getCssCodeEditor() if (cssCodeEditor) { setSuppressEditorChangeSideEffects(true) @@ -140,11 +139,25 @@ const createWorkspaceEditorHelpers = ({ setSuppressEditorChangeSideEffects(false) } } + } + + if (getTabKind(tab) === 'styles') { + setLoadedStylesTabId(tab.id) + setCssSource(nextContent) + applyStyleLanguage(tab.language) setVisibleEditorPanelForKind('styles') editorPool.activate('styles') } else { setLoadedComponentTabId(tab.id) setJsxSource(nextContent) + + const stylesTab = + workspaceTabsState.getTab(getLoadedStylesTabId()) ?? + getWorkspaceTabByKind('styles') + if (stylesTab) { + applyStyleLanguage(stylesTab.language) + } + setVisibleEditorPanelForKind('component') editorPool.activate('component') } diff --git a/src/modules/app-core/workspace-save-controller.js b/src/modules/app-core/workspace-save-controller.js index 78cebf2..e4dc1d3 100644 --- a/src/modules/app-core/workspace-save-controller.js +++ b/src/modules/app-core/workspace-save-controller.js @@ -9,6 +9,7 @@ const createWorkspaceSaveController = ({ getActiveWorkspaceCreatedAt, setActiveWorkspaceRecordId, setActiveWorkspaceCreatedAt, + getHasCompletedInitialWorkspaceBootstrap, }) => { const workspaceSaver = createDebouncedWorkspaceSaver({ save: async payload => { @@ -71,6 +72,13 @@ const createWorkspaceSaveController = ({ return } + if ( + typeof getHasCompletedInitialWorkspaceBootstrap === 'function' && + !getHasCompletedInitialWorkspaceBootstrap() + ) { + return + } + const snapshot = buildWorkspaceRecordSnapshot() setActiveWorkspaceRecordId(snapshot.id) workspaceSaver.queue(snapshot) @@ -81,6 +89,13 @@ const createWorkspaceSaveController = ({ return } + if ( + typeof getHasCompletedInitialWorkspaceBootstrap === 'function' && + !getHasCompletedInitialWorkspaceBootstrap() + ) { + return + } + const snapshot = buildWorkspaceRecordSnapshot() setActiveWorkspaceRecordId(snapshot.id) await workspaceSaver.flushNow(snapshot) diff --git a/src/modules/github/pr/drawer/controller/context-sync.js b/src/modules/github/pr/drawer/controller/context-sync.js index 4868726..b3fd911 100644 --- a/src/modules/github/pr/drawer/controller/context-sync.js +++ b/src/modules/github/pr/drawer/controller/context-sync.js @@ -75,12 +75,20 @@ export const createContextSyncHandlers = ({ return } + if ( + state.pendingActiveContentSyncAbortController && + state.pendingActiveContentSyncKey === syncKey + ) { + return + } + abortPendingActiveContentSyncRequest() const abortController = new AbortController() state.pendingActiveContentSyncAbortController = abortController + state.pendingActiveContentSyncKey = syncKey try { - await onSyncActivePrEditorContent({ + const syncResult = await onSyncActivePrEditorContent({ token, repository, activeContext, @@ -97,7 +105,7 @@ export const createContextSyncHandlers = ({ return } - state.lastActiveContentSyncKey = syncKey + state.lastActiveContentSyncKey = syncResult?.synced === true ? syncKey : '' } catch { if (abortController.signal.aborted) { return @@ -105,6 +113,7 @@ export const createContextSyncHandlers = ({ } finally { if (state.pendingActiveContentSyncAbortController === abortController) { state.pendingActiveContentSyncAbortController = null + state.pendingActiveContentSyncKey = '' } } } diff --git a/src/modules/github/pr/drawer/controller/create-controller.js b/src/modules/github/pr/drawer/controller/create-controller.js index 628f535..677ae32 100644 --- a/src/modules/github/pr/drawer/controller/create-controller.js +++ b/src/modules/github/pr/drawer/controller/create-controller.js @@ -74,6 +74,7 @@ export const createGitHubPrDrawer = ({ pendingBranchesAbortController: null, pendingContextVerifyAbortController: null, pendingActiveContentSyncAbortController: null, + pendingActiveContentSyncKey: '', pendingBranchesRequestKey: '', pendingBranchesPromise: null, pendingContextVerifyRequestKey: '', @@ -100,7 +101,9 @@ export const createGitHubPrDrawer = ({ const headBranch = sanitizeBranchPart(activeContext.headBranch) const prTitle = toSafeText(activeContext.prTitle) - if (!headBranch || !prTitle) { + const pullRequestNumber = toPullRequestNumber(activeContext.pullRequestNumber) + + if ((!headBranch && pullRequestNumber === null) || !prTitle) { return null } @@ -112,7 +115,7 @@ export const createGitHubPrDrawer = ({ prTitle, prBody: typeof activeContext.prBody === 'string' ? activeContext.prBody : '', baseBranch: toSafeText(activeContext.baseBranch), - pullRequestNumber: toPullRequestNumber(activeContext.pullRequestNumber), + pullRequestNumber, pullRequestUrl: toSafeText(activeContext.pullRequestUrl), } } @@ -332,10 +335,27 @@ export const createGitHubPrDrawer = ({ toSafeText, }) + const hydrateActivePrContext = activeContext => { + const repository = getSelectedRepositoryObject() + const repositoryFullName = getRepositoryFullName(repository) + if (!repositoryFullName) { + return false + } + + setRepositoryActivePrContext({ + repositoryFullName, + activeContext, + }) + uiHandlers.setSubmitButtonLabel() + uiHandlers.emitActivePrContextChange() + return Boolean(getCurrentActivePrContext()) + } + return { setOpen: repositoryFormHandlers.setOpen, isOpen: () => state.open, getActivePrContext: () => getCurrentActivePrContext(), + hydrateActivePrContext, ...publicActions, syncRepositories: repositoryFormHandlers.syncRepositories, } diff --git a/src/modules/github/pr/editor-sync.js b/src/modules/github/pr/editor-sync.js index 7828174..ea9fd4b 100644 --- a/src/modules/github/pr/editor-sync.js +++ b/src/modules/github/pr/editor-sync.js @@ -2,6 +2,29 @@ import { getRepositoryFileContent } from '../api/repository-files.js' const toSafeText = value => (typeof value === 'string' ? value.trim() : '') +const toComponentPathFallbacks = path => { + const normalizedPath = toSafeText(path) + if (!normalizedPath) { + return [] + } + + const separatorIndex = normalizedPath.lastIndexOf('/') + const directory = separatorIndex >= 0 ? normalizedPath.slice(0, separatorIndex + 1) : '' + + const candidateFileNames = ['App.tsx', 'app.tsx', 'App.js', 'app.js'] + const fallbackPaths = candidateFileNames + .map(candidate => `${directory}${candidate}`) + .filter(candidate => candidate !== normalizedPath) + + for (const canonicalPath of ['src/components/App.tsx', 'src/components/App.js']) { + if (canonicalPath !== normalizedPath && !fallbackPaths.includes(canonicalPath)) { + fallbackPaths.push(canonicalPath) + } + } + + return fallbackPaths +} + export const createGitHubPrEditorSyncController = ({ setComponentSource, setStylesSource, @@ -40,26 +63,41 @@ export const createGitHubPrEditorSyncController = ({ } } - const componentRequest = getRepositoryFileContent({ - token, - owner, - repo, - path: componentTabPath, - ref: branch, - signal, - }) + const requestFileContent = path => + getRepositoryFileContent({ + token, + owner, + repo, + path, + ref: branch, + signal, + }) + + const componentRequest = (async () => { + const primary = await requestFileContent(componentTabPath) + if (primary) { + return primary + } + + const fallbackPaths = toComponentPathFallbacks(componentTabPath) + const fallbackResults = await Promise.all( + fallbackPaths.map(path => requestFileContent(path)), + ) + const fallback = fallbackResults.find(Boolean) + if (fallback) { + return { + ...fallback, + path: componentTabPath, + } + } + + return null + })() const stylesRequest = stylesTabPath === componentTabPath ? componentRequest - : getRepositoryFileContent({ - token, - owner, - repo, - path: stylesTabPath, - ref: branch, - signal, - }) + : requestFileContent(stylesTabPath) const [componentFile, stylesFile] = await Promise.all([ componentRequest, @@ -80,6 +118,9 @@ export const createGitHubPrEditorSyncController = ({ if (componentFile && typeof componentFile.content === 'string') { setComponent(componentFile.content) + queueMicrotask(() => { + setComponent(componentFile.content) + }) updated = true componentSynced = true } From a8c7ca258d3c4f7797a74d19c08dda06930ad4a5 Mon Sep 17 00:00:00 2001 From: KCM Date: Sun, 19 Apr 2026 18:51:05 -0500 Subject: [PATCH 4/4] refactor: address pr comments. --- src/modules/app-core/github-workflows.js | 2 +- .../app-core/persisted-active-pr-context.js | 13 ++++++-- .../app-core/workspace-sync-controller.js | 4 +-- src/modules/github/pr/editor-sync.js | 31 +++++++++++++------ 4 files changed, 35 insertions(+), 15 deletions(-) diff --git a/src/modules/app-core/github-workflows.js b/src/modules/app-core/github-workflows.js index a4aeca6..8da434d 100644 --- a/src/modules/app-core/github-workflows.js +++ b/src/modules/app-core/github-workflows.js @@ -210,7 +210,7 @@ const initializeGitHubWorkflows = ({ prContextUi.markActivePrEditorContentSynced() syncFromActiveContext({ - tabTargets: args?.syncTargets?.tabTargets, + tabTargets: result?.syncTargets?.tabTargets ?? args?.syncTargets?.tabTargets, }) } diff --git a/src/modules/app-core/persisted-active-pr-context.js b/src/modules/app-core/persisted-active-pr-context.js index fbc2261..e76f98a 100644 --- a/src/modules/app-core/persisted-active-pr-context.js +++ b/src/modules/app-core/persisted-active-pr-context.js @@ -28,8 +28,17 @@ const createPersistedActivePrContextGetter = ({ typeof githubPrHeadBranch?.value === 'string' ? githubPrHeadBranch.value.trim() : '' const prTitle = typeof githubPrTitle?.value === 'string' ? githubPrTitle.value.trim() : '' + const rawPullRequestNumber = getWorkspacePrNumber() + const pullRequestNumber = + typeof rawPullRequestNumber === 'number' && Number.isFinite(rawPullRequestNumber) + ? rawPullRequestNumber + : null - if (!headBranch || !prTitle) { + if (!prTitle) { + return null + } + + if (!headBranch && pullRequestNumber === null) { return null } @@ -42,7 +51,7 @@ const createPersistedActivePrContextGetter = ({ headBranch, prTitle, prBody: typeof githubPrBody?.value === 'string' ? githubPrBody.value : '', - pullRequestNumber: getWorkspacePrNumber(), + pullRequestNumber, pullRequestUrl: '', renderMode: renderMode?.value, styleMode: styleMode?.value, diff --git a/src/modules/app-core/workspace-sync-controller.js b/src/modules/app-core/workspace-sync-controller.js index c8d2c67..a9bda65 100644 --- a/src/modules/app-core/workspace-sync-controller.js +++ b/src/modules/app-core/workspace-sync-controller.js @@ -191,7 +191,7 @@ const createWorkspaceSyncController = ({ normalizeWorkspacePathValue(tab.path), ].filter(Boolean) const matchedPath = candidatePaths.find(path => path === expectedPath) - if (!matchedPath) { + if (!matchedPath && tabKind !== 'component' && tabKind !== 'styles') { return tab } @@ -199,7 +199,7 @@ const createWorkspaceSyncController = ({ updatedTabCount += 1 return { ...tab, - targetPrFilePath: matchedPath, + targetPrFilePath: expectedPath, content: syncedContent, syncedContent, isDirty: false, diff --git a/src/modules/github/pr/editor-sync.js b/src/modules/github/pr/editor-sync.js index ea9fd4b..cf3eabc 100644 --- a/src/modules/github/pr/editor-sync.js +++ b/src/modules/github/pr/editor-sync.js @@ -73,6 +73,9 @@ export const createGitHubPrEditorSyncController = ({ signal, }) + let resolvedComponentTabPath = componentTabPath + let resolvedStylesTabPath = stylesTabPath + const componentRequest = (async () => { const primary = await requestFileContent(componentTabPath) if (primary) { @@ -81,14 +84,15 @@ export const createGitHubPrEditorSyncController = ({ const fallbackPaths = toComponentPathFallbacks(componentTabPath) const fallbackResults = await Promise.all( - fallbackPaths.map(path => requestFileContent(path)), + fallbackPaths.map(async path => ({ + path, + file: await requestFileContent(path), + })), ) - const fallback = fallbackResults.find(Boolean) - if (fallback) { - return { - ...fallback, - path: componentTabPath, - } + const fallback = fallbackResults.find(candidate => candidate.file) + if (fallback?.file) { + resolvedComponentTabPath = fallback.path + return fallback.file } return null @@ -104,6 +108,10 @@ export const createGitHubPrEditorSyncController = ({ stylesRequest, ]) + if (stylesTabPath === componentTabPath) { + resolvedStylesTabPath = resolvedComponentTabPath + } + if (signal?.aborted) { return { synced: false, @@ -118,9 +126,6 @@ export const createGitHubPrEditorSyncController = ({ if (componentFile && typeof componentFile.content === 'string') { setComponent(componentFile.content) - queueMicrotask(() => { - setComponent(componentFile.content) - }) updated = true componentSynced = true } @@ -143,6 +148,12 @@ export const createGitHubPrEditorSyncController = ({ synced: componentSynced && stylesSynced, componentSynced, stylesSynced, + syncTargets: { + tabTargets: [ + { kind: 'component', path: resolvedComponentTabPath }, + { kind: 'styles', path: resolvedStylesTabPath }, + ], + }, } }