Skip to content

fix active tab - #139

Merged
Kitenite merged 4 commits into
mainfrom
fix-active-tab
Nov 24, 2025
Merged

Kitenite merged 4 commits into
mainfrom
fix-active-tab

Conversation

@Kitenite

@Kitenite Kitenite commented Nov 24, 2025 •

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Tests

    • Expanded test coverage for tab selection and group behaviors, including history/fallback, sibling navigation, cross-workspace cases, and removal scenarios.
  • Bug Fixes

    • More reliable and predictable active-tab selection when closing or removing tabs and groups; history is kept consistent after removals.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Nov 24, 2025 •

Copy link
Copy Markdown
Contributor

Caution

Review failed

The pull request is closed.

Walkthrough

Refactors next-tab selection logic and adds extensive tests: history-based selection, sibling/group navigation, top-level positional fallback, and cross-workspace edge cases. Group removal now uses findNextTab to pick a new active tab and prunes history entries when a child tab is deleted.

Changes

Cohort / File(s) Summary
Next-tab tests
apps/desktop/src/renderer/stores/tabs/helpers/next-tab-finder.test.ts
Added a large suite of tests covering history preference, positional fallback, group/sibling navigation, mixed top-level & grouped scenarios, cross-workspace behavior, and edge cases ensuring a next tab is returned.
Next-tab logic
apps/desktop/src/renderer/stores/tabs/helpers/next-tab-finder.ts
Reordered and simplified selection priority: iterate strategies (history → sibling in same group → top-level by position → final workspace fallback) and short-circuit on first valid result. Reworked history and group selection helpers and removed legacy fallback logic.
Group operations tests
apps/desktop/src/renderer/stores/tabs/helpers/group-operations.test.ts
Added tests for removing a child from a group: active tab updates (next/previous sibling), history pruning, group removal when last child gone, and ensuring removed entries are absent from state.
Group operations logic
apps/desktop/src/renderer/stores/tabs/helpers/group-operations.ts
When removing an active child, select new active tab via findNextTab and update activeTabIds and tabHistoryStacks (prune removed tab). Tabs validation unchanged.
Store tests
apps/desktop/src/renderer/stores/tabs/store.test.ts
Removed assertions expecting the new split tabs to have title "New Tab" (test assertion relaxation only).
Manifest
package.json
Present in diff metadata (no code-summary changes noted).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant UI as UI / Close Action
    participant Finder as findNextTab
    participant Hist as Tab History Stack
    participant Group as Group Siblings
    participant Top as Top-level Tabs
    participant Work as Workspace Tabs

    Note over Finder: Priority order (checked iteratively)
    UI->>Finder: request next for closedTab in workspace
    Finder->>Hist: check most-recent entries for valid tab
    alt history hit
        Hist-->>Finder: return recent valid tab
    else no history hit
        Finder->>Group: look for sibling (same parent)
        alt sibling found
            Group-->>Finder: return sibling
        else no sibling
            Finder->>Top: determine relative top-level position
            alt next/prev top-level found
                Top-->>Finder: return top-level tab
            else fallback
                Finder->>Work: pick first remaining workspace tab
                Work-->>Finder: return fallback tab
            end
        end
    end
    Finder-->>UI: nextTabId (or undefined)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Areas needing extra attention:
    • findNextTab priority short-circuiting and correctness across all edge cases (history vs group vs top-level).
    • findNextTopLevelTab index calculations and boundary behavior when closed tab is first/last.
    • Group removal flow in group-operations.ts ensuring history stacks and activeTabIds are consistently updated and no dangling references remain.
    • New tests in next-tab-finder.test.ts — verify they accurately reflect intended behavior and are not brittle.

Possibly related PRs

  • tabs interaction #111 — Overlaps changes to tab activation and group-removal code paths; likely touches the same helpers and tests.
  • refactor tabs store #110 — Refactor of group-operations and active-tab helpers; directly related to how removed children determine next active tab.
  • fix active tab #139 — Modifications to next-tab selection logic and top-level selection behavior; highly connected to these changes.

Poem

🐰
I hop through stacks and tabs anew,
I favor history, then siblings true,
If none remain, I scan the top,
Else pick the first and never stop.
A little hop, a careful cue. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The description is a list of unrelated commits rather than a structured explanation of this PR's changes and lacks required template sections like description, testing, and type of change. Replace the commit list with a clear description of the actual changes, fill in all required template sections, and link related issues using GitHub keywords.
Title check ❓ Inconclusive The title is vague and does not clearly specify what aspect of the active tab behavior is being fixed or how it relates to the detailed changes in the changeset. Consider a more descriptive title that clarifies the specific fix, such as 'Fix next tab selection when closing a tab' or 'Refactor tab fallback logic in next-tab-finder'.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c3e5df and e1b18c5.

📒 Files selected for processing (5)
  • apps/desktop/src/renderer/stores/tabs/helpers/group-operations.test.ts (1 hunks)
  • apps/desktop/src/renderer/stores/tabs/helpers/group-operations.ts (1 hunks)
  • apps/desktop/src/renderer/stores/tabs/helpers/next-tab-finder.test.ts (1 hunks)
  • apps/desktop/src/renderer/stores/tabs/helpers/next-tab-finder.ts (4 hunks)
  • apps/desktop/src/renderer/stores/tabs/store.test.ts (0 hunks)

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.

@Kitenite
Kitenite merged commit 0507561 into main Nov 24, 2025
0 of 5 checks passed
@Kitenite
Kitenite deleted the fix-active-tab branch November 24, 2025 07:08
@Kitenite Kitenite mentioned this pull request Nov 24, 2025
AviPeltz added a commit that referenced this pull request May 13, 2026
* chore(deps): bump simple-git to 3.36.0 to patch GHSA-hffm-xvc3-vprc

Closes Dependabot alerts #65 and #139 (RCE via --config option injection).
Current call sites pass static option arrays, so exposure was theoretical,
but bumping is defense-in-depth.

* chore(deps): bump next 16.2.1 -> 16.2.6 across all apps

16.2.6 patches every flagged Next.js Dependabot CVE, including the
proxy/middleware bypass advisories (GHSA-26hh-7cqf-hhc6, etc.) that
matter most given web/admin/api enforce auth via proxy.ts. Patch-level
bump, no breaking changes in the 16.2.x range.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant