fix(editor): address review feedback on the split editor slice (#14850)

* fix(editor): address review feedback on the split editor slice

- Localize the conflict-placeholder guidance string (en/es/ja/ko/zh).
- Filter target-worktree tabs by migrated tab id so an owner transition
  cannot leave two tabs sharing one id.
- Resolve a pending editor reveal by fileId first; the oldFilePath scan
  could pick another worktree's rekey.
- Return before opening a workspace editor item when conflict metadata
  is missing, so no tab is created for a file that never entered openFiles.
- Skip a persisted open file whose resolved id is already used; the
  session schema allows repeated (path, worktree, runtime) tuples.

* fix(editor): remove migrated tab ids from sibling groups

When tabs migrate to a target group during editor owner transition, the
same tab IDs can be left in sibling groups, causing state corruption.
Strip these IDs from all sibling groups to ensure each tab ID exists
only once across the editor layout.

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.",

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.",

src/renderer/src/i18n/locales/ja.json

@@ -196,6 +196,7 @@
         },
         "editor": {
           "dcb521ed29": "このファイルは競合状態にありますが、編集できる作業ツリーファイルがありません。",
+          "conflictPlaceholderGuidance": "Git で競合を解決するか、いずれかの側を復元してから開き直してください。",
           "51f15c37d3": "ディレクトリを開けません: {{value0}}",
           "f2e00db373": "ファイルが見つかりません: {{value0}}",
           "checkRunDetailsUnavailable": "このチェックの詳細はありません。",

src/renderer/src/i18n/locales/ko.json

@@ -196,6 +196,7 @@
         },
         "editor": {
           "dcb521ed29": "이 파일은 충돌 상태에 있지만 편집할 수 있는 작업 트리 파일이 없습니다.",
+          "conflictPlaceholderGuidance": "Git에서 충돌을 해결하거나 한쪽을 복원한 후 다시 여세요.",
           "51f15c37d3": "디렉터리를 열 수 없습니다: {{value0}}",
           "f2e00db373": "파일을 찾을 수 없습니다: {{value0}}",
           "checkRunDetailsUnavailable": "이 체크에 사용할 수 있는 세부 정보가 없습니다.",

src/renderer/src/i18n/locales/zh.json

@@ -196,6 +196,7 @@
         },
         "editor": {
           "dcb521ed29": "该文件处于冲突状态，但没有可编辑的工作树文件。",
+          "conflictPlaceholderGuidance": "请在 Git 中解决冲突，或恢复其中一侧后再重新打开。",
           "51f15c37d3": "无法打开目录：{{value0}}",
           "f2e00db373": "未找到文件：{{value0}}",
           "checkRunDetailsUnavailable": "此检查没有可用的详细信息。",

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

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)

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,

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(),

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,

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,

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<T>(
   record: Record<string, T>,
@@ -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<string>): 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[],

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.'
+        )
       }
 }
 

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')

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'