feat: unified tab system hardening, window chrome settings, file explorer icons#418
Merged
feat: unified tab system hardening, window chrome settings, file explorer icons#418
Conversation
Add native title bar and auto-hide menu bar toggles under Settings > Display > Window Chrome (implements PR #300 and #293 as proper settings). Both are read as early settings before window creation and conditionally applied to BrowserWindow options. Extract buildUnifiedTabs and ensureInUnifiedTabOrder into tabHelpers as shared utilities, replacing duplicate implementations in useTabHandlers and tabStore. Fix orphaned tab repair in reopenUnifiedClosedTab and include file tabs in session restoration fallback for unifiedTabOrder.
…nces The unified tab system had no documentation about its critical invariant: every tab must exist in both its data array and unifiedTabOrder, or it becomes a ghost tab (content renders but no tab visible in TabBar). Added the invariant, correct/wrong code examples, and documented the shared utilities (buildUnifiedTabs, ensureInUnifiedTabOrder). Fixed stale field name and key file references in ARCHITECTURE.md.
📝 WalkthroughWalkthroughRefactors tab management into public utilities (buildUnifiedTabs, ensureInUnifiedTabOrder) and hooks (useTabHandlers), promotes unifiedTabOrder as the canonical tab order, renames closedUnifiedTabHistory -> unifiedClosedTabHistory, and adds window chrome settings (useNativeTitleBar, autoHideMenuBar) across main/renderer, store, UI, and tests. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
Greptile SummaryThis PR systematically fixes ghost-tab bugs in the unified tab system by ensuring Key improvements:
Implementation quality:
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start[Tab Operation: Open/Close/Reopen/Restore] --> CheckType{Operation Type?}
CheckType -->|New AI Tab| CreateAI[createTab in tabHelpers.ts]
CreateAI --> AddAIToOrder[Add tab to aiTabs AND unifiedTabOrder]
AddAIToOrder --> Success[Tab visible in TabBar]
CheckType -->|Open File Tab| OpenFile[handleOpenFileTab in useTabHandlers.ts]
OpenFile --> FileExists{Tab exists?}
FileExists -->|Yes| Repair[ensureInUnifiedTabOrder]
FileExists -->|No| CreateFile[Create new FilePreviewTab]
CreateFile --> AddFileToOrder[Add to filePreviewTabs AND unifiedTabOrder]
Repair --> Success
AddFileToOrder --> Success
CheckType -->|Reopen Closed Tab| Reopen[reopenUnifiedClosedTab in tabHelpers.ts]
Reopen --> DuplicateCheck{Duplicate exists?}
DuplicateCheck -->|Yes| RepairDup[ensureInUnifiedTabOrder for existing tab]
DuplicateCheck -->|No| RestoreTab[Restore tab data with new ID]
RestoreTab --> InsertOrder[Insert at original position in unifiedTabOrder]
RepairDup --> Success
InsertOrder --> Success
CheckType -->|Session Restore| SessionRestore[useSessionRestoration.ts]
SessionRestore --> BuildOrder[Build unifiedTabOrder from AI + file tabs]
BuildOrder --> Success
Success --> Render[buildUnifiedTabs builds final tab list]
Render --> TabBar[TabBar renders all tabs correctly]
TabBar --> Orphan{Orphaned tabs?}
Orphan -->|Yes| Append[buildUnifiedTabs appends as safety net]
Orphan -->|No| Done[All tabs visible]
Append --> Done
Last reviewed commit: 2a46999 |
This was referenced Feb 19, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/__tests__/main/app-lifecycle/window-manager.test.ts (1)
144-146: Tests updated correctly, but coverage for enabled window chrome settings is missing.All tests pass
useNativeTitleBar: falseandautoHideMenuBar: false. Consider adding test cases that verify:
useNativeTitleBar: trueomitstitleBarStylefrom BrowserWindow optionsautoHideMenuBar: trueincludesautoHideMenuBar: truein optionsThis would ensure the conditional logic in
createWindowis exercised for both branches.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/main/app-lifecycle/window-manager.test.ts` around lines 144 - 146, Add test cases for createWindow to exercise both branches of the window chrome conditionals: create one test that calls createWindow with useNativeTitleBar: true and asserts the constructed BrowserWindow options do not contain titleBarStyle, and another test that calls createWindow with autoHideMenuBar: true and asserts the BrowserWindow options include autoHideMenuBar: true; locate where current tests stub BrowserWindow construction (the code that inspects the options passed when createWindow is invoked) and extend it to cover these two scenarios so the conditional logic around useNativeTitleBar and autoHideMenuBar is asserted.src/renderer/utils/tabHelpers.ts (1)
27-61: Consider defensive checks for consistency with file patterns.Other functions in this file (e.g.,
closeTab,reopenClosedTab) guard against undefined session properties. For a "hardening" PR, adding similar guards here would provide robustness against partial session state during initialization or corruption recovery.🛡️ Optional defensive check
export function buildUnifiedTabs(session: Session): UnifiedTab[] { + if (!session) return []; const { aiTabs, filePreviewTabs, unifiedTabOrder } = session; - const aiTabMap = new Map(aiTabs.map((tab) => [tab.id, tab])); - const fileTabMap = new Map(filePreviewTabs.map((tab) => [tab.id, tab])); + const aiTabMap = new Map((aiTabs || []).map((tab) => [tab.id, tab])); + const fileTabMap = new Map((filePreviewTabs || []).map((tab) => [tab.id, tab])); const result: UnifiedTab[] = []; // Follow unified order for tabs that have entries - for (const ref of unifiedTabOrder) { + for (const ref of unifiedTabOrder || []) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/utils/tabHelpers.ts` around lines 27 - 61, buildUnifiedTabs assumes session.aiTabs, session.filePreviewTabs, and session.unifiedTabOrder are defined; add defensive guards at the start of buildUnifiedTabs to default any of those undefined/null properties to empty arrays (e.g., const aiTabs = session.aiTabs ?? []; const filePreviewTabs = session.filePreviewTabs ?? []; const unifiedTabOrder = session.unifiedTabOrder ?? [];), then proceed using those local variables when creating aiTabMap, fileTabMap and iterating the order so the function tolerates partial/corrupt session state without throwing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/components/SettingsModal.tsx`:
- Around line 2559-2651: The two custom toggle buttons in SettingsModal (the
button toggling useNativeTitleBar using setUseNativeTitleBar/useNativeTitleBar
and the button toggling autoHideMenuBar using
setAutoHideMenuBar/autoHideMenuBar) need explicit focus handling: add
tabIndex={0} (or tabIndex={-1} if you intend them not focusable) and include the
"outline-none" class on each button so they follow renderer focus guidelines; if
either toggle should auto-focus, add a ref callback like ref={(el) =>
el?.focus()} to that button. Ensure you update both the native title bar button
and the auto-hide menu bar button.
---
Nitpick comments:
In `@src/__tests__/main/app-lifecycle/window-manager.test.ts`:
- Around line 144-146: Add test cases for createWindow to exercise both branches
of the window chrome conditionals: create one test that calls createWindow with
useNativeTitleBar: true and asserts the constructed BrowserWindow options do not
contain titleBarStyle, and another test that calls createWindow with
autoHideMenuBar: true and asserts the BrowserWindow options include
autoHideMenuBar: true; locate where current tests stub BrowserWindow
construction (the code that inspects the options passed when createWindow is
invoked) and extend it to cover these two scenarios so the conditional logic
around useNativeTitleBar and autoHideMenuBar is asserted.
In `@src/renderer/utils/tabHelpers.ts`:
- Around line 27-61: buildUnifiedTabs assumes session.aiTabs,
session.filePreviewTabs, and session.unifiedTabOrder are defined; add defensive
guards at the start of buildUnifiedTabs to default any of those undefined/null
properties to empty arrays (e.g., const aiTabs = session.aiTabs ?? []; const
filePreviewTabs = session.filePreviewTabs ?? []; const unifiedTabOrder =
session.unifiedTabOrder ?? [];), then proceed using those local variables when
creating aiTabMap, fileTabMap and iterating the order so the function tolerates
partial/corrupt session state without throwing.
… tab guards Add tabIndex and outline-none to Window Chrome toggle buttons for keyboard accessibility. Add defensive null guards to buildUnifiedTabs for robustness during partial session state. Add test coverage for useNativeTitleBar and autoHideMenuBar enabled branches.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/utils/tabHelpers.ts`:
- Around line 68-76: ensureInUnifiedTabOrder lacks a null/undefined guard for
its unifiedTabOrder parameter and will throw if called with undefined (unlike
buildUnifiedTabs). Update ensureInUnifiedTabOrder to defensively handle
unifiedTabOrder being null/undefined: treat undefined as an empty array, then
perform the existing existence check and append logic (so callers like
session.unifiedTabOrder can be passed safely). Keep the function signature but
internally coalesce unifiedTabOrder to [] before using .some and returning the
new array.
Comment on lines
+68
to
+76
| export function ensureInUnifiedTabOrder( | ||
| unifiedTabOrder: UnifiedTabRef[], | ||
| type: 'ai' | 'file', | ||
| id: string | ||
| ): UnifiedTabRef[] { | ||
| const exists = unifiedTabOrder.some((ref) => ref.type === type && ref.id === id); | ||
| if (exists) return unifiedTabOrder; | ||
| return [...unifiedTabOrder, { type, id }]; | ||
| } |
There was a problem hiding this comment.
Missing null guard for unifiedTabOrder parameter.
Unlike buildUnifiedTabs which defensively handles undefined arrays, this function will throw a TypeError if unifiedTabOrder is null/undefined. The call sites at lines 767 and 836 pass session.unifiedTabOrder directly, which could be undefined during migration or partial state scenarios.
🛡️ Proposed fix to add defensive null handling
export function ensureInUnifiedTabOrder(
unifiedTabOrder: UnifiedTabRef[],
type: 'ai' | 'file',
id: string
): UnifiedTabRef[] {
+ const order = unifiedTabOrder || [];
- const exists = unifiedTabOrder.some((ref) => ref.type === type && ref.id === id);
- if (exists) return unifiedTabOrder;
- return [...unifiedTabOrder, { type, id }];
+ const exists = order.some((ref) => ref.type === type && ref.id === id);
+ if (exists) return order;
+ return [...order, { type, id }];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export function ensureInUnifiedTabOrder( | |
| unifiedTabOrder: UnifiedTabRef[], | |
| type: 'ai' | 'file', | |
| id: string | |
| ): UnifiedTabRef[] { | |
| const exists = unifiedTabOrder.some((ref) => ref.type === type && ref.id === id); | |
| if (exists) return unifiedTabOrder; | |
| return [...unifiedTabOrder, { type, id }]; | |
| } | |
| export function ensureInUnifiedTabOrder( | |
| unifiedTabOrder: UnifiedTabRef[], | |
| type: 'ai' | 'file', | |
| id: string | |
| ): UnifiedTabRef[] { | |
| const order = unifiedTabOrder || []; | |
| const exists = order.some((ref) => ref.type === type && ref.id === id); | |
| if (exists) return order; | |
| return [...order, { type, id }]; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/renderer/utils/tabHelpers.ts` around lines 68 - 76,
ensureInUnifiedTabOrder lacks a null/undefined guard for its unifiedTabOrder
parameter and will throw if called with undefined (unlike buildUnifiedTabs).
Update ensureInUnifiedTabOrder to defensively handle unifiedTabOrder being
null/undefined: treat undefined as an empty array, then perform the existing
existence check and append logic (so callers like session.unifiedTabOrder can be
passed safely). Keep the function signature but internally coalesce
unifiedTabOrder to [] before using .some and returning the new array.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
buildUnifiedTabs()andensureInUnifiedTabOrder()into sharedtabHelpers.ts, fixing ghost-tab bugs where tabs existed in state but were invisible in the TabBar due to missingunifiedTabOrderentries. All tab mutation paths (open, close, reopen, restore, transfer) now consistently maintain the invariant.BrowserWindowintegration and tests.unifiedTabOrdercritical invariant, correct type names, and updated key files table.Changes
Tab System (
tabHelpers.ts,useTabHandlers.ts,tabStore.ts,App.tsx)buildUnifiedTabs(session),ensureInUnifiedTabOrder(session, ref)handleOpenFileTabnow repairsunifiedTabOrderwhen selecting an existing file tabreopenUnifiedClosedTabrepairs order for both AI and file tab duplicate pathsselectUnifiedTabsin tabStore replaced with sharedbuildUnifiedTabsunifiedTabOrderon creationWindow Chrome (
settingsStore.ts,useSettings.ts,SettingsModal.tsx,window-manager.ts)useNativeTitleBarandautoHideMenuBarsettings with IPC round-tripFile Explorer Icons (
FileExplorerPanel.tsx,theme.tsx)Tests
tabHelpers.test.ts— 180 lines coveringbuildUnifiedTabsandensureInUnifiedTabOrderwindow-manager.test.ts— native titlebar and autoHideMenuBar testsutils.test.tsandFileExplorerPanel.test.tsx— supporting testsTest plan
npm run lintpassesnpm run testpassesSummary by CodeRabbit
New Features
Improvements
Bug Fixes