feat(sidebar): nest forked sessions under their parent (#3224)

[rodboev]

Thinking Path

• The sidebar already subgroups two session relationship classes: compression-lineage segments (collapsed into one row) and subagent child sessions (rendered as an expandable session-child-sessions group via _expandedChildSessionKeys). Forks are the third relationship class and receive only a branch-indicator icon, no nesting.
• The fork early-return in _sessionLineageKey (static/sessions.js:3482) was intentional: PRs fix: keep explicit fork sessions out of compression lineage #2014 and fix: keep explicit forks out of lineage report #2063 added it to keep forks out of compression-lineage collapse when the parent row is absent or is a compression-continuation. Removing it outright would reintroduce that regression.
• The right approach is conditional: keep the _sessionLineageKey guard (so forks never merge into compression-lineage chains), but extend _attachChildSessionsToSidebarRows to also route fork sessions that have a resolvable in-list parent through the existing session-child-sessions machinery.
• A new _isForkWithResolvableParent(s, sessionIdsInList) helper guards the extension: it returns true only when session_source==='fork' and parent_session_id is present in the current sidebar payload. CLI-imported forks whose parent is not a WebUI session carry _cross_surface_child_session; the orphan path is preserved for those via if(!isForkChild&&child._cross_surface_child_session).
• _resolveSessionIdFromSidebarLineage skips both fork and child-session rows in its candidate scan (static/sessions.js:3526). The child-session skip stays (those rows are nested, not selectable top-level rows), but the fork skip must be removed: once a fork is nested, it may be the active session and must be resolvable.
• The flat timestamp sort at static/sessions.js:4216 controls sidebar row order. A parent with a recent fork child should not jump to the top of its date bucket; the parent's own timestamp determines its position, not its children's.
• The existing renderer at static/sessions.js:4583 and the _expandedChildSessionKeys set at static/sessions.js:1844 are reused without modification; the only new wire-up is feeding fork children into _child_sessions arrays during _attachChildSessionsToSidebarRows.
• tests/test_465_session_branching.py:85-93 asserts that the if(s.session_source==='fork') return null; guard is present in _sessionLineageKey; it will fail the moment the guard is touched. The guard remains in place, but the test assertion wording is updated to reflect the conditional intent (prevent lineage merging, not prevent nesting). A new test asserts the subgrouping path.

What Changed

• static/sessions.js (after line 3477): new _isForkWithResolvableParent(s, sessionIdsInList) helper that returns true when a session is a fork with its parent in the current sidebar payload.
• static/sessions.js (line 3702 region): _attachChildSessionsToSidebarRows inner loop extended to include fork children alongside _isChildSession children; _cross_surface_child_session orphan path preserved for CLI-imported forks.
• static/sessions.js (line 3526): _resolveSessionIdFromSidebarLineage candidate scan no longer skips fork rows wholesale; fork rows nested as children must remain resolvable when active.
• static/sessions.js (line 4216): flat timestamp sort preserved at session level; parent rows sort by their own timestamp, not their fork children's activity.
• tests/test_465_session_branching.py (lines 85-93): test_branch_fork_sessions_do_not_collapse_into_parent_lineage reworded to assert the guard prevents compression-lineage merging (not nesting); new test_branch_fork_sessions_nest_under_parent asserts _isForkWithResolvableParent exists, that _attachChildSessionsToSidebarRows uses it, and that _resolveSessionIdFromSidebarLineage no longer skips fork rows.
• tests/test_session_lineage_collapse.py: added eval(extractFunc('_isForkWithResolvableParent')); to all three Node subprocess test functions that evaluate _attachChildSessionsToSidebarRows, preventing ReferenceError on CI.

Why It Matters

Forked sessions created with /branch appear as flat top-level rows sorted by timestamp, so a branch off an old conversation jumps to the top of the list, detached from the session it came from. After this change, forked sessions with a resolvable parent are nested under that parent (collapsible via the existing child-count badge), keeping the sidebar readable for power users with branch-heavy trees. Closes #3224.

Verification

C:\Apps\hermes\hermes-agent\venv\Scripts\python.exe -m pytest tests/test_465_session_branching.py -v --timeout=60
C:\Apps\hermes\hermes-agent\venv\Scripts\python.exe -m pytest tests/ -v --timeout=60

On Windows, add --noconftest if collection fails due to the pre-existing WinError 1314 symlink-privilege issue; the static-analysis tests do not depend on conftest fixtures. The real gate is CI on Linux (Python 3.11, 3.12, 3.13).

Manual steps: open the WebUI with at least one forked session (use /branch from an older conversation). Confirm: the fork appears nested under its parent row, not at the top of the flat list; the parent row's date-bucket position did not change; the fork is collapsed by default; clicking the child-count badge expands it; opening the fork session resolves correctly. Also confirm CLI-imported forks (without a WebUI parent) still appear as flat orphan rows and are not silently dropped.

Risks / Follow-ups

• The if(s.session_source==='fork') return null; guard in _sessionLineageKey is deliberately preserved; removing it would allow a fork whose parent is a compression-continuation row to merge into the lineage chain, reintroducing the regression fixed by PRs fix: keep explicit fork sessions out of compression lineage #2014 and fix: keep explicit forks out of lineage report #2063.
• CLI-imported forks (_cross_surface_child_session) are not affected: the orphan path for true subagent children is preserved via the !isForkChild&&child._cross_surface_child_session condition.
• Sort order uses the parent's own timestamp. A richer fix anchoring parents to their creation time requires a separate created_at field from the backend; left as a follow-up.
• The session-branch-indicator icon added by feat: session branching (/branch) — fork conversation from any point #465 remains on both the parent row and on the fork child row inside the expanded group; this is consistent with the existing subagent-child rendering pattern.
• The test rewrite is coordinated with the implementation: the guard assertion wording is updated, not the guard itself. The new test relies only on static JS source analysis (no runtime fixtures), matching the existing pattern in the file.

Model Used

Claude Opus 4.8 via Claude Code CLI

[greptile-apps]

Greptile Summary

Forked sessions (created via /branch) are now nested under their parent row in the sidebar rather than appearing as flat top-level entries sorted by their own timestamp, resolving the clutter and disorientation for branch-heavy workflows.

• Core nesting logic (sessions.js): _isForkWithResolvableParent gates which forks are nested; _attachChildSessionsToSidebarRows filters them from top-level rows and depth-sorts them into parent _child_sessions; fork-of-fork chains are flattened under the common visible ancestor via visibleBySegmentSid. The _sessionLineageKey fork guard is deliberately preserved to prevent compression-lineage merging.
• Full interactivity on nested rows: Fork child rows get swipe-to-archive/delete, long-press context menu, rename, batch-select checkbox, and streaming/unread/attention state indicators; parent rows bubble child streaming/unread/attention state (but not last_message_at, preserving sort order).
• Test coverage: New runtime Node-subprocess tests verify nesting, chain-flattening, timestamp preservation, attention bubbling, pinned-fork exemption, and duplicate-row absence; existing tests updated for the ownStreaming split and rename refactor.

Confidence Score: 5/5

Safe to merge; the one style gap (missing selected-state highlight for fork child rows) is cosmetic and does not break functionality.

The nesting logic is well-guarded: unpinned forks with a resolvable parent are filtered from top-level rows and attached to their parent's _child_sessions, so the previously reported duplicate-row issue is resolved. Timestamp sort order is preserved (last_message_at is not mutated). Active-session resolution and auto-expand both correctly traverse _child_sessions. The only gap found is that .session-child-session-fork.selected has no CSS rule, so batch-selected fork children won't visually highlight even though the checkbox checks and the session is correctly included in batch operations.

static/style.css — missing .session-child-session-fork.selected rule for batch-select visual feedback.

Important Files Changed

Filename	Overview
static/sessions.js	Large, well-structured change: adds _isForkWithResolvableParent helper, filters nested forks from top-level rows, attaches them to parent _child_sessions with depth-ordered queue, bubbles state (streaming/unread/attention but NOT timestamp), renders fork children with full interactivity (swipe, rename, context menu), and registers them in _sessionVisibleSidebarIds for batch select.
static/style.css	Comprehensive fork-row CSS added (swipe, streaming, attention, long-press); missing .session-child-session-fork.selected rule for batch-select visual feedback.
tests/test_session_lineage_collapse.py	New Node-subprocess runtime tests verify fork nesting, chain-flattening under root ancestor, timestamp preservation, attention bubbling, and pinned-fork exemption; also adds _isForkWithResolvableParent eval to existing test harnesses to prevent ReferenceError.
tests/test_465_session_branching.py	Updates existing test wording to reflect that the fork guard in _sessionLineageKey prevents lineage merging (not nesting); adds structural static-analysis tests for the new nesting path, search expansion, and state indicators.
tests/test_session_rename_lifecycle.py	Updated to match refactored _buildSessionRenameStarter helper; adds blur-commits-title assertion.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[_attachChildSessionsToSidebarRows] --> B{Build sessionIdsInList}
    B --> C[Filter collapsedRows: remove child_sessions and unpinned resolvable forks]
    C --> D[rows = top-level visible rows]
    B --> E[attachDepthFor: depth 0=root, 1=fork child, 2=fork-of-fork]
    E --> F[attachQueue sorted by depth]
    F --> G{for each child}
    G --> H{isChildSession OR isForkChild?}
    H -- No --> I[skip]
    H -- Yes --> J{cross_surface and not fork?}
    J -- Yes --> K[orphans]
    J -- No --> L{find parentRow via visibleBySid or visibleBySegmentSid}
    L -- found --> M[push to _child_sessions, bubbleSidebarState, update visibleBySegmentSid]
    L -- not found --> K
    M --> N[return rows + orphans]
    D --> N

Loading

_{Reviews (11): Last reviewed commit: "fix(sidebar): add attention and drag-hov..." | Re-trigger Greptile}

[nesquena-hermes]

Thanks @rodboev — the concept is good and reusing the existing _attachChildSessionsToSidebarRows machinery is the right approach. A deep review (Codex + Opus, both independently) surfaced two issues to address before this can ship. The full test suite passed, so these are behavioral, not test failures.

🔴 Required: forks render twice (duplicate rows)

_attachChildSessionsToSidebarRows builds the top-level rows filtering only !_isChildSession(s) (static/sessions.js:4192), then later pushes _isForkWithResolvableParent rows into parentRow._child_sessions — without removing them from the top-level list. So a /branch fork whose parent is visible renders both as a normal top-level row AND nested under its parent. This is immediately visible the first time you branch and the sidebar re-renders.

Fix: compute sessionIdsInList before building rows, and exclude resolvable forks from the top-level list:

const sessionIdsInList = new Set((rawSessions||[]).map(s=>s&&s.session_id).filter(Boolean));
const rows = (collapsedRows||[])
  .filter(s => !_isChildSession(s) && !_isForkWithResolvableParent(s, sessionIdsInList))
  .map(s => ({...s}));

Please also add a runtime test asserting a fork with a visible parent appears exactly once (nested), and a fork with an absent parent stays top-level.

🟠 Design question: nested forks lose their per-row actions

The nested-child render path (static/sessions.js:5173) renders children as plain .session-child-session buttons whose only action is loadSession(). Swipe-delete explicitly excludes .session-child-session (line 5548), and there's no three-dot menu / rename / pin / archive on nested rows. That's intentional for subagent child sessions (effectively read-only). But a fork is a normal, editable session — nesting it this way strips the actions a user expects on it.

So there's a design decision to make:

• Option A: give nested fork rows the full session-row action surface (three-dot menu, swipe-delete, rename, pin) — more work, but keeps forks first-class.
• Option B: accept that nested forks are click-to-open only, and document that (e.g. actions available after opening). Simpler, but a real capability reduction vs a top-level fork.

We'd lean toward A (forks staying first-class) but it's your call as the author — let us know which direction you want and we'll re-review. Once the duplicate-render fix + the action decision are in, this should be a clean merge.

[rodboev]

Addressed the follow-up regressions in the nested fork path.

1. Kept inline rename behavior consistent with the existing sidebar flow: blur now saves, and the shared rename helper updates title, display_title, and _state_db_title together so renamed fork rows do not snap back on the next cache render.
2. Kept nested fork search hits visible by expanding child groups while a sidebar search is active.
3. Restored standard state affordances on expanded nested fork rows so unread / streaming / attention state stays visible on the specific fork row, not only on the parent.
4. Kept the Node extraction tests UTF-8 safe on Windows.

Validation:

• npx eslint --no-config-lookup -c eslint.runtime-guard.config.mjs "static/**/*.js"
• python -m pytest tests/test_1764_context_menu_essentials.py::TestSessionRenameMenuItem tests/test_465_session_branching.py tests/test_session_lineage_collapse.py tests/test_session_touch_actions.py tests/test_session_rename_lifecycle.py

[nesquena-hermes]

Re-reviewed the new commits (0740f60d…b67d764f) against origin/master. Both items from the prior review are addressed cleanly — reading the actual diff, not just the description.

🔴 Duplicate-render — fixed and verified

_attachChildSessionsToSidebarRows now computes sessionIdsInList up front and excludes resolvable forks from the top-level list (static/sessions.js:3784-3787):

const sessionIdsInList=new Set((rawSessions||[]).map(s=>s&&s.session_id).filter(Boolean));
const rows=(collapsedRows||[])
  .filter(s=>!_isChildSession(s)&&((s&&s.pinned)||!_isForkWithResolvableParent(s, sessionIdsInList)))
  .map(s=>({...s}));

The same pinned-exception predicate is reused consistently at the attach guard (sessions.js:3827) and in attachDepthFor (3844-3845), so a pinned fork stays top-level and an unpinned resolvable fork nests — neither path can double-render. The regression test the review asked for is exactly here: test_fork_child_with_visible_parent_is_nested_once asserts rows == ["parent"] with one nested child, and test_fork_child_without_visible_parent_stays_top_level covers the absent-parent case. The pinned carve-out even has its own coverage in test_pinned_fork_with_visible_parent_stays_top_level (rows == ["parent", "fork1"], no _child_sessions).

🟠 Per-row actions — you went with Option A

The new session-child-session-fork render branch (sessions.js:4948) gives nested forks the full action surface, gated on _isReadOnlySession(child) so subagent children stay click-to-open:

const readOnlyChild=_isReadOnlySession(child);
if(!readOnlyChild){
  // three-dot menu → _openSessionActionMenu(child, menuBtn)
  // swipe affordances (archive / trash-2)
  installForkChildSwipe(row, child, actions);
}
row._startRename=_buildSessionRenameStarter(child, mainBtn, ...);

That's the first-class direction we hoped for: menu, rename, swipe-delete/archive, context menu, plus a live session-state-indicator for streaming/unread/attention bubbled from the child. The "rename snaps back on next cache render" concern is handled by _buildSessionRenameStarter updating title/display_title/_state_db_title together, covered in test_session_rename_lifecycle.py.

One small thing to confirm

In attachDepthFor (sessions.js:3821) the recursion uses a seen set for cycle-guarding, but attachDepthCache is keyed by session_id and populated after the recursive call returns — for a deep fork chain that's fine, but worth a sanity check that a fork whose parent_session_id points at itself (corrupt state) resolves to depth 0 rather than looping. The seen.has() guard at 3823 looks like it covers this; just calling it out.

Test surface looks solid: test_session_lineage_collapse.py (+247 lines), the branching and rename-lifecycle suites, and the touch-actions test all exercise the new paths. From a read-only review this looks ready for a maintainer merge pass. Nice iteration.

[rodboev]

Rebase update: I resolved the branch rebase conflicts by keeping the fork-specific nested behavior (forks now attach under parents and keep full session action/swipe affordances) while preserving current master behavior for non-fork child rows.

Validation run:

• npx eslint --no-config-lookup -c eslint.runtime-guard.config.mjs "static/**/*.js"
• python -m pytest tests/test_465_session_branching.py tests/test_session_lineage_collapse.py -v --timeout=60

All tests passed, and I force-pushed pr/fork-session-subgrouping on fork to cebd6e24.

[nesquena-hermes]

Picked this up to ship (the nesting UX is maintainer-approved, and the prior two rounds' findings are genuinely resolved — dedup vs top-level, cycle-safe depth walk, full action surface on nested forks, rename lifecycle all verified correct). I rebased it onto current master myself (it was 236 commits behind; net diff applied clean, sessions.js+style.css byte-identical to your head) and ran the full gate. Two silent behavioral regressions surfaced that I want fixed before it ships — both confirmed against the rebased code, and both invisible to a screenshot and to the green suite.

Must-fix

1. Parent rows get false unread/completion state when a nested fork is streaming. (static/sessions.js:5388)

const isStreaming=_isSessionEffectivelyStreaming(s)||!!s._child_session_streaming;
_rememberRenderedStreamingState(s, isStreaming);   // ← records the bubbled child state under the PARENT id

The parent spinner correctly bubbles _child_session_streaming, but then _rememberRenderedStreamingState(s, isStreaming) persists that bubbled value under the parent's id. On the next poll the raw parent isn't streaming, so the streaming→not-streaming transition fires and the parent is marked unread / completed even though nothing happened on the parent itself.
→ Split own-vs-bubbled: const ownStreaming=_isSessionEffectivelyStreaming(s); const isStreaming=ownStreaming||!!s._child_session_streaming; — render the spinner on isStreaming, but call _rememberRenderedStreamingState(s, ownStreaming) (own state only).

2. Nested forks drop out of batch-select. (static/sessions.js:5276 / nested render ~:5815)
_sessionVisibleSidebarIds is built only from top-level flatSessionRows, and the nested .session-child-session-fork rows render with no select-mode checkbox. So a fork that used to be batch archive/delete/move-selectable when it was top-level loses that path once it's nested under its parent.
→ Render the same select checkbox for writable nested fork rows in select mode, and include expanded/search-visible nested fork ids in the selectable id set.

One scope confirm (non-blocking)

The diff also makes subagent-parent badges reflect child streaming/unread/attention (change beyond the stated "nest forks" goal). Reads like a deliberate improvement — fine to keep, just flagging it's a behavior change to the existing subagent-child class, and finding #1 above is the concrete bug that rides along with it.

Everything else is ready — these are localized fixes. Both gates agree the nesting itself is correct (Opus: "safe to ship" modulo the badge-scope note; Codex: SHIP-ONLY-WITH-FIXES on the two above). Full suite green (8894). Ping me when pushed and I'll re-gate + ship. Thanks @rodboev. 🙏

[rodboev]

Addressed both must-fix items plus an additional call-site issue an adversarial review caught.

1. Parent streaming state isolation — _rememberRenderedStreamingState now receives the parent's own streaming state (ownStreaming), not the composite own+child value. The spinner still renders on the composite, but the transition map only tracks what the parent itself is doing, so poll completion won't false-positive unread/completed on the parent when a nested fork stops streaming.

2. Batch-select for nested forks — expanded writable fork children are now included in _sessionVisibleSidebarIds and get checkboxes in the fork child render path. Additionally, setSessionSelected, toggleSessionSelect, selectAllSessions, and deselectAllSessions all used .closest('.session-item') which skipped .session-child-session-fork rows entirely, toggling the selected class on the parent row instead. Widened the selector to .session-item,.session-child-session-fork.

3. Scope confirm — the subagent-parent badge change (child streaming/unread/attention bubbling) is intentional, and the streaming fix above keeps it clean.

Validation:

• tests/test_session_lineage_collapse.py, tests/test_465_session_branching.py, tests/test_session_touch_actions.py, tests/test_session_rename_lifecycle.py, tests/test_issue856_background_completion_unread.py, tests/test_issue856_pinned_indicator_layout.py (93 passed)
• npx eslint --no-config-lookup -c eslint.runtime-guard.config.mjs "static/**/*.js"