diff --git a/.github/workflows/playwright.yml b/.github/workflows/playwright.yml index b3a1a72..6073249 100644 --- a/.github/workflows/playwright.yml +++ b/.github/workflows/playwright.yml @@ -4,7 +4,6 @@ on: pull_request: branches: - main - - next types: - opened - synchronize diff --git a/playwright/github-pr-drawer/active-context-sync.spec.ts b/playwright/github-pr-drawer/active-context-sync.spec.ts index 87f4b4f..89c3c55 100644 --- a/playwright/github-pr-drawer/active-context-sync.spec.ts +++ b/playwright/github-pr-drawer/active-context-sync.spec.ts @@ -433,6 +433,290 @@ test('Renaming a synced module tab keeps plain tab label and includes renamed pa }) }) +test('Removing a synced module tab includes a delete entry when pushing to an active PR', async ({ + page, +}) => { + const treeRequests: Array> = [] + + await page.route('https://api.github.com/user/repos**', async route => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify([ + { + id: 11, + owner: { login: 'knightedcodemonkey' }, + name: 'develop', + full_name: 'knightedcodemonkey/develop', + default_branch: 'main', + permissions: { push: true }, + }, + ]), + }) + }) + + await mockRepositoryBranches(page, { + 'knightedcodemonkey/develop': ['main', 'release', 'develop/open-pr-test'], + }) + + await page.route( + 'https://api.github.com/repos/knightedcodemonkey/develop/pulls/2', + async route => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + number: 2, + state: 'open', + title: 'Existing PR context from storage', + html_url: 'https://github.com/knightedcodemonkey/develop/pull/2', + head: { ref: 'develop/open-pr-test' }, + base: { ref: 'main' }, + }), + }) + }, + ) + + await page.route( + 'https://api.github.com/repos/knightedcodemonkey/develop/git/ref/**', + async route => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + ref: 'refs/heads/develop/open-pr-test', + object: { type: 'commit', sha: 'existing-head-sha' }, + }), + }) + }, + ) + + await page.route( + 'https://api.github.com/repos/knightedcodemonkey/develop/git/commits/existing-head-sha', + async route => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + sha: 'existing-head-sha', + tree: { sha: 'base-tree-sha' }, + }), + }) + }, + ) + + await page.route( + 'https://api.github.com/repos/knightedcodemonkey/develop/contents/**', + async route => { + const url = new URL(route.request().url()) + const path = decodeURIComponent(url.pathname.split('/contents/')[1] ?? '').trim() + const responseByPath: Record = { + 'src/components/boop.tsx': { + status: 200, + body: JSON.stringify({ sha: 'boop-existing-sha' }), + }, + } + const response = responseByPath[path] ?? { + status: 404, + body: JSON.stringify({ message: 'Not Found' }), + } + + await route.fulfill({ + status: response.status, + contentType: 'application/json', + body: response.body, + }) + }, + ) + + await page.route( + 'https://api.github.com/repos/knightedcodemonkey/develop/git/trees', + async route => { + treeRequests.push(route.request().postDataJSON() as Record) + + await route.fulfill({ + status: 201, + contentType: 'application/json', + body: JSON.stringify({ sha: 'remove-tree-sha' }), + }) + }, + ) + + await page.route( + 'https://api.github.com/repos/knightedcodemonkey/develop/git/commits', + async route => { + await route.fulfill({ + status: 201, + contentType: 'application/json', + body: JSON.stringify({ sha: 'remove-commit-sha' }), + }) + }, + ) + + await page.route( + 'https://api.github.com/repos/knightedcodemonkey/develop/git/refs/**', + async route => { + await route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ + ref: 'refs/heads/develop/open-pr-test', + object: { type: 'commit', sha: 'remove-commit-sha' }, + }), + }) + }, + ) + + await waitForAppReady(page, `${appEntryPath}`) + + const now = Date.now() + await seedLocalWorkspaceContexts(page, [ + { + id: buildWorkspaceRecordId({ + repositoryFullName: 'knightedcodemonkey/develop', + headBranch: 'develop/open-pr-test', + }), + repo: 'knightedcodemonkey/develop', + base: 'main', + head: 'develop/open-pr-test', + prTitle: 'Existing PR context from storage', + prNumber: 2, + prContextState: 'active', + renderMode: 'react', + 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
', + targetPrFilePath: 'src/components/App.tsx', + syncedContent: 'export const App = () =>
Hello from Knighted
', + syncedAt: now, + isDirty: false, + }, + { + id: 'styles', + name: 'app.css', + path: 'src/styles/app.css', + language: 'css', + role: 'module', + isActive: false, + content: 'main { color: #111; }', + targetPrFilePath: 'src/styles/app.css', + syncedContent: 'main { color: #111; }', + syncedAt: now, + isDirty: false, + }, + { + id: 'boop', + name: 'boop.tsx', + path: 'src/components/boop.tsx', + language: 'javascript-jsx', + role: 'module', + isActive: false, + content: 'export const Boop = () =>

boop

', + targetPrFilePath: 'src/components/boop.tsx', + syncedContent: 'export const Boop = () =>

boop

', + syncedAt: now, + isDirty: false, + }, + ], + activeTabId: 'component', + createdAt: now, + lastModified: now, + }, + ]) + + await connectByotWithSingleRepo(page) + await openMostRecentStoredWorkspaceContext(page) + + await page.getByRole('button', { name: 'Remove tab boop.tsx' }).click() + const removeDialog = page.locator('#clear-confirm-dialog') + await expect(removeDialog).toBeVisible() + await removeDialog.locator('button[value="confirm"]').evaluate(element => { + if (element instanceof HTMLButtonElement) { + element.click() + } + }) + await expect(page.getByRole('button', { name: 'Open tab boop.tsx' })).toHaveCount(0) + + await setComponentEditorSource( + page, + 'export const App = () =>
Updated entry after removal
', + ) + + await ensureOpenPrDrawerOpen(page) + const pushCommitButton = page + .locator('#github-pr-drawer') + .getByRole('button', { name: 'Push commit', exact: true }) + await expect(pushCommitButton).toBeEnabled() + await pushCommitButton.evaluate(element => { + if (element instanceof HTMLButtonElement) { + element.click() + } + }) + + const pushDialog = page.locator('#clear-confirm-dialog') + await expect(pushDialog).toBeVisible() + await expect(pushDialog.getByText('Files to commit:', { exact: true })).toBeVisible() + await expect( + pushDialog.getByText(/src\/components\/boop\.tsx.*\(delete\)/, { exact: false }), + ).toBeVisible() + + await pushDialog.locator('button[value="confirm"]').evaluate(element => { + if (element instanceof HTMLButtonElement) { + element.click() + } + }) + + await expect( + page.getByRole('status', { name: 'Open pull request status', includeHidden: true }), + ).toContainText('Commit pushed to develop/open-pr-test') + + await expect + .poll(async () => { + const workspaceRecord = await getWorkspaceTabsRecord(page, { + headBranch: 'develop/open-pr-test', + }) + const tabs = Array.isArray(workspaceRecord?.tabs) + ? (workspaceRecord.tabs as Array>) + : [] + + return tabs.some(tab => { + const path = typeof tab?.path === 'string' ? tab.path.trim() : '' + return path === 'src/components/boop.tsx' + }) + }) + .toBe(false) + + expect(treeRequests).toHaveLength(1) + const treePayload = treeRequests[0]?.tree as Array> + const updatedEntryBlob = treePayload?.find( + file => file.path === 'src/components/App.tsx', + ) + const deletedBlob = treePayload?.find(file => file.path === 'src/components/boop.tsx') + + expect(updatedEntryBlob).toMatchObject({ + path: 'src/components/App.tsx', + mode: '100644', + type: 'blob', + }) + expect(typeof updatedEntryBlob?.content).toBe('string') + expect( + (updatedEntryBlob?.content as string).includes('Updated entry after removal'), + ).toBe(true) + + expect(deletedBlob).toEqual({ + path: 'src/components/boop.tsx', + mode: '100644', + type: 'blob', + sha: null, + }) +}) + test('Push commit prunes stale delete entries before Git tree creation', async ({ page, }) => { diff --git a/src/app.js b/src/app.js index 4143d1d..3fd1a38 100644 --- a/src/app.js +++ b/src/app.js @@ -870,6 +870,7 @@ const { confirmAction: options => confirmAction(options), isStyleWorkspaceTab, clearTrackedWorkspaceTab, + trackRemovedWorkspaceTab: tab => workspaceSyncController.trackRemovedWorkspaceTab(tab), getWorkspaceTabByKind, makeUniqueTabPath, createWorkspaceTabId, diff --git a/src/modules/app-core/workspace-controllers-setup.js b/src/modules/app-core/workspace-controllers-setup.js index 60b3475..77298e9 100644 --- a/src/modules/app-core/workspace-controllers-setup.js +++ b/src/modules/app-core/workspace-controllers-setup.js @@ -67,6 +67,7 @@ const createWorkspaceControllersSetup = ({ confirmAction, isStyleWorkspaceTab, clearTrackedWorkspaceTab, + trackRemovedWorkspaceTab, getWorkspaceTabByKind, makeUniqueTabPath, createWorkspaceTabId, @@ -151,6 +152,7 @@ const createWorkspaceControllersSetup = ({ isStyleWorkspaceTab, persistActiveTabEditorContent, clearTrackedWorkspaceTab, + trackRemovedWorkspaceTab, getActiveWorkspaceTab, loadWorkspaceTabIntoEditor, getWorkspaceTabByKind, diff --git a/src/modules/app-core/workspace-sync-controller.js b/src/modules/app-core/workspace-sync-controller.js index a4c9175..1a07c7c 100644 --- a/src/modules/app-core/workspace-sync-controller.js +++ b/src/modules/app-core/workspace-sync-controller.js @@ -19,6 +19,74 @@ const createWorkspaceSyncController = ({ getRenderModeValue, normalizeRenderMode, }) => { + const removedWorkspaceTabPathsByWorkspaceKey = new Map() + + const getCurrentWorkspaceKey = () => { + const context = getWorkspaceContextSnapshot() + return toWorkspaceRecordKey({ + repositoryFullName: context?.repositoryFullName, + headBranch: context?.headBranch, + }) + } + + const getRemovedWorkspaceTabPathsForCurrentWorkspace = () => { + const workspaceKey = getCurrentWorkspaceKey() + if (!workspaceKey) { + return new Set() + } + + const trackedPaths = removedWorkspaceTabPathsByWorkspaceKey.get(workspaceKey) + return trackedPaths instanceof Set ? trackedPaths : new Set() + } + + const trackRemovedWorkspaceTab = tab => { + if (!hasTabCommittedSyncState(tab)) { + return false + } + + const removedPath = normalizeWorkspacePathValue( + getTabTargetPrFilePath(tab) || tab?.path, + ) + if (!removedPath) { + return false + } + + const workspaceKey = getCurrentWorkspaceKey() + if (!workspaceKey) { + return false + } + + const existingPaths = removedWorkspaceTabPathsByWorkspaceKey.get(workspaceKey) + const nextPaths = existingPaths instanceof Set ? existingPaths : new Set() + nextPaths.add(removedPath) + removedWorkspaceTabPathsByWorkspaceKey.set(workspaceKey, nextPaths) + return true + } + + const clearTrackedRemovedWorkspaceTabPath = path => { + const normalizedPath = normalizeWorkspacePathValue(path) + if (!normalizedPath) { + return false + } + + const workspaceKey = getCurrentWorkspaceKey() + if (!workspaceKey) { + return false + } + + const trackedPaths = removedWorkspaceTabPathsByWorkspaceKey.get(workspaceKey) + if (!(trackedPaths instanceof Set) || !trackedPaths.has(normalizedPath)) { + return false + } + + trackedPaths.delete(normalizedPath) + if (trackedPaths.size === 0) { + removedWorkspaceTabPathsByWorkspaceKey.delete(workspaceKey) + } + + return true + } + const resolveCanonicalDirtyState = ({ tab, content }) => { const syncedContent = toWorkspaceSyncedContent(tab?.syncedContent) if (syncedContent !== null) { @@ -64,6 +132,12 @@ const createWorkspaceSyncController = ({ const reconcileWorkspaceTabsWithPushUpdates = fileUpdates => { const updates = Array.isArray(fileUpdates) ? fileUpdates : [] + for (const update of updates) { + if (update?.deleted === true) { + clearTrackedRemovedWorkspaceTabPath(update?.path) + } + } + if (updates.length === 0) { return 0 } @@ -176,6 +250,23 @@ const createWorkspaceSyncController = ({ } } + if (!includeAllWorkspaceFiles) { + const removedPaths = getRemovedWorkspaceTabPathsForCurrentWorkspace() + for (const removedPath of removedPaths) { + if (currentPaths.has(removedPath) || dedupedByPath.has(removedPath)) { + continue + } + + dedupedByPath.set(removedPath, { + path: removedPath, + content: '', + tabLabel: removedPath, + isEntry: false, + deleted: true, + }) + } + } + return [...dedupedByPath.values()] } @@ -320,6 +411,7 @@ const createWorkspaceSyncController = ({ buildWorkspaceTabsSnapshot, getEditorSyncTargets, getWorkspacePrFileCommits, + trackRemovedWorkspaceTab, reconcileWorkspaceTabsWithEditorSync, reconcileWorkspaceTabsWithPushUpdates, } diff --git a/src/modules/app-core/workspace-tab-mutations-controller.js b/src/modules/app-core/workspace-tab-mutations-controller.js index 7a1a10b..5349605 100644 --- a/src/modules/app-core/workspace-tab-mutations-controller.js +++ b/src/modules/app-core/workspace-tab-mutations-controller.js @@ -20,6 +20,7 @@ const createWorkspaceTabMutationsController = ({ isStyleWorkspaceTab, persistActiveTabEditorContent, clearTrackedWorkspaceTab, + trackRemovedWorkspaceTab, getActiveWorkspaceTab, loadWorkspaceTabIntoEditor, getWorkspaceTabByKind, @@ -219,6 +220,10 @@ const createWorkspaceTabMutationsController = ({ return } + if (typeof trackRemovedWorkspaceTab === 'function') { + trackRemovedWorkspaceTab(tab) + } + if (typeof clearTrackedWorkspaceTab === 'function') { clearTrackedWorkspaceTab(tab.id) }