Skip to content

fix(window_panel): dispose outgoing tree tabs in clearAllPanels - #1344

Merged
fluck-boss merged 1 commit into
risa-labs-inc:devfrom
Mihir-Rabari:wfix/clear-panels-disposal-1310
Sep 24, 2026
Merged

fluck-boss merged 1 commit into
risa-labs-inc:devfrom
Mihir-Rabari:wfix/clear-panels-disposal-1310

Conversation

@Mihir-Rabari

Copy link
Copy Markdown
Contributor

Root cause

SplitViewState.clearAllPanels reset the split tree by assigning a fresh SplitNode.Panel("main") to _rootNode and nothing else. The outgoing root's BossTabsComponents were simply dereferenced: their tabs' LifecycleRegistrys stayed RESUMED, so onDestroy never fired - and for browser and terminal tabs those handlers are what release the Chromium process and the shell process. Since WorkspaceApplier.applyWorkspace calls clearAllPanels() between every apply (#1310), each restore over an unpreserved tree leaked one native process per open tab, compounding across session restores and workspace applies.

Design

The outgoing tree is disposed before the fresh main panel is assigned, using the exact primitive closeCurrentWorkspace already uses - clearAllTabs() on every panel - so per-tab lifecycle destruction (removeTab(i, recordForOpen = false) -> tabLifecycles.remove(it.id)?.destroy()) finally fires for the dropped tree.

One guard makes this safe: disposal is skipped when the outgoing root is still identity-held by preservedWorkspaceStates. A tree kept by preserveCurrentState must survive to be restored on switch-back, and its live tabs keep moving through moveTabToWorkspace and collectAllActiveTabs while preserved - destroying them would break the cross-workspace switch that SplitViewWorkspaceTransferTest and SplitViewActiveTabsTest pin. The identity check (it.rootNode === outgoingRoot) distinguishes the two cases without touching any preserved-state semantics.

getAllPanels() is read before _rootNode is reassigned, so it walks exactly the outgoing tree.

Tests

New SplitViewClearAllPanelsDisposalTest with a destroy-counting tab component (lifecycle onDestroy -> counter):

  • clearAllPanels destroys outgoing tree tabs when no preserved state holds it - both tabs of the old root destroyed exactly once; fresh main panel starts empty.
  • clearAllPanels keeps a preserved outgoing tree alive for switch-back - a tree recorded by preserveCurrentState keeps its tab undestroyed and reachable through panelsInWorkspace.
  • clearAllPanels disposes every panel of a split outgoing tree - disposal covers both panes of a vertical split.

Gates

One gradlew --offline ktlintCheck :composeApp:detekt :composeApp:desktopTest --tests 'ai.rever.boss.components.window_panel.*' --max-workers=2 invocation:

tests="3" skipped="0" failures="0" errors="0"   SplitViewClearAllPanelsDisposalTest (new)

Full ai.rever.boss.components.window_panel.* filter green - 222 tests completed, 0 failures (all pre-existing suites unchanged, including the preserved-workspace and transfer pins).

Fixes #1310

@Mihir-Rabari

Copy link
Copy Markdown
Contributor Author

CI note: the only red on this PR is a pre-existing macos timing flake — expected to clear on a failed-job re-run.

Everything else: 8/10 checks green, MERGEABLE. A re-run of the failed job should clear it (PR-author tokens cannot re-run repo workflows).

clearAllPanels replaced the split tree wholesale, dropping the old
root's BossTabsComponents without destroying their tabs' lifecycles.
For browser and terminal tabs those lifecycle destroys are what release
the Chromium process and the shell process, so every applyWorkspace over
an unpreserved tree leaked one native process per open tab - compounding
across workspace restores and applies (risa-labs-inc#1310).

The outgoing tree is now disposed through clearAllTabs() on every panel
before the fresh main panel is assigned - the same path
closeCurrentWorkspace already uses - guarded by an identity check: a
tree still held by preserveCurrentState is exempt, because it has to
survive to be restored on switch-back and keeps moving live tabs through
moveTabToWorkspace while preserved.

Tests use a destroy-counting tab component to cover disposal of an
unreferenced tree, survival of a preserved one, and disposal across a
split outgoing tree.

Fixes risa-labs-inc#1310
@Mihir-Rabari
Mihir-Rabari force-pushed the wfix/clear-panels-disposal-1310 branch from 93ee88e to 8cbad70 Compare September 22, 2026 07:55
@Mihir-Rabari

Copy link
Copy Markdown
Contributor Author

Rebuilt on current dev (bc9aa3b) as an isolated 2-file change (SplitView.kt, SplitViewClearAllPanelsDisposalTest.kt); the branch no longer carries unrelated history. Fresh CI is starting on 8cbad70.

@Mihir-Rabari

Copy link
Copy Markdown
Contributor Author

Fresh CI on 8cbad70: build-test (macOS/Ubuntu/Windows), code quality, compatibility, database, contracts, version, and shell checks all passed. Ready for re-review.

@fluck-boss fluck-boss closed this Sep 23, 2026
@fluck-boss fluck-boss reopened this Sep 23, 2026
@fluck-boss
fluck-boss merged commit 7ec2ed8 into risa-labs-inc:dev Sep 24, 2026
20 checks passed
@fluck-boss

Copy link
Copy Markdown
Collaborator

Merged to dev (squash). Gate: full CI green on the merge preview (10/10 checks) + maintainer diff verification - clearAllPanels now disposes the outgoing tree's tabs before reassigning _rootNode, releasing browser/terminal native processes (#1310), and the exemption for a tree still held by preserveCurrentState is explicit so switch-back keeps working. The new test pins both the disposal and the preserved-tree survival. No follow-ups filed. Thanks @Mihir-Rabari!

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.

2 participants