diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index c23f0903c75..f965961e5ea 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -262,6 +262,7 @@ }, "editor": { "dcb521ed29": "This file is in a conflict state, but no working-tree file is available to edit.", + "conflictPlaceholderGuidance": "Resolve the conflict in Git or restore one side before reopening it.", "51f15c37d3": "Cannot open directory: {{value0}}", "f2e00db373": "File not found: {{value0}}", "checkRunDetailsUnavailable": "No details are available for this check.", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index 5e090ea65c1..6cc500a1eb5 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -196,6 +196,7 @@ }, "editor": { "dcb521ed29": "Este archivo está en conflicto, pero no hay una copia editable en el worktree.", + "conflictPlaceholderGuidance": "Resuelve el conflicto en Git o restaura uno de los lados antes de volver a abrirlo.", "51f15c37d3": "No se puede abrir la carpeta: {{value0}}", "f2e00db373": "No se encontró el archivo: {{value0}}", "checkRunDetailsUnavailable": "No hay detalles disponibles para este check.", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index f52411f9c2f..fa7e623b28a 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -196,6 +196,7 @@ }, "editor": { "dcb521ed29": "このファイルは競合状態にありますが、編集できる作業ツリーファイルがありません。", + "conflictPlaceholderGuidance": "Git で競合を解決するか、いずれかの側を復元してから開き直してください。", "51f15c37d3": "ディレクトリを開けません: {{value0}}", "f2e00db373": "ファイルが見つかりません: {{value0}}", "checkRunDetailsUnavailable": "このチェックの詳細はありません。", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 3214982cd47..6e84ad2595b 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -196,6 +196,7 @@ }, "editor": { "dcb521ed29": "이 파일은 충돌 상태에 있지만 편집할 수 있는 작업 트리 파일이 없습니다.", + "conflictPlaceholderGuidance": "Git에서 충돌을 해결하거나 한쪽을 복원한 후 다시 여세요.", "51f15c37d3": "디렉터리를 열 수 없습니다: {{value0}}", "f2e00db373": "파일을 찾을 수 없습니다: {{value0}}", "checkRunDetailsUnavailable": "이 체크에 사용할 수 있는 세부 정보가 없습니다.", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 39a9e876433..58f0a7e5f2e 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -196,6 +196,7 @@ }, "editor": { "dcb521ed29": "该文件处于冲突状态,但没有可编辑的工作树文件。", + "conflictPlaceholderGuidance": "请在 Git 中解决冲突,或恢复其中一侧后再重新打开。", "51f15c37d3": "无法打开目录:{{value0}}", "f2e00db373": "未找到文件:{{value0}}", "checkRunDetailsUnavailable": "此检查没有可用的详细信息。", diff --git a/src/renderer/src/store/slices/editor-rekey-open-files.test.ts b/src/renderer/src/store/slices/editor-rekey-open-files.test.ts index 2099c826e9b..5597eb748dd 100644 --- a/src/renderer/src/store/slices/editor-rekey-open-files.test.ts +++ b/src/renderer/src/store/slices/editor-rekey-open-files.test.ts @@ -269,6 +269,40 @@ describe('rekeyOpenFilesForPathChange', () => { expect(useAppStore.getState().openFiles[0]!.mirroredFromRuntimeSession).toBeUndefined() }) + it('leaves a reveal keyed by another worktree file id untouched when only the path matches', () => { + seedEditTab() + // A same-path tab in a second worktree: the reveal belongs to it, not to the rekeyed tab. + useAppStore.getState().openFile( + { + filePath: '/repo/a.md', + relativePath: 'a.md', + worktreeId: 'wt-2', + runtimeEnvironmentId: null, + language: 'markdown', + mode: 'edit' + }, + { suppressActiveRuntimeFallback: true } + ) + const rekeyedId = useAppStore.getState().openFiles[0]!.id + const otherWorktreeId = useAppStore.getState().openFiles[1]!.id + expect(otherWorktreeId).not.toBe(rekeyedId) + useAppStore.setState({ + pendingEditorReveal: { fileId: otherWorktreeId, filePath: '/repo/a.md', line: 40 } + } as never) + + const result = useAppStore.getState().rekeyOpenFilesForPathChange({ + rekeys: [rekeyFor(rekeyedId, '/repo/sub/a.md', 'sub/a.md')] + }) + + expect(result).toEqual({ ok: true }) + expect( + useAppStore.getState().openFiles.find((file) => file.id === '/repo/sub/a.md') + ).toMatchObject({ filePath: '/repo/sub/a.md' }) + const reveal = useAppStore.getState().pendingEditorReveal! + expect(reveal.fileId).toBe(otherWorktreeId) + expect(reveal.filePath).toBe('/repo/a.md') + }) + it('migrates a pending editor reveal to the new path', () => { seedEditTab() const oldId = useAppStore.getState().openFiles[0]!.id diff --git a/src/renderer/src/store/slices/editor/actions/hydrate-editor-session.ts b/src/renderer/src/store/slices/editor/actions/hydrate-editor-session.ts index 4b07d85a919..3b76d3df384 100644 --- a/src/renderer/src/store/slices/editor/actions/hydrate-editor-session.ts +++ b/src/renderer/src/store/slices/editor/actions/hydrate-editor-session.ts @@ -61,6 +61,10 @@ export function createHydrateEditorSession( usedOpenFileIds.has(pf.filePath) ? ownedId : pf.filePath + // Why: the persisted schema allows repeated (path, worktree, runtime) tuples, and an owned id repeats verbatim — restoring both would put two files under one id. + if (usedOpenFileIds.has(id)) { + continue + } usedOpenFileIds.add(id) // Why: map from the collision-derived legacy id; keying by filePath would collapse same-path local/runtime tabs onto the last owner to hydrate. addEditorFileIdMigration(editorFileIdMigrationsByWorktree, worktreeId, legacyId, id) diff --git a/src/renderer/src/store/slices/editor/actions/open-conflict-file.ts b/src/renderer/src/store/slices/editor/actions/open-conflict-file.ts index d816cd8263c..c6ad14d5afc 100644 --- a/src/renderer/src/store/slices/editor/actions/open-conflict-file.ts +++ b/src/renderer/src/store/slices/editor/actions/open-conflict-file.ts @@ -19,6 +19,7 @@ export function createOpenConflictFile( const absolutePath = joinPath(worktreePath, entry.path) const isPreview = options?.preview ?? false let editorItemTargetGroupId = options?.targetGroupId + let openedConflictFile = true set((s) => { const id = absolutePath const conflict = toOpenConflictMetadata(entry) @@ -35,6 +36,7 @@ export function createOpenConflictFile( : s.trackedConflictPathsByWorktree[worktreeId] if (!conflict) { + openedConflictFile = false return s } @@ -115,6 +117,10 @@ export function createOpenConflictFile( : { ...s.trackedConflictPathsByWorktree, [worktreeId]: nextTracked } } }) + // Why: no conflict metadata means no OpenFile was added, so a workspace tab would point at nothing. + if (!openedConflictFile) { + return + } void openWorkspaceEditorItem( get(), absolutePath, diff --git a/src/renderer/src/store/slices/editor/actions/open-conflict-review.ts b/src/renderer/src/store/slices/editor/actions/open-conflict-review.ts index 4ae331b213c..a91c98c2b73 100644 --- a/src/renderer/src/store/slices/editor/actions/open-conflict-review.ts +++ b/src/renderer/src/store/slices/editor/actions/open-conflict-review.ts @@ -15,6 +15,7 @@ export function createOpenConflictReview( const reviewTab = (get().unifiedTabsByWorktree?.[worktreeId] ?? []).find( (tab) => tab.entityId === reviewFileId && tab.contentType === 'conflict-review' ) + let openedConflictFile = true set((s) => { const conflict = toOpenConflictMetadata(entry) const existing = s.openFiles.find((f) => f.id === absolutePath) @@ -27,6 +28,7 @@ export function createOpenConflictReview( : s.trackedConflictPathsByWorktree[worktreeId] if (!conflict) { + openedConflictFile = false return s } @@ -91,6 +93,10 @@ export function createOpenConflictReview( } }) + // Why: no conflict metadata means no OpenFile was added, so a workspace tab would point at nothing. + if (!openedConflictFile) { + return + } // Why: the conflict file needs a normal editor backing tab for save/close, but selecting from Conflict Review must keep the review tab visible; restore focus after. void openWorkspaceEditorItem( get(), diff --git a/src/renderer/src/store/slices/editor/actions/rekey-open-files-action.ts b/src/renderer/src/store/slices/editor/actions/rekey-open-files-action.ts index c8ad0eae79e..a591aef8356 100644 --- a/src/renderer/src/store/slices/editor/actions/rekey-open-files-action.ts +++ b/src/renderer/src/store/slices/editor/actions/rekey-open-files-action.ts @@ -105,9 +105,12 @@ export function createRekeyOpenFilesAction( } const reveal = s.pendingEditorReveal - const rekeyForReveal = reveal - ? rekeys.find((r) => r.oldFilePath === reveal.filePath) - : undefined + // Why: two worktrees can rekey the same oldFilePath, so an id-keyed reveal must match its own file, not the first path match. + const rekeyForReveal = !reveal + ? undefined + : reveal.fileId + ? rekeyByOldId.get(reveal.fileId) + : rekeys.find((r) => r.oldFilePath === reveal.filePath) return { openFiles: nextOpenFiles, diff --git a/src/renderer/src/store/slices/editor/actions/restored-editor-owner-transition.ts b/src/renderer/src/store/slices/editor/actions/restored-editor-owner-transition.ts index 77a91bbe2d5..3ec23130388 100644 --- a/src/renderer/src/store/slices/editor/actions/restored-editor-owner-transition.ts +++ b/src/renderer/src/store/slices/editor/actions/restored-editor-owner-transition.ts @@ -6,6 +6,7 @@ import { sanitizeRecentTabIds } from '../../tab-group-state' import { nextActiveIdAfterRemoval, removeEmptyEditorGroups, + removeTabIdsFromGroup, rekeyFileIdRecord } from '../file-ids/open-file-path-rekey' import type { @@ -44,6 +45,7 @@ export function buildRestoredEditorOwnerTransition( movedTabs.map((tab) => [tab.id, migrations.get(tab.id) ?? tab.id]) ) const mappedMovedTabIds = movedTabs.map((tab) => tabIdMigration.get(tab.id) ?? tab.id) + const mappedMovedTabIdSet = new Set(mappedMovedTabIds) const mappedMovedTabBarIds = movedTabs.map( (tab) => migrations.get(tab.entityId) ?? tab.entityId ) @@ -93,16 +95,27 @@ export function buildRestoredEditorOwnerTransition( destinationOrder ) } + // Why: the migrated ids land in targetGroup only, so any sibling group holding the same id is left dangling. const nextTargetGroups = targetGroups.some((group) => group.id === targetGroupId) - ? targetGroups.map((group) => (group.id === targetGroupId ? updatedTargetGroup : group)) - : [...targetGroups, updatedTargetGroup] + ? targetGroups.map((group) => + group.id === targetGroupId + ? updatedTargetGroup + : removeTabIdsFromGroup(group, mappedMovedTabIdSet) + ) + : [ + ...targetGroups.map((group) => removeTabIdsFromGroup(group, mappedMovedTabIdSet)), + updatedTargetGroup + ] const nextUnifiedTabsByWorktree = { ...s.unifiedTabsByWorktree } nextUnifiedTabsByWorktree[sourceWorktreeId] = ( nextUnifiedTabsByWorktree[sourceWorktreeId] ?? [] ).filter((tab) => !movedTabIds.has(tab.id)) nextUnifiedTabsByWorktree[targetWorktreeId] = [ - ...(nextUnifiedTabsByWorktree[targetWorktreeId] ?? []), + // Why: a leftover target tab carrying a migrated id would duplicate the id that destinationOrder keeps only once. + ...(nextUnifiedTabsByWorktree[targetWorktreeId] ?? []).filter( + (tab) => !mappedMovedTabIdSet.has(tab.id) + ), ...movedTabs.map((tab) => ({ ...tab, id: tabIdMigration.get(tab.id) ?? tab.id, diff --git a/src/renderer/src/store/slices/editor/file-ids/open-file-path-rekey.ts b/src/renderer/src/store/slices/editor/file-ids/open-file-path-rekey.ts index 3f7da9c78de..007684497ce 100644 --- a/src/renderer/src/store/slices/editor/file-ids/open-file-path-rekey.ts +++ b/src/renderer/src/store/slices/editor/file-ids/open-file-path-rekey.ts @@ -1,6 +1,7 @@ import type { AppState } from '../../../types' import type { TabGroup } from '../../../../../../shared/tab-types' import { pruneTabGroupLayoutForGroups } from '../../tabs-hydration' +import { sanitizeRecentTabIds } from '../../tab-group-state' export function rekeyFileIdRecord( record: Record, @@ -29,6 +30,29 @@ export function nextActiveIdAfterRemoval( return recent ?? ids.find((id) => !removedIds.has(id)) ?? null } +/** Strip `removedIds` from a group's order, MRU stack and active id; returns the + * same object when the group never referenced them. */ +export function removeTabIdsFromGroup(group: TabGroup, removedIds: ReadonlySet): TabGroup { + const recentTabIds = group.recentTabIds ?? [] + const references = + group.tabOrder.some((id) => removedIds.has(id)) || + recentTabIds.some((id) => removedIds.has(id)) || + (group.activeTabId !== null && removedIds.has(group.activeTabId)) + if (!references) { + return group + } + const tabOrder = group.tabOrder.filter((id) => !removedIds.has(id)) + return { + ...group, + activeTabId: + group.activeTabId !== null && removedIds.has(group.activeTabId) + ? nextActiveIdAfterRemoval(group.tabOrder, recentTabIds, removedIds) + : group.activeTabId, + tabOrder, + recentTabIds: sanitizeRecentTabIds(recentTabIds, tabOrder) + } +} + export function removeEmptyEditorGroups( previousGroups: TabGroup[], groups: TabGroup[], diff --git a/src/renderer/src/store/slices/editor/git/git-status-reconciliation.ts b/src/renderer/src/store/slices/editor/git/git-status-reconciliation.ts index 37426cb7b28..3e4c098210f 100644 --- a/src/renderer/src/store/slices/editor/git/git-status-reconciliation.ts +++ b/src/renderer/src/store/slices/editor/git/git-status-reconciliation.ts @@ -76,7 +76,10 @@ export function toOpenConflictMetadata(entry: GitStatusEntry): OpenConflictMetad 'auto.store.slices.editor.dcb521ed29', 'This file is in a conflict state, but no working-tree file is available to edit.' ), - guidance: 'Resolve the conflict in Git or restore one side before reopening it.' + guidance: translate( + 'auto.store.slices.editor.conflictPlaceholderGuidance', + 'Resolve the conflict in Git or restore one side before reopening it.' + ) } } diff --git a/src/renderer/src/store/slices/restored-editor-owner-reparent.test.ts b/src/renderer/src/store/slices/restored-editor-owner-reparent.test.ts index 9e7cf558ba2..3302e041c45 100644 --- a/src/renderer/src/store/slices/restored-editor-owner-reparent.test.ts +++ b/src/renderer/src/store/slices/restored-editor-owner-reparent.test.ts @@ -315,6 +315,80 @@ describe('restored editor owner reparent', () => { expect(refreshGitHubForWorktreeIfStale).toHaveBeenCalledWith(TARGET) }) + it('repairs a duplicate migrated tab id held by a non-selected destination group', () => { + const oldId = openRestoredSource() + const movedTabId = useAppStore.getState().unifiedTabsByWorktree[SOURCE]![0]!.id + const keptFileId = useAppStore.getState().openFile( + { + filePath: '/repo-b/docs/other.md', + relativePath: 'docs/other.md', + worktreeId: TARGET, + runtimeEnvironmentId: null, + language: 'markdown', + mode: 'edit' + }, + { suppressActiveRuntimeFallback: true } + ) + const keptTab = useAppStore + .getState() + .unifiedTabsByWorktree[TARGET]!.find((tab) => tab.entityId === keptFileId)! + useAppStore.setState((state) => ({ + unifiedTabsByWorktree: { + ...state.unifiedTabsByWorktree, + [TARGET]: [ + { ...keptTab, groupId: 'other-group' }, + // A stale duplicate carrying the id the source tab keeps through the move. + { ...keptTab, id: movedTabId, groupId: 'other-group' } + ] + }, + groupsByWorktree: { + ...state.groupsByWorktree, + [TARGET]: [ + { + id: 'target-group', + worktreeId: TARGET, + activeTabId: null, + tabOrder: [], + recentTabIds: [] + }, + { + id: 'other-group', + worktreeId: TARGET, + activeTabId: movedTabId, + tabOrder: [keptTab.id, movedTabId], + recentTabIds: [keptTab.id, movedTabId] + } + ] + }, + layoutByWorktree: { + ...state.layoutByWorktree, + [TARGET]: { + type: 'split', + direction: 'horizontal', + first: { type: 'leaf', groupId: 'target-group' }, + second: { type: 'leaf', groupId: 'other-group' } + } + }, + activeGroupIdByWorktree: { ...state.activeGroupIdByWorktree, [TARGET]: 'target-group' } + })) + + expect(reparent(oldId).ok).toBe(true) + + const next = useAppStore.getState() + const groups = next.groupsByWorktree[TARGET]! + const otherGroup = groups.find((group) => group.id === 'other-group')! + expect(otherGroup).toMatchObject({ + activeTabId: keptTab.id, + tabOrder: [keptTab.id], + recentTabIds: [keptTab.id] + }) + expect(groups.find((group) => group.id === 'target-group')?.tabOrder).toContain(movedTabId) + // The migrated id survives exactly once, in the group that now owns the tab. + expect(next.unifiedTabsByWorktree[TARGET]!.filter((tab) => tab.id === movedTabId)).toHaveLength( + 1 + ) + }) + it('persists and restores the destination owner and dirty hot-exit draft', () => { const oldId = openRestoredSource() useAppStore.getState().setEditorDraft(oldId, 'hot exit') diff --git a/src/renderer/src/store/slices/store-session-editor-hydration.test.ts b/src/renderer/src/store/slices/store-session-editor-hydration.test.ts index f9ce41093c0..dddc005bc81 100644 --- a/src/renderer/src/store/slices/store-session-editor-hydration.test.ts +++ b/src/renderer/src/store/slices/store-session-editor-hydration.test.ts @@ -383,6 +383,47 @@ describe('hydrateEditorSession', () => { ) }) + it('drops a duplicate persisted file that would restore under an already used id', () => { + const store = createTestStore() + const wt = 'repo1::/path/wt1' + const filePath = '/path/wt1/src/app.ts' + const runtimeEnvironmentId = 'runtime-1' + + store.setState({ + repos: [ + { id: 'repo1', path: '/repo1', displayName: 'Repo 1', badgeColor: '#000', addedAt: 0 } + ], + worktreesByRepo: { + repo1: [makeWorktree({ id: wt, repoId: 'repo1', path: '/path/wt1' })] + }, + activeWorktreeId: wt + }) + + const persistedFile = { + filePath, + relativePath: 'src/app.ts', + worktreeId: wt, + language: 'typescript', + runtimeEnvironmentId + } + store.getState().hydrateEditorSession({ + activeRepoId: 'repo1', + activeWorktreeId: wt, + activeTabId: null, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + // The schema allows a repeated (path, worktree, runtime) tuple; both entries resolve to one owned id. + openFilesByWorktree: { [wt]: [persistedFile, { ...persistedFile }] }, + activeFileIdByWorktree: {}, + activeTabTypeByWorktree: { [wt]: 'editor' as const } + }) + + const s = store.getState() + expect(s.openFiles.map((file) => file.id)).toEqual([ + ownedEditorFileId(filePath, wt, runtimeEnvironmentId) + ]) + }) + it('keeps floating owner-qualified editor ids aligned with restored unified tabs', () => { const store = createTestStore() const sharedPath = '/path/wt1/README.md'