fix(terminal): number tabs by creation order, not visual position by chr1syy · Pull Request #956 · RunMaestro/Maestro

chr1syy · 2026-05-05T06:12:41Z

Summary

Opening a 2nd terminal while an AI tab is active caused the new terminal to display as "Terminal 1" and the original to be re-labeled "Terminal 2"
Root cause: TabBar.tsx computed the Terminal N index from the visual unifiedTabs order, but addTerminalTab inserts the new terminal directly after the currently-active tab — placing it visually to the LEFT of the existing terminal when an AI tab is active
Replaced the visual-order findIndex with a useMemo-cached terminalIndexById map sorted by data.createdAt, aligning TabBar with the storage-order indexing already used in MainPanel.tsx and AppSessionModals.tsx

Test plan

New regression test in TabBar.test.tsx renders a unified tab list where the newer terminal sits before the older one in visual order, asserts older stays "Terminal 1" / newer becomes "Terminal 2"
TabBar test suite: 171 pass / 0 fail
terminalTabHelpers + AppSessionModals tests: 57 pass / 0 fail
TypeScript clean on changed files
Prettier + ESLint clean
Manual: open agent → open Terminal → click AI tab → open another Terminal → verify they read "Terminal 1" / "Terminal 2" in creation order

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Fixed terminal tab numbering to correctly reflect creation order, ensuring "Terminal 1" and "Terminal 2" labels remain consistent with their actual terminal tabs even when displayed in different visual order.

When a user clicks back to an AI tab and then opens a 2nd terminal, addTerminalTab inserts the new terminal directly after the active AI tab — visually to the LEFT of the existing terminal. The TabBar computed "Terminal N" labels from visual order, so the new tab became "Terminal 1" and the original was renamed to "Terminal 2". Pre-compute a terminalIndexById map sorted by createdAt and use it when resolving the display name. This matches the storage-order indexing already used in MainPanel and AppSessionModals. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-05-05T06:13:02Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4cf5c6a0-2196-4cdf-954f-0d2a56d30d0b

📥 Commits

Reviewing files that changed from the base of the PR and between 6ecf5ae and 6ed771d.

📒 Files selected for processing (2)

src/__tests__/renderer/components/TabBar.test.tsx
src/renderer/components/TabBar/TabBar.tsx

📝 Walkthrough

Walkthrough

Terminal tabs in unified mode are now numbered by creation order rather than visual position in the tabs array. This is implemented via a memoized Map that sorts terminals by createdAt, with a regression test ensuring the fix holds.

Changes

Terminal Tab Numbering by Creation Order

Layer / File(s)	Summary
Core Implementation `src/renderer/components/TabBar/TabBar.tsx` (lines 311–325)	Introduces memoized `terminalIndexById` Map that collects terminal tabs from `allTabs`, sorts by `createdAt`, and maps each tab's `id` to a stable display index.
Usage Integration `src/renderer/components/TabBar/TabBar.tsx` (line 538)	Terminal tab's `terminalIndex` now reads from `terminalIndexById` instead of performing per-render filtering and `findIndex` lookup.
Regression Test `src/__tests__/renderer/components/TabBar.test.tsx` (lines 4634–4693)	New test case verifies that "Terminal 1" and "Terminal 2" labels map to the correct terminal IDs by creation order, even when unified tabs array presents them in reverse visual order.

Estimated Code Review Effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested Labels

ready to merge

Poem

🐰 Tabs dance in the light,
By creation, not sight,
Terminal One stays true,
Numbering born anew!
Order preserved right. ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(terminal): number tabs by creation order, not visual position' accurately and specifically summarizes the main change—fixing terminal numbering to use creation order instead of visual position.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

greptile-apps · 2026-05-05T06:14:48Z

Greptile Summary

This PR fixes a terminal tab mislabeling bug where opening a second terminal while an AI tab is active caused the new terminal to appear as "Terminal 1" and the existing one to be renamed "Terminal 2". The fix replaces a visual-order findIndex lookup in TabBar.tsx with a useMemo-cached Map keyed by createdAt, matching the creation-order indexing already used in MainPanel.tsx and AppSessionModals.tsx. A focused regression test covers the inverted-visual-order scenario.

Confidence Score: 5/5

Safe to merge — the change is narrowly scoped to terminal tab label computation with no side-effects on tab state or ordering.

The fix is straightforward: one well-bounded useMemo replaces a single findIndex call, the createdAt field is a required non-nullable number on TerminalTab, and a targeted regression test validates the exact failure scenario. The only leftover is a now-harmless dead ternary guard at line 544.

No files require special attention.

Important Files Changed

Filename	Overview
src/renderer/components/TabBar/TabBar.tsx	Replaces visual-order findIndex with a useMemo-cached Map keyed by createdAt; fix is correct and the only leftover is a now-dead ternary guard.
src/tests/renderer/components/TabBar.test.tsx	New regression test correctly exercises the inverted-visual-order scenario and asserts creation-order numbering; well-structured and clear.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["unifiedTabs prop changes"] --> B["allTabs = unifiedTabs ?? []"]
    B --> C["useMemo: build terminalIndexById"]
    C --> D["Filter allTabs for type === 'terminal'"]
    D --> E["Sort filtered terminals by createdAt (ascending)"]
    E --> F["Map: id → sorted index (0, 1, 2…)"]
    F --> G["Render loop over allTabs"]
    G -->|terminal tab| H["terminalIndex = terminalIndexById.get(id) ?? 0"]
    H --> I["TerminalTabItem rendered as 'Terminal N+1'"]
    G -->|ai/file/browser tab| J["Render other tab type"]

Comments Outside Diff (1)

src/renderer/components/TabBar/TabBar.tsx, line 544 (link)

Dead guard left from old findIndex logic

terminalIndexById.get(unifiedTab.id) ?? 0 already guarantees a non-negative value — ?? 0 handles the missing-id case, and every value stored by forEach is >= 0. The terminalIndex >= 0 ? terminalIndex : 0 conditional is unreachable and can be simplified to just terminalIndex.

_{Reviews (1): Last reviewed commit: "fix(terminal): number tabs by creation o..." | Re-trigger Greptile}

pedramamini · 2026-05-05T06:44:13Z

Thanks for the contribution, @chr1syy! Nice clean fix — the useMemo Map keyed by createdAt is the right call, and the regression test pinpoints the inverted-visual-order scenario perfectly. Aligns with the storage-order indexing already used in MainPanel.tsx / AppSessionModals.tsx, which is a nice consistency win. CI is green and the diff is mergeable with no conflicts.

Approving as-is. One tiny non-blocking nit if you want to tidy up in a follow-up: the existing terminalIndex={terminalIndex >= 0 ? terminalIndex : 0} at the TerminalTabItem call site is now dead — terminalIndexById.get(unifiedTab.id) ?? 0 already guarantees a non-negative number, so it can simplify to just terminalIndex={terminalIndex}. Totally optional.

chr1syy added the ready to merge This PR is ready to merge label May 5, 2026

pedramamini added the approved label May 5, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(terminal): number tabs by creation order, not visual position#956

fix(terminal): number tabs by creation order, not visual position#956
chr1syy wants to merge 1 commit intoRunMaestro:rcfrom
chr1syy:fix/terminal-naming

chr1syy commented May 5, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 5, 2026 •

edited

Loading

Walkthrough

Changes

Estimated Code Review Effort

Suggested Labels

Poem

Uh oh!

greptile-apps Bot commented May 5, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

pedramamini commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

chr1syy commented May 5, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated Code Review Effort

Suggested Labels

Poem

Uh oh!

greptile-apps Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Comments Outside Diff (1)

Uh oh!

pedramamini commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chr1syy commented May 5, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 5, 2026 •

edited

Loading

greptile-apps Bot commented May 5, 2026 •

edited

Loading