Skip to content

fix(app): prevent Solid cleanNode crash on session switch#37832

Open
cyrasafia wants to merge 5 commits into
anomalyco:devfrom
cyrasafia:fix/legacy-session-refresh
Open

fix(app): prevent Solid cleanNode crash on session switch#37832
cyrasafia wants to merge 5 commits into
anomalyco:devfrom
cyrasafia:fix/legacy-session-refresh

Conversation

@cyrasafia

@cyrasafia cyrasafia commented Jul 20, 2026

Copy link
Copy Markdown

Issue for this PR

Closes #37534

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

Fixes a freeze/crash when switching sessions in the desktop app. The crash manifested as an uncaught TypeError: Cannot read properties of null inside Solid's cleanNode, causing the UI to freeze and become unresponsive.

Root cause: Solid's cleanNode iterates node.owned[], node.tOwned[], and node.cleanups[] with a for-loop, then nulls the field after the loop. During a router transition (session switch), a cleanup or owned-child disposal can re-enter cleanNode on the same owner. The inner call nulls the field; the outer loop then reads null[i] and crashes. This affects all Solid 1.9.x releases (including 1.9.14); the upstream runComputation patch (#2046) included in this repo does not cover cleanNode.

Fix — two parts:

  1. Drop all session-id keyed remounts (session.tsx, app.tsx): The original approach used <Show keyed> on the session ID to refresh panel content on switch. This deepened the disposal tree during transitions, making the reentrant crash more likely. All session-id keyed remounts are removed — SessionPage and Page stay alive across switches; timeline messages, file tabs, and review diffs update reactively via params.id/sessionKey. View state resets are handled by existing effects (on(sessionKey) for store, on(params.id) for autoScroll/editor draft/focus).

  2. Guard cleanNode against reentrant disposal (patches/solid-js@1.9.10.patch): Extended the existing patch to capture owned/tOwned/cleanups to a local variable and null the node field before iterating, so reentrant calls no longer read a nulled array. Covers all four dist files (dev.js, dev.cjs, solid.js, solid.cjs).

Additional changes:

  • WorkspaceSessionPage keyed only on scope+directory (not session ID) so TerminalProvider survives session switches — eliminates PTY 404 errors on switch.
  • editor-dom.ts: setCursorPosition checks parent.isConnected to avoid writing a selection range into a detached editor.

How did you verify your code works?

  • Ran bun typecheck in packages/app
  • Built and installed opencode-desktop-bin locally (1.18.4)
  • Switched sessions repeatedly — no freeze or cleanNode crash
  • Verified the patch is applied after bun install (guards present in all dist files and in the built bundle)

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@onelpawarai

Copy link
Copy Markdown

Please support me with my ZYRAXON-AI project. https://github.com/onelpawarai/ZYRAXON-AI.git

@cyrasafia
cyrasafia marked this pull request as draft July 20, 2026 05:47
付晨昱 added 5 commits July 21, 2026 18:20
1.18.3-8 moved the session-id keyed remount from the session panel up to
the whole Page owner. Switching sessions still triggered Solid's
cleanNode on the same owner twice, crashing at node.cleanups[i]() when
an inner cleanup had already nulled cleanups.

Remove every session-id keyed remount inside Page: keep SessionPage
stable, keep WorkspaceSessionPage keyed only on scope+directory so
TerminalProvider survives, and let MessageTimeline/SessionPanelFrame
react to params.id without disposing. Session switches already reset
view state (on(sessionKey)), autoScroll + scroll-to-bottom (on(params.id)),
editor draft (restorePromptModel), and focus (on(params.id)); timeline
messages, file tabs and review diffs are reactive on params.id/sessionKey.

Also stop tracking the local AUR packaging files (.SRCINFO, PKGBUILD);
they are build artifacts moved under tmp/aur (gitignored).
Add reentrant-disposal guards to cleanNode in solid-js 1.9.10: capture
owned/tOwned/cleanups to a local, null the node field, then iterate. A
cleanup or owned-child disposal can re-enter cleanNode on the same owner
during a router transition, nulling the field mid-loop and crashing on
the next index read.
@cyrasafia
cyrasafia force-pushed the fix/legacy-session-refresh branch from 9a8716a to 13e731b Compare July 22, 2026 01:35
@cyrasafia
cyrasafia marked this pull request as ready for review July 22, 2026 01:35
@cyrasafia cyrasafia changed the title fix(desktop): refresh legacy session panel on session switch fix(app): prevent Solid cleanNode crash on session switch Jul 22, 2026
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.

[Desktop] Main content occasionally doesn't refresh when switching projects on the same server

2 participants