fix: Filter worktree child sessions from web UI sidebar#382
fix: Filter worktree child sessions from web UI sidebar#382EEmayank wants to merge 1 commit intoRunMaestro:mainfrom
Conversation
pedramamini
left a comment
pedramamini
left a comment
There was a problem hiding this comment.
PR Review: Filter worktree child sessions from web UI sidebar
Critical: sessionGrouping.ts is dead code — nothing imports it
The new utility module (src/web/utils/sessionGrouping.ts) is created but never imported anywhere. Grepping for sessionGrouping across the entire src/ tree returns zero matches. The module's docstring claims it's "Used by Sidebar, AllSessionsView, and SessionPillBar" — but none of those components import or use it.
The PR doesn't actually fix the bug. The filtering logic exists but is disconnected.
Major: Duplicate code with AllSessionsView.tsx
AllSessionsView.tsx (lines 200-295) already contains identical inline implementations of:
findParentSession()(with extra debugconsole.logstatements)getSessionDisplayName()getSessionEffectiveGroup()
The new sessionGrouping.ts is a cleaner copy of the same functions. The right approach: create the shared module and refactor AllSessionsView.tsx to import from it, deleting the inline copies. This also removes the debug console.log sprinkled throughout the AllSessionsView.tsx versions.
Minor: SessionPillBar.tsx doesn't filter either
SessionPillBar.tsx renders sessions without any parentSessionId filtering. If the utility module is meant to be shared across all web views, it should also be wired into the pill bar.
Valid: SessionData type change
The parentSessionId/worktreeBranch fields exist on SessionBroadcastData but were missing from SessionData. This addition aligns the two types — good change.
Suggested action items
- Wire up imports — Import
groupSessions,filterSessions,getSessionDisplayNameinto the web sidebar/views that actually render session lists - Refactor
AllSessionsView.tsx— Replace inlinefindParentSession/getSessionDisplayName/getSessionEffectiveGroupwith imports from the shared module (also removes debugconsole.logspam) - Filter in
SessionPillBar.tsx— ApplyparentSessionIdfilter so worktree children don't appear as top-level pills - Add a test for
groupSessions()confirming worktree children are excluded from top-level listing
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThe changes extend the SessionData interface with two new optional fields ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/web/utils/sessionGrouping.ts`:
- Around line 138-154: filterSessions currently only checks fields on each
session row (using getSessionDisplayName and
session.name/cwd/toolType/worktreeBranch) but because groupSessions removes
child rows a query that only matches a child (e.g., child's worktreeBranch) will
exclude the parent and return no results; fix by extending filterSessions to
also consider children: for each parent session being tested, look up its child
sessions from allSessions (e.g., by parent id relationship used in
groupSessions) and treat the parent as a match if any child’s displayName, name,
cwd, toolType, or worktreeBranch matches the query (i.e., "promote" a matching
child to include its parent in results); keep all existing checks (including
getSessionDisplayName) and short-circuit when query is empty.
- Around line 19-33: The inferred basePath from the worktree regex may include a
trailing path separator which breaks equality and startsWith checks; normalize
basePath (e.g., strip any trailing '/' or '\\' or run through path
normalization) into a new variable like basePathNormalized and use that in the
sessions.find comparisons (replace uses of basePath with basePathNormalized and
build startsWith checks with basePathNormalized + '/' and basePathNormalized +
'\\') so parent inference works reliably across path layouts.
| // Try to infer parent from worktree path patterns | ||
| const cwd = session.cwd; | ||
| const worktreeMatch = cwd.match(/^(.+?)[-]?WorkTrees[\/\\]([^\/\\]+)/i); | ||
|
|
||
| if (worktreeMatch) { | ||
| const basePath = worktreeMatch[1]; | ||
| return ( | ||
| sessions.find( | ||
| (s) => | ||
| s.id !== session.id && | ||
| !s.parentSessionId && | ||
| (s.cwd === basePath || | ||
| s.cwd.startsWith(basePath + '/') || | ||
| s.cwd.startsWith(basePath + '\\')) | ||
| ) || null |
There was a problem hiding this comment.
Normalize inferred basePath before comparisons.
The regex capture can include a trailing separator (e.g., .../WorkTrees/...), which makes the equality and startsWith checks miss the parent path. This breaks parent inference in common path layouts.
🛠️ Proposed fix
- const basePath = worktreeMatch[1];
+ const basePath = worktreeMatch[1].replace(/[\/\\]+$/, '');📝 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.
| // Try to infer parent from worktree path patterns | |
| const cwd = session.cwd; | |
| const worktreeMatch = cwd.match(/^(.+?)[-]?WorkTrees[\/\\]([^\/\\]+)/i); | |
| if (worktreeMatch) { | |
| const basePath = worktreeMatch[1]; | |
| return ( | |
| sessions.find( | |
| (s) => | |
| s.id !== session.id && | |
| !s.parentSessionId && | |
| (s.cwd === basePath || | |
| s.cwd.startsWith(basePath + '/') || | |
| s.cwd.startsWith(basePath + '\\')) | |
| ) || null | |
| // Try to infer parent from worktree path patterns | |
| const cwd = session.cwd; | |
| const worktreeMatch = cwd.match(/^(.+?)[-]?WorkTrees[\/\\]([^\/\\]+)/i); | |
| if (worktreeMatch) { | |
| const basePath = worktreeMatch[1].replace(/[\/\\]+$/, ''); | |
| return ( | |
| sessions.find( | |
| (s) => | |
| s.id !== session.id && | |
| !s.parentSessionId && | |
| (s.cwd === basePath || | |
| s.cwd.startsWith(basePath + '/') || | |
| s.cwd.startsWith(basePath + '\\')) | |
| ) || null |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/web/utils/sessionGrouping.ts` around lines 19 - 33, The inferred basePath
from the worktree regex may include a trailing path separator which breaks
equality and startsWith checks; normalize basePath (e.g., strip any trailing '/'
or '\\' or run through path normalization) into a new variable like
basePathNormalized and use that in the sessions.find comparisons (replace uses
of basePath with basePathNormalized and build startsWith checks with
basePathNormalized + '/' and basePathNormalized + '\\') so parent inference
works reliably across path layouts.
| export function filterSessions( | ||
| sessions: Session[], | ||
| query: string, | ||
| allSessions: Session[] | ||
| ): Session[] { | ||
| if (!query.trim()) return sessions; | ||
| const q = query.toLowerCase(); | ||
| return sessions.filter((session) => { | ||
| const displayName = getSessionDisplayName(session, allSessions); | ||
| return ( | ||
| displayName.toLowerCase().includes(q) || | ||
| session.name.toLowerCase().includes(q) || | ||
| session.cwd.toLowerCase().includes(q) || | ||
| (session.toolType && session.toolType.toLowerCase().includes(q)) || | ||
| (session.worktreeBranch && session.worktreeBranch.toLowerCase().includes(q)) | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Search can drop matches that only exist in child sessions.
Because groupSessions() removes children, a query that matches only a worktree child (e.g., by worktreeBranch) yields an empty UI result. Consider promoting a matching child to its parent (or otherwise ensuring the parent appears) so search remains useful.
🛠️ Proposed fix (promote matching child to parent)
export function filterSessions(
sessions: Session[],
query: string,
allSessions: Session[]
): Session[] {
if (!query.trim()) return sessions;
const q = query.toLowerCase();
- return sessions.filter((session) => {
- const displayName = getSessionDisplayName(session, allSessions);
- return (
- displayName.toLowerCase().includes(q) ||
- session.name.toLowerCase().includes(q) ||
- session.cwd.toLowerCase().includes(q) ||
- (session.toolType && session.toolType.toLowerCase().includes(q)) ||
- (session.worktreeBranch && session.worktreeBranch.toLowerCase().includes(q))
- );
- });
+ const results = new Map<string, Session>();
+ for (const session of sessions) {
+ const displayName = getSessionDisplayName(session, allSessions);
+ const matches =
+ displayName.toLowerCase().includes(q) ||
+ session.name.toLowerCase().includes(q) ||
+ session.cwd.toLowerCase().includes(q) ||
+ (session.toolType && session.toolType.toLowerCase().includes(q)) ||
+ (session.worktreeBranch && session.worktreeBranch.toLowerCase().includes(q));
+ if (matches) {
+ const parent = findParentSession(session, allSessions);
+ const effective = parent ?? session;
+ results.set(effective.id, effective);
+ }
+ }
+ return Array.from(results.values());
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/web/utils/sessionGrouping.ts` around lines 138 - 154, filterSessions
currently only checks fields on each session row (using getSessionDisplayName
and session.name/cwd/toolType/worktreeBranch) but because groupSessions removes
child rows a query that only matches a child (e.g., child's worktreeBranch) will
exclude the parent and return no results; fix by extending filterSessions to
also consider children: for each parent session being tested, look up its child
sessions from allSessions (e.g., by parent id relationship used in
groupSessions) and treat the parent as a match if any child’s displayName, name,
cwd, toolType, or worktreeBranch matches the query (i.e., "promote" a matching
child to include its parent in results); keep all existing checks (including
getSessionDisplayName) and short-circuit when query is empty.
2 participants
fix: Filter worktree child sessions from web UI sidebar
Problem
The web UI sidebar displayed significantly more agents than the Electron desktop app. Every worktree child session (branches spawned from a parent agent's worktree config) appeared as a top-level item in the sidebar, inflating the visible agent count. For example, a parent agent with 2 worktree children showed as 3 separate entries in the web sidebar, but only 1 (with expandable children) in the desktop app.
Root Cause
The Electron desktop sidebar in
SessionList.tsxfilters out sessions whereparentSessionIdis set — worktree children are excluded from the main list and rendered as nested sub-items under their parent. The web sidebar had no equivalent filtering:groupSessions()insessionGrouping.tspassed every session through to the rendered output without checking for parent-child relationships.Additionally, the server-side
SessionDatatype intypes.tsdid not declareparentSessionIdorworktreeBranch, even though the factory was already serializing and sending those fields to web clients — a type-safety gap.Fix
src/main/web-server/types.tsparentSessionId?: string | nullandworktreeBranch?: string | nullto theSessionDatainterface, aligning the type definition with the data already being sent by the server factory.src/web/utils/sessionGrouping.tstopLevelSessionsfilter at the top ofgroupSessions()that excludes sessions with aparentSessionId, matching the Electron sidebar behavior.topLevelSessionsinstead of the unfiltered input.Summary by CodeRabbit