From 2bcee895a627e8919768cddcf39ced015b3d1d4b Mon Sep 17 00:00:00 2001 From: KCM Date: Thu, 23 Apr 2026 10:13:01 -0500 Subject: [PATCH] fix: properly commit tab name changes. --- playwright/github-pr-drawer.spec.ts | 236 ++++++++++++++++++ .../app-core/workspace-sync-controller.js | 28 +++ .../workspace-tab-mutations-controller.js | 12 +- src/modules/github/api/editor-content.js | 24 +- .../github/pr/drawer/controller/run-submit.js | 7 + src/modules/github/pr/drawer/file-commits.js | 1 + src/modules/github/pr/drawer/summary.js | 7 +- 7 files changed, 304 insertions(+), 11 deletions(-) diff --git a/playwright/github-pr-drawer.spec.ts b/playwright/github-pr-drawer.spec.ts index 23f036a..f9e964c 100644 --- a/playwright/github-pr-drawer.spec.ts +++ b/playwright/github-pr-drawer.spec.ts @@ -18,6 +18,22 @@ import { const getOpenPrDrawer = (page: Page) => page.getByRole('complementary', { name: /Open Pull Request|Push Commit/ }) +const renameWorkspaceTab = async ( + page: Page, + { + from, + to, + }: { + from: string + to: string + }, +) => { + await page.getByRole('button', { name: `Rename tab ${from}` }).click() + const renameInput = page.getByLabel(`Rename ${from}`) + await renameInput.fill(to) + await renameInput.press('Enter') +} + const clickOpenPrDrawerSubmit = async (page: Page) => { const drawer = getOpenPrDrawer(page) await expect(drawer).toBeVisible() @@ -2596,6 +2612,226 @@ test('Dirty tabs expose Edited in accessible names during active PR context', as ).toBeVisible() }) +test('Renaming a synced module tab marks it Edited and includes renamed path in Push commit confirmation', 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/git/trees', + async route => { + treeRequests.push(route.request().postDataJSON() as Record) + + await route.fulfill({ + status: 201, + contentType: 'application/json', + body: JSON.stringify({ sha: 'rename-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: 'rename-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: 'rename-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 renameWorkspaceTab(page, { from: 'boop.tsx', to: 'beep.tsx' }) + + await expect( + page.getByRole('button', { name: 'Open tab beep.tsx (Edited)' }), + ).toBeVisible() + + await ensureOpenPrDrawerOpen(page) + await page.getByRole('button', { name: 'Push commit' }).last().click() + + const dialog = page.getByRole('dialog') + await expect(dialog).toBeVisible() + await expect(page.getByText('Files to commit:', { exact: true })).toBeVisible() + await expect( + page.getByText('beep.tsx -> src/components/beep.tsx', { exact: true }), + ).toBeVisible() + await expect( + page.getByText('beep.tsx -> src/components/boop.tsx (delete)', { exact: true }), + ).toBeVisible() + + await dialog.getByRole('button', { name: 'Push commit' }).click() + + await expect( + page.getByRole('status', { name: 'Open pull request status', includeHidden: true }), + ).toContainText('Commit pushed to develop/open-pr-test') + + expect(treeRequests).toHaveLength(1) + const treePayload = treeRequests[0]?.tree as Array> + const renamedBlob = treePayload?.find(file => file.path === 'src/components/beep.tsx') + const deletedBlob = treePayload?.find(file => file.path === 'src/components/boop.tsx') + + expect(renamedBlob).toMatchObject({ + path: 'src/components/beep.tsx', + mode: '100644', + type: 'blob', + }) + expect(typeof renamedBlob?.content).toBe('string') + + expect(deletedBlob).toEqual({ + path: 'src/components/boop.tsx', + mode: '100644', + type: 'blob', + sha: null, + }) +}) + test('Active PR context push commit uses Git Database API atomic path by default', async ({ page, }) => { diff --git a/src/modules/app-core/workspace-sync-controller.js b/src/modules/app-core/workspace-sync-controller.js index 51be597..93bf60e 100644 --- a/src/modules/app-core/workspace-sync-controller.js +++ b/src/modules/app-core/workspace-sync-controller.js @@ -113,8 +113,13 @@ const createWorkspaceSyncController = ({ const getWorkspacePrFileCommits = options => { const includeAllWorkspaceFiles = options?.includeAllWorkspaceFiles === true || options?.includeAll === true + const sourceTabs = workspaceTabsState.getTabs() + const sourceTabById = new Map(sourceTabs.map(tab => [tab?.id, tab])) const snapshotTabs = buildWorkspaceTabsSnapshot() const dedupedByPath = new Map() + const currentPaths = new Set( + snapshotTabs.map(tab => normalizeWorkspacePathValue(tab?.path)).filter(Boolean), + ) const primaryTabPaths = new Set( snapshotTabs .filter(tab => tab?.id === 'component' || tab?.id === 'styles') @@ -147,6 +152,29 @@ const createWorkspaceSyncController = ({ tabLabel: toNonEmptyWorkspaceText(tab?.name) || toNonEmptyWorkspaceText(tab?.id), isEntry: tab?.role === 'entry', }) + + const sourceTab = sourceTabById.get(tab?.id) ?? tab + const previousPath = getTabTargetPrFilePath(sourceTab) + const isCommittedRename = + hasTabCommittedSyncState(sourceTab) && + Boolean(previousPath) && + Boolean(normalizedPath) && + previousPath !== normalizedPath + + if ( + isCommittedRename && + !currentPaths.has(previousPath) && + !dedupedByPath.has(previousPath) + ) { + dedupedByPath.set(previousPath, { + path: previousPath, + content: '', + tabLabel: + toNonEmptyWorkspaceText(tab?.name) || toNonEmptyWorkspaceText(tab?.id), + isEntry: false, + deleted: true, + }) + } } return [...dedupedByPath.values()] diff --git a/src/modules/app-core/workspace-tab-mutations-controller.js b/src/modules/app-core/workspace-tab-mutations-controller.js index 66e1e76..206b703 100644 --- a/src/modules/app-core/workspace-tab-mutations-controller.js +++ b/src/modules/app-core/workspace-tab-mutations-controller.js @@ -74,15 +74,19 @@ const createWorkspaceTabMutationsController = ({ tab.role === 'entry' ? getPathFileName(normalizedEntryPath) || defaultComponentTabName : getPathFileName(normalizedEntryPath) || normalizedName + const didPathChange = + typeof tab?.path === 'string' && normalizedEntryPath !== tab.path workspaceTabsState.upsertTab({ ...tab, name: normalizedTabName, path: normalizedEntryPath, - isDirty: getDirtyStateForTabChange( - tab, - typeof tab?.content === 'string' ? tab.content : '', - ), + isDirty: + didPathChange || + getDirtyStateForTabChange( + tab, + typeof tab?.content === 'string' ? tab.content : '', + ), lastModified: Date.now(), }) diff --git a/src/modules/github/api/editor-content.js b/src/modules/github/api/editor-content.js index 525aa18..add2b1e 100644 --- a/src/modules/github/api/editor-content.js +++ b/src/modules/github/api/editor-content.js @@ -59,6 +59,7 @@ const normalizeFileUpdateInput = (file, index) => { return { path: validation.value, content: typeof file.content === 'string' ? file.content : '', + deleted: file.deleted === true, } } @@ -100,12 +101,23 @@ const createRepositoryTree = async ({ files, signal, }) => { - const tree = files.map(file => ({ - path: file.path, - mode: '100644', - type: 'blob', - content: file.content, - })) + const tree = files.map(file => { + if (file.deleted === true) { + return { + path: file.path, + mode: '100644', + type: 'blob', + sha: null, + } + } + + return { + path: file.path, + mode: '100644', + type: 'blob', + content: file.content, + } + }) const response = await requestGitHubJson({ token, diff --git a/src/modules/github/pr/drawer/controller/run-submit.js b/src/modules/github/pr/drawer/controller/run-submit.js index 3eb81c2..3fd74e1 100644 --- a/src/modules/github/pr/drawer/controller/run-submit.js +++ b/src/modules/github/pr/drawer/controller/run-submit.js @@ -165,6 +165,13 @@ export const createRunSubmit = ({ const fileUpdates = await Promise.all( normalizedFileCommits.map(async fileCommit => { + if (fileCommit.deleted === true) { + return { + path: fileCommit.path, + deleted: true, + } + } + const shouldStripEntryWrapper = !includeAppWrapper && fileCommit.isEntry const nextContent = shouldStripEntryWrapper ? await stripTopLevelAppWrapper({ diff --git a/src/modules/github/pr/drawer/file-commits.js b/src/modules/github/pr/drawer/file-commits.js index 1bf0ec6..9ee0935 100644 --- a/src/modules/github/pr/drawer/file-commits.js +++ b/src/modules/github/pr/drawer/file-commits.js @@ -77,6 +77,7 @@ const normalizeFileCommits = fileCommits => { content: typeof item?.content === 'string' ? item.content : '', tabLabel: toSafeText(item?.tabLabel), isEntry: item?.isEntry === true, + deleted: item?.deleted === true, }) } diff --git a/src/modules/github/pr/drawer/summary.js b/src/modules/github/pr/drawer/summary.js index 93f7101..ef3f259 100644 --- a/src/modules/github/pr/drawer/summary.js +++ b/src/modules/github/pr/drawer/summary.js @@ -28,7 +28,12 @@ const buildSummary = ({ } const tabLabel = toSafeText(fileCommit?.tabLabel) - lines.push(tabLabel ? `- ${tabLabel} -> ${path}` : `- ${path}`) + const operationLabel = fileCommit?.deleted === true ? ' (delete)' : '' + lines.push( + tabLabel + ? `- ${tabLabel} -> ${path}${operationLabel}` + : `- ${path}${operationLabel}`, + ) } }